From 7be0036938cf1fd91beed858a134447f5a3168c5 Mon Sep 17 00:00:00 2001 From: garyqian Date: Mon, 30 Dec 2019 14:37:51 -0500 Subject: [PATCH 01/33] Begin adding API for boundary line height behavior --- lib/ui/text.dart | 17 +++++++++++------ lib/ui/text/paragraph_builder.cc | 20 ++++++++++++++------ third_party/txt/src/txt/paragraph_style.h | 1 + 3 files changed, 26 insertions(+), 12 deletions(-) diff --git a/lib/ui/text.dart b/lib/ui/text.dart index 19b06a906e878..4540fa17c7821 100644 --- a/lib/ui/text.dart +++ b/lib/ui/text.dart @@ -753,6 +753,7 @@ Int32List _encodeParagraphStyle( String fontFamily, double fontSize, double height, + int boundingLineHeightBehavior, FontWeight fontWeight, FontStyle fontStyle, StrutStyle strutStyle, @@ -780,28 +781,32 @@ Int32List _encodeParagraphStyle( result[0] |= 1 << 5; result[5] = maxLines; } - if (fontFamily != null) { + if (boundingLineHeightBehavior != null) { result[0] |= 1 << 6; + result[6] = boundingLineHeightBehavior; + } + if (fontFamily != null) { + result[0] |= 1 << 7; // Passed separately to native. } if (fontSize != null) { - result[0] |= 1 << 7; + result[0] |= 1 << 8; // Passed separately to native. } if (height != null) { - result[0] |= 1 << 8; + result[0] |= 1 << 9; // Passed separately to native. } if (strutStyle != null) { - result[0] |= 1 << 9; + result[0] |= 1 << 10; // Passed separately to native. } if (ellipsis != null) { - result[0] |= 1 << 10; + result[0] |= 1 << 11; // Passed separately to native. } if (locale != null) { - result[0] |= 1 << 11; + result[0] |= 1 << 12; // Passed separately to native. } return result; diff --git a/lib/ui/text/paragraph_builder.cc b/lib/ui/text/paragraph_builder.cc index 8c858e6953ba9..1c654a78ba9b2 100644 --- a/lib/ui/text/paragraph_builder.cc +++ b/lib/ui/text/paragraph_builder.cc @@ -75,12 +75,13 @@ const int psTextDirectionIndex = 2; const int psFontWeightIndex = 3; const int psFontStyleIndex = 4; const int psMaxLinesIndex = 5; -const int psFontFamilyIndex = 6; -const int psFontSizeIndex = 7; -const int psHeightIndex = 8; -const int psStrutStyleIndex = 9; -const int psEllipsisIndex = 10; -const int psLocaleIndex = 11; +const int psBoundingLineHeightBehaviorIndex = 6; +const int psFontFamilyIndex = 7; +const int psFontSizeIndex = 8; +const int psHeightIndex = 9; +const int psStrutStyleIndex = 10; +const int psEllipsisIndex = 11; +const int psLocaleIndex = 12; const int psTextAlignMask = 1 << psTextAlignIndex; const int psTextDirectionMask = 1 << psTextDirectionIndex; @@ -90,6 +91,8 @@ const int psMaxLinesMask = 1 << psMaxLinesIndex; const int psFontFamilyMask = 1 << psFontFamilyIndex; const int psFontSizeMask = 1 << psFontSizeIndex; const int psHeightMask = 1 << psHeightIndex; +const int psBoundingLineHeightBehaviorMask = + 1 << psBoundingLineHeightBehaviorIndex; const int psStrutStyleMask = 1 << psStrutStyleIndex; const int psEllipsisMask = 1 << psEllipsisIndex; const int psLocaleMask = 1 << psLocaleIndex; @@ -265,6 +268,11 @@ ParagraphBuilder::ParagraphBuilder( style.has_height_override = true; } + if (mask & psBoundingLineHeightBehaviorMask) { + style.bounding_line_height_behavior = + encoded[psBoundingLineHeightBehaviorIndex]; + } + if (mask & psStrutStyleMask) { decodeStrut(strutData, strutFontFamilies, style); } diff --git a/third_party/txt/src/txt/paragraph_style.h b/third_party/txt/src/txt/paragraph_style.h index 9ca5fae3ca9c4..f011f72d4d68b 100644 --- a/third_party/txt/src/txt/paragraph_style.h +++ b/third_party/txt/src/txt/paragraph_style.h @@ -50,6 +50,7 @@ class ParagraphStyle { std::string font_family = ""; double font_size = 14; double height = 1; + size_t bounding_line_height_behavior = 1 << 0 + 1 << 1 + 1 << 2; bool has_height_override = false; // Strut properties. strut_enabled must be set to true for the rest of the From 82f9361ee0de9a4269567cd92280dcce55bbb88e Mon Sep 17 00:00:00 2001 From: garyqian Date: Mon, 30 Dec 2019 15:04:50 -0500 Subject: [PATCH 02/33] Add enum --- third_party/txt/src/txt/paragraph_style.h | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/third_party/txt/src/txt/paragraph_style.h b/third_party/txt/src/txt/paragraph_style.h index f011f72d4d68b..7c8b8bde8bfb9 100644 --- a/third_party/txt/src/txt/paragraph_style.h +++ b/third_party/txt/src/txt/paragraph_style.h @@ -41,6 +41,17 @@ enum class TextDirection { ltr, }; +// Multiple behaviors can be applied at once with a bitwise & operator. For +// example, disabling first and last can achieved with: +// +// (kDisableFirst & kDisableLast). +enum BoundaryLineHeightBehavior { + kDisableAll = 0x0, + kAll = 0x1 | 0x2, + kDisableFirst = 0x2, + kDisableLast = 0x1, +}; + class ParagraphStyle { public: // Default TextStyle. Used in GetTextStyle() to obtain the base TextStyle to @@ -50,7 +61,7 @@ class ParagraphStyle { std::string font_family = ""; double font_size = 14; double height = 1; - size_t bounding_line_height_behavior = 1 << 0 + 1 << 1 + 1 << 2; + size_t bounding_line_height_behavior = BoundaryLineHeightBehavior::kAll; bool has_height_override = false; // Strut properties. strut_enabled must be set to true for the rest of the From 882397b5f45986e7f6fc8f751a13ade1f42a4c4b Mon Sep 17 00:00:00 2001 From: garyqian Date: Thu, 2 Jan 2020 17:09:48 -0500 Subject: [PATCH 03/33] LibTxt begin impl --- lib/ui/text.dart | 4 +++- third_party/txt/src/txt/paragraph_txt.cc | 21 ++++++++++++++++++--- third_party/txt/src/txt/paragraph_txt.h | 5 ++++- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/lib/ui/text.dart b/lib/ui/text.dart index 4540fa17c7821..40a43f4f8e4f5 100644 --- a/lib/ui/text.dart +++ b/lib/ui/text.dart @@ -760,7 +760,7 @@ Int32List _encodeParagraphStyle( String ellipsis, Locale locale, ) { - final Int32List result = Int32List(6); // also update paragraph_builder.cc + final Int32List result = Int32List(7); // also update paragraph_builder.cc if (textAlign != null) { result[0] |= 1 << 1; result[1] = textAlign.index; @@ -874,6 +874,7 @@ class ParagraphStyle { String fontFamily, double fontSize, double height, + int boundingLineHeightBehavior, FontWeight fontWeight, FontStyle fontStyle, StrutStyle strutStyle, @@ -886,6 +887,7 @@ class ParagraphStyle { fontFamily, fontSize, height, + boundingLineHeightBehavior, fontWeight, fontStyle, strutStyle, diff --git a/third_party/txt/src/txt/paragraph_txt.cc b/third_party/txt/src/txt/paragraph_txt.cc index 4b28234e83876..18a4c918cf847 100644 --- a/third_party/txt/src/txt/paragraph_txt.cc +++ b/third_party/txt/src/txt/paragraph_txt.cc @@ -1113,7 +1113,8 @@ void ParagraphTxt::Layout(double width) { for (const PaintRecord& paint_record : paint_records) { UpdateLineMetrics(paint_record.metrics(), paint_record.style(), max_ascent, max_descent, max_unscaled_ascent, - paint_record.GetPlaceholderRun()); + paint_record.GetPlaceholderRun(), line_number, + line_limit); } // If no fonts were actually rendered, then compute a baseline based on the @@ -1125,7 +1126,7 @@ void ParagraphTxt::Layout(double width) { font.setSize(style.font_size); font.getMetrics(&metrics); UpdateLineMetrics(metrics, style, max_ascent, max_descent, - max_unscaled_ascent, nullptr); + max_unscaled_ascent, nullptr, line_number, line_limit); } // Calculate the baselines. This is only done on the first line. @@ -1181,7 +1182,9 @@ void ParagraphTxt::UpdateLineMetrics(const SkFontMetrics& metrics, double& max_ascent, double& max_descent, double& max_unscaled_ascent, - PlaceholderRun* placeholder_run) { + PlaceholderRun* placeholder_run, + size_t line_number, + size_t line_limit) { if (!strut_.force_strut) { double ascent; double descent; @@ -1242,6 +1245,18 @@ void ParagraphTxt::UpdateLineMetrics(const SkFontMetrics& metrics, ascent = (-metrics.fAscent + metrics.fLeading / 2); descent = (metrics.fDescent + metrics.fLeading / 2); } + + // Account for bounding_line_height_behavior in parargaph_style_. + if (line_number == 0 && !(paragraph_style_.bounding_line_height_behavior & + ~BoundaryLineHeightBehavior::kDisableFirst)) { + ascent = -metrics.fAscent; + } + if (line_number == line_limit - 1 && + !(paragraph_style_.bounding_line_height_behavior & + ~BoundaryLineHeightBehavior::kDisableLast)) { + descent = metrics.fDescent; + } + ComputePlaceholder(placeholder_run, ascent, descent); max_ascent = std::max(ascent, max_ascent); diff --git a/third_party/txt/src/txt/paragraph_txt.h b/third_party/txt/src/txt/paragraph_txt.h index 7cb1cce73f8e2..803e760c9628e 100644 --- a/third_party/txt/src/txt/paragraph_txt.h +++ b/third_party/txt/src/txt/paragraph_txt.h @@ -368,7 +368,10 @@ class ParagraphTxt : public Paragraph { double& max_ascent, double& max_descent, double& max_unscaled_ascent, - PlaceholderRun* placeholder_run); + PlaceholderRun* placeholder_run, + size_t line_number, + size_t line_limit); + // Calculate the starting X offset of a line based on the line's width and // alignment. double GetLineXOffset(double line_total_advance, From 5a92bbb2839d3a0f721083d5c51b8f6bcea9eb0e Mon Sep 17 00:00:00 2001 From: garyqian Date: Thu, 2 Jan 2020 17:38:19 -0500 Subject: [PATCH 04/33] Add docs/comments --- third_party/txt/src/txt/paragraph_style.h | 8 ++++++++ third_party/txt/src/txt/paragraph_txt.cc | 3 +++ 2 files changed, 11 insertions(+) diff --git a/third_party/txt/src/txt/paragraph_style.h b/third_party/txt/src/txt/paragraph_style.h index 7c8b8bde8bfb9..c7c1e79f8d45a 100644 --- a/third_party/txt/src/txt/paragraph_style.h +++ b/third_party/txt/src/txt/paragraph_style.h @@ -41,6 +41,14 @@ enum class TextDirection { ltr, }; +// Allows disabling height adjustments to first first line's ascent and the +// last line's descent. If disabled, the line will use the default font +// metric provided ascent/descent and ParagraphStyle.height will not take +// effect. +// +// The default behavior is kAll where height adjustments are enabled for all +// lines. +// // Multiple behaviors can be applied at once with a bitwise & operator. For // example, disabling first and last can achieved with: // diff --git a/third_party/txt/src/txt/paragraph_txt.cc b/third_party/txt/src/txt/paragraph_txt.cc index 18a4c918cf847..df1e1eaa8ca67 100644 --- a/third_party/txt/src/txt/paragraph_txt.cc +++ b/third_party/txt/src/txt/paragraph_txt.cc @@ -1247,10 +1247,13 @@ void ParagraphTxt::UpdateLineMetrics(const SkFontMetrics& metrics, } // Account for bounding_line_height_behavior in parargaph_style_. + // + // Disable first line ascent modifications. if (line_number == 0 && !(paragraph_style_.bounding_line_height_behavior & ~BoundaryLineHeightBehavior::kDisableFirst)) { ascent = -metrics.fAscent; } + // Disable last line descent modifications. if (line_number == line_limit - 1 && !(paragraph_style_.bounding_line_height_behavior & ~BoundaryLineHeightBehavior::kDisableLast)) { From 3125514f710caf2a69aebd442c9d7382ab82e8ab Mon Sep 17 00:00:00 2001 From: GaryQian Date: Mon, 6 Jan 2020 15:13:41 -0800 Subject: [PATCH 05/33] add tests --- .../lib/src/engine/compositor/text.dart | 7 ++ lib/web_ui/lib/src/engine/text/paragraph.dart | 5 + lib/web_ui/lib/src/ui/text.dart | 3 + third_party/txt/src/txt/paragraph_txt.h | 1 + third_party/txt/tests/paragraph_unittests.cc | 97 +++++++++++++++++++ 5 files changed, 113 insertions(+) diff --git a/lib/web_ui/lib/src/engine/compositor/text.dart b/lib/web_ui/lib/src/engine/compositor/text.dart index 06f6805b88020..ace85a05a1e72 100644 --- a/lib/web_ui/lib/src/engine/compositor/text.dart +++ b/lib/web_ui/lib/src/engine/compositor/text.dart @@ -25,6 +25,7 @@ class SkParagraphStyle implements ui.ParagraphStyle { String fontFamily, double fontSize, double height, + int boundingLineHeightBehavior, ui.FontWeight fontWeight, ui.FontStyle fontStyle, ui.StrutStyle strutStyle, @@ -38,6 +39,7 @@ class SkParagraphStyle implements ui.ParagraphStyle { fontFamily, fontSize, height, + boundingLineHeightBehavior, fontWeight, fontStyle, ellipsis, @@ -85,6 +87,7 @@ class SkParagraphStyle implements ui.ParagraphStyle { String fontFamily, double fontSize, double height, + int boundingLineHeightBehavior, ui.FontWeight fontWeight, ui.FontStyle fontStyle, String ellipsis, @@ -129,6 +132,10 @@ class SkParagraphStyle implements ui.ParagraphStyle { skParagraphStyle['heightMultiplier'] = height; } + if (boundingLineHeightBehavior != null) { + skParagraphStyle['boundingLineHeightBehavior'] = boundingLineHeightBehavior; + } + if (maxLines != null) { skParagraphStyle['maxLines'] = maxLines; } diff --git a/lib/web_ui/lib/src/engine/text/paragraph.dart b/lib/web_ui/lib/src/engine/text/paragraph.dart index b4c5912c7946e..2c9f1fc579b5e 100644 --- a/lib/web_ui/lib/src/engine/text/paragraph.dart +++ b/lib/web_ui/lib/src/engine/text/paragraph.dart @@ -457,6 +457,7 @@ class EngineParagraphStyle implements ui.ParagraphStyle { String fontFamily, double fontSize, double height, + int boundingLineHeightBehavior, ui.FontWeight fontWeight, ui.FontStyle fontStyle, ui.StrutStyle strutStyle, @@ -470,6 +471,7 @@ class EngineParagraphStyle implements ui.ParagraphStyle { _fontFamily = fontFamily, _fontSize = fontSize, _height = height, + _boundingLineHeightBehavior = boundingLineHeightBehavior, // TODO(b/128317744): add support for strut style. _strutStyle = strutStyle, _ellipsis = ellipsis, @@ -483,6 +485,7 @@ class EngineParagraphStyle implements ui.ParagraphStyle { final String _fontFamily; final double _fontSize; final double _height; + final int _boundingLineHeightBehavior; final EngineStrutStyle _strutStyle; final String _ellipsis; final ui.Locale _locale; @@ -536,6 +539,7 @@ class EngineParagraphStyle implements ui.ParagraphStyle { _fontFamily == typedOther._fontFamily || _fontSize == typedOther._fontSize || _height == typedOther._height || + _boundingLineHeightBehavior == typedOther._boundingLineHeightBehavior || _ellipsis == typedOther._ellipsis || _locale == typedOther._locale; } @@ -556,6 +560,7 @@ class EngineParagraphStyle implements ui.ParagraphStyle { 'fontFamily: ${_fontFamily ?? "unspecified"}, ' 'fontSize: ${_fontSize != null ? _fontSize.toStringAsFixed(1) : "unspecified"}, ' 'height: ${_height != null ? "${_height.toStringAsFixed(1)}x" : "unspecified"}, ' + 'boundingLineHeightBehavior: ${_boundingLineHeightBehavior ?? "unspecified"}, ' 'ellipsis: ${_ellipsis != null ? "\"$_ellipsis\"" : "unspecified"}, ' 'locale: ${_locale ?? "unspecified"}' ')'; diff --git a/lib/web_ui/lib/src/ui/text.dart b/lib/web_ui/lib/src/ui/text.dart index 6b57d37f2f168..022e681d9a8ea 100644 --- a/lib/web_ui/lib/src/ui/text.dart +++ b/lib/web_ui/lib/src/ui/text.dart @@ -584,6 +584,7 @@ abstract class ParagraphStyle { String fontFamily, double fontSize, double height, + int boundingLineHeightBehavior, FontWeight fontWeight, FontStyle fontStyle, StrutStyle strutStyle, @@ -598,6 +599,7 @@ abstract class ParagraphStyle { fontFamily: fontFamily, fontSize: fontSize, height: height, + boundingLineHeightBehavior: boundingLineHeightBehavior, fontWeight: fontWeight, fontStyle: fontStyle, strutStyle: strutStyle, @@ -612,6 +614,7 @@ abstract class ParagraphStyle { fontFamily: fontFamily, fontSize: fontSize, height: height, + boundingLineHeightBehavior: boundingLineHeightBehavior, fontWeight: fontWeight, fontStyle: fontStyle, strutStyle: strutStyle, diff --git a/third_party/txt/src/txt/paragraph_txt.h b/third_party/txt/src/txt/paragraph_txt.h index 803e760c9628e..4eea57bb489c3 100644 --- a/third_party/txt/src/txt/paragraph_txt.h +++ b/third_party/txt/src/txt/paragraph_txt.h @@ -162,6 +162,7 @@ class ParagraphTxt : public Paragraph { FRIEND_TEST(ParagraphTest, FontFeaturesParagraph); FRIEND_TEST(ParagraphTest, GetGlyphPositionAtCoordinateSegfault); FRIEND_TEST(ParagraphTest, KhmerLineBreaker); + FRIEND_TEST(ParagraphTest, BoundingLineHeightBehaviorRectsParagraph); // Starting data to layout. std::vector text_; diff --git a/third_party/txt/tests/paragraph_unittests.cc b/third_party/txt/tests/paragraph_unittests.cc index 057273eddf9c7..2ab8c93c5e686 100644 --- a/third_party/txt/tests/paragraph_unittests.cc +++ b/third_party/txt/tests/paragraph_unittests.cc @@ -5877,4 +5877,101 @@ TEST_F(ParagraphTest, KhmerLineBreaker) { ASSERT_TRUE(Snapshot()); } +TEST_F(ParagraphTest, BoundingLineHeightBehaviorRectsParagraph) { + // clang-format off + const char* text = + "line1\nline2\nline3"; + // clang-format on + auto icu_text = icu::UnicodeString::fromUTF8(text); + std::u16string u16_text(icu_text.getBuffer(), + icu_text.getBuffer() + icu_text.length()); + + txt::ParagraphStyle paragraph_style; + paragraph_style.bounding_line_height_behavior = + txt::BoundaryLineHeightBehavior::kDisableFirst & + txt::BoundaryLineHeightBehavior::kDisableLast; + + txt::ParagraphBuilderTxt builder(paragraph_style, GetTestFontCollection()); + + txt::TextStyle text_style; + text_style.color = SK_ColorBLACK; + text_style.font_families = std::vector(1, "Roboto"); + text_style.font_size = 30; + text_style.height = 5; + text_style.has_height_override = true; + builder.PushStyle(text_style); + builder.AddText(u16_text); + + builder.Pop(); + + auto paragraph = BuildParagraph(builder); + paragraph->Layout(GetTestCanvasWidth() - 300); + + paragraph->Paint(GetCanvas(), 0, 0); + + for (size_t i = 0; i < u16_text.length(); i++) { + ASSERT_EQ(paragraph->text_[i], u16_text[i]); + } + + ASSERT_EQ(paragraph->records_.size(), 3ull); + + SkPaint paint; + paint.setStyle(SkPaint::kStroke_Style); + paint.setAntiAlias(true); + paint.setStrokeWidth(1); + + // Tests for GetRectsForRange() + Paragraph::RectHeightStyle rect_height_style = + Paragraph::RectHeightStyle::kMax; + Paragraph::RectWidthStyle rect_width_style = + Paragraph::RectWidthStyle::kTight; + paint.setColor(SK_ColorRED); + std::vector boxes = + paragraph->GetRectsForRange(0, 0, rect_height_style, rect_width_style); + for (size_t i = 0; i < boxes.size(); ++i) { + GetCanvas()->drawRect(boxes[i].rect, paint); + } + EXPECT_EQ(boxes.size(), 0ull); + + // First line. Shorter due to disabled height modifications on first ascent. + boxes = + paragraph->GetRectsForRange(0, 3, rect_height_style, rect_width_style); + for (size_t i = 0; i < boxes.size(); ++i) { + GetCanvas()->drawRect(boxes[i].rect, paint); + } + EXPECT_EQ(boxes.size(), 1ull); + EXPECT_FLOAT_EQ(boxes[0].rect.left(), 0); + EXPECT_FLOAT_EQ(boxes[0].rect.right(), 31.117188); + EXPECT_FLOAT_EQ(boxes[0].rect.top(), -0.08203125); + EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 59); + EXPECT_FLOAT_EQ(boxes[0].rect.bottom() - boxes[0].rect.top(), 59.082031); + + // Second line. Normal. + boxes = + paragraph->GetRectsForRange(6, 10, rect_height_style, rect_width_style); + for (size_t i = 0; i < boxes.size(); ++i) { + GetCanvas()->drawRect(boxes[i].rect, paint); + } + EXPECT_EQ(boxes.size(), 1ull); + EXPECT_FLOAT_EQ(boxes[0].rect.left(), 0); + EXPECT_FLOAT_EQ(boxes[0].rect.right(), 47.011719); + EXPECT_FLOAT_EQ(boxes[0].rect.top(), 59); + EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 209); + EXPECT_FLOAT_EQ(boxes[0].rect.bottom() - boxes[0].rect.top(), 150); + + boxes = + paragraph->GetRectsForRange(12, 17, rect_height_style, rect_width_style); + for (size_t i = 0; i < boxes.size(); ++i) { + GetCanvas()->drawRect(boxes[i].rect, paint); + } + EXPECT_EQ(boxes.size(), 1ull); + EXPECT_FLOAT_EQ(boxes[0].rect.left(), 0); + EXPECT_FLOAT_EQ(boxes[0].rect.right(), 63.859375); + EXPECT_FLOAT_EQ(boxes[0].rect.top(), 208.92578); + EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 335); + EXPECT_FLOAT_EQ(boxes[0].rect.bottom() - boxes[0].rect.top(), 126.07422); + + ASSERT_TRUE(Snapshot()); +} + } // namespace txt From 2757cf1da733751bf7ecb4f7e51ae99345b1eeb1 Mon Sep 17 00:00:00 2001 From: GaryQian Date: Tue, 7 Jan 2020 10:00:44 -0800 Subject: [PATCH 06/33] boundaryLineHeight class --- lib/ui/text.dart | 56 +++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 51 insertions(+), 5 deletions(-) diff --git a/lib/ui/text.dart b/lib/ui/text.dart index 40a43f4f8e4f5..57ab98f9e44b5 100644 --- a/lib/ui/text.dart +++ b/lib/ui/text.dart @@ -429,6 +429,52 @@ enum TextDecorationStyle { wavy } +/// Defines how the paragraph will handle the ascent of the first line and +/// descent of the last line. These lines are referred to as "Boundary" lines. +class BoundaryLineHeightBehavior { + + /// Creates a new BoundaryLineHeightBehavior object. + /// + /// * first: When true, the [TextStyle.height] modifier will be applied to + /// to the ascent of the first line. When false, the font's default ascent + /// will be used. + /// * last: When true, the [TextStyle.height] modifier will be applied to + /// to the descent of the last line. When false, the font's default descent + /// will be used. + BoundaryLineHeightBehavior({ + this.first = true, + this.last = true, + }); + + /// Creates a new BoundaryLineHeightBehavior object from an encoded form. + /// + /// See [encode] for the creation of the encoded form. + BoundaryLineHeightBehavior.fromEncoded(int encoded) : first = (encoded & 0x1) > 0, + last = (encoded & 0x2) > 0; + + + /// Whether to apply the [TextStyle.height] modifier or not to the ascent of + /// the first line in the paragraph. + /// + /// When true, the [TextStyle.height] modifier will be applied to to the ascent + /// of the first line. When false, the font's default ascent will be used and + /// the [TextStyle.height] will have no effect on the ascent of the first line. + final bool first; + + /// Whether to apply the [TextStyle.height] modifier or not to the descent of + /// the last line in the paragraph. + /// + /// When true, the [TextStyle.height] modifier will be applied to to the descent + /// of the last line. When false, the font's default descent will be used and + /// the [TextStyle.height] will have no effect on the descent of the last line. + final bool last; + + /// Returns an encoded int representation of this object. + int encode() { + return 0 + (first ? 1 << 0 : 0) + (last ? 1 << 1 : 0); + } +} + /// Determines if lists [a] and [b] are deep equivalent. /// /// Returns true if the lists are both null, or if they are both non-null, have @@ -753,7 +799,7 @@ Int32List _encodeParagraphStyle( String fontFamily, double fontSize, double height, - int boundingLineHeightBehavior, + BoundaryLineHeightBehavior boundaryLineHeightBehavior, FontWeight fontWeight, FontStyle fontStyle, StrutStyle strutStyle, @@ -781,9 +827,9 @@ Int32List _encodeParagraphStyle( result[0] |= 1 << 5; result[5] = maxLines; } - if (boundingLineHeightBehavior != null) { + if (boundaryLineHeightBehavior != null) { result[0] |= 1 << 6; - result[6] = boundingLineHeightBehavior; + result[6] = boundaryLineHeightBehavior.encode(); } if (fontFamily != null) { result[0] |= 1 << 7; @@ -874,7 +920,7 @@ class ParagraphStyle { String fontFamily, double fontSize, double height, - int boundingLineHeightBehavior, + BoundaryLineHeightBehavior boundaryLineHeightBehavior, FontWeight fontWeight, FontStyle fontStyle, StrutStyle strutStyle, @@ -887,7 +933,7 @@ class ParagraphStyle { fontFamily, fontSize, height, - boundingLineHeightBehavior, + boundaryLineHeightBehavior, fontWeight, fontStyle, strutStyle, From 86bf1f20fb8309c4760d9a28f3b7da555b776307 Mon Sep 17 00:00:00 2001 From: GaryQian Date: Tue, 7 Jan 2020 12:28:10 -0800 Subject: [PATCH 07/33] Caps --- lib/ui/text.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ui/text.dart b/lib/ui/text.dart index 57ab98f9e44b5..b5ccf083d5d93 100644 --- a/lib/ui/text.dart +++ b/lib/ui/text.dart @@ -430,7 +430,7 @@ enum TextDecorationStyle { } /// Defines how the paragraph will handle the ascent of the first line and -/// descent of the last line. These lines are referred to as "Boundary" lines. +/// descent of the last line. These lines are referred to as "boundary" lines. class BoundaryLineHeightBehavior { /// Creates a new BoundaryLineHeightBehavior object. From 5ec5618406848106b035b4245970700c2ca25c56 Mon Sep 17 00:00:00 2001 From: GaryQian Date: Tue, 7 Jan 2020 14:15:16 -0800 Subject: [PATCH 08/33] Add BoundaryLineHeightBehavior to web_ui --- lib/ui/text.dart | 19 +++++++++ lib/web_ui/lib/src/ui/text.dart | 71 +++++++++++++++++++++++++++++++-- 2 files changed, 87 insertions(+), 3 deletions(-) diff --git a/lib/ui/text.dart b/lib/ui/text.dart index b5ccf083d5d93..9fb1c8e130d20 100644 --- a/lib/ui/text.dart +++ b/lib/ui/text.dart @@ -473,6 +473,25 @@ class BoundaryLineHeightBehavior { int encode() { return 0 + (first ? 1 << 0 : 0) + (last ? 1 << 1 : 0); } + + @override + bool operator ==(dynamic other) { + if (identical(this, other)) + return true; + if (other.runtimeType != runtimeType) + return false; + return other is BoundaryLineHeightBehavior + && other.first == first + && other.second == second; + } + + @override + int get hashCode { + return hashValues( + first, + last, + ); + } } /// Determines if lists [a] and [b] are deep equivalent. diff --git a/lib/web_ui/lib/src/ui/text.dart b/lib/web_ui/lib/src/ui/text.dart index 022e681d9a8ea..99ed82e35dd20 100644 --- a/lib/web_ui/lib/src/ui/text.dart +++ b/lib/web_ui/lib/src/ui/text.dart @@ -424,6 +424,71 @@ enum TextDecorationStyle { wavy } +/// Defines how the paragraph will handle the ascent of the first line and +/// descent of the last line. These lines are referred to as "boundary" lines. +class BoundaryLineHeightBehavior { + + /// Creates a new BoundaryLineHeightBehavior object. + /// + /// * first: When true, the [TextStyle.height] modifier will be applied to + /// to the ascent of the first line. When false, the font's default ascent + /// will be used. + /// * last: When true, the [TextStyle.height] modifier will be applied to + /// to the descent of the last line. When false, the font's default descent + /// will be used. + BoundaryLineHeightBehavior({ + this.first = true, + this.last = true, + }); + + /// Creates a new BoundaryLineHeightBehavior object from an encoded form. + /// + /// See [encode] for the creation of the encoded form. + BoundaryLineHeightBehavior.fromEncoded(int encoded) : first = (encoded & 0x1) > 0, + last = (encoded & 0x2) > 0; + + + /// Whether to apply the [TextStyle.height] modifier or not to the ascent of + /// the first line in the paragraph. + /// + /// When true, the [TextStyle.height] modifier will be applied to to the ascent + /// of the first line. When false, the font's default ascent will be used and + /// the [TextStyle.height] will have no effect on the ascent of the first line. + final bool first; + + /// Whether to apply the [TextStyle.height] modifier or not to the descent of + /// the last line in the paragraph. + /// + /// When true, the [TextStyle.height] modifier will be applied to to the descent + /// of the last line. When false, the font's default descent will be used and + /// the [TextStyle.height] will have no effect on the descent of the last line. + final bool last; + + /// Returns an encoded int representation of this object. + int encode() { + return 0 + (first ? 1 << 0 : 0) + (last ? 1 << 1 : 0); + } + + @override + bool operator ==(dynamic other) { + if (identical(this, other)) + return true; + if (other.runtimeType != runtimeType) + return false; + return other is BoundaryLineHeightBehavior + && other.first == first + && other.second == second; + } + + @override + int get hashCode { + return hashValues( + first, + last, + ); + } +} + /// An opaque object that determines the size, position, and rendering of text. abstract class TextStyle { /// Creates a new TextStyle object. @@ -584,7 +649,7 @@ abstract class ParagraphStyle { String fontFamily, double fontSize, double height, - int boundingLineHeightBehavior, + BoundaryLineHeightBehavior boundaryLineHeightBehavior, FontWeight fontWeight, FontStyle fontStyle, StrutStyle strutStyle, @@ -599,7 +664,7 @@ abstract class ParagraphStyle { fontFamily: fontFamily, fontSize: fontSize, height: height, - boundingLineHeightBehavior: boundingLineHeightBehavior, + boundaryLineHeightBehavior: boundaryLineHeightBehavior, fontWeight: fontWeight, fontStyle: fontStyle, strutStyle: strutStyle, @@ -614,7 +679,7 @@ abstract class ParagraphStyle { fontFamily: fontFamily, fontSize: fontSize, height: height, - boundingLineHeightBehavior: boundingLineHeightBehavior, + boundaryLineHeightBehavior: boundaryLineHeightBehavior, fontWeight: fontWeight, fontStyle: fontStyle, strutStyle: strutStyle, From 3e7e84658dc27d341f325cf179eda26b21ebf8c5 Mon Sep 17 00:00:00 2001 From: GaryQian Date: Tue, 7 Jan 2020 14:21:38 -0800 Subject: [PATCH 09/33] More web_ui impl --- lib/web_ui/lib/src/engine/compositor/text.dart | 10 +++++----- lib/web_ui/lib/src/engine/text/paragraph.dart | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/web_ui/lib/src/engine/compositor/text.dart b/lib/web_ui/lib/src/engine/compositor/text.dart index ace85a05a1e72..fabb643db690f 100644 --- a/lib/web_ui/lib/src/engine/compositor/text.dart +++ b/lib/web_ui/lib/src/engine/compositor/text.dart @@ -25,7 +25,7 @@ class SkParagraphStyle implements ui.ParagraphStyle { String fontFamily, double fontSize, double height, - int boundingLineHeightBehavior, + ui.BoundaryLineHeightBehavior boundaryLineHeightBehavior, ui.FontWeight fontWeight, ui.FontStyle fontStyle, ui.StrutStyle strutStyle, @@ -39,7 +39,7 @@ class SkParagraphStyle implements ui.ParagraphStyle { fontFamily, fontSize, height, - boundingLineHeightBehavior, + boundaryLineHeightBehavior, fontWeight, fontStyle, ellipsis, @@ -87,7 +87,7 @@ class SkParagraphStyle implements ui.ParagraphStyle { String fontFamily, double fontSize, double height, - int boundingLineHeightBehavior, + ui.BoundaryLineHeightBehavior boundaryLineHeightBehavior, ui.FontWeight fontWeight, ui.FontStyle fontStyle, String ellipsis, @@ -132,8 +132,8 @@ class SkParagraphStyle implements ui.ParagraphStyle { skParagraphStyle['heightMultiplier'] = height; } - if (boundingLineHeightBehavior != null) { - skParagraphStyle['boundingLineHeightBehavior'] = boundingLineHeightBehavior; + if (boundaryLineHeightBehavior != null) { + skParagraphStyle['boundaryLineHeightBehavior'] = boundaryLineHeightBehavior.encode(); } if (maxLines != null) { diff --git a/lib/web_ui/lib/src/engine/text/paragraph.dart b/lib/web_ui/lib/src/engine/text/paragraph.dart index 2c9f1fc579b5e..e5beb6329e8fa 100644 --- a/lib/web_ui/lib/src/engine/text/paragraph.dart +++ b/lib/web_ui/lib/src/engine/text/paragraph.dart @@ -457,7 +457,7 @@ class EngineParagraphStyle implements ui.ParagraphStyle { String fontFamily, double fontSize, double height, - int boundingLineHeightBehavior, + ui.BoundaryLineHeightBehavior boundaryLineHeightBehavior, ui.FontWeight fontWeight, ui.FontStyle fontStyle, ui.StrutStyle strutStyle, @@ -471,7 +471,7 @@ class EngineParagraphStyle implements ui.ParagraphStyle { _fontFamily = fontFamily, _fontSize = fontSize, _height = height, - _boundingLineHeightBehavior = boundingLineHeightBehavior, + _boundaryLineHeightBehavior = boundaryLineHeightBehavior, // TODO(b/128317744): add support for strut style. _strutStyle = strutStyle, _ellipsis = ellipsis, @@ -485,7 +485,7 @@ class EngineParagraphStyle implements ui.ParagraphStyle { final String _fontFamily; final double _fontSize; final double _height; - final int _boundingLineHeightBehavior; + final ui.BoundaryLineHeightBehavior _boundaryLineHeightBehavior; final EngineStrutStyle _strutStyle; final String _ellipsis; final ui.Locale _locale; @@ -539,7 +539,7 @@ class EngineParagraphStyle implements ui.ParagraphStyle { _fontFamily == typedOther._fontFamily || _fontSize == typedOther._fontSize || _height == typedOther._height || - _boundingLineHeightBehavior == typedOther._boundingLineHeightBehavior || + _boundaryLineHeightBehavior == typedOther._boundaryLineHeightBehavior || _ellipsis == typedOther._ellipsis || _locale == typedOther._locale; } @@ -560,7 +560,7 @@ class EngineParagraphStyle implements ui.ParagraphStyle { 'fontFamily: ${_fontFamily ?? "unspecified"}, ' 'fontSize: ${_fontSize != null ? _fontSize.toStringAsFixed(1) : "unspecified"}, ' 'height: ${_height != null ? "${_height.toStringAsFixed(1)}x" : "unspecified"}, ' - 'boundingLineHeightBehavior: ${_boundingLineHeightBehavior ?? "unspecified"}, ' + 'boundaryLineHeightBehavior: ${_boundaryLineHeightBehavior ?? "unspecified"}, ' 'ellipsis: ${_ellipsis != null ? "\"$_ellipsis\"" : "unspecified"}, ' 'locale: ${_locale ?? "unspecified"}' ')'; From b8e6f030a9df86414aae650bd71a4b61a052b3cf Mon Sep 17 00:00:00 2001 From: GaryQian Date: Tue, 7 Jan 2020 14:56:52 -0800 Subject: [PATCH 10/33] toString --- lib/ui/text.dart | 22 ++++++++++++++++------ lib/web_ui/lib/src/ui/text.dart | 10 +++++++++- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/lib/ui/text.dart b/lib/ui/text.dart index 9fb1c8e130d20..42341cc0d842f 100644 --- a/lib/ui/text.dart +++ b/lib/ui/text.dart @@ -482,7 +482,7 @@ class BoundaryLineHeightBehavior { return false; return other is BoundaryLineHeightBehavior && other.first == first - && other.second == second; + && other.last == last; } @override @@ -492,6 +492,14 @@ class BoundaryLineHeightBehavior { last, ); } + + @override + String toString() { + return 'BoundaryLineHeightBehavior(' + 'first: $first, ' + 'last: $last, ' + ')'; + } } /// Determines if lists [a] and [b] are deep equivalent. @@ -1001,11 +1009,13 @@ class ParagraphStyle { 'fontWeight: ${ _encoded[0] & 0x008 == 0x008 ? FontWeight.values[_encoded[3]] : "unspecified"}, ' 'fontStyle: ${ _encoded[0] & 0x010 == 0x010 ? FontStyle.values[_encoded[4]] : "unspecified"}, ' 'maxLines: ${ _encoded[0] & 0x020 == 0x020 ? _encoded[5] : "unspecified"}, ' - 'fontFamily: ${ _encoded[0] & 0x040 == 0x040 ? _fontFamily : "unspecified"}, ' - 'fontSize: ${ _encoded[0] & 0x080 == 0x080 ? _fontSize : "unspecified"}, ' - 'height: ${ _encoded[0] & 0x100 == 0x100 ? "${_height}x" : "unspecified"}, ' - 'ellipsis: ${ _encoded[0] & 0x200 == 0x200 ? "\"$_ellipsis\"" : "unspecified"}, ' - 'locale: ${ _encoded[0] & 0x400 == 0x400 ? _locale : "unspecified"}' + 'boundaryLineHeightBehavior: ${ + _encoded[0] & 0x040 == 0x040 ? _encoded[6] : "unspecified"}, ' + 'fontFamily: ${ _encoded[0] & 0x080 == 0x080 ? _fontFamily : "unspecified"}, ' + 'fontSize: ${ _encoded[0] & 0x100 == 0x100 ? _fontSize : "unspecified"}, ' + 'height: ${ _encoded[0] & 0x200 == 0x200 ? "${_height}x" : "unspecified"}, ' + 'ellipsis: ${ _encoded[0] & 0x400 == 0x400 ? "\"$_ellipsis\"" : "unspecified"}, ' + 'locale: ${ _encoded[0] & 0x800 == 0x800 ? _locale : "unspecified"}' ')'; } } diff --git a/lib/web_ui/lib/src/ui/text.dart b/lib/web_ui/lib/src/ui/text.dart index 99ed82e35dd20..48eb26303226a 100644 --- a/lib/web_ui/lib/src/ui/text.dart +++ b/lib/web_ui/lib/src/ui/text.dart @@ -477,7 +477,7 @@ class BoundaryLineHeightBehavior { return false; return other is BoundaryLineHeightBehavior && other.first == first - && other.second == second; + && other.last == last; } @override @@ -487,6 +487,14 @@ class BoundaryLineHeightBehavior { last, ); } + + @override + String toString() { + return 'BoundaryLineHeightBehavior(' + 'first: $first, ' + 'last: $last, ' + ')'; + } } /// An opaque object that determines the size, position, and rendering of text. From 338516d2f5ad46b6fddb906e00568b786bd0a36b Mon Sep 17 00:00:00 2001 From: GaryQian Date: Tue, 7 Jan 2020 14:58:30 -0800 Subject: [PATCH 11/33] COmments --- third_party/txt/tests/paragraph_unittests.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/third_party/txt/tests/paragraph_unittests.cc b/third_party/txt/tests/paragraph_unittests.cc index 2ab8c93c5e686..4d87881cb483a 100644 --- a/third_party/txt/tests/paragraph_unittests.cc +++ b/third_party/txt/tests/paragraph_unittests.cc @@ -5959,6 +5959,7 @@ TEST_F(ParagraphTest, BoundingLineHeightBehaviorRectsParagraph) { EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 209); EXPECT_FLOAT_EQ(boxes[0].rect.bottom() - boxes[0].rect.top(), 150); + // Third line. Shorter due to disabled height modifications on last descent boxes = paragraph->GetRectsForRange(12, 17, rect_height_style, rect_width_style); for (size_t i = 0; i < boxes.size(); ++i) { From d683d822d65bfc3dcc108394832cf4e8f93821fb Mon Sep 17 00:00:00 2001 From: GaryQian Date: Wed, 8 Jan 2020 10:43:38 -0800 Subject: [PATCH 12/33] Add info on defaults --- lib/ui/text.dart | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/ui/text.dart b/lib/ui/text.dart index 42341cc0d842f..5ce34181ddd84 100644 --- a/lib/ui/text.dart +++ b/lib/ui/text.dart @@ -441,6 +441,8 @@ class BoundaryLineHeightBehavior { /// * last: When true, the [TextStyle.height] modifier will be applied to /// to the descent of the last line. When false, the font's default descent /// will be used. + /// + /// All properties default to true (height modificaitons applied as normal). BoundaryLineHeightBehavior({ this.first = true, this.last = true, @@ -459,6 +461,8 @@ class BoundaryLineHeightBehavior { /// When true, the [TextStyle.height] modifier will be applied to to the ascent /// of the first line. When false, the font's default ascent will be used and /// the [TextStyle.height] will have no effect on the ascent of the first line. + /// + /// Defaults to true (height modificaitons applied as normal). final bool first; /// Whether to apply the [TextStyle.height] modifier or not to the descent of @@ -467,6 +471,8 @@ class BoundaryLineHeightBehavior { /// When true, the [TextStyle.height] modifier will be applied to to the descent /// of the last line. When false, the font's default descent will be used and /// the [TextStyle.height] will have no effect on the descent of the last line. + /// + /// Defaults to true (height modificaitons applied as normal). final bool last; /// Returns an encoded int representation of this object. From 68820815e5e8aa499c85cc7eb23f23ae06560218 Mon Sep 17 00:00:00 2001 From: GaryQian Date: Wed, 8 Jan 2020 13:29:32 -0800 Subject: [PATCH 13/33] Rename first/last to be more descriptive --- lib/ui/text.dart | 64 +++++++++++--------- lib/ui/text/paragraph_builder.cc | 12 ++-- third_party/txt/src/txt/paragraph_style.h | 6 +- third_party/txt/src/txt/paragraph_txt.cc | 11 ++-- third_party/txt/src/txt/paragraph_txt.h | 2 +- third_party/txt/tests/paragraph_unittests.cc | 8 +-- 6 files changed, 56 insertions(+), 47 deletions(-) diff --git a/lib/ui/text.dart b/lib/ui/text.dart index 5ce34181ddd84..9d9190b7cf0c3 100644 --- a/lib/ui/text.dart +++ b/lib/ui/text.dart @@ -429,55 +429,63 @@ enum TextDecorationStyle { wavy } -/// Defines how the paragraph will handle the ascent of the first line and -/// descent of the last line. These lines are referred to as "boundary" lines. +/// {@template flutter.dart:ui.boundaryLineHeightBehavior} +/// Defines how the paragraph will apply [TextStyle.height] the ascent of the +/// first line and descent of the last line. +/// +/// These lines are referred to as "boundary" lines. The boolean value +/// represents whether the [TextStyle.height] modifier will be applied to the +/// corresponding metric. By default, all properties are true, and +/// [TextStyle.height] is applied as normal. When set to false, the font's +/// default ascent will be used. +/// {@endtemplate} class BoundaryLineHeightBehavior { /// Creates a new BoundaryLineHeightBehavior object. /// - /// * first: When true, the [TextStyle.height] modifier will be applied to - /// to the ascent of the first line. When false, the font's default ascent - /// will be used. - /// * last: When true, the [TextStyle.height] modifier will be applied to - /// to the descent of the last line. When false, the font's default descent - /// will be used. + /// * applyHeightToFirstLineAscent: When true, the [TextStyle.height] modifier + /// will be applied to the ascent of the first line. When false, the font's + /// default ascent will be used. + /// * applyHeightToLastLineDescent: When true, the [TextStyle.height] modifier + /// will be applied to the descent of the last line. When false, the font's + /// default descent will be used. /// - /// All properties default to true (height modificaitons applied as normal). + /// All properties default to true (height modifications applied as normal). BoundaryLineHeightBehavior({ - this.first = true, - this.last = true, + this.applyHeightToFirstLineAscent = true, + this.applyHeightToLastLineDescent = true, }); /// Creates a new BoundaryLineHeightBehavior object from an encoded form. /// /// See [encode] for the creation of the encoded form. - BoundaryLineHeightBehavior.fromEncoded(int encoded) : first = (encoded & 0x1) > 0, - last = (encoded & 0x2) > 0; + BoundaryLineHeightBehavior.fromEncoded(int encoded) : applyHeightToFirstLineAscent = (encoded & 0x1) > 0, + applyHeightToLastLineDescent = (encoded & 0x2) > 0; - /// Whether to apply the [TextStyle.height] modifier or not to the ascent of - /// the first line in the paragraph. + /// Whether to apply the [TextStyle.height] modifier to the ascent of the first + /// line in the paragraph. /// /// When true, the [TextStyle.height] modifier will be applied to to the ascent /// of the first line. When false, the font's default ascent will be used and /// the [TextStyle.height] will have no effect on the ascent of the first line. /// - /// Defaults to true (height modificaitons applied as normal). - final bool first; + /// Defaults to true (height modifications applied as normal). + final bool applyHeightToFirstLineAscent; - /// Whether to apply the [TextStyle.height] modifier or not to the descent of - /// the last line in the paragraph. + /// Whether to apply the [TextStyle.height] modifier to the descent of the last + /// line in the paragraph. /// /// When true, the [TextStyle.height] modifier will be applied to to the descent /// of the last line. When false, the font's default descent will be used and /// the [TextStyle.height] will have no effect on the descent of the last line. /// - /// Defaults to true (height modificaitons applied as normal). - final bool last; + /// Defaults to true (height modifications applied as normal). + final bool applyHeightToLastLineDescent; /// Returns an encoded int representation of this object. int encode() { - return 0 + (first ? 1 << 0 : 0) + (last ? 1 << 1 : 0); + return 0 + (applyHeightToFirstLineAscent ? 1 << 0 : 0) + (applyHeightToLastLineDescent ? 1 << 1 : 0); } @override @@ -487,23 +495,23 @@ class BoundaryLineHeightBehavior { if (other.runtimeType != runtimeType) return false; return other is BoundaryLineHeightBehavior - && other.first == first - && other.last == last; + && other.applyHeightToFirstLineAscent == applyHeightToLastLineDescent + && other.applyHeightToLastLineDescent == applyHeightToLastLineDescent; } @override int get hashCode { return hashValues( - first, - last, + applyHeightToFirstLineAscent, + applyHeightToLastLineDescent, ); } @override String toString() { return 'BoundaryLineHeightBehavior(' - 'first: $first, ' - 'last: $last, ' + 'applyHeightToFirstLineAscent: $applyHeightToFirstLineAscent, ' + 'applyHeightToLastLineDescent: $applyHeightToLastLineDescent, ' ')'; } } diff --git a/lib/ui/text/paragraph_builder.cc b/lib/ui/text/paragraph_builder.cc index 1c654a78ba9b2..9b75c8736e4e9 100644 --- a/lib/ui/text/paragraph_builder.cc +++ b/lib/ui/text/paragraph_builder.cc @@ -75,7 +75,7 @@ const int psTextDirectionIndex = 2; const int psFontWeightIndex = 3; const int psFontStyleIndex = 4; const int psMaxLinesIndex = 5; -const int psBoundingLineHeightBehaviorIndex = 6; +const int psBoundaryLineHeightBehaviorIndex = 6; const int psFontFamilyIndex = 7; const int psFontSizeIndex = 8; const int psHeightIndex = 9; @@ -91,8 +91,8 @@ const int psMaxLinesMask = 1 << psMaxLinesIndex; const int psFontFamilyMask = 1 << psFontFamilyIndex; const int psFontSizeMask = 1 << psFontSizeIndex; const int psHeightMask = 1 << psHeightIndex; -const int psBoundingLineHeightBehaviorMask = - 1 << psBoundingLineHeightBehaviorIndex; +const int psBoundaryLineHeightBehaviorMask = + 1 << psBoundaryLineHeightBehaviorIndex; const int psStrutStyleMask = 1 << psStrutStyleIndex; const int psEllipsisMask = 1 << psEllipsisIndex; const int psLocaleMask = 1 << psLocaleIndex; @@ -268,9 +268,9 @@ ParagraphBuilder::ParagraphBuilder( style.has_height_override = true; } - if (mask & psBoundingLineHeightBehaviorMask) { - style.bounding_line_height_behavior = - encoded[psBoundingLineHeightBehaviorIndex]; + if (mask & psBoundaryLineHeightBehaviorMask) { + style.boundary_line_height_behavior = + encoded[psBoundaryLineHeightBehaviorIndex]; } if (mask & psStrutStyleMask) { diff --git a/third_party/txt/src/txt/paragraph_style.h b/third_party/txt/src/txt/paragraph_style.h index c7c1e79f8d45a..abd096412a1bd 100644 --- a/third_party/txt/src/txt/paragraph_style.h +++ b/third_party/txt/src/txt/paragraph_style.h @@ -56,8 +56,8 @@ enum class TextDirection { enum BoundaryLineHeightBehavior { kDisableAll = 0x0, kAll = 0x1 | 0x2, - kDisableFirst = 0x2, - kDisableLast = 0x1, + kDisableFirstAscent = 0x2, + kDisableLastDescent = 0x1, }; class ParagraphStyle { @@ -69,7 +69,7 @@ class ParagraphStyle { std::string font_family = ""; double font_size = 14; double height = 1; - size_t bounding_line_height_behavior = BoundaryLineHeightBehavior::kAll; + size_t boundary_line_height_behavior = BoundaryLineHeightBehavior::kAll; bool has_height_override = false; // Strut properties. strut_enabled must be set to true for the rest of the diff --git a/third_party/txt/src/txt/paragraph_txt.cc b/third_party/txt/src/txt/paragraph_txt.cc index df1e1eaa8ca67..24bfb5abc8d10 100644 --- a/third_party/txt/src/txt/paragraph_txt.cc +++ b/third_party/txt/src/txt/paragraph_txt.cc @@ -1246,17 +1246,18 @@ void ParagraphTxt::UpdateLineMetrics(const SkFontMetrics& metrics, descent = (metrics.fDescent + metrics.fLeading / 2); } - // Account for bounding_line_height_behavior in parargaph_style_. + // Account for boundary_line_height_behavior in parargaph_style_. // // Disable first line ascent modifications. - if (line_number == 0 && !(paragraph_style_.bounding_line_height_behavior & - ~BoundaryLineHeightBehavior::kDisableFirst)) { + if (line_number == 0 && + !(paragraph_style_.boundary_line_height_behavior & + ~BoundaryLineHeightBehavior::kDisableFirstAscent)) { ascent = -metrics.fAscent; } // Disable last line descent modifications. if (line_number == line_limit - 1 && - !(paragraph_style_.bounding_line_height_behavior & - ~BoundaryLineHeightBehavior::kDisableLast)) { + !(paragraph_style_.boundary_line_height_behavior & + ~BoundaryLineHeightBehavior::kDisableLastDescent)) { descent = metrics.fDescent; } diff --git a/third_party/txt/src/txt/paragraph_txt.h b/third_party/txt/src/txt/paragraph_txt.h index 4eea57bb489c3..25d5e3eaf003a 100644 --- a/third_party/txt/src/txt/paragraph_txt.h +++ b/third_party/txt/src/txt/paragraph_txt.h @@ -162,7 +162,7 @@ class ParagraphTxt : public Paragraph { FRIEND_TEST(ParagraphTest, FontFeaturesParagraph); FRIEND_TEST(ParagraphTest, GetGlyphPositionAtCoordinateSegfault); FRIEND_TEST(ParagraphTest, KhmerLineBreaker); - FRIEND_TEST(ParagraphTest, BoundingLineHeightBehaviorRectsParagraph); + FRIEND_TEST(ParagraphTest, BoundaryLineHeightBehaviorRectsParagraph); // Starting data to layout. std::vector text_; diff --git a/third_party/txt/tests/paragraph_unittests.cc b/third_party/txt/tests/paragraph_unittests.cc index 4d87881cb483a..6ee8d3475fa50 100644 --- a/third_party/txt/tests/paragraph_unittests.cc +++ b/third_party/txt/tests/paragraph_unittests.cc @@ -5877,7 +5877,7 @@ TEST_F(ParagraphTest, KhmerLineBreaker) { ASSERT_TRUE(Snapshot()); } -TEST_F(ParagraphTest, BoundingLineHeightBehaviorRectsParagraph) { +TEST_F(ParagraphTest, BoundaryLineHeightBehaviorRectsParagraph) { // clang-format off const char* text = "line1\nline2\nline3"; @@ -5887,9 +5887,9 @@ TEST_F(ParagraphTest, BoundingLineHeightBehaviorRectsParagraph) { icu_text.getBuffer() + icu_text.length()); txt::ParagraphStyle paragraph_style; - paragraph_style.bounding_line_height_behavior = - txt::BoundaryLineHeightBehavior::kDisableFirst & - txt::BoundaryLineHeightBehavior::kDisableLast; + paragraph_style.boundary_line_height_behavior = + txt::BoundaryLineHeightBehavior::kDisableFirstAscent & + txt::BoundaryLineHeightBehavior::kDisableLastDescent; txt::ParagraphBuilderTxt builder(paragraph_style, GetTestFontCollection()); From ba29ea34a79670901914a9ed9d356f451cb3d138 Mon Sep 17 00:00:00 2001 From: GaryQian Date: Wed, 8 Jan 2020 14:09:11 -0800 Subject: [PATCH 14/33] Rename BoundaryLineHeightBehavior -> HeightBehavior, address comments --- lib/ui/text.dart | 44 +++++++++++-------- lib/ui/text/paragraph_builder.cc | 10 ++--- .../lib/src/engine/compositor/text.dart | 10 ++--- lib/web_ui/lib/src/engine/text/paragraph.dart | 10 ++--- lib/web_ui/lib/src/ui/text.dart | 20 ++++----- third_party/txt/src/txt/paragraph_style.h | 4 +- third_party/txt/src/txt/paragraph_txt.cc | 11 +++-- third_party/txt/src/txt/paragraph_txt.h | 2 +- third_party/txt/tests/paragraph_unittests.cc | 8 ++-- 9 files changed, 62 insertions(+), 57 deletions(-) diff --git a/lib/ui/text.dart b/lib/ui/text.dart index 9d9190b7cf0c3..c51858fba4d69 100644 --- a/lib/ui/text.dart +++ b/lib/ui/text.dart @@ -429,7 +429,7 @@ enum TextDecorationStyle { wavy } -/// {@template flutter.dart:ui.boundaryLineHeightBehavior} +/// {@template flutter.dart:ui.heightBehavior} /// Defines how the paragraph will apply [TextStyle.height] the ascent of the /// first line and descent of the last line. /// @@ -439,9 +439,9 @@ enum TextDecorationStyle { /// [TextStyle.height] is applied as normal. When set to false, the font's /// default ascent will be used. /// {@endtemplate} -class BoundaryLineHeightBehavior { +class HeightBehavior { - /// Creates a new BoundaryLineHeightBehavior object. + /// Creates a new HeightBehavior object. /// /// * applyHeightToFirstLineAscent: When true, the [TextStyle.height] modifier /// will be applied to the ascent of the first line. When false, the font's @@ -451,15 +451,15 @@ class BoundaryLineHeightBehavior { /// default descent will be used. /// /// All properties default to true (height modifications applied as normal). - BoundaryLineHeightBehavior({ + HeightBehavior({ this.applyHeightToFirstLineAscent = true, this.applyHeightToLastLineDescent = true, }); - /// Creates a new BoundaryLineHeightBehavior object from an encoded form. + /// Creates a new HeightBehavior object from an encoded form. /// /// See [encode] for the creation of the encoded form. - BoundaryLineHeightBehavior.fromEncoded(int encoded) : applyHeightToFirstLineAscent = (encoded & 0x1) > 0, + HeightBehavior.fromEncoded(int encoded) : applyHeightToFirstLineAscent = (encoded & 0x1) > 0, applyHeightToLastLineDescent = (encoded & 0x2) > 0; @@ -470,6 +470,8 @@ class BoundaryLineHeightBehavior { /// of the first line. When false, the font's default ascent will be used and /// the [TextStyle.height] will have no effect on the ascent of the first line. /// + /// This property only has effect if a non-null [TextStyle.height] is specified. + /// /// Defaults to true (height modifications applied as normal). final bool applyHeightToFirstLineAscent; @@ -480,6 +482,8 @@ class BoundaryLineHeightBehavior { /// of the last line. When false, the font's default descent will be used and /// the [TextStyle.height] will have no effect on the descent of the last line. /// + /// This property only has effect if a non-null [TextStyle.height] is specified. + /// /// Defaults to true (height modifications applied as normal). final bool applyHeightToLastLineDescent; @@ -490,11 +494,9 @@ class BoundaryLineHeightBehavior { @override bool operator ==(dynamic other) { - if (identical(this, other)) - return true; if (other.runtimeType != runtimeType) return false; - return other is BoundaryLineHeightBehavior + return other is HeightBehavior && other.applyHeightToFirstLineAscent == applyHeightToLastLineDescent && other.applyHeightToLastLineDescent == applyHeightToLastLineDescent; } @@ -509,9 +511,9 @@ class BoundaryLineHeightBehavior { @override String toString() { - return 'BoundaryLineHeightBehavior(' + return 'HeightBehavior(' 'applyHeightToFirstLineAscent: $applyHeightToFirstLineAscent, ' - 'applyHeightToLastLineDescent: $applyHeightToLastLineDescent, ' + 'applyHeightToLastLineDescent: $applyHeightToLastLineDescent ' ')'; } } @@ -833,6 +835,8 @@ class TextStyle { // // - Element 5: The value of |maxLines|. // +// - Element 6: The encoded value of |heightBehavior|. +// Int32List _encodeParagraphStyle( TextAlign textAlign, TextDirection textDirection, @@ -840,7 +844,7 @@ Int32List _encodeParagraphStyle( String fontFamily, double fontSize, double height, - BoundaryLineHeightBehavior boundaryLineHeightBehavior, + HeightBehavior heightBehavior, FontWeight fontWeight, FontStyle fontStyle, StrutStyle strutStyle, @@ -868,9 +872,9 @@ Int32List _encodeParagraphStyle( result[0] |= 1 << 5; result[5] = maxLines; } - if (boundaryLineHeightBehavior != null) { + if (heightBehavior != null) { result[0] |= 1 << 6; - result[6] = boundaryLineHeightBehavior.encode(); + result[6] = heightBehavior.encode(); } if (fontFamily != null) { result[0] |= 1 << 7; @@ -934,6 +938,9 @@ class ParagraphStyle { /// the line height to take the height as defined by the font, which may not /// be exactly the height of the `fontSize`. /// + /// * `heightBehavior`: Specifies how the `height` multiplier is + /// applied to ascent of the first line and the descent of the last line. + /// /// * `fontWeight`: The typeface thickness to use when painting the text /// (e.g., bold). /// @@ -961,7 +968,7 @@ class ParagraphStyle { String fontFamily, double fontSize, double height, - BoundaryLineHeightBehavior boundaryLineHeightBehavior, + HeightBehavior heightBehavior, FontWeight fontWeight, FontStyle fontStyle, StrutStyle strutStyle, @@ -974,7 +981,7 @@ class ParagraphStyle { fontFamily, fontSize, height, - boundaryLineHeightBehavior, + heightBehavior, fontWeight, fontStyle, strutStyle, @@ -1023,8 +1030,9 @@ class ParagraphStyle { 'fontWeight: ${ _encoded[0] & 0x008 == 0x008 ? FontWeight.values[_encoded[3]] : "unspecified"}, ' 'fontStyle: ${ _encoded[0] & 0x010 == 0x010 ? FontStyle.values[_encoded[4]] : "unspecified"}, ' 'maxLines: ${ _encoded[0] & 0x020 == 0x020 ? _encoded[5] : "unspecified"}, ' - 'boundaryLineHeightBehavior: ${ - _encoded[0] & 0x040 == 0x040 ? _encoded[6] : "unspecified"}, ' + 'heightBehavior: ${ + _encoded[0] & 0x040 == 0x040 ? + HeightBehavior.fromEncoded(_encoded[6]).toString() : "unspecified"}, ' 'fontFamily: ${ _encoded[0] & 0x080 == 0x080 ? _fontFamily : "unspecified"}, ' 'fontSize: ${ _encoded[0] & 0x100 == 0x100 ? _fontSize : "unspecified"}, ' 'height: ${ _encoded[0] & 0x200 == 0x200 ? "${_height}x" : "unspecified"}, ' diff --git a/lib/ui/text/paragraph_builder.cc b/lib/ui/text/paragraph_builder.cc index 9b75c8736e4e9..a8741095b832f 100644 --- a/lib/ui/text/paragraph_builder.cc +++ b/lib/ui/text/paragraph_builder.cc @@ -75,7 +75,7 @@ const int psTextDirectionIndex = 2; const int psFontWeightIndex = 3; const int psFontStyleIndex = 4; const int psMaxLinesIndex = 5; -const int psBoundaryLineHeightBehaviorIndex = 6; +const int psHeightBehaviorIndex = 6; const int psFontFamilyIndex = 7; const int psFontSizeIndex = 8; const int psHeightIndex = 9; @@ -91,8 +91,7 @@ const int psMaxLinesMask = 1 << psMaxLinesIndex; const int psFontFamilyMask = 1 << psFontFamilyIndex; const int psFontSizeMask = 1 << psFontSizeIndex; const int psHeightMask = 1 << psHeightIndex; -const int psBoundaryLineHeightBehaviorMask = - 1 << psBoundaryLineHeightBehaviorIndex; +const int psHeightBehaviorMask = 1 << psHeightBehaviorIndex; const int psStrutStyleMask = 1 << psStrutStyleIndex; const int psEllipsisMask = 1 << psEllipsisIndex; const int psLocaleMask = 1 << psLocaleIndex; @@ -268,9 +267,8 @@ ParagraphBuilder::ParagraphBuilder( style.has_height_override = true; } - if (mask & psBoundaryLineHeightBehaviorMask) { - style.boundary_line_height_behavior = - encoded[psBoundaryLineHeightBehaviorIndex]; + if (mask & psHeightBehaviorMask) { + style.height_behavior = encoded[psHeightBehaviorIndex]; } if (mask & psStrutStyleMask) { diff --git a/lib/web_ui/lib/src/engine/compositor/text.dart b/lib/web_ui/lib/src/engine/compositor/text.dart index fabb643db690f..de373dc31b25a 100644 --- a/lib/web_ui/lib/src/engine/compositor/text.dart +++ b/lib/web_ui/lib/src/engine/compositor/text.dart @@ -25,7 +25,7 @@ class SkParagraphStyle implements ui.ParagraphStyle { String fontFamily, double fontSize, double height, - ui.BoundaryLineHeightBehavior boundaryLineHeightBehavior, + ui.HeightBehavior heightBehavior, ui.FontWeight fontWeight, ui.FontStyle fontStyle, ui.StrutStyle strutStyle, @@ -39,7 +39,7 @@ class SkParagraphStyle implements ui.ParagraphStyle { fontFamily, fontSize, height, - boundaryLineHeightBehavior, + heightBehavior, fontWeight, fontStyle, ellipsis, @@ -87,7 +87,7 @@ class SkParagraphStyle implements ui.ParagraphStyle { String fontFamily, double fontSize, double height, - ui.BoundaryLineHeightBehavior boundaryLineHeightBehavior, + ui.HeightBehavior heightBehavior, ui.FontWeight fontWeight, ui.FontStyle fontStyle, String ellipsis, @@ -132,8 +132,8 @@ class SkParagraphStyle implements ui.ParagraphStyle { skParagraphStyle['heightMultiplier'] = height; } - if (boundaryLineHeightBehavior != null) { - skParagraphStyle['boundaryLineHeightBehavior'] = boundaryLineHeightBehavior.encode(); + if (heightBehavior != null) { + skParagraphStyle['heightBehavior'] = heightBehavior.encode(); } if (maxLines != null) { diff --git a/lib/web_ui/lib/src/engine/text/paragraph.dart b/lib/web_ui/lib/src/engine/text/paragraph.dart index e5beb6329e8fa..12ab5622968a0 100644 --- a/lib/web_ui/lib/src/engine/text/paragraph.dart +++ b/lib/web_ui/lib/src/engine/text/paragraph.dart @@ -457,7 +457,7 @@ class EngineParagraphStyle implements ui.ParagraphStyle { String fontFamily, double fontSize, double height, - ui.BoundaryLineHeightBehavior boundaryLineHeightBehavior, + ui.HeightBehavior heightBehavior, ui.FontWeight fontWeight, ui.FontStyle fontStyle, ui.StrutStyle strutStyle, @@ -471,7 +471,7 @@ class EngineParagraphStyle implements ui.ParagraphStyle { _fontFamily = fontFamily, _fontSize = fontSize, _height = height, - _boundaryLineHeightBehavior = boundaryLineHeightBehavior, + _heightBehavior = heightBehavior, // TODO(b/128317744): add support for strut style. _strutStyle = strutStyle, _ellipsis = ellipsis, @@ -485,7 +485,7 @@ class EngineParagraphStyle implements ui.ParagraphStyle { final String _fontFamily; final double _fontSize; final double _height; - final ui.BoundaryLineHeightBehavior _boundaryLineHeightBehavior; + final ui.HeightBehavior _heightBehavior; final EngineStrutStyle _strutStyle; final String _ellipsis; final ui.Locale _locale; @@ -539,7 +539,7 @@ class EngineParagraphStyle implements ui.ParagraphStyle { _fontFamily == typedOther._fontFamily || _fontSize == typedOther._fontSize || _height == typedOther._height || - _boundaryLineHeightBehavior == typedOther._boundaryLineHeightBehavior || + _heightBehavior == typedOther._heightBehavior || _ellipsis == typedOther._ellipsis || _locale == typedOther._locale; } @@ -560,7 +560,7 @@ class EngineParagraphStyle implements ui.ParagraphStyle { 'fontFamily: ${_fontFamily ?? "unspecified"}, ' 'fontSize: ${_fontSize != null ? _fontSize.toStringAsFixed(1) : "unspecified"}, ' 'height: ${_height != null ? "${_height.toStringAsFixed(1)}x" : "unspecified"}, ' - 'boundaryLineHeightBehavior: ${_boundaryLineHeightBehavior ?? "unspecified"}, ' + 'heightBehavior: ${_heightBehavior ?? "unspecified"}, ' 'ellipsis: ${_ellipsis != null ? "\"$_ellipsis\"" : "unspecified"}, ' 'locale: ${_locale ?? "unspecified"}' ')'; diff --git a/lib/web_ui/lib/src/ui/text.dart b/lib/web_ui/lib/src/ui/text.dart index 48eb26303226a..6fef7b21b5b3d 100644 --- a/lib/web_ui/lib/src/ui/text.dart +++ b/lib/web_ui/lib/src/ui/text.dart @@ -426,9 +426,9 @@ enum TextDecorationStyle { /// Defines how the paragraph will handle the ascent of the first line and /// descent of the last line. These lines are referred to as "boundary" lines. -class BoundaryLineHeightBehavior { +class HeightBehavior { - /// Creates a new BoundaryLineHeightBehavior object. + /// Creates a new HeightBehavior object. /// /// * first: When true, the [TextStyle.height] modifier will be applied to /// to the ascent of the first line. When false, the font's default ascent @@ -436,15 +436,15 @@ class BoundaryLineHeightBehavior { /// * last: When true, the [TextStyle.height] modifier will be applied to /// to the descent of the last line. When false, the font's default descent /// will be used. - BoundaryLineHeightBehavior({ + HeightBehavior({ this.first = true, this.last = true, }); - /// Creates a new BoundaryLineHeightBehavior object from an encoded form. + /// Creates a new HeightBehavior object from an encoded form. /// /// See [encode] for the creation of the encoded form. - BoundaryLineHeightBehavior.fromEncoded(int encoded) : first = (encoded & 0x1) > 0, + HeightBehavior.fromEncoded(int encoded) : first = (encoded & 0x1) > 0, last = (encoded & 0x2) > 0; @@ -475,7 +475,7 @@ class BoundaryLineHeightBehavior { return true; if (other.runtimeType != runtimeType) return false; - return other is BoundaryLineHeightBehavior + return other is HeightBehavior && other.first == first && other.last == last; } @@ -490,7 +490,7 @@ class BoundaryLineHeightBehavior { @override String toString() { - return 'BoundaryLineHeightBehavior(' + return 'HeightBehavior(' 'first: $first, ' 'last: $last, ' ')'; @@ -657,7 +657,7 @@ abstract class ParagraphStyle { String fontFamily, double fontSize, double height, - BoundaryLineHeightBehavior boundaryLineHeightBehavior, + HeightBehavior heightBehavior, FontWeight fontWeight, FontStyle fontStyle, StrutStyle strutStyle, @@ -672,7 +672,7 @@ abstract class ParagraphStyle { fontFamily: fontFamily, fontSize: fontSize, height: height, - boundaryLineHeightBehavior: boundaryLineHeightBehavior, + heightBehavior: heightBehavior, fontWeight: fontWeight, fontStyle: fontStyle, strutStyle: strutStyle, @@ -687,7 +687,7 @@ abstract class ParagraphStyle { fontFamily: fontFamily, fontSize: fontSize, height: height, - boundaryLineHeightBehavior: boundaryLineHeightBehavior, + heightBehavior: heightBehavior, fontWeight: fontWeight, fontStyle: fontStyle, strutStyle: strutStyle, diff --git a/third_party/txt/src/txt/paragraph_style.h b/third_party/txt/src/txt/paragraph_style.h index abd096412a1bd..dd3ccf9249276 100644 --- a/third_party/txt/src/txt/paragraph_style.h +++ b/third_party/txt/src/txt/paragraph_style.h @@ -53,7 +53,7 @@ enum class TextDirection { // example, disabling first and last can achieved with: // // (kDisableFirst & kDisableLast). -enum BoundaryLineHeightBehavior { +enum HeightBehavior { kDisableAll = 0x0, kAll = 0x1 | 0x2, kDisableFirstAscent = 0x2, @@ -69,7 +69,7 @@ class ParagraphStyle { std::string font_family = ""; double font_size = 14; double height = 1; - size_t boundary_line_height_behavior = BoundaryLineHeightBehavior::kAll; + size_t height_behavior = HeightBehavior::kAll; bool has_height_override = false; // Strut properties. strut_enabled must be set to true for the rest of the diff --git a/third_party/txt/src/txt/paragraph_txt.cc b/third_party/txt/src/txt/paragraph_txt.cc index 24bfb5abc8d10..f13f9eb51ef51 100644 --- a/third_party/txt/src/txt/paragraph_txt.cc +++ b/third_party/txt/src/txt/paragraph_txt.cc @@ -1246,18 +1246,17 @@ void ParagraphTxt::UpdateLineMetrics(const SkFontMetrics& metrics, descent = (metrics.fDescent + metrics.fLeading / 2); } - // Account for boundary_line_height_behavior in parargaph_style_. + // Account for height_behavior in parargaph_style_. // // Disable first line ascent modifications. - if (line_number == 0 && - !(paragraph_style_.boundary_line_height_behavior & - ~BoundaryLineHeightBehavior::kDisableFirstAscent)) { + if (line_number == 0 && !(paragraph_style_.height_behavior & + ~HeightBehavior::kDisableFirstAscent)) { ascent = -metrics.fAscent; } // Disable last line descent modifications. if (line_number == line_limit - 1 && - !(paragraph_style_.boundary_line_height_behavior & - ~BoundaryLineHeightBehavior::kDisableLastDescent)) { + !(paragraph_style_.height_behavior & + ~HeightBehavior::kDisableLastDescent)) { descent = metrics.fDescent; } diff --git a/third_party/txt/src/txt/paragraph_txt.h b/third_party/txt/src/txt/paragraph_txt.h index 25d5e3eaf003a..2b5fbd39c9203 100644 --- a/third_party/txt/src/txt/paragraph_txt.h +++ b/third_party/txt/src/txt/paragraph_txt.h @@ -162,7 +162,7 @@ class ParagraphTxt : public Paragraph { FRIEND_TEST(ParagraphTest, FontFeaturesParagraph); FRIEND_TEST(ParagraphTest, GetGlyphPositionAtCoordinateSegfault); FRIEND_TEST(ParagraphTest, KhmerLineBreaker); - FRIEND_TEST(ParagraphTest, BoundaryLineHeightBehaviorRectsParagraph); + FRIEND_TEST(ParagraphTest, HeightBehaviorRectsParagraph); // Starting data to layout. std::vector text_; diff --git a/third_party/txt/tests/paragraph_unittests.cc b/third_party/txt/tests/paragraph_unittests.cc index 6ee8d3475fa50..debde2aa08113 100644 --- a/third_party/txt/tests/paragraph_unittests.cc +++ b/third_party/txt/tests/paragraph_unittests.cc @@ -5877,7 +5877,7 @@ TEST_F(ParagraphTest, KhmerLineBreaker) { ASSERT_TRUE(Snapshot()); } -TEST_F(ParagraphTest, BoundaryLineHeightBehaviorRectsParagraph) { +TEST_F(ParagraphTest, HeightBehaviorRectsParagraph) { // clang-format off const char* text = "line1\nline2\nline3"; @@ -5887,9 +5887,9 @@ TEST_F(ParagraphTest, BoundaryLineHeightBehaviorRectsParagraph) { icu_text.getBuffer() + icu_text.length()); txt::ParagraphStyle paragraph_style; - paragraph_style.boundary_line_height_behavior = - txt::BoundaryLineHeightBehavior::kDisableFirstAscent & - txt::BoundaryLineHeightBehavior::kDisableLastDescent; + paragraph_style.height_behavior = + txt::HeightBehavior::kDisableFirstAscent & + txt::HeightBehavior::kDisableLastDescent; txt::ParagraphBuilderTxt builder(paragraph_style, GetTestFontCollection()); From 8423170d10b908ee47c1f9c0c3bd117ab26b407f Mon Sep 17 00:00:00 2001 From: GaryQian Date: Wed, 8 Jan 2020 14:44:08 -0800 Subject: [PATCH 15/33] Add unit tests for HeightBehavior --- testing/dart/text_test.dart | 47 +++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/testing/dart/text_test.dart b/testing/dart/text_test.dart index df1f9a8384af3..39759175fc790 100644 --- a/testing/dart/text_test.dart +++ b/testing/dart/text_test.dart @@ -26,6 +26,53 @@ void main() { expect(FontWeight.lerp(FontWeight.w400, null, 1), equals(FontWeight.w400)); }); }); + + group('HeightBehavior', () { + HeightBehavior behavior0 = HeightBehavior(); + HeightBehavior behavior1 = HeightBehavior( + applyHeightToFirstAscent: false, + applyHeightToLastDescent: false + ); + HeightBehavior behavior2 = HeightBehavior( + applyHeightToFirstAscent: false, + ); + HeightBehavior behavior3 = HeightBehavior( + applyHeightToLastDescent: false + ); + + test('default constructor works', () { + expect(behavior0.applyHeightToFirstAscent, equals(true)); + expect(behavior0.applyHeightToLastDescent, equals(true)); + + expect(behavior1.applyHeightToFirstAscent, equals(false)); + expect(behavior1.applyHeightToLastDescent, equals(false)); + + expect(behavior2.applyHeightToFirstAscent, equals(true)); + expect(behavior2.applyHeightToLastDescent, equals(false)); + }); + + test('encode works', () { + expect(behavior0.encode(), equals(3)); + expect(behavior1.encode(), equals(0)); + expect(behavior2.encode(), equals(2)); + expect(behavior3.encode(), equals(1)); + }); + + test('encode works', () { + expect(HeightBehavior.fromEncoded(3), equals(behavior0)); + expect(HeightBehavior.fromEncoded(0), equals(behavior1)); + expect(HeightBehavior.fromEncoded(2), equals(behavior2)); + expect(HeightBehavior.fromEncoded(1), equals(behavior3)); + }); + + test('toString works', () { + expect(behavior0.toString(), equals('HeightBehavior(applyHeightToFirstAscent: true, applyHeightToLastDescent: true)')); + expect(behavior1.toString(), equals('HeightBehavior(applyHeightToFirstAscent: false, applyHeightToLastDescent: false)')); + expect(behavior2.toString(), equals('HeightBehavior(applyHeightToFirstAscent: false, applyHeightToLastDescent: true)')); + expect(behavior3.toString(), equals('HeightBehavior(applyHeightToFirstAscent: true, applyHeightToLastDescent: false)')); + }); + }); + group('TextRange', () { test('empty ranges are correct', () { const TextRange range = TextRange(start: -1, end: -1); From 0c4c3abc73b9b69606b78c9408e18429a43ddbb0 Mon Sep 17 00:00:00 2001 From: GaryQian Date: Wed, 8 Jan 2020 15:11:16 -0800 Subject: [PATCH 16/33] simpler property naming --- lib/ui/text.dart | 37 ++++++++++---------- third_party/txt/tests/paragraph_unittests.cc | 5 ++- 2 files changed, 20 insertions(+), 22 deletions(-) diff --git a/lib/ui/text.dart b/lib/ui/text.dart index c51858fba4d69..1099a59ee0b5d 100644 --- a/lib/ui/text.dart +++ b/lib/ui/text.dart @@ -433,34 +433,33 @@ enum TextDecorationStyle { /// Defines how the paragraph will apply [TextStyle.height] the ascent of the /// first line and descent of the last line. /// -/// These lines are referred to as "boundary" lines. The boolean value -/// represents whether the [TextStyle.height] modifier will be applied to the -/// corresponding metric. By default, all properties are true, and -/// [TextStyle.height] is applied as normal. When set to false, the font's +/// The boolean value represents whether the [TextStyle.height] modifier will +/// be applied to the corresponding metric. By default, all properties are true, +/// and [TextStyle.height] is applied as normal. When set to false, the font's /// default ascent will be used. /// {@endtemplate} class HeightBehavior { /// Creates a new HeightBehavior object. /// - /// * applyHeightToFirstLineAscent: When true, the [TextStyle.height] modifier + /// * applyHeightToFirstAscent: When true, the [TextStyle.height] modifier /// will be applied to the ascent of the first line. When false, the font's /// default ascent will be used. - /// * applyHeightToLastLineDescent: When true, the [TextStyle.height] modifier + /// * applyHeightToLastDescent: When true, the [TextStyle.height] modifier /// will be applied to the descent of the last line. When false, the font's /// default descent will be used. /// /// All properties default to true (height modifications applied as normal). HeightBehavior({ - this.applyHeightToFirstLineAscent = true, - this.applyHeightToLastLineDescent = true, + this.applyHeightToFirstAscent = true, + this.applyHeightToLastDescent = true, }); /// Creates a new HeightBehavior object from an encoded form. /// /// See [encode] for the creation of the encoded form. - HeightBehavior.fromEncoded(int encoded) : applyHeightToFirstLineAscent = (encoded & 0x1) > 0, - applyHeightToLastLineDescent = (encoded & 0x2) > 0; + HeightBehavior.fromEncoded(int encoded) : applyHeightToFirstAscent = (encoded & 0x1) > 0, + applyHeightToLastDescent = (encoded & 0x2) > 0; /// Whether to apply the [TextStyle.height] modifier to the ascent of the first @@ -473,7 +472,7 @@ class HeightBehavior { /// This property only has effect if a non-null [TextStyle.height] is specified. /// /// Defaults to true (height modifications applied as normal). - final bool applyHeightToFirstLineAscent; + final bool applyHeightToFirstAscent; /// Whether to apply the [TextStyle.height] modifier to the descent of the last /// line in the paragraph. @@ -485,11 +484,11 @@ class HeightBehavior { /// This property only has effect if a non-null [TextStyle.height] is specified. /// /// Defaults to true (height modifications applied as normal). - final bool applyHeightToLastLineDescent; + final bool applyHeightToLastDescent; /// Returns an encoded int representation of this object. int encode() { - return 0 + (applyHeightToFirstLineAscent ? 1 << 0 : 0) + (applyHeightToLastLineDescent ? 1 << 1 : 0); + return 0 + (applyHeightToFirstAscent ? 1 << 0 : 0) + (applyHeightToLastDescent ? 1 << 1 : 0); } @override @@ -497,23 +496,23 @@ class HeightBehavior { if (other.runtimeType != runtimeType) return false; return other is HeightBehavior - && other.applyHeightToFirstLineAscent == applyHeightToLastLineDescent - && other.applyHeightToLastLineDescent == applyHeightToLastLineDescent; + && other.applyHeightToFirstAscent == applyHeightToLastDescent + && other.applyHeightToLastDescent == applyHeightToLastDescent; } @override int get hashCode { return hashValues( - applyHeightToFirstLineAscent, - applyHeightToLastLineDescent, + applyHeightToFirstAscent, + applyHeightToLastDescent, ); } @override String toString() { return 'HeightBehavior(' - 'applyHeightToFirstLineAscent: $applyHeightToFirstLineAscent, ' - 'applyHeightToLastLineDescent: $applyHeightToLastLineDescent ' + 'applyHeightToFirstAscent: $applyHeightToFirstAscent, ' + 'applyHeightToLastDescent: $applyHeightToLastDescent ' ')'; } } diff --git a/third_party/txt/tests/paragraph_unittests.cc b/third_party/txt/tests/paragraph_unittests.cc index debde2aa08113..dfbca75bca8ca 100644 --- a/third_party/txt/tests/paragraph_unittests.cc +++ b/third_party/txt/tests/paragraph_unittests.cc @@ -5887,9 +5887,8 @@ TEST_F(ParagraphTest, HeightBehaviorRectsParagraph) { icu_text.getBuffer() + icu_text.length()); txt::ParagraphStyle paragraph_style; - paragraph_style.height_behavior = - txt::HeightBehavior::kDisableFirstAscent & - txt::HeightBehavior::kDisableLastDescent; + paragraph_style.height_behavior = txt::HeightBehavior::kDisableFirstAscent & + txt::HeightBehavior::kDisableLastDescent; txt::ParagraphBuilderTxt builder(paragraph_style, GetTestFontCollection()); From 8f2b9eba9b46c3b9acf1a99363c2175a901cbebc Mon Sep 17 00:00:00 2001 From: GaryQian Date: Wed, 8 Jan 2020 17:04:19 -0800 Subject: [PATCH 17/33] Fix tests, feed linter --- lib/ui/text.dart | 2 +- testing/dart/text_test.dart | 15 +++++++++------ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/ui/text.dart b/lib/ui/text.dart index 1099a59ee0b5d..ac6bfc2153ea7 100644 --- a/lib/ui/text.dart +++ b/lib/ui/text.dart @@ -512,7 +512,7 @@ class HeightBehavior { String toString() { return 'HeightBehavior(' 'applyHeightToFirstAscent: $applyHeightToFirstAscent, ' - 'applyHeightToLastDescent: $applyHeightToLastDescent ' + 'applyHeightToLastDescent: $applyHeightToLastDescent' ')'; } } diff --git a/testing/dart/text_test.dart b/testing/dart/text_test.dart index 39759175fc790..431fb76a2cebf 100644 --- a/testing/dart/text_test.dart +++ b/testing/dart/text_test.dart @@ -28,15 +28,15 @@ void main() { }); group('HeightBehavior', () { - HeightBehavior behavior0 = HeightBehavior(); - HeightBehavior behavior1 = HeightBehavior( + final HeightBehavior behavior0 = HeightBehavior(); + final HeightBehavior behavior1 = HeightBehavior( applyHeightToFirstAscent: false, applyHeightToLastDescent: false ); - HeightBehavior behavior2 = HeightBehavior( + final HeightBehavior behavior2 = HeightBehavior( applyHeightToFirstAscent: false, ); - HeightBehavior behavior3 = HeightBehavior( + final HeightBehavior behavior3 = HeightBehavior( applyHeightToLastDescent: false ); @@ -47,8 +47,11 @@ void main() { expect(behavior1.applyHeightToFirstAscent, equals(false)); expect(behavior1.applyHeightToLastDescent, equals(false)); - expect(behavior2.applyHeightToFirstAscent, equals(true)); - expect(behavior2.applyHeightToLastDescent, equals(false)); + expect(behavior2.applyHeightToFirstAscent, equals(false)); + expect(behavior2.applyHeightToLastDescent, equals(true)); + + expect(behavior3.applyHeightToFirstAscent, equals(true)); + expect(behavior3.applyHeightToLastDescent, equals(false)); }); test('encode works', () { From bb7cf30c727a3dce20c43436e832ca8bec0def07 Mon Sep 17 00:00:00 2001 From: GaryQian Date: Wed, 8 Jan 2020 17:14:04 -0800 Subject: [PATCH 18/33] Sync changes with web_ui --- lib/web_ui/lib/src/ui/text.dart | 67 ++++++++++++++++++++------------- 1 file changed, 40 insertions(+), 27 deletions(-) diff --git a/lib/web_ui/lib/src/ui/text.dart b/lib/web_ui/lib/src/ui/text.dart index 6fef7b21b5b3d..a846e89012753 100644 --- a/lib/web_ui/lib/src/ui/text.dart +++ b/lib/web_ui/lib/src/ui/text.dart @@ -424,75 +424,88 @@ enum TextDecorationStyle { wavy } -/// Defines how the paragraph will handle the ascent of the first line and -/// descent of the last line. These lines are referred to as "boundary" lines. +/// Defines how the paragraph will apply [TextStyle.height] the ascent of the +/// first line and descent of the last line. +/// +/// The boolean value represents whether the [TextStyle.height] modifier will +/// be applied to the corresponding metric. By default, all properties are true, +/// and [TextStyle.height] is applied as normal. When set to false, the font's +/// default ascent will be used. class HeightBehavior { /// Creates a new HeightBehavior object. /// - /// * first: When true, the [TextStyle.height] modifier will be applied to - /// to the ascent of the first line. When false, the font's default ascent - /// will be used. - /// * last: When true, the [TextStyle.height] modifier will be applied to - /// to the descent of the last line. When false, the font's default descent - /// will be used. + /// * applyHeightToFirstAscent: When true, the [TextStyle.height] modifier + /// will be applied to the ascent of the first line. When false, the font's + /// default ascent will be used. + /// * applyHeightToLastDescent: When true, the [TextStyle.height] modifier + /// will be applied to the descent of the last line. When false, the font's + /// default descent will be used. + /// + /// All properties default to true (height modifications applied as normal). HeightBehavior({ - this.first = true, - this.last = true, + this.applyHeightToFirstAscent = true, + this.applyHeightToLastDescent = true, }); /// Creates a new HeightBehavior object from an encoded form. /// /// See [encode] for the creation of the encoded form. - HeightBehavior.fromEncoded(int encoded) : first = (encoded & 0x1) > 0, - last = (encoded & 0x2) > 0; + HeightBehavior.fromEncoded(int encoded) : applyHeightToFirstAscent = (encoded & 0x1) > 0, + applyHeightToLastDescent = (encoded & 0x2) > 0; - /// Whether to apply the [TextStyle.height] modifier or not to the ascent of - /// the first line in the paragraph. + /// Whether to apply the [TextStyle.height] modifier to the ascent of the first + /// line in the paragraph. /// /// When true, the [TextStyle.height] modifier will be applied to to the ascent /// of the first line. When false, the font's default ascent will be used and /// the [TextStyle.height] will have no effect on the ascent of the first line. - final bool first; + /// + /// This property only has effect if a non-null [TextStyle.height] is specified. + /// + /// Defaults to true (height modifications applied as normal). + final bool applyHeightToFirstAscent; - /// Whether to apply the [TextStyle.height] modifier or not to the descent of - /// the last line in the paragraph. + /// Whether to apply the [TextStyle.height] modifier to the descent of the last + /// line in the paragraph. /// /// When true, the [TextStyle.height] modifier will be applied to to the descent /// of the last line. When false, the font's default descent will be used and /// the [TextStyle.height] will have no effect on the descent of the last line. - final bool last; + /// + /// This property only has effect if a non-null [TextStyle.height] is specified. + /// + /// Defaults to true (height modifications applied as normal). + final bool applyHeightToLastDescent; /// Returns an encoded int representation of this object. int encode() { - return 0 + (first ? 1 << 0 : 0) + (last ? 1 << 1 : 0); + return 0 + (applyHeightToFirstAscent ? 1 << 0 : 0) + (applyHeightToLastDescent ? 1 << 1 : 0); } @override bool operator ==(dynamic other) { - if (identical(this, other)) - return true; if (other.runtimeType != runtimeType) return false; return other is HeightBehavior - && other.first == first - && other.last == last; + && other.applyHeightToFirstAscent == applyHeightToLastDescent + && other.applyHeightToLastDescent == applyHeightToLastDescent; } @override int get hashCode { return hashValues( - first, - last, + applyHeightToFirstAscent, + applyHeightToLastDescent, ); } @override String toString() { return 'HeightBehavior(' - 'first: $first, ' - 'last: $last, ' + 'applyHeightToFirstAscent: $applyHeightToFirstAscent, ' + 'applyHeightToLastDescent: $applyHeightToLastDescent' ')'; } } From fa18e73f1b84e767168a92544ae6d7dc57a7f1fd Mon Sep 17 00:00:00 2001 From: GaryQian Date: Wed, 8 Jan 2020 17:36:12 -0800 Subject: [PATCH 19/33] Fix typo/bug --- lib/ui/text.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ui/text.dart b/lib/ui/text.dart index ac6bfc2153ea7..1bb6d00001c64 100644 --- a/lib/ui/text.dart +++ b/lib/ui/text.dart @@ -496,7 +496,7 @@ class HeightBehavior { if (other.runtimeType != runtimeType) return false; return other is HeightBehavior - && other.applyHeightToFirstAscent == applyHeightToLastDescent + && other.applyHeightToFirstAscent == applyHeightToFirstAscent && other.applyHeightToLastDescent == applyHeightToLastDescent; } From c53137311ff5e6e447d18539820bca1b7654a8b8 Mon Sep 17 00:00:00 2001 From: GaryQian Date: Thu, 9 Jan 2020 15:13:01 -0800 Subject: [PATCH 20/33] Add const to contructors --- lib/ui/text.dart | 4 ++-- lib/web_ui/lib/src/ui/text.dart | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/ui/text.dart b/lib/ui/text.dart index 1bb6d00001c64..681aea3ca3419 100644 --- a/lib/ui/text.dart +++ b/lib/ui/text.dart @@ -450,7 +450,7 @@ class HeightBehavior { /// default descent will be used. /// /// All properties default to true (height modifications applied as normal). - HeightBehavior({ + const HeightBehavior({ this.applyHeightToFirstAscent = true, this.applyHeightToLastDescent = true, }); @@ -458,7 +458,7 @@ class HeightBehavior { /// Creates a new HeightBehavior object from an encoded form. /// /// See [encode] for the creation of the encoded form. - HeightBehavior.fromEncoded(int encoded) : applyHeightToFirstAscent = (encoded & 0x1) > 0, + const HeightBehavior.fromEncoded(int encoded) : applyHeightToFirstAscent = (encoded & 0x1) > 0, applyHeightToLastDescent = (encoded & 0x2) > 0; diff --git a/lib/web_ui/lib/src/ui/text.dart b/lib/web_ui/lib/src/ui/text.dart index a846e89012753..1563328a58974 100644 --- a/lib/web_ui/lib/src/ui/text.dart +++ b/lib/web_ui/lib/src/ui/text.dart @@ -443,7 +443,7 @@ class HeightBehavior { /// default descent will be used. /// /// All properties default to true (height modifications applied as normal). - HeightBehavior({ + const HeightBehavior({ this.applyHeightToFirstAscent = true, this.applyHeightToLastDescent = true, }); @@ -451,7 +451,7 @@ class HeightBehavior { /// Creates a new HeightBehavior object from an encoded form. /// /// See [encode] for the creation of the encoded form. - HeightBehavior.fromEncoded(int encoded) : applyHeightToFirstAscent = (encoded & 0x1) > 0, + const HeightBehavior.fromEncoded(int encoded) : applyHeightToFirstAscent = (encoded & 0x1) > 0, applyHeightToLastDescent = (encoded & 0x2) > 0; From d9829fab3a65c7ade1edbcadc26672fbedb52300 Mon Sep 17 00:00:00 2001 From: GaryQian Date: Thu, 9 Jan 2020 15:33:20 -0800 Subject: [PATCH 21/33] Fix wording issues --- lib/ui/text.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ui/text.dart b/lib/ui/text.dart index 681aea3ca3419..4070d0c08b6f8 100644 --- a/lib/ui/text.dart +++ b/lib/ui/text.dart @@ -430,10 +430,10 @@ enum TextDecorationStyle { } /// {@template flutter.dart:ui.heightBehavior} -/// Defines how the paragraph will apply [TextStyle.height] the ascent of the +/// Defines how the paragraph will apply [TextStyle.height] to the ascent of the /// first line and descent of the last line. /// -/// The boolean value represents whether the [TextStyle.height] modifier will +/// Each boolean value represents whether the [TextStyle.height] modifier will /// be applied to the corresponding metric. By default, all properties are true, /// and [TextStyle.height] is applied as normal. When set to false, the font's /// default ascent will be used. From 5764a3643825ddfe83d6a27c3b9fd58827bb3843 Mon Sep 17 00:00:00 2001 From: GaryQian Date: Thu, 9 Jan 2020 16:00:29 -0800 Subject: [PATCH 22/33] Fix analyzer --- testing/dart/text_test.dart | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/testing/dart/text_test.dart b/testing/dart/text_test.dart index 431fb76a2cebf..e63f73e9aefe1 100644 --- a/testing/dart/text_test.dart +++ b/testing/dart/text_test.dart @@ -28,15 +28,15 @@ void main() { }); group('HeightBehavior', () { - final HeightBehavior behavior0 = HeightBehavior(); - final HeightBehavior behavior1 = HeightBehavior( + final HeightBehavior behavior0 = const HeightBehavior(); + final HeightBehavior behavior1 = const HeightBehavior( applyHeightToFirstAscent: false, applyHeightToLastDescent: false ); - final HeightBehavior behavior2 = HeightBehavior( + final HeightBehavior behavior2 = const HeightBehavior( applyHeightToFirstAscent: false, ); - final HeightBehavior behavior3 = HeightBehavior( + final HeightBehavior behavior3 = const HeightBehavior( applyHeightToLastDescent: false ); @@ -62,10 +62,10 @@ void main() { }); test('encode works', () { - expect(HeightBehavior.fromEncoded(3), equals(behavior0)); - expect(HeightBehavior.fromEncoded(0), equals(behavior1)); - expect(HeightBehavior.fromEncoded(2), equals(behavior2)); - expect(HeightBehavior.fromEncoded(1), equals(behavior3)); + expect(const HeightBehavior.fromEncoded(3), equals(behavior0)); + expect(const HeightBehavior.fromEncoded(0), equals(behavior1)); + expect(const HeightBehavior.fromEncoded(2), equals(behavior2)); + expect(const HeightBehavior.fromEncoded(1), equals(behavior3)); }); test('toString works', () { From 9cdc51a7118156a0aaeaa62d87c492f28d198c8f Mon Sep 17 00:00:00 2001 From: GaryQian Date: Thu, 9 Jan 2020 16:09:25 -0800 Subject: [PATCH 23/33] Analyzer part 2 --- testing/dart/text_test.dart | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/testing/dart/text_test.dart b/testing/dart/text_test.dart index e63f73e9aefe1..47e3c3c92f69e 100644 --- a/testing/dart/text_test.dart +++ b/testing/dart/text_test.dart @@ -28,15 +28,15 @@ void main() { }); group('HeightBehavior', () { - final HeightBehavior behavior0 = const HeightBehavior(); - final HeightBehavior behavior1 = const HeightBehavior( + const HeightBehavior behavior0 = HeightBehavior(); + const HeightBehavior behavior1 = HeightBehavior( applyHeightToFirstAscent: false, applyHeightToLastDescent: false ); - final HeightBehavior behavior2 = const HeightBehavior( + const HeightBehavior behavior2 = HeightBehavior( applyHeightToFirstAscent: false, ); - final HeightBehavior behavior3 = const HeightBehavior( + const HeightBehavior behavior3 = HeightBehavior( applyHeightToLastDescent: false ); From 889b95d360f7e54975bd11025f32b1fdfb23b03f Mon Sep 17 00:00:00 2001 From: GaryQian Date: Wed, 22 Jan 2020 17:18:23 -0800 Subject: [PATCH 24/33] REname HeightBehavior -> TextHeighBehavior --- lib/ui/text.dart | 34 +++++++++---------- lib/ui/text/paragraph_builder.cc | 8 ++--- lib/web_ui/lib/src/engine/text/paragraph.dart | 10 +++--- testing/dart/text_test.dart | 26 +++++++------- third_party/txt/src/txt/paragraph_style.h | 4 +-- third_party/txt/src/txt/paragraph_txt.cc | 4 +-- third_party/txt/src/txt/paragraph_txt.h | 2 +- third_party/txt/tests/paragraph_unittests.cc | 6 ++-- 8 files changed, 47 insertions(+), 47 deletions(-) diff --git a/lib/ui/text.dart b/lib/ui/text.dart index 4070d0c08b6f8..3e6312364b2b5 100644 --- a/lib/ui/text.dart +++ b/lib/ui/text.dart @@ -429,7 +429,7 @@ enum TextDecorationStyle { wavy } -/// {@template flutter.dart:ui.heightBehavior} +/// {@template flutter.dart:ui.textHeightBehavior} /// Defines how the paragraph will apply [TextStyle.height] to the ascent of the /// first line and descent of the last line. /// @@ -438,9 +438,9 @@ enum TextDecorationStyle { /// and [TextStyle.height] is applied as normal. When set to false, the font's /// default ascent will be used. /// {@endtemplate} -class HeightBehavior { +class TextHeightBehavior { - /// Creates a new HeightBehavior object. + /// Creates a new TextHeightBehavior object. /// /// * applyHeightToFirstAscent: When true, the [TextStyle.height] modifier /// will be applied to the ascent of the first line. When false, the font's @@ -450,15 +450,15 @@ class HeightBehavior { /// default descent will be used. /// /// All properties default to true (height modifications applied as normal). - const HeightBehavior({ + const TextHeightBehavior({ this.applyHeightToFirstAscent = true, this.applyHeightToLastDescent = true, }); - /// Creates a new HeightBehavior object from an encoded form. + /// Creates a new TextHeightBehavior object from an encoded form. /// /// See [encode] for the creation of the encoded form. - const HeightBehavior.fromEncoded(int encoded) : applyHeightToFirstAscent = (encoded & 0x1) > 0, + const TextHeightBehavior.fromEncoded(int encoded) : applyHeightToFirstAscent = (encoded & 0x1) > 0, applyHeightToLastDescent = (encoded & 0x2) > 0; @@ -495,7 +495,7 @@ class HeightBehavior { bool operator ==(dynamic other) { if (other.runtimeType != runtimeType) return false; - return other is HeightBehavior + return other is TextHeightBehavior && other.applyHeightToFirstAscent == applyHeightToFirstAscent && other.applyHeightToLastDescent == applyHeightToLastDescent; } @@ -510,7 +510,7 @@ class HeightBehavior { @override String toString() { - return 'HeightBehavior(' + return 'TextHeightBehavior(' 'applyHeightToFirstAscent: $applyHeightToFirstAscent, ' 'applyHeightToLastDescent: $applyHeightToLastDescent' ')'; @@ -834,7 +834,7 @@ class TextStyle { // // - Element 5: The value of |maxLines|. // -// - Element 6: The encoded value of |heightBehavior|. +// - Element 6: The encoded value of |textHeightBehavior|. // Int32List _encodeParagraphStyle( TextAlign textAlign, @@ -843,7 +843,7 @@ Int32List _encodeParagraphStyle( String fontFamily, double fontSize, double height, - HeightBehavior heightBehavior, + TextHeightBehavior textHeightBehavior, FontWeight fontWeight, FontStyle fontStyle, StrutStyle strutStyle, @@ -871,9 +871,9 @@ Int32List _encodeParagraphStyle( result[0] |= 1 << 5; result[5] = maxLines; } - if (heightBehavior != null) { + if (textHeightBehavior != null) { result[0] |= 1 << 6; - result[6] = heightBehavior.encode(); + result[6] = textHeightBehavior.encode(); } if (fontFamily != null) { result[0] |= 1 << 7; @@ -937,7 +937,7 @@ class ParagraphStyle { /// the line height to take the height as defined by the font, which may not /// be exactly the height of the `fontSize`. /// - /// * `heightBehavior`: Specifies how the `height` multiplier is + /// * `textHeightBehavior`: Specifies how the `height` multiplier is /// applied to ascent of the first line and the descent of the last line. /// /// * `fontWeight`: The typeface thickness to use when painting the text @@ -967,7 +967,7 @@ class ParagraphStyle { String fontFamily, double fontSize, double height, - HeightBehavior heightBehavior, + TextHeightBehavior textHeightBehavior, FontWeight fontWeight, FontStyle fontStyle, StrutStyle strutStyle, @@ -980,7 +980,7 @@ class ParagraphStyle { fontFamily, fontSize, height, - heightBehavior, + textHeightBehavior, fontWeight, fontStyle, strutStyle, @@ -1029,9 +1029,9 @@ class ParagraphStyle { 'fontWeight: ${ _encoded[0] & 0x008 == 0x008 ? FontWeight.values[_encoded[3]] : "unspecified"}, ' 'fontStyle: ${ _encoded[0] & 0x010 == 0x010 ? FontStyle.values[_encoded[4]] : "unspecified"}, ' 'maxLines: ${ _encoded[0] & 0x020 == 0x020 ? _encoded[5] : "unspecified"}, ' - 'heightBehavior: ${ + 'textHeightBehavior: ${ _encoded[0] & 0x040 == 0x040 ? - HeightBehavior.fromEncoded(_encoded[6]).toString() : "unspecified"}, ' + TextHeightBehavior.fromEncoded(_encoded[6]).toString() : "unspecified"}, ' 'fontFamily: ${ _encoded[0] & 0x080 == 0x080 ? _fontFamily : "unspecified"}, ' 'fontSize: ${ _encoded[0] & 0x100 == 0x100 ? _fontSize : "unspecified"}, ' 'height: ${ _encoded[0] & 0x200 == 0x200 ? "${_height}x" : "unspecified"}, ' diff --git a/lib/ui/text/paragraph_builder.cc b/lib/ui/text/paragraph_builder.cc index a8741095b832f..d7514ace9f8cc 100644 --- a/lib/ui/text/paragraph_builder.cc +++ b/lib/ui/text/paragraph_builder.cc @@ -75,7 +75,7 @@ const int psTextDirectionIndex = 2; const int psFontWeightIndex = 3; const int psFontStyleIndex = 4; const int psMaxLinesIndex = 5; -const int psHeightBehaviorIndex = 6; +const int psTextHeightBehaviorIndex = 6; const int psFontFamilyIndex = 7; const int psFontSizeIndex = 8; const int psHeightIndex = 9; @@ -91,7 +91,7 @@ const int psMaxLinesMask = 1 << psMaxLinesIndex; const int psFontFamilyMask = 1 << psFontFamilyIndex; const int psFontSizeMask = 1 << psFontSizeIndex; const int psHeightMask = 1 << psHeightIndex; -const int psHeightBehaviorMask = 1 << psHeightBehaviorIndex; +const int psTextHeightBehaviorMask = 1 << psTextHeightBehaviorIndex; const int psStrutStyleMask = 1 << psStrutStyleIndex; const int psEllipsisMask = 1 << psEllipsisIndex; const int psLocaleMask = 1 << psLocaleIndex; @@ -267,8 +267,8 @@ ParagraphBuilder::ParagraphBuilder( style.has_height_override = true; } - if (mask & psHeightBehaviorMask) { - style.height_behavior = encoded[psHeightBehaviorIndex]; + if (mask & psTextHeightBehaviorMask) { + style.height_behavior = encoded[psTextHeightBehaviorIndex]; } if (mask & psStrutStyleMask) { diff --git a/lib/web_ui/lib/src/engine/text/paragraph.dart b/lib/web_ui/lib/src/engine/text/paragraph.dart index 12ab5622968a0..80ba65dbd8bdb 100644 --- a/lib/web_ui/lib/src/engine/text/paragraph.dart +++ b/lib/web_ui/lib/src/engine/text/paragraph.dart @@ -457,7 +457,7 @@ class EngineParagraphStyle implements ui.ParagraphStyle { String fontFamily, double fontSize, double height, - ui.HeightBehavior heightBehavior, + ui.TextHeightBehavior textHeightBehavior, ui.FontWeight fontWeight, ui.FontStyle fontStyle, ui.StrutStyle strutStyle, @@ -471,7 +471,7 @@ class EngineParagraphStyle implements ui.ParagraphStyle { _fontFamily = fontFamily, _fontSize = fontSize, _height = height, - _heightBehavior = heightBehavior, + _textHeightBehavior = textHeightBehavior, // TODO(b/128317744): add support for strut style. _strutStyle = strutStyle, _ellipsis = ellipsis, @@ -485,7 +485,7 @@ class EngineParagraphStyle implements ui.ParagraphStyle { final String _fontFamily; final double _fontSize; final double _height; - final ui.HeightBehavior _heightBehavior; + final ui.TextHeightBehavior _textHeightBehavior; final EngineStrutStyle _strutStyle; final String _ellipsis; final ui.Locale _locale; @@ -539,7 +539,7 @@ class EngineParagraphStyle implements ui.ParagraphStyle { _fontFamily == typedOther._fontFamily || _fontSize == typedOther._fontSize || _height == typedOther._height || - _heightBehavior == typedOther._heightBehavior || + _textHeightBehavior == typedOther._textHeightBehavior || _ellipsis == typedOther._ellipsis || _locale == typedOther._locale; } @@ -560,7 +560,7 @@ class EngineParagraphStyle implements ui.ParagraphStyle { 'fontFamily: ${_fontFamily ?? "unspecified"}, ' 'fontSize: ${_fontSize != null ? _fontSize.toStringAsFixed(1) : "unspecified"}, ' 'height: ${_height != null ? "${_height.toStringAsFixed(1)}x" : "unspecified"}, ' - 'heightBehavior: ${_heightBehavior ?? "unspecified"}, ' + 'textHeightBehavior: ${_textHeightBehavior ?? "unspecified"}, ' 'ellipsis: ${_ellipsis != null ? "\"$_ellipsis\"" : "unspecified"}, ' 'locale: ${_locale ?? "unspecified"}' ')'; diff --git a/testing/dart/text_test.dart b/testing/dart/text_test.dart index 47e3c3c92f69e..988cfada3428e 100644 --- a/testing/dart/text_test.dart +++ b/testing/dart/text_test.dart @@ -27,16 +27,16 @@ void main() { }); }); - group('HeightBehavior', () { - const HeightBehavior behavior0 = HeightBehavior(); - const HeightBehavior behavior1 = HeightBehavior( + group('TextHeightBehavior', () { + const TextHeightBehavior behavior0 = TextHeightBehavior(); + const TextHeightBehavior behavior1 = TextHeightBehavior( applyHeightToFirstAscent: false, applyHeightToLastDescent: false ); - const HeightBehavior behavior2 = HeightBehavior( + const TextHeightBehavior behavior2 = TextHeightBehavior( applyHeightToFirstAscent: false, ); - const HeightBehavior behavior3 = HeightBehavior( + const TextHeightBehavior behavior3 = TextHeightBehavior( applyHeightToLastDescent: false ); @@ -62,17 +62,17 @@ void main() { }); test('encode works', () { - expect(const HeightBehavior.fromEncoded(3), equals(behavior0)); - expect(const HeightBehavior.fromEncoded(0), equals(behavior1)); - expect(const HeightBehavior.fromEncoded(2), equals(behavior2)); - expect(const HeightBehavior.fromEncoded(1), equals(behavior3)); + expect(const TextHeightBehavior.fromEncoded(3), equals(behavior0)); + expect(const TextHeightBehavior.fromEncoded(0), equals(behavior1)); + expect(const TextHeightBehavior.fromEncoded(2), equals(behavior2)); + expect(const TextHeightBehavior.fromEncoded(1), equals(behavior3)); }); test('toString works', () { - expect(behavior0.toString(), equals('HeightBehavior(applyHeightToFirstAscent: true, applyHeightToLastDescent: true)')); - expect(behavior1.toString(), equals('HeightBehavior(applyHeightToFirstAscent: false, applyHeightToLastDescent: false)')); - expect(behavior2.toString(), equals('HeightBehavior(applyHeightToFirstAscent: false, applyHeightToLastDescent: true)')); - expect(behavior3.toString(), equals('HeightBehavior(applyHeightToFirstAscent: true, applyHeightToLastDescent: false)')); + expect(behavior0.toString(), equals('TextHeightBehavior(applyHeightToFirstAscent: true, applyHeightToLastDescent: true)')); + expect(behavior1.toString(), equals('TextHeightBehavior(applyHeightToFirstAscent: false, applyHeightToLastDescent: false)')); + expect(behavior2.toString(), equals('TextHeightBehavior(applyHeightToFirstAscent: false, applyHeightToLastDescent: true)')); + expect(behavior3.toString(), equals('TextHeightBehavior(applyHeightToFirstAscent: true, applyHeightToLastDescent: false)')); }); }); diff --git a/third_party/txt/src/txt/paragraph_style.h b/third_party/txt/src/txt/paragraph_style.h index dd3ccf9249276..859a92e73786a 100644 --- a/third_party/txt/src/txt/paragraph_style.h +++ b/third_party/txt/src/txt/paragraph_style.h @@ -53,7 +53,7 @@ enum class TextDirection { // example, disabling first and last can achieved with: // // (kDisableFirst & kDisableLast). -enum HeightBehavior { +enum TextHeightBehavior { kDisableAll = 0x0, kAll = 0x1 | 0x2, kDisableFirstAscent = 0x2, @@ -69,7 +69,7 @@ class ParagraphStyle { std::string font_family = ""; double font_size = 14; double height = 1; - size_t height_behavior = HeightBehavior::kAll; + size_t height_behavior = TextHeightBehavior::kAll; bool has_height_override = false; // Strut properties. strut_enabled must be set to true for the rest of the diff --git a/third_party/txt/src/txt/paragraph_txt.cc b/third_party/txt/src/txt/paragraph_txt.cc index f13f9eb51ef51..1e49d2dc325bb 100644 --- a/third_party/txt/src/txt/paragraph_txt.cc +++ b/third_party/txt/src/txt/paragraph_txt.cc @@ -1250,13 +1250,13 @@ void ParagraphTxt::UpdateLineMetrics(const SkFontMetrics& metrics, // // Disable first line ascent modifications. if (line_number == 0 && !(paragraph_style_.height_behavior & - ~HeightBehavior::kDisableFirstAscent)) { + ~TextHeightBehavior::kDisableFirstAscent)) { ascent = -metrics.fAscent; } // Disable last line descent modifications. if (line_number == line_limit - 1 && !(paragraph_style_.height_behavior & - ~HeightBehavior::kDisableLastDescent)) { + ~TextHeightBehavior::kDisableLastDescent)) { descent = metrics.fDescent; } diff --git a/third_party/txt/src/txt/paragraph_txt.h b/third_party/txt/src/txt/paragraph_txt.h index 2b5fbd39c9203..bb95e6e7a0053 100644 --- a/third_party/txt/src/txt/paragraph_txt.h +++ b/third_party/txt/src/txt/paragraph_txt.h @@ -162,7 +162,7 @@ class ParagraphTxt : public Paragraph { FRIEND_TEST(ParagraphTest, FontFeaturesParagraph); FRIEND_TEST(ParagraphTest, GetGlyphPositionAtCoordinateSegfault); FRIEND_TEST(ParagraphTest, KhmerLineBreaker); - FRIEND_TEST(ParagraphTest, HeightBehaviorRectsParagraph); + FRIEND_TEST(ParagraphTest, TextHeightBehaviorRectsParagraph); // Starting data to layout. std::vector text_; diff --git a/third_party/txt/tests/paragraph_unittests.cc b/third_party/txt/tests/paragraph_unittests.cc index dfbca75bca8ca..b01bae33fbbb1 100644 --- a/third_party/txt/tests/paragraph_unittests.cc +++ b/third_party/txt/tests/paragraph_unittests.cc @@ -5877,7 +5877,7 @@ TEST_F(ParagraphTest, KhmerLineBreaker) { ASSERT_TRUE(Snapshot()); } -TEST_F(ParagraphTest, HeightBehaviorRectsParagraph) { +TEST_F(ParagraphTest, TextHeightBehaviorRectsParagraph) { // clang-format off const char* text = "line1\nline2\nline3"; @@ -5887,8 +5887,8 @@ TEST_F(ParagraphTest, HeightBehaviorRectsParagraph) { icu_text.getBuffer() + icu_text.length()); txt::ParagraphStyle paragraph_style; - paragraph_style.height_behavior = txt::HeightBehavior::kDisableFirstAscent & - txt::HeightBehavior::kDisableLastDescent; + paragraph_style.height_behavior = txt::TextHeightBehavior::kDisableFirstAscent & + txt::TextHeightBehavior::kDisableLastDescent; txt::ParagraphBuilderTxt builder(paragraph_style, GetTestFontCollection()); From 2b46f70429cb24ac2b5073b021a9bf300b605b2b Mon Sep 17 00:00:00 2001 From: GaryQian Date: Wed, 22 Jan 2020 17:41:46 -0800 Subject: [PATCH 25/33] Fix indentation --- lib/ui/text.dart | 2 +- lib/web_ui/lib/src/ui/text.dart | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ui/text.dart b/lib/ui/text.dart index 3e6312364b2b5..570ef6b5c46c8 100644 --- a/lib/ui/text.dart +++ b/lib/ui/text.dart @@ -459,7 +459,7 @@ class TextHeightBehavior { /// /// See [encode] for the creation of the encoded form. const TextHeightBehavior.fromEncoded(int encoded) : applyHeightToFirstAscent = (encoded & 0x1) > 0, - applyHeightToLastDescent = (encoded & 0x2) > 0; + applyHeightToLastDescent = (encoded & 0x2) > 0; /// Whether to apply the [TextStyle.height] modifier to the ascent of the first diff --git a/lib/web_ui/lib/src/ui/text.dart b/lib/web_ui/lib/src/ui/text.dart index 1563328a58974..24bcb6f9e685c 100644 --- a/lib/web_ui/lib/src/ui/text.dart +++ b/lib/web_ui/lib/src/ui/text.dart @@ -452,7 +452,7 @@ class HeightBehavior { /// /// See [encode] for the creation of the encoded form. const HeightBehavior.fromEncoded(int encoded) : applyHeightToFirstAscent = (encoded & 0x1) > 0, - applyHeightToLastDescent = (encoded & 0x2) > 0; + applyHeightToLastDescent = (encoded & 0x2) > 0; /// Whether to apply the [TextStyle.height] modifier to the ascent of the first From ddda7e62326fffd3ade7a72c2804da41f6711c4d Mon Sep 17 00:00:00 2001 From: GaryQian Date: Thu, 23 Jan 2020 12:45:22 -0800 Subject: [PATCH 26/33] Invert enum bits, fix namiming change --- lib/ui/text.dart | 6 +++--- lib/web_ui/lib/src/ui/text.dart | 20 ++++++++++---------- third_party/txt/src/txt/paragraph_style.h | 10 +++++----- third_party/txt/src/txt/paragraph_txt.cc | 10 +++++----- 4 files changed, 23 insertions(+), 23 deletions(-) diff --git a/lib/ui/text.dart b/lib/ui/text.dart index 570ef6b5c46c8..deca93096d85b 100644 --- a/lib/ui/text.dart +++ b/lib/ui/text.dart @@ -458,8 +458,8 @@ class TextHeightBehavior { /// Creates a new TextHeightBehavior object from an encoded form. /// /// See [encode] for the creation of the encoded form. - const TextHeightBehavior.fromEncoded(int encoded) : applyHeightToFirstAscent = (encoded & 0x1) > 0, - applyHeightToLastDescent = (encoded & 0x2) > 0; + const TextHeightBehavior.fromEncoded(int encoded) : applyHeightToFirstAscent = (encoded & 0x1) == 0, + applyHeightToLastDescent = (encoded & 0x2) == 0; /// Whether to apply the [TextStyle.height] modifier to the ascent of the first @@ -488,7 +488,7 @@ class TextHeightBehavior { /// Returns an encoded int representation of this object. int encode() { - return 0 + (applyHeightToFirstAscent ? 1 << 0 : 0) + (applyHeightToLastDescent ? 1 << 1 : 0); + return 0 + (applyHeightToFirstAscent ? 0 : 1 << 0) + (applyHeightToLastDescent ? 0 : 1 << 1); } @override diff --git a/lib/web_ui/lib/src/ui/text.dart b/lib/web_ui/lib/src/ui/text.dart index 24bcb6f9e685c..3ac8646dcb2b8 100644 --- a/lib/web_ui/lib/src/ui/text.dart +++ b/lib/web_ui/lib/src/ui/text.dart @@ -431,9 +431,9 @@ enum TextDecorationStyle { /// be applied to the corresponding metric. By default, all properties are true, /// and [TextStyle.height] is applied as normal. When set to false, the font's /// default ascent will be used. -class HeightBehavior { +class TextHeightBehavior { - /// Creates a new HeightBehavior object. + /// Creates a new TextHeightBehavior object. /// /// * applyHeightToFirstAscent: When true, the [TextStyle.height] modifier /// will be applied to the ascent of the first line. When false, the font's @@ -443,16 +443,16 @@ class HeightBehavior { /// default descent will be used. /// /// All properties default to true (height modifications applied as normal). - const HeightBehavior({ + const TextHeightBehavior({ this.applyHeightToFirstAscent = true, this.applyHeightToLastDescent = true, }); - /// Creates a new HeightBehavior object from an encoded form. + /// Creates a new TextHeightBehavior object from an encoded form. /// /// See [encode] for the creation of the encoded form. - const HeightBehavior.fromEncoded(int encoded) : applyHeightToFirstAscent = (encoded & 0x1) > 0, - applyHeightToLastDescent = (encoded & 0x2) > 0; + const TextHeightBehavior.fromEncoded(int encoded) : applyHeightToFirstAscent = (encoded & 0x1) == 0, + applyHeightToLastDescent = (encoded & 0x2) == 0; /// Whether to apply the [TextStyle.height] modifier to the ascent of the first @@ -481,15 +481,15 @@ class HeightBehavior { /// Returns an encoded int representation of this object. int encode() { - return 0 + (applyHeightToFirstAscent ? 1 << 0 : 0) + (applyHeightToLastDescent ? 1 << 1 : 0); + return 0 + (applyHeightToFirstAscent ? 0 : 1 << 0) + (applyHeightToLastDescent ? 0 : 1 << 1); } @override bool operator ==(dynamic other) { if (other.runtimeType != runtimeType) return false; - return other is HeightBehavior - && other.applyHeightToFirstAscent == applyHeightToLastDescent + return other is TextHeightBehavior + && other.applyHeightToFirstAscent == applyHeightToFirstAscent && other.applyHeightToLastDescent == applyHeightToLastDescent; } @@ -503,7 +503,7 @@ class HeightBehavior { @override String toString() { - return 'HeightBehavior(' + return 'TextHeightBehavior(' 'applyHeightToFirstAscent: $applyHeightToFirstAscent, ' 'applyHeightToLastDescent: $applyHeightToLastDescent' ')'; diff --git a/third_party/txt/src/txt/paragraph_style.h b/third_party/txt/src/txt/paragraph_style.h index 859a92e73786a..af8b9e1a40345 100644 --- a/third_party/txt/src/txt/paragraph_style.h +++ b/third_party/txt/src/txt/paragraph_style.h @@ -41,7 +41,7 @@ enum class TextDirection { ltr, }; -// Allows disabling height adjustments to first first line's ascent and the +// Allows disabling height adjustments to first line's ascent and the // last line's descent. If disabled, the line will use the default font // metric provided ascent/descent and ParagraphStyle.height will not take // effect. @@ -54,10 +54,10 @@ enum class TextDirection { // // (kDisableFirst & kDisableLast). enum TextHeightBehavior { - kDisableAll = 0x0, - kAll = 0x1 | 0x2, - kDisableFirstAscent = 0x2, - kDisableLastDescent = 0x1, + kAll = 0x0, + kDisableFirstAscent = 0x1, + kDisableLastDescent = 0x2, + kDisableAll = 0x1 | 0x2, }; class ParagraphStyle { diff --git a/third_party/txt/src/txt/paragraph_txt.cc b/third_party/txt/src/txt/paragraph_txt.cc index 1e49d2dc325bb..491f0f4015926 100644 --- a/third_party/txt/src/txt/paragraph_txt.cc +++ b/third_party/txt/src/txt/paragraph_txt.cc @@ -1246,17 +1246,17 @@ void ParagraphTxt::UpdateLineMetrics(const SkFontMetrics& metrics, descent = (metrics.fDescent + metrics.fLeading / 2); } - // Account for height_behavior in parargaph_style_. + // Account for height_behavior in paragraph_style_. // // Disable first line ascent modifications. - if (line_number == 0 && !(paragraph_style_.height_behavior & - ~TextHeightBehavior::kDisableFirstAscent)) { + if (line_number == 0 && paragraph_style_.height_behavior & + TextHeightBehavior::kDisableFirstAscent) { ascent = -metrics.fAscent; } // Disable last line descent modifications. if (line_number == line_limit - 1 && - !(paragraph_style_.height_behavior & - ~TextHeightBehavior::kDisableLastDescent)) { + paragraph_style_.height_behavior & + TextHeightBehavior::kDisableLastDescent) { descent = metrics.fDescent; } From ba9f09a69c9f6064ce3429792fd11e10da05409b Mon Sep 17 00:00:00 2001 From: GaryQian Date: Thu, 23 Jan 2020 12:52:01 -0800 Subject: [PATCH 27/33] Fix tests: --- third_party/txt/tests/paragraph_unittests.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/third_party/txt/tests/paragraph_unittests.cc b/third_party/txt/tests/paragraph_unittests.cc index b01bae33fbbb1..db06b9dfe7511 100644 --- a/third_party/txt/tests/paragraph_unittests.cc +++ b/third_party/txt/tests/paragraph_unittests.cc @@ -5887,7 +5887,7 @@ TEST_F(ParagraphTest, TextHeightBehaviorRectsParagraph) { icu_text.getBuffer() + icu_text.length()); txt::ParagraphStyle paragraph_style; - paragraph_style.height_behavior = txt::TextHeightBehavior::kDisableFirstAscent & + paragraph_style.height_behavior = txt::TextHeightBehavior::kDisableFirstAscent | txt::TextHeightBehavior::kDisableLastDescent; txt::ParagraphBuilderTxt builder(paragraph_style, GetTestFontCollection()); From 24dbc5666cdab200c2cfb45a70ab69e2fea4f35f Mon Sep 17 00:00:00 2001 From: GaryQian Date: Thu, 23 Jan 2020 13:26:58 -0800 Subject: [PATCH 28/33] Formatting --- third_party/txt/tests/paragraph_unittests.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/third_party/txt/tests/paragraph_unittests.cc b/third_party/txt/tests/paragraph_unittests.cc index db06b9dfe7511..c9b339ef15d4a 100644 --- a/third_party/txt/tests/paragraph_unittests.cc +++ b/third_party/txt/tests/paragraph_unittests.cc @@ -5887,8 +5887,9 @@ TEST_F(ParagraphTest, TextHeightBehaviorRectsParagraph) { icu_text.getBuffer() + icu_text.length()); txt::ParagraphStyle paragraph_style; - paragraph_style.height_behavior = txt::TextHeightBehavior::kDisableFirstAscent | - txt::TextHeightBehavior::kDisableLastDescent; + paragraph_style.height_behavior = + txt::TextHeightBehavior::kDisableFirstAscent | + txt::TextHeightBehavior::kDisableLastDescent; txt::ParagraphBuilderTxt builder(paragraph_style, GetTestFontCollection()); From 8a0d421a15460a771a1920746a272c673920bdfa Mon Sep 17 00:00:00 2001 From: GaryQian Date: Thu, 23 Jan 2020 14:36:50 -0800 Subject: [PATCH 29/33] Rename to TextHeightBehavior in Web_ui --- lib/web_ui/lib/src/engine/compositor/text.dart | 10 +++++----- lib/web_ui/lib/src/ui/text.dart | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/web_ui/lib/src/engine/compositor/text.dart b/lib/web_ui/lib/src/engine/compositor/text.dart index de373dc31b25a..a9cf6578f5c96 100644 --- a/lib/web_ui/lib/src/engine/compositor/text.dart +++ b/lib/web_ui/lib/src/engine/compositor/text.dart @@ -25,7 +25,7 @@ class SkParagraphStyle implements ui.ParagraphStyle { String fontFamily, double fontSize, double height, - ui.HeightBehavior heightBehavior, + ui.TextHeightBehavior textHeightBehavior, ui.FontWeight fontWeight, ui.FontStyle fontStyle, ui.StrutStyle strutStyle, @@ -39,7 +39,7 @@ class SkParagraphStyle implements ui.ParagraphStyle { fontFamily, fontSize, height, - heightBehavior, + textHeightBehavior, fontWeight, fontStyle, ellipsis, @@ -87,7 +87,7 @@ class SkParagraphStyle implements ui.ParagraphStyle { String fontFamily, double fontSize, double height, - ui.HeightBehavior heightBehavior, + ui.TextHeightBehavior textHeightBehavior, ui.FontWeight fontWeight, ui.FontStyle fontStyle, String ellipsis, @@ -132,8 +132,8 @@ class SkParagraphStyle implements ui.ParagraphStyle { skParagraphStyle['heightMultiplier'] = height; } - if (heightBehavior != null) { - skParagraphStyle['heightBehavior'] = heightBehavior.encode(); + if (textHeightBehavior != null) { + skParagraphStyle['textHeightBehavior'] = textHeightBehavior.encode(); } if (maxLines != null) { diff --git a/lib/web_ui/lib/src/ui/text.dart b/lib/web_ui/lib/src/ui/text.dart index 3ac8646dcb2b8..47767b543d0bc 100644 --- a/lib/web_ui/lib/src/ui/text.dart +++ b/lib/web_ui/lib/src/ui/text.dart @@ -670,7 +670,7 @@ abstract class ParagraphStyle { String fontFamily, double fontSize, double height, - HeightBehavior heightBehavior, + TextHeightBehavior textHeightBehavior, FontWeight fontWeight, FontStyle fontStyle, StrutStyle strutStyle, @@ -685,7 +685,7 @@ abstract class ParagraphStyle { fontFamily: fontFamily, fontSize: fontSize, height: height, - heightBehavior: heightBehavior, + textHeightBehavior: textHeightBehavior, fontWeight: fontWeight, fontStyle: fontStyle, strutStyle: strutStyle, @@ -700,7 +700,7 @@ abstract class ParagraphStyle { fontFamily: fontFamily, fontSize: fontSize, height: height, - heightBehavior: heightBehavior, + textHeightBehavior: textHeightBehavior, fontWeight: fontWeight, fontStyle: fontStyle, strutStyle: strutStyle, From 5668145d17e6a49b06d261dea4aaceea52749a08 Mon Sep 17 00:00:00 2001 From: GaryQian Date: Thu, 23 Jan 2020 14:45:07 -0800 Subject: [PATCH 30/33] Address nets --- lib/ui/text.dart | 2 +- lib/web_ui/lib/src/ui/text.dart | 2 +- third_party/txt/src/txt/paragraph_style.h | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/ui/text.dart b/lib/ui/text.dart index deca93096d85b..fa5092cac172f 100644 --- a/lib/ui/text.dart +++ b/lib/ui/text.dart @@ -488,7 +488,7 @@ class TextHeightBehavior { /// Returns an encoded int representation of this object. int encode() { - return 0 + (applyHeightToFirstAscent ? 0 : 1 << 0) + (applyHeightToLastDescent ? 0 : 1 << 1); + return (applyHeightToFirstAscent ? 0 : 1 << 0) | (applyHeightToLastDescent ? 0 : 1 << 1); } @override diff --git a/lib/web_ui/lib/src/ui/text.dart b/lib/web_ui/lib/src/ui/text.dart index 47767b543d0bc..b479db85c51c7 100644 --- a/lib/web_ui/lib/src/ui/text.dart +++ b/lib/web_ui/lib/src/ui/text.dart @@ -481,7 +481,7 @@ class TextHeightBehavior { /// Returns an encoded int representation of this object. int encode() { - return 0 + (applyHeightToFirstAscent ? 0 : 1 << 0) + (applyHeightToLastDescent ? 0 : 1 << 1); + return (applyHeightToFirstAscent ? 0 : 1 << 0) | (applyHeightToLastDescent ? 0 : 1 << 1); } @override diff --git a/third_party/txt/src/txt/paragraph_style.h b/third_party/txt/src/txt/paragraph_style.h index af8b9e1a40345..63836d9c05c63 100644 --- a/third_party/txt/src/txt/paragraph_style.h +++ b/third_party/txt/src/txt/paragraph_style.h @@ -49,10 +49,10 @@ enum class TextDirection { // The default behavior is kAll where height adjustments are enabled for all // lines. // -// Multiple behaviors can be applied at once with a bitwise & operator. For +// Multiple behaviors can be applied at once with a bitwise | operator. For // example, disabling first and last can achieved with: // -// (kDisableFirst & kDisableLast). +// (kDisableFirst | kDisableLast). enum TextHeightBehavior { kAll = 0x0, kDisableFirstAscent = 0x1, From 5ff8cf761f4f66cd97d8c4daf01a84d575a6fcfe Mon Sep 17 00:00:00 2001 From: GaryQian Date: Thu, 23 Jan 2020 14:55:33 -0800 Subject: [PATCH 31/33] MOre missed renaming --- lib/ui/text/paragraph_builder.cc | 2 +- third_party/txt/src/txt/paragraph_style.h | 2 +- third_party/txt/src/txt/paragraph_txt.cc | 6 +++--- third_party/txt/tests/paragraph_unittests.cc | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/ui/text/paragraph_builder.cc b/lib/ui/text/paragraph_builder.cc index d7514ace9f8cc..51739fc2cf3fd 100644 --- a/lib/ui/text/paragraph_builder.cc +++ b/lib/ui/text/paragraph_builder.cc @@ -268,7 +268,7 @@ ParagraphBuilder::ParagraphBuilder( } if (mask & psTextHeightBehaviorMask) { - style.height_behavior = encoded[psTextHeightBehaviorIndex]; + style.text_height_behavior = encoded[psTextHeightBehaviorIndex]; } if (mask & psStrutStyleMask) { diff --git a/third_party/txt/src/txt/paragraph_style.h b/third_party/txt/src/txt/paragraph_style.h index 63836d9c05c63..948cdf2dac5d8 100644 --- a/third_party/txt/src/txt/paragraph_style.h +++ b/third_party/txt/src/txt/paragraph_style.h @@ -69,7 +69,7 @@ class ParagraphStyle { std::string font_family = ""; double font_size = 14; double height = 1; - size_t height_behavior = TextHeightBehavior::kAll; + size_t text_height_behavior = TextHeightBehavior::kAll; bool has_height_override = false; // Strut properties. strut_enabled must be set to true for the rest of the diff --git a/third_party/txt/src/txt/paragraph_txt.cc b/third_party/txt/src/txt/paragraph_txt.cc index 491f0f4015926..0554091ceeb72 100644 --- a/third_party/txt/src/txt/paragraph_txt.cc +++ b/third_party/txt/src/txt/paragraph_txt.cc @@ -1246,16 +1246,16 @@ void ParagraphTxt::UpdateLineMetrics(const SkFontMetrics& metrics, descent = (metrics.fDescent + metrics.fLeading / 2); } - // Account for height_behavior in paragraph_style_. + // Account for text_height_behavior in paragraph_style_. // // Disable first line ascent modifications. - if (line_number == 0 && paragraph_style_.height_behavior & + if (line_number == 0 && paragraph_style_.text_height_behavior & TextHeightBehavior::kDisableFirstAscent) { ascent = -metrics.fAscent; } // Disable last line descent modifications. if (line_number == line_limit - 1 && - paragraph_style_.height_behavior & + paragraph_style_.text_height_behavior & TextHeightBehavior::kDisableLastDescent) { descent = metrics.fDescent; } diff --git a/third_party/txt/tests/paragraph_unittests.cc b/third_party/txt/tests/paragraph_unittests.cc index c9b339ef15d4a..6122be579929b 100644 --- a/third_party/txt/tests/paragraph_unittests.cc +++ b/third_party/txt/tests/paragraph_unittests.cc @@ -5887,7 +5887,7 @@ TEST_F(ParagraphTest, TextHeightBehaviorRectsParagraph) { icu_text.getBuffer() + icu_text.length()); txt::ParagraphStyle paragraph_style; - paragraph_style.height_behavior = + paragraph_style.text_height_behavior = txt::TextHeightBehavior::kDisableFirstAscent | txt::TextHeightBehavior::kDisableLastDescent; From 30bfb5465ca68b72abd161d67023aaa91766f9fb Mon Sep 17 00:00:00 2001 From: GaryQian Date: Thu, 23 Jan 2020 14:58:52 -0800 Subject: [PATCH 32/33] Apply naming changes to comments --- third_party/txt/src/txt/paragraph_style.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/third_party/txt/src/txt/paragraph_style.h b/third_party/txt/src/txt/paragraph_style.h index 948cdf2dac5d8..46ea7a1e7c1a8 100644 --- a/third_party/txt/src/txt/paragraph_style.h +++ b/third_party/txt/src/txt/paragraph_style.h @@ -50,9 +50,9 @@ enum class TextDirection { // lines. // // Multiple behaviors can be applied at once with a bitwise | operator. For -// example, disabling first and last can achieved with: +// example, disabling first ascent and last descent can achieved with: // -// (kDisableFirst | kDisableLast). +// (kDisableFirstAscent | kDisableLastDescent). enum TextHeightBehavior { kAll = 0x0, kDisableFirstAscent = 0x1, From abbaafbc67c9fab6904b049a71d4029aa141ce32 Mon Sep 17 00:00:00 2001 From: GaryQian Date: Thu, 23 Jan 2020 15:34:28 -0800 Subject: [PATCH 33/33] Fix dart unittests --- testing/dart/text_test.dart | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/testing/dart/text_test.dart b/testing/dart/text_test.dart index 988cfada3428e..2ca0d726f9fdb 100644 --- a/testing/dart/text_test.dart +++ b/testing/dart/text_test.dart @@ -55,17 +55,17 @@ void main() { }); test('encode works', () { - expect(behavior0.encode(), equals(3)); - expect(behavior1.encode(), equals(0)); - expect(behavior2.encode(), equals(2)); - expect(behavior3.encode(), equals(1)); + expect(behavior0.encode(), equals(0)); + expect(behavior1.encode(), equals(3)); + expect(behavior2.encode(), equals(1)); + expect(behavior3.encode(), equals(2)); }); - test('encode works', () { - expect(const TextHeightBehavior.fromEncoded(3), equals(behavior0)); - expect(const TextHeightBehavior.fromEncoded(0), equals(behavior1)); - expect(const TextHeightBehavior.fromEncoded(2), equals(behavior2)); - expect(const TextHeightBehavior.fromEncoded(1), equals(behavior3)); + test('decode works', () { + expect(const TextHeightBehavior.fromEncoded(0), equals(behavior0)); + expect(const TextHeightBehavior.fromEncoded(3), equals(behavior1)); + expect(const TextHeightBehavior.fromEncoded(1), equals(behavior2)); + expect(const TextHeightBehavior.fromEncoded(2), equals(behavior3)); }); test('toString works', () {