From 79fc484c16189d28b936c143a106bcd066053812 Mon Sep 17 00:00:00 2001 From: "auto-submit[bot]" Date: Thu, 1 Feb 2024 03:46:47 +0000 Subject: [PATCH] Revert "Remove migration flag and unused header files (#50216)" This reverts commit 5f380fffdba2b9c2399c000c81d9e75555907dce. --- ci/licenses_golden/licenses_flutter | 2 + lib/ui/BUILD.gn | 1 + lib/ui/text.dart | 22 ++++++- lib/ui/text/line_metrics.h | 62 +++++++++++++++++++ lib/ui/text/paragraph.h | 1 + lib/ui/text/paragraph_builder.cc | 9 ++- lib/ui/text/paragraph_builder.h | 6 +- .../lib/src/engine/canvaskit/renderer.dart | 1 + lib/web_ui/lib/src/engine/canvaskit/text.dart | 5 +- .../lib/src/engine/text/canvas_paragraph.dart | 11 ++++ lib/web_ui/lib/text.dart | 7 +++ lib/web_ui/test/canvaskit/text_test.dart | 30 ++++++++- .../test/html/text/canvas_paragraph_test.dart | 15 +++++ lib/web_ui/test/html/text_test.dart | 7 ++- testing/dart/paragraph_test.dart | 34 +++++++++- .../txt/src/skia/paragraph_builder_skia.cc | 2 +- third_party/txt/src/txt/paragraph_style.h | 8 +++ 17 files changed, 211 insertions(+), 12 deletions(-) create mode 100644 lib/ui/text/line_metrics.h diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index ed427001cdda1..ac8392aaebc70 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -5859,6 +5859,7 @@ ORIGIN: ../../../flutter/lib/ui/text/asset_manager_font_provider.cc + ../../../f ORIGIN: ../../../flutter/lib/ui/text/asset_manager_font_provider.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/ui/text/font_collection.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/ui/text/font_collection.h + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/lib/ui/text/line_metrics.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/ui/text/paragraph.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/ui/text/paragraph.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/ui/text/paragraph_builder.cc + ../../../flutter/LICENSE @@ -8721,6 +8722,7 @@ FILE: ../../../flutter/lib/ui/text/asset_manager_font_provider.cc FILE: ../../../flutter/lib/ui/text/asset_manager_font_provider.h FILE: ../../../flutter/lib/ui/text/font_collection.cc FILE: ../../../flutter/lib/ui/text/font_collection.h +FILE: ../../../flutter/lib/ui/text/line_metrics.h FILE: ../../../flutter/lib/ui/text/paragraph.cc FILE: ../../../flutter/lib/ui/text/paragraph.h FILE: ../../../flutter/lib/ui/text/paragraph_builder.cc diff --git a/lib/ui/BUILD.gn b/lib/ui/BUILD.gn index a762c39ddad64..2f4dfccb2b3ab 100644 --- a/lib/ui/BUILD.gn +++ b/lib/ui/BUILD.gn @@ -123,6 +123,7 @@ source_set("ui") { "text/asset_manager_font_provider.h", "text/font_collection.cc", "text/font_collection.h", + "text/line_metrics.h", "text/paragraph.cc", "text/paragraph.h", "text/paragraph_builder.cc", diff --git a/lib/ui/text.dart b/lib/ui/text.dart index 9a833f51cea0e..ff4665162577f 100644 --- a/lib/ui/text.dart +++ b/lib/ui/text.dart @@ -3355,6 +3355,23 @@ abstract class ParagraphBuilder { /// [Paragraph]. factory ParagraphBuilder(ParagraphStyle style) = _NativeParagraphBuilder; + /// Whether the rounding hack enabled by default in SkParagraph and TextPainter + /// is disabled. + /// + /// Do not rely on this getter as it exists for migration purposes only and + /// will soon be removed. + @Deprecated(''' + The shouldDisableRoundingHack flag is for internal migration purposes only and should not be used. + ''') + static bool get shouldDisableRoundingHack => _shouldDisableRoundingHack; + static bool _shouldDisableRoundingHack = true; + /// Do not call this method as it is for migration purposes only and will soon + /// be removed. + // ignore: use_setters_to_change_properties + static void setDisableRoundingHack(bool disableRoundingHack) { + _shouldDisableRoundingHack = disableRoundingHack; + } + /// The number of placeholders currently in the paragraph. int get placeholderCount; @@ -3472,10 +3489,11 @@ base class _NativeParagraphBuilder extends NativeFieldWrapperClass1 implements P style._height ?? 0, style._ellipsis ?? '', _encodeLocale(style._locale), + !ParagraphBuilder.shouldDisableRoundingHack, ); } - @Native(symbol: 'ParagraphBuilder::Create') + @Native(symbol: 'ParagraphBuilder::Create') external void _constructor( Int32List encoded, ByteData? strutData, @@ -3485,7 +3503,7 @@ base class _NativeParagraphBuilder extends NativeFieldWrapperClass1 implements P double height, String ellipsis, String locale, - ); + bool applyRoundingHack); @override int get placeholderCount => _placeholderCount; diff --git a/lib/ui/text/line_metrics.h b/lib/ui/text/line_metrics.h new file mode 100644 index 0000000000000..f5f656cd816f9 --- /dev/null +++ b/lib/ui/text/line_metrics.h @@ -0,0 +1,62 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef FLUTTER_LIB_UI_TEXT_LINE_METRICS_H_ +#define FLUTTER_LIB_UI_TEXT_LINE_METRICS_H_ + +#include "third_party/dart/runtime/include/dart_api.h" +#include "third_party/tonic/converter/dart_converter.h" + +namespace flutter { + +struct LineMetrics { + const bool* hard_break; + + // The final computed ascent and descent for the line. This can be impacted by + // the strut, height, scaling, as well as outlying runs that are very tall. + // + // The top edge is `baseline - ascent` and the bottom edge is `baseline + + // descent`. Ascent and descent are provided as positive numbers. Raw numbers + // for specific runs of text can be obtained in run_metrics_map. These values + // are the cumulative metrics for the entire line. + const double* ascent; + const double* descent; + const double* unscaled_ascent; + // Height of the line. + const double* height; + // Width of the line. + const double* width; + // The left edge of the line. The right edge can be obtained with `left + + // width` + const double* left; + // The y position of the baseline for this line from the top of the paragraph. + const double* baseline; + // Zero indexed line number. + const size_t* line_number; + + LineMetrics(); + + LineMetrics(const bool* hard_break, + const double* ascent, + const double* descent, + const double* unscaled_ascent, + const double* height, + const double* width, + const double* left, + const double* baseline, + const size_t* line_number) + : hard_break(hard_break), + ascent(ascent), + descent(descent), + unscaled_ascent(unscaled_ascent), + height(height), + width(width), + left(left), + baseline(baseline), + line_number(line_number) {} +}; + +} // namespace flutter + +#endif // FLUTTER_LIB_UI_TEXT_LINE_METRICS_H_ diff --git a/lib/ui/text/paragraph.h b/lib/ui/text/paragraph.h index d37e1ae62a13f..a1f4c96eb49e5 100644 --- a/lib/ui/text/paragraph.h +++ b/lib/ui/text/paragraph.h @@ -8,6 +8,7 @@ #include "flutter/fml/message_loop.h" #include "flutter/lib/ui/dart_wrapper.h" #include "flutter/lib/ui/painting/canvas.h" +#include "flutter/lib/ui/text/line_metrics.h" #include "flutter/third_party/txt/src/txt/paragraph.h" namespace flutter { diff --git a/lib/ui/text/paragraph_builder.cc b/lib/ui/text/paragraph_builder.cc index c79c09110b6e9..1e84a90debcf8 100644 --- a/lib/ui/text/paragraph_builder.cc +++ b/lib/ui/text/paragraph_builder.cc @@ -151,11 +151,12 @@ void ParagraphBuilder::Create(Dart_Handle wrapper, double fontSize, double height, const std::u16string& ellipsis, - const std::string& locale) { + const std::string& locale, + bool applyRoundingHack) { UIDartState::ThrowIfUIOperationsProhibited(); auto res = fml::MakeRefCounted( encoded_handle, strutData, fontFamily, strutFontFamilies, fontSize, - height, ellipsis, locale); + height, ellipsis, locale, applyRoundingHack); res->AssociateWithDartWrapper(wrapper); } @@ -230,7 +231,8 @@ ParagraphBuilder::ParagraphBuilder( double fontSize, double height, const std::u16string& ellipsis, - const std::string& locale) { + const std::string& locale, + bool applyRoundingHack) { int32_t mask = 0; txt::ParagraphStyle style; { @@ -291,6 +293,7 @@ ParagraphBuilder::ParagraphBuilder( if (mask & kPSLocaleMask) { style.locale = locale; } + style.apply_rounding_hack = applyRoundingHack; FontCollection& font_collection = UIDartState::Current() ->platform_configuration() diff --git a/lib/ui/text/paragraph_builder.h b/lib/ui/text/paragraph_builder.h index 809f91bc3fd20..4b341a739f026 100644 --- a/lib/ui/text/paragraph_builder.h +++ b/lib/ui/text/paragraph_builder.h @@ -30,7 +30,8 @@ class ParagraphBuilder : public RefCountedDartWrappable { double fontSize, double height, const std::u16string& ellipsis, - const std::string& locale); + const std::string& locale, + bool applyRoundingHack); ~ParagraphBuilder() override; @@ -76,7 +77,8 @@ class ParagraphBuilder : public RefCountedDartWrappable { double fontSize, double height, const std::u16string& ellipsis, - const std::string& locale); + const std::string& locale, + bool applyRoundingHack); std::unique_ptr m_paragraph_builder_; }; diff --git a/lib/web_ui/lib/src/engine/canvaskit/renderer.dart b/lib/web_ui/lib/src/engine/canvaskit/renderer.dart index 3917fdc59c550..f4fdaef67c0f9 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/renderer.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/renderer.dart @@ -379,6 +379,7 @@ class CanvasKitRenderer implements Renderer { strutStyle: strutStyle, ellipsis: ellipsis, locale: locale, + applyRoundingHack: !ui.ParagraphBuilder.shouldDisableRoundingHack, ); @override diff --git a/lib/web_ui/lib/src/engine/canvaskit/text.dart b/lib/web_ui/lib/src/engine/canvaskit/text.dart index 4a928cb3092fb..235690f2487aa 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/text.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/text.dart @@ -33,6 +33,7 @@ class CkParagraphStyle implements ui.ParagraphStyle { ui.StrutStyle? strutStyle, String? ellipsis, ui.Locale? locale, + bool applyRoundingHack = true, }) : skParagraphStyle = toSkParagraphStyle( textAlign, textDirection, @@ -46,6 +47,7 @@ class CkParagraphStyle implements ui.ParagraphStyle { strutStyle, ellipsis, locale, + applyRoundingHack, ), _textAlign = textAlign, _textDirection = textDirection, @@ -161,6 +163,7 @@ class CkParagraphStyle implements ui.ParagraphStyle { ui.StrutStyle? strutStyle, String? ellipsis, ui.Locale? locale, + bool applyRoundingHack, ) { final SkParagraphStyleProperties properties = SkParagraphStyleProperties(); @@ -197,7 +200,7 @@ class CkParagraphStyle implements ui.ParagraphStyle { properties.replaceTabCharacters = true; properties.textStyle = toSkTextStyleProperties( fontFamily, fontSize, height, fontWeight, fontStyle); - properties.applyRoundingHack = false; + properties.applyRoundingHack = applyRoundingHack; return canvasKit.ParagraphStyle(properties); } diff --git a/lib/web_ui/lib/src/engine/text/canvas_paragraph.dart b/lib/web_ui/lib/src/engine/text/canvas_paragraph.dart index 55db09a32410b..a692a3698b0b0 100644 --- a/lib/web_ui/lib/src/engine/text/canvas_paragraph.dart +++ b/lib/web_ui/lib/src/engine/text/canvas_paragraph.dart @@ -87,6 +87,17 @@ class CanvasParagraph implements ui.Paragraph { @override void layout(ui.ParagraphConstraints constraints) { + // When constraint width has a decimal place, we floor it to avoid getting + // a layout width that's higher than the constraint width. + // + // For example, if constraint width is `30.8` and the text has a width of + // `30.5` then the TextPainter in the framework will ceil the `30.5` width + // which will result in a width of `40.0` that's higher than the constraint + // width. + if (!ui.ParagraphBuilder.shouldDisableRoundingHack) { + constraints = ui.ParagraphConstraints(width: constraints.width.floorToDouble()); + } + if (constraints == _lastUsedConstraints) { return; } diff --git a/lib/web_ui/lib/text.dart b/lib/web_ui/lib/text.dart index c4b2fdb3268d1..adddffae4f863 100644 --- a/lib/web_ui/lib/text.dart +++ b/lib/web_ui/lib/text.dart @@ -733,6 +733,13 @@ abstract class ParagraphBuilder { factory ParagraphBuilder(ParagraphStyle style) => engine.renderer.createParagraphBuilder(style); + static bool get shouldDisableRoundingHack => _shouldDisableRoundingHack; + static bool _shouldDisableRoundingHack = true; + // ignore: use_setters_to_change_properties + static void setDisableRoundingHack(bool disableRoundingHack) { + _shouldDisableRoundingHack = disableRoundingHack; + } + void pushStyle(TextStyle style); void pop(); void addText(String text); diff --git a/lib/web_ui/test/canvaskit/text_test.dart b/lib/web_ui/test/canvaskit/text_test.dart index 6d71362152574..bc0f58070125e 100644 --- a/lib/web_ui/test/canvaskit/text_test.dart +++ b/lib/web_ui/test/canvaskit/text_test.dart @@ -204,7 +204,9 @@ void testMain() { expect(bottomRight?.writingDirection, ui.TextDirection.ltr); }); - test('rounding hack disabled', () { + test('rounding hack disabled by default', () { + expect(ui.ParagraphBuilder.shouldDisableRoundingHack, isTrue); + const double fontSize = 1.25; const String text = '12345'; assert((fontSize * text.length).truncate() != fontSize * text.length); @@ -224,6 +226,32 @@ void testMain() { } }); + test('setDisableRoundinghHack to false works in tests', () { + bool assertsEnabled = false; + assert(() { + assertsEnabled = true; + return true; + }()); + if (!assertsEnabled){ + return; + } + + if (ui.ParagraphBuilder.shouldDisableRoundingHack) { + ui.ParagraphBuilder.setDisableRoundingHack(false); + addTearDown(() => ui.ParagraphBuilder.setDisableRoundingHack(true)); + } + + assert(!ui.ParagraphBuilder.shouldDisableRoundingHack); + const double fontSize = 1.25; + const String text = '12345'; + assert((fontSize * text.length).truncate() != fontSize * text.length); + final ui.ParagraphBuilder builder = ui.ParagraphBuilder(ui.ParagraphStyle(fontSize: fontSize, fontFamily: 'FlutterTest')); + builder.addText(text); + final ui.Paragraph paragraph = builder.build() + ..layout(const ui.ParagraphConstraints(width: text.length * fontSize)); + expect(paragraph.computeLineMetrics().length, greaterThan(1)); + }); + // TODO(hterkelsen): https://github.com/flutter/flutter/issues/71520 }, skip: isSafari || isFirefox); } diff --git a/lib/web_ui/test/html/text/canvas_paragraph_test.dart b/lib/web_ui/test/html/text/canvas_paragraph_test.dart index a00c6d16f9357..c738fa4103d84 100644 --- a/lib/web_ui/test/html/text/canvas_paragraph_test.dart +++ b/lib/web_ui/test/html/text/canvas_paragraph_test.dart @@ -777,6 +777,21 @@ Future testMain() async { expect(paragraph.longestLine, 50.0); }); + test('$CanvasParagraph.width should be a whole integer when shouldDisableRoundingHack is false', () { + if (ui.ParagraphBuilder.shouldDisableRoundingHack) { + ui.ParagraphBuilder.setDisableRoundingHack(false); + addTearDown(() => ui.ParagraphBuilder.setDisableRoundingHack(true)); + } + // The paragraph width is only rounded to a whole integer if + // shouldDisableRoundingHack is false. + assert(!ui.ParagraphBuilder.shouldDisableRoundingHack); + final ui.Paragraph paragraph = plain(ahemStyle, 'abc'); + paragraph.layout(const ui.ParagraphConstraints(width: 30.8)); + + expect(paragraph.width, 30); + expect(paragraph.height, 10); + }); + test('Render after dispose', () async { final ui.Paragraph paragraph = plain(ahemStyle, 'abc'); paragraph.layout(const ui.ParagraphConstraints(width: 30.8)); diff --git a/lib/web_ui/test/html/text_test.dart b/lib/web_ui/test/html/text_test.dart index b287352ec2959..d935a0ae15cfe 100644 --- a/lib/web_ui/test/html/text_test.dart +++ b/lib/web_ui/test/html/text_test.dart @@ -200,7 +200,12 @@ Future testMain() async { expect(bottomRight?.graphemeClusterLayoutBounds, const Rect.fromLTWH(0.0, 0.0, 10.0, 10.0)); }, skip: domIntl.v8BreakIterator == null); // Intended: Intl.v8breakiterator is needed for correctly breaking grapheme clusters. - test('disable rounding hack', () { + test('Can disable rounding hack', () { + if (!ParagraphBuilder.shouldDisableRoundingHack) { + ParagraphBuilder.setDisableRoundingHack(true); + addTearDown(() => ParagraphBuilder.setDisableRoundingHack(false)); + } + assert(ParagraphBuilder.shouldDisableRoundingHack); const double fontSize = 1; const String text = '12345'; const double letterSpacing = 0.25; diff --git a/testing/dart/paragraph_test.dart b/testing/dart/paragraph_test.dart index d8acb74c50c34..35bd0ccb68007 100644 --- a/testing/dart/paragraph_test.dart +++ b/testing/dart/paragraph_test.dart @@ -333,10 +333,42 @@ void main() { } }); - test('rounding hack disabled', () { + test('can set disableRoundingHack to false in tests', () { + bool assertsEnabled = false; + assert(() { + assertsEnabled = true; + return true; + }()); + if (!assertsEnabled){ + return; + } + const double fontSize = 1.25; + const String text = '12345'; + assert((fontSize * text.length).truncate() != fontSize * text.length); + // ignore: deprecated_member_use + final bool roundingHackWasDisabled = ParagraphBuilder.shouldDisableRoundingHack; + if (roundingHackWasDisabled) { + ParagraphBuilder.setDisableRoundingHack(false); + } + // ignore: deprecated_member_use + assert(!ParagraphBuilder.shouldDisableRoundingHack); + final ParagraphBuilder builder = ParagraphBuilder(ParagraphStyle(fontSize: fontSize)); + builder.addText(text); + final Paragraph paragraph = builder.build() + ..layout(const ParagraphConstraints(width: text.length * fontSize)); + expect(paragraph.computeLineMetrics().length, greaterThan(1)); + + if (roundingHackWasDisabled) { + ParagraphBuilder.setDisableRoundingHack(true); + } + }); + + test('rounding hack disabled by default', () { const double fontSize = 1.25; const String text = '12345'; assert((fontSize * text.length).truncate() != fontSize * text.length); + // ignore: deprecated_member_use + expect(ParagraphBuilder.shouldDisableRoundingHack, isTrue); final ParagraphBuilder builder = ParagraphBuilder(ParagraphStyle(fontSize: fontSize)); builder.addText(text); final Paragraph paragraph = builder.build() diff --git a/third_party/txt/src/skia/paragraph_builder_skia.cc b/third_party/txt/src/skia/paragraph_builder_skia.cc index a062a357773f6..443c2b9ca2b08 100644 --- a/third_party/txt/src/skia/paragraph_builder_skia.cc +++ b/third_party/txt/src/skia/paragraph_builder_skia.cc @@ -139,7 +139,7 @@ skt::ParagraphStyle ParagraphBuilderSkia::TxtToSkia(const ParagraphStyle& txt) { skia.turnHintingOff(); skia.setReplaceTabCharacters(true); - skia.setApplyRoundingHack(false); + skia.setApplyRoundingHack(txt.apply_rounding_hack); return skia; } diff --git a/third_party/txt/src/txt/paragraph_style.h b/third_party/txt/src/txt/paragraph_style.h index 7c3043b8ad899..e1915323a0bd3 100644 --- a/third_party/txt/src/txt/paragraph_style.h +++ b/third_party/txt/src/txt/paragraph_style.h @@ -95,6 +95,14 @@ class ParagraphStyle { std::u16string ellipsis; std::string locale; + // Temporary flag that indicates whether the Paragraph should report its + // metrics with rounding hacks applied. + // + // This flag currently defaults to true and will be flipped to false once the + // migration is complete. + // TODO(LongCatIsLooong): https://github.com/flutter/flutter/issues/31707 + bool apply_rounding_hack = true; + TextStyle GetTextStyle() const; bool unlimited_lines() const;