From 912cb0aa6bf4e65862bd36adeec34c0bcaec3fdc Mon Sep 17 00:00:00 2001
From: GaryQian <garyq@google.com>
Date: Tue, 21 May 2019 17:22:04 -0700
Subject: [PATCH 01/10] Initial height override impl

---
 lib/ui/text/paragraph_builder.cc             |   3 +
 third_party/txt/src/txt/paragraph.cc         |  24 +++-
 third_party/txt/src/txt/paragraph_style.cc   |   5 +-
 third_party/txt/src/txt/paragraph_style.h    |   2 +
 third_party/txt/src/txt/text_style.cc        |   3 +
 third_party/txt/src/txt/text_style.h         |   1 +
 third_party/txt/tests/paragraph_unittests.cc | 140 +++++++++----------
 7 files changed, 96 insertions(+), 82 deletions(-)

diff --git a/lib/ui/text/paragraph_builder.cc b/lib/ui/text/paragraph_builder.cc
index 1682cbe41473c..bef66a700b98e 100644
--- a/lib/ui/text/paragraph_builder.cc
+++ b/lib/ui/text/paragraph_builder.cc
@@ -204,6 +204,7 @@ void decodeStrut(Dart_Handle strut_data,
   }
   if (mask & sHeightMask) {
     paragraph_style.strut_height = float_data[float_count++];
+    paragraph_style.strut_has_height_override = true;
   }
   if (mask & sLeadingMask) {
     paragraph_style.strut_leading = float_data[float_count++];
@@ -261,6 +262,7 @@ ParagraphBuilder::ParagraphBuilder(
 
   if (mask & psHeightMask) {
     style.height = height;
+    style.has_height_override = true;
   }
 
   if (mask & psStrutStyleMask) {
@@ -401,6 +403,7 @@ void ParagraphBuilder::pushStyle(tonic::Int32List& encoded,
 
   if (mask & tsHeightMask) {
     style.height = height;
+    style.has_height_override = true;
   }
 
   if (mask & tsLocaleMask) {
diff --git a/third_party/txt/src/txt/paragraph.cc b/third_party/txt/src/txt/paragraph.cc
index 33712f77161ef..79dfb8c93f5ec 100644
--- a/third_party/txt/src/txt/paragraph.cc
+++ b/third_party/txt/src/txt/paragraph.cc
@@ -1042,15 +1042,25 @@ void Paragraph::Layout(double width, bool force) {
                                    const TextStyle& style,
                                    PlaceholderRun* placeholder_run) {
       if (!strut_.force_strut) {
-        double ascent =
-            (-metrics.fAscent + metrics.fLeading / 2) * style.height;
-        double descent =
-            (metrics.fDescent + metrics.fLeading / 2) * style.height;
-
-        ComputePlaceholder(placeholder_run, ascent, descent);
-
+        double ascent;
+        double descent;
+        if (style.has_height_override) {
+          // Scale the ascent and descent such that the sum of ascent and
+          // descent is `fontsize * style.height * style.font_size`.
+          double metrics_height = -metrics.fAscent + metrics.fDescent;
+          ascent = (-metrics.fAscent / metrics_height) * style.height *
+                   style.font_size;
+          descent = (metrics.fDescent / metrics_height) * style.height *
+                    style.font_size;
+        } else {
+          // Use the font-provided ascent, descent, and leading directly.
+          ascent = (-metrics.fAscent + metrics.fLeading / 2);
+          descent = (metrics.fDescent + metrics.fLeading / 2);
+        }
         max_ascent = std::max(ascent, max_ascent);
         max_descent = std::max(descent, max_descent);
+
+        ComputePlaceholder(placeholder_run, ascent, descent);
       }
 
       max_unscaled_ascent = std::max(placeholder_run == nullptr
diff --git a/third_party/txt/src/txt/paragraph_style.cc b/third_party/txt/src/txt/paragraph_style.cc
index 933a8db6d6123..06a4feca71c87 100644
--- a/third_party/txt/src/txt/paragraph_style.cc
+++ b/third_party/txt/src/txt/paragraph_style.cc
@@ -14,10 +14,10 @@
  * limitations under the License.
  */
 
-#include <vector>
-
 #include "paragraph_style.h"
 
+#include <vector>
+
 namespace txt {
 
 TextStyle ParagraphStyle::GetTextStyle() const {
@@ -30,6 +30,7 @@ TextStyle ParagraphStyle::GetTextStyle() const {
   }
   result.locale = locale;
   result.height = height;
+  result.has_height_override = has_height_override;
   return result;
 }
 
diff --git a/third_party/txt/src/txt/paragraph_style.h b/third_party/txt/src/txt/paragraph_style.h
index 514037d41fd93..9ca5fae3ca9c4 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;
+  bool has_height_override = false;
 
   // Strut properties. strut_enabled must be set to true for the rest of the
   // properties to take effect.
@@ -60,6 +61,7 @@ class ParagraphStyle {
   std::vector<std::string> strut_font_families;
   double strut_font_size = 14;
   double strut_height = 1;
+  bool strut_has_height_override = false;
   double strut_leading = -1;  // Negative to use font's default leading. [0,inf)
                               // to use custom leading as a ratio of font size.
   bool force_strut_height = false;
diff --git a/third_party/txt/src/txt/text_style.cc b/third_party/txt/src/txt/text_style.cc
index 194dde47882e3..8ca0c42a1e4cb 100644
--- a/third_party/txt/src/txt/text_style.cc
+++ b/third_party/txt/src/txt/text_style.cc
@@ -15,6 +15,7 @@
  */
 
 #include "text_style.h"
+
 #include "font_style.h"
 #include "font_weight.h"
 #include "third_party/skia/include/core/SkColor.h"
@@ -46,6 +47,8 @@ bool TextStyle::equals(const TextStyle& other) const {
     return false;
   if (height != other.height)
     return false;
+  if (has_height_override != other.has_height_override)
+    return false;
   if (locale != other.locale)
     return false;
   if (foreground != other.foreground)
diff --git a/third_party/txt/src/txt/text_style.h b/third_party/txt/src/txt/text_style.h
index 83c814fd2027f..0d4a92a099814 100644
--- a/third_party/txt/src/txt/text_style.h
+++ b/third_party/txt/src/txt/text_style.h
@@ -51,6 +51,7 @@ class TextStyle {
   double letter_spacing = 0.0;
   double word_spacing = 0.0;
   double height = 1.0;
+  bool has_height_override = false;
   std::string locale;
   bool has_background = false;
   SkPaint background;
diff --git a/third_party/txt/tests/paragraph_unittests.cc b/third_party/txt/tests/paragraph_unittests.cc
index 1a073080b5bb0..7503bf7d1baf5 100644
--- a/third_party/txt/tests/paragraph_unittests.cc
+++ b/third_party/txt/tests/paragraph_unittests.cc
@@ -2389,9 +2389,6 @@ TEST_F(ParagraphTest, DISABLE_ON_WINDOWS(GetRectsForRangeTight)) {
 
 TEST_F(ParagraphTest,
        DISABLE_ON_WINDOWS(GetRectsForRangeIncludeLineSpacingMiddle)) {
-  // const char* text =
-  //     "12345,  \"67890\" 12345 67890 12345 67890 12345 67890 12345 67890
-  //     12345 " "67890 12345";
   const char* text =
       "(　´･‿･｀)(　´･‿･｀)(　´･‿･｀)(　´･‿･｀)(　´･‿･｀)(　´･‿･｀)(　´･‿･｀)("
       "　´･‿･｀)(　´･‿･｀)(　´･‿･｀)(　´･‿･｀)(　´･‿･｀)(　´･‿･｀)(　´･‿･｀)("
@@ -2407,13 +2404,13 @@ TEST_F(ParagraphTest,
 
   txt::TextStyle text_style;
   text_style.font_families = std::vector<std::string>(1, "Roboto");
-  // text_style.font_families = std::vector<std::string>(1, "Roboto");
   text_style.font_size = 50;
   text_style.letter_spacing = 0;
   text_style.font_weight = FontWeight::w500;
   text_style.word_spacing = 0;
   text_style.color = SK_ColorBLACK;
-  text_style.height = 1.3;
+  text_style.height = 1.6;
+  text_style.has_height_override = true;
   builder.PushStyle(text_style);
 
   builder.AddText(u16_text);
@@ -2451,9 +2448,9 @@ TEST_F(ParagraphTest,
   }
   EXPECT_EQ(boxes.size(), 1ull);
   EXPECT_FLOAT_EQ(boxes[0].rect.left(), 0);
-  EXPECT_FLOAT_EQ(boxes[0].rect.top(), 13.744141);
+  EXPECT_FLOAT_EQ(boxes[0].rect.top(), 16.946615);
   EXPECT_FLOAT_EQ(boxes[0].rect.right(), 17.429688);
-  EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 82.958008);
+  EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 88.473305);
 
   paint.setColor(SK_ColorBLUE);
   boxes =
@@ -2463,9 +2460,9 @@ TEST_F(ParagraphTest,
   }
   EXPECT_EQ(boxes.size(), 1ull);
   EXPECT_FLOAT_EQ(boxes[0].rect.left(), 67.429688);
-  EXPECT_FLOAT_EQ(boxes[0].rect.top(), 13.744141);
+  EXPECT_FLOAT_EQ(boxes[0].rect.top(), 16.946615);
   EXPECT_FLOAT_EQ(boxes[0].rect.right(), 190.00781);
-  EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 82.958008);
+  EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 88.473305);
 
   paint.setColor(SK_ColorGREEN);
   boxes =
@@ -2475,9 +2472,9 @@ TEST_F(ParagraphTest,
   }
   EXPECT_EQ(boxes.size(), 1ull);
   EXPECT_FLOAT_EQ(boxes[0].rect.left(), 190.00781);
-  EXPECT_FLOAT_EQ(boxes[0].rect.top(), 13.744141);
+  EXPECT_FLOAT_EQ(boxes[0].rect.top(), 16.946615);
   EXPECT_FLOAT_EQ(boxes[0].rect.right(), 508.0625);
-  EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 82.958008);
+  EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 88.473312);
 
   paint.setColor(SK_ColorRED);
   boxes =
@@ -2487,34 +2484,34 @@ TEST_F(ParagraphTest,
   }
   EXPECT_EQ(boxes.size(), 8ull);
   EXPECT_FLOAT_EQ(boxes[0].rect.left(), 190.00781);
-  EXPECT_FLOAT_EQ(boxes[0].rect.top(), 82.786133);
+  EXPECT_FLOAT_EQ(boxes[0].rect.top(), 88.473312);
   EXPECT_FLOAT_EQ(boxes[0].rect.right(), 525.6875);
-  EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 158.95801);
+  EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 168.47331);
 
   EXPECT_FLOAT_EQ(boxes[1].rect.left(), 525.6875);
-  EXPECT_FLOAT_EQ(boxes[1].rect.top(), 82.786133);
+  EXPECT_FLOAT_EQ(boxes[1].rect.top(), 88.473312);
   EXPECT_FLOAT_EQ(boxes[1].rect.right(), 570.02344);
-  EXPECT_FLOAT_EQ(boxes[1].rect.bottom(), 158.95801);
+  EXPECT_FLOAT_EQ(boxes[1].rect.bottom(), 168.4733);
 
   EXPECT_FLOAT_EQ(boxes[2].rect.left(), 0);
-  EXPECT_FLOAT_EQ(boxes[2].rect.top(), 158.78613);
+  EXPECT_FLOAT_EQ(boxes[2].rect.top(), 168.4733);
   EXPECT_FLOAT_EQ(boxes[2].rect.right(), 531.57422);
-  EXPECT_FLOAT_EQ(boxes[2].rect.bottom(), 234.95801);
+  EXPECT_FLOAT_EQ(boxes[2].rect.bottom(), 248.47331);
 
   EXPECT_FLOAT_EQ(boxes[3].rect.left(), 531.57422);
-  EXPECT_FLOAT_EQ(boxes[3].rect.top(), 158.78613);
+  EXPECT_FLOAT_EQ(boxes[3].rect.top(), 168.4733);
   EXPECT_FLOAT_EQ(boxes[3].rect.right(), 570.02344);
-  EXPECT_FLOAT_EQ(boxes[3].rect.bottom(), 234.95801);
+  EXPECT_FLOAT_EQ(boxes[3].rect.bottom(), 248.47331);
 
   EXPECT_FLOAT_EQ(boxes[4].rect.left(), 0);
-  EXPECT_FLOAT_EQ(boxes[4].rect.top(), 234.78613);
+  EXPECT_FLOAT_EQ(boxes[4].rect.top(), 248.47331);
   EXPECT_FLOAT_EQ(boxes[4].rect.right(), 570.02344);
-  EXPECT_FLOAT_EQ(boxes[4].rect.bottom(), 310.95801);
+  EXPECT_FLOAT_EQ(boxes[4].rect.bottom(), 328.4733);
 
   EXPECT_FLOAT_EQ(boxes[5].rect.left(), 0);
-  EXPECT_FLOAT_EQ(boxes[5].rect.top(), 310.78613);
+  EXPECT_FLOAT_EQ(boxes[5].rect.top(), 328.47333);
   EXPECT_FLOAT_EQ(boxes[5].rect.right(), 570.02344);
-  EXPECT_FLOAT_EQ(boxes[5].rect.bottom(), 386.95801);
+  EXPECT_FLOAT_EQ(boxes[5].rect.bottom(), 408.4733);
 
   paint.setColor(SK_ColorBLUE);
   boxes =
@@ -2524,9 +2521,9 @@ TEST_F(ParagraphTest,
   }
   EXPECT_EQ(boxes.size(), 1ull);
   EXPECT_FLOAT_EQ(boxes[0].rect.left(), 463.72656);
-  EXPECT_FLOAT_EQ(boxes[0].rect.top(), 13.744141);
+  EXPECT_FLOAT_EQ(boxes[0].rect.top(), 16.946615);
   EXPECT_FLOAT_EQ(boxes[0].rect.right(), 530.23047);
-  EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 82.958008);
+  EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 88.473305);
 
   paint.setColor(SK_ColorRED);
   boxes =
@@ -2541,9 +2538,6 @@ TEST_F(ParagraphTest,
 
 TEST_F(ParagraphTest,
        DISABLE_ON_WINDOWS(GetRectsForRangeIncludeLineSpacingTop)) {
-  // const char* text =
-  //     "12345,  \"67890\" 12345 67890 12345 67890 12345 67890 12345 67890
-  //     12345 " "67890 12345";
   const char* text =
       "(　´･‿･｀)(　´･‿･｀)(　´･‿･｀)(　´･‿･｀)(　´･‿･｀)(　´･‿･｀)(　´･‿･｀)("
       "　´･‿･｀)(　´･‿･｀)(　´･‿･｀)(　´･‿･｀)(　´･‿･｀)(　´･‿･｀)(　´･‿･｀)("
@@ -2564,7 +2558,8 @@ TEST_F(ParagraphTest,
   text_style.font_weight = FontWeight::w500;
   text_style.word_spacing = 0;
   text_style.color = SK_ColorBLACK;
-  text_style.height = 1.3;
+  text_style.height = 1.6;
+  text_style.has_height_override = true;
   builder.PushStyle(text_style);
 
   builder.AddText(u16_text);
@@ -2602,9 +2597,9 @@ TEST_F(ParagraphTest,
   }
   EXPECT_EQ(boxes.size(), 1ull);
   EXPECT_FLOAT_EQ(boxes[0].rect.left(), 0);
-  EXPECT_FLOAT_EQ(boxes[0].rect.top(), 13.744141);
+  EXPECT_FLOAT_EQ(boxes[0].rect.top(), 16.946615);
   EXPECT_FLOAT_EQ(boxes[0].rect.right(), 17.429688);
-  EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 76);
+  EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 80);
 
   paint.setColor(SK_ColorBLUE);
   boxes =
@@ -2614,9 +2609,9 @@ TEST_F(ParagraphTest,
   }
   EXPECT_EQ(boxes.size(), 1ull);
   EXPECT_FLOAT_EQ(boxes[0].rect.left(), 67.429688);
-  EXPECT_FLOAT_EQ(boxes[0].rect.top(), 13.744141);
+  EXPECT_FLOAT_EQ(boxes[0].rect.top(), 16.946615);
   EXPECT_FLOAT_EQ(boxes[0].rect.right(), 190.00781);
-  EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 76);
+  EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 80);
 
   paint.setColor(SK_ColorGREEN);
   boxes =
@@ -2626,9 +2621,9 @@ TEST_F(ParagraphTest,
   }
   EXPECT_EQ(boxes.size(), 1ull);
   EXPECT_FLOAT_EQ(boxes[0].rect.left(), 190.00781);
-  EXPECT_FLOAT_EQ(boxes[0].rect.top(), 13.744141);
+  EXPECT_FLOAT_EQ(boxes[0].rect.top(), 16.946615);
   EXPECT_FLOAT_EQ(boxes[0].rect.right(), 508.0625);
-  EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 76);
+  EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 80);
 
   paint.setColor(SK_ColorRED);
   boxes =
@@ -2638,34 +2633,34 @@ TEST_F(ParagraphTest,
   }
   EXPECT_EQ(boxes.size(), 8ull);
   EXPECT_FLOAT_EQ(boxes[0].rect.left(), 190.00781);
-  EXPECT_FLOAT_EQ(boxes[0].rect.top(), 75.828125);
+  EXPECT_FLOAT_EQ(boxes[0].rect.top(), 80);
   EXPECT_FLOAT_EQ(boxes[0].rect.right(), 525.6875);
-  EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 152);
+  EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 160);
 
   EXPECT_FLOAT_EQ(boxes[1].rect.left(), 525.6875);
-  EXPECT_FLOAT_EQ(boxes[1].rect.top(), 75.828125);
+  EXPECT_FLOAT_EQ(boxes[1].rect.top(), 80);
   EXPECT_FLOAT_EQ(boxes[1].rect.right(), 570.02344);
-  EXPECT_FLOAT_EQ(boxes[1].rect.bottom(), 152);
+  EXPECT_FLOAT_EQ(boxes[1].rect.bottom(), 160);
 
   EXPECT_FLOAT_EQ(boxes[2].rect.left(), 0);
-  EXPECT_FLOAT_EQ(boxes[2].rect.top(), 151.82812);
+  EXPECT_FLOAT_EQ(boxes[2].rect.top(), 160);
   EXPECT_FLOAT_EQ(boxes[2].rect.right(), 531.57422);
-  EXPECT_FLOAT_EQ(boxes[2].rect.bottom(), 228);
+  EXPECT_FLOAT_EQ(boxes[2].rect.bottom(), 240);
 
   EXPECT_FLOAT_EQ(boxes[3].rect.left(), 531.57422);
-  EXPECT_FLOAT_EQ(boxes[3].rect.top(), 151.82812);
+  EXPECT_FLOAT_EQ(boxes[3].rect.top(), 160);
   EXPECT_FLOAT_EQ(boxes[3].rect.right(), 570.02344);
-  EXPECT_FLOAT_EQ(boxes[3].rect.bottom(), 228);
+  EXPECT_FLOAT_EQ(boxes[3].rect.bottom(), 240);
 
   EXPECT_FLOAT_EQ(boxes[4].rect.left(), 0);
-  EXPECT_FLOAT_EQ(boxes[4].rect.top(), 227.82812);
+  EXPECT_FLOAT_EQ(boxes[4].rect.top(), 240);
   EXPECT_FLOAT_EQ(boxes[4].rect.right(), 570.02344);
-  EXPECT_FLOAT_EQ(boxes[4].rect.bottom(), 304);
+  EXPECT_FLOAT_EQ(boxes[4].rect.bottom(), 320);
 
   EXPECT_FLOAT_EQ(boxes[5].rect.left(), 0);
-  EXPECT_FLOAT_EQ(boxes[5].rect.top(), 303.82812);
+  EXPECT_FLOAT_EQ(boxes[5].rect.top(), 320);
   EXPECT_FLOAT_EQ(boxes[5].rect.right(), 570.02344);
-  EXPECT_FLOAT_EQ(boxes[5].rect.bottom(), 380);
+  EXPECT_FLOAT_EQ(boxes[5].rect.bottom(), 400);
 
   paint.setColor(SK_ColorBLUE);
   boxes =
@@ -2675,9 +2670,9 @@ TEST_F(ParagraphTest,
   }
   EXPECT_EQ(boxes.size(), 1ull);
   EXPECT_FLOAT_EQ(boxes[0].rect.left(), 463.72656);
-  EXPECT_FLOAT_EQ(boxes[0].rect.top(), 13.744141);
+  EXPECT_FLOAT_EQ(boxes[0].rect.top(), 16.946615);
   EXPECT_FLOAT_EQ(boxes[0].rect.right(), 530.23047);
-  EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 76);
+  EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 80);
 
   paint.setColor(SK_ColorRED);
   boxes =
@@ -2692,9 +2687,6 @@ TEST_F(ParagraphTest,
 
 TEST_F(ParagraphTest,
        DISABLE_ON_WINDOWS(GetRectsForRangeIncludeLineSpacingBottom)) {
-  // const char* text =
-  //     "12345,  \"67890\" 12345 67890 12345 67890 12345 67890 12345 67890
-  //     12345 " "67890 12345";
   const char* text =
       "(　´･‿･｀)(　´･‿･｀)(　´･‿･｀)(　´･‿･｀)(　´･‿･｀)(　´･‿･｀)(　´･‿･｀)("
       "　´･‿･｀)(　´･‿･｀)(　´･‿･｀)(　´･‿･｀)(　´･‿･｀)(　´･‿･｀)(　´･‿･｀)("
@@ -2715,7 +2707,8 @@ TEST_F(ParagraphTest,
   text_style.font_weight = FontWeight::w500;
   text_style.word_spacing = 0;
   text_style.color = SK_ColorBLACK;
-  text_style.height = 1.3;
+  text_style.height = 1.6;
+  text_style.has_height_override = true;
   builder.PushStyle(text_style);
 
   builder.AddText(u16_text);
@@ -2753,9 +2746,9 @@ TEST_F(ParagraphTest,
   }
   EXPECT_EQ(boxes.size(), 1ull);
   EXPECT_FLOAT_EQ(boxes[0].rect.left(), 0);
-  EXPECT_FLOAT_EQ(boxes[0].rect.top(), 13.744141);
+  EXPECT_FLOAT_EQ(boxes[0].rect.top(), 16.946615);
   EXPECT_FLOAT_EQ(boxes[0].rect.right(), 17.429688);
-  EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 89.916016);
+  EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 96.946609);
 
   paint.setColor(SK_ColorBLUE);
   boxes =
@@ -2765,9 +2758,9 @@ TEST_F(ParagraphTest,
   }
   EXPECT_EQ(boxes.size(), 1ull);
   EXPECT_FLOAT_EQ(boxes[0].rect.left(), 67.429688);
-  EXPECT_FLOAT_EQ(boxes[0].rect.top(), 13.744141);
+  EXPECT_FLOAT_EQ(boxes[0].rect.top(), 16.946615);
   EXPECT_FLOAT_EQ(boxes[0].rect.right(), 190.00781);
-  EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 89.916016);
+  EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 96.946609);
 
   paint.setColor(SK_ColorGREEN);
   boxes =
@@ -2777,9 +2770,9 @@ TEST_F(ParagraphTest,
   }
   EXPECT_EQ(boxes.size(), 1ull);
   EXPECT_FLOAT_EQ(boxes[0].rect.left(), 190.00781);
-  EXPECT_FLOAT_EQ(boxes[0].rect.top(), 13.744141);
+  EXPECT_FLOAT_EQ(boxes[0].rect.top(), 16.946615);
   EXPECT_FLOAT_EQ(boxes[0].rect.right(), 508.0625);
-  EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 89.916016);
+  EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 96.946609);
 
   paint.setColor(SK_ColorRED);
   boxes =
@@ -2789,34 +2782,34 @@ TEST_F(ParagraphTest,
   }
   EXPECT_EQ(boxes.size(), 8ull);
   EXPECT_FLOAT_EQ(boxes[0].rect.left(), 190.00781);
-  EXPECT_FLOAT_EQ(boxes[0].rect.top(), 89.744141);
+  EXPECT_FLOAT_EQ(boxes[0].rect.top(), 96.946617);
   EXPECT_FLOAT_EQ(boxes[0].rect.right(), 525.6875);
-  EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 165.91602);
+  EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 176.94661);
 
   EXPECT_FLOAT_EQ(boxes[1].rect.left(), 525.6875);
-  EXPECT_FLOAT_EQ(boxes[1].rect.top(), 89.744141);
+  EXPECT_FLOAT_EQ(boxes[1].rect.top(), 96.946617);
   EXPECT_FLOAT_EQ(boxes[1].rect.right(), 570.02344);
-  EXPECT_FLOAT_EQ(boxes[1].rect.bottom(), 165.91602);
+  EXPECT_FLOAT_EQ(boxes[1].rect.bottom(), 176.94661);
 
   EXPECT_FLOAT_EQ(boxes[2].rect.left(), 0);
-  EXPECT_FLOAT_EQ(boxes[2].rect.top(), 165.74414);
+  EXPECT_FLOAT_EQ(boxes[2].rect.top(), 176.94661);
   EXPECT_FLOAT_EQ(boxes[2].rect.right(), 531.57422);
-  EXPECT_FLOAT_EQ(boxes[2].rect.bottom(), 241.91602);
+  EXPECT_FLOAT_EQ(boxes[2].rect.bottom(), 256.94662);
 
   EXPECT_FLOAT_EQ(boxes[3].rect.left(), 531.57422);
-  EXPECT_FLOAT_EQ(boxes[3].rect.top(), 165.74414);
+  EXPECT_FLOAT_EQ(boxes[3].rect.top(), 176.94661);
   EXPECT_FLOAT_EQ(boxes[3].rect.right(), 570.02344);
-  EXPECT_FLOAT_EQ(boxes[3].rect.bottom(), 241.91602);
+  EXPECT_FLOAT_EQ(boxes[3].rect.bottom(), 256.94662);
 
   EXPECT_FLOAT_EQ(boxes[4].rect.left(), 0);
-  EXPECT_FLOAT_EQ(boxes[4].rect.top(), 241.74414);
+  EXPECT_FLOAT_EQ(boxes[4].rect.top(), 256.94662);
   EXPECT_FLOAT_EQ(boxes[4].rect.right(), 570.02344);
-  EXPECT_FLOAT_EQ(boxes[4].rect.bottom(), 317.91602);
+  EXPECT_FLOAT_EQ(boxes[4].rect.bottom(), 336.94662);
 
   EXPECT_FLOAT_EQ(boxes[5].rect.left(), 0);
-  EXPECT_FLOAT_EQ(boxes[5].rect.top(), 317.74414);
+  EXPECT_FLOAT_EQ(boxes[5].rect.top(), 336.94662);
   EXPECT_FLOAT_EQ(boxes[5].rect.right(), 570.02344);
-  EXPECT_FLOAT_EQ(boxes[5].rect.bottom(), 393.91602);
+  EXPECT_FLOAT_EQ(boxes[5].rect.bottom(), 416.94662);
 
   paint.setColor(SK_ColorBLUE);
   boxes =
@@ -2826,9 +2819,9 @@ TEST_F(ParagraphTest,
   }
   EXPECT_EQ(boxes.size(), 1ull);
   EXPECT_FLOAT_EQ(boxes[0].rect.left(), 463.72656);
-  EXPECT_FLOAT_EQ(boxes[0].rect.top(), 13.744141);
+  EXPECT_FLOAT_EQ(boxes[0].rect.top(), 16.946615);
   EXPECT_FLOAT_EQ(boxes[0].rect.right(), 530.23047);
-  EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 89.916016);
+  EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 96.946609);
 
   paint.setColor(SK_ColorRED);
   boxes =
@@ -3365,6 +3358,7 @@ TEST_F(ParagraphTest, GetWordBoundaryParagraph) {
   text_style.word_spacing = 5;
   text_style.color = SK_ColorBLACK;
   text_style.height = 1.5;
+  text_style.has_height_override = true;
   builder.PushStyle(text_style);
 
   builder.AddText(u16_text);

From 7eb7b7c5ff505c27e31ec9f104e311f47b57775a Mon Sep 17 00:00:00 2001
From: GaryQian <garyq@google.com>
Date: Wed, 22 May 2019 16:28:51 -0700
Subject: [PATCH 02/10] update docs to explain behavior

---
 lib/stub_ui/lib/src/ui/text.dart | 31 ++++++++++++++++---------------
 lib/ui/text.dart                 | 15 ++++++++++-----
 2 files changed, 26 insertions(+), 20 deletions(-)

diff --git a/lib/stub_ui/lib/src/ui/text.dart b/lib/stub_ui/lib/src/ui/text.dart
index 8f9d3bdfd8f25..21ccd5e218d54 100644
--- a/lib/stub_ui/lib/src/ui/text.dart
+++ b/lib/stub_ui/lib/src/ui/text.dart
@@ -298,7 +298,8 @@ class TextStyle {
   /// * `letterSpacing`: The amount of space (in logical pixels) to add between each letter.
   /// * `wordSpacing`: The amount of space (in logical pixels) to add at each sequence of white-space (i.e. between each word).
   /// * `textBaseline`: The common baseline that should be aligned between this text span and its parent text span, or, for the root text spans, with the line box.
-  /// * `height`: The height of this text span, as a multiple of the font size.
+  /// * `height`: The height of this text span, as a multiplier of the font size. Omitting `height` will allow the text
+  ///   to take the height as defined by the font, which may not be exactly the height of the fontSize.
   /// * `locale`: The locale used to select region-specific glyphs.
   /// * `background`: The paint drawn as a background for the text.
   /// * `foreground`: The paint used to draw the text. If this is specified, `color` must be null.
@@ -623,13 +624,11 @@ class ParagraphStyle {
   /// * `fontSize`: The size of glyphs (in logical pixels) to use when painting
   ///   the text.
   ///
-  /// * `height`: The minimum height of the line boxes, as a multiple of the
-  ///   font size. The lines of the paragraph will be at least
-  ///   `(height + leading) * fontSize` tall when fontSize
-  ///   is not null. When fontSize is null, there is no minimum line height. Tall
-  ///   glyphs due to baseline alignment or large [TextStyle.fontSize] may cause
-  ///   the actual line height after layout to be taller than specified here.
-  ///   [fontSize] must be provided for this property to take effect.
+  /// * `height`: The fallback height of the spans as a multiplier of the font
+  ///   size. The fallback height is used when no height is provided in through
+  ///   [TextStyle.height]. Omitting `height` here and in [TextStyle] will allow
+  ///   the text to take the height as defined by the font, which may not be
+  ///   exactly the height of the fontSize.
   ///
   /// * `fontWeight`: The typeface thickness to use when painting the text
   ///   (e.g., bold).
@@ -770,13 +769,15 @@ class StrutStyle {
   /// * `fontSize`: The size of glyphs (in logical pixels) to use when painting
   ///   the text.
   ///
-  /// * `lineHeight`: The minimum height of the line boxes, as a multiple of the
-  ///   font size. The lines of the paragraph will be at least
-  ///   `(lineHeight + leading) * fontSize` tall when fontSize
-  ///   is not null. When fontSize is null, there is no minimum line height. Tall
-  ///   glyphs due to baseline alignment or large [TextStyle.fontSize] may cause
-  ///   the actual line height after layout to be taller than specified here.
-  ///   [fontSize] must be provided for this property to take effect.
+  /// * `height`: The minimum height of the line boxes, as a multiplier of the
+  ///   font size. The lines of the paragraph will be at least `(height + leading)
+  ///   * fontSize` tall when fontSize is not null. Omitting `height` will allow
+  ///   the minimum line height to take the height as defined by the font, which
+  ///   may not be exactly the height of the fontSize. When fontSize is null, there
+  ///   is no minimum line height. Tall glyphs due to baseline alignment or large
+  ///   [TextStyle.fontSize] may cause the actual line height after layout to be
+  ///   taller than specified here. [fontSize] must be provided for this property
+  ///   to take effect.
   ///
   /// * `leading`: The minimum amount of leading between lines as a multiple of
   ///   the font size. [fontSize] must be provided for this property to take effect.
diff --git a/lib/ui/text.dart b/lib/ui/text.dart
index bfdfd898e9c3b..6d98610e799a5 100644
--- a/lib/ui/text.dart
+++ b/lib/ui/text.dart
@@ -527,7 +527,8 @@ class TextStyle {
   /// * `letterSpacing`: The amount of space (in logical pixels) to add between each letter.
   /// * `wordSpacing`: The amount of space (in logical pixels) to add at each sequence of white-space (i.e. between each word).
   /// * `textBaseline`: The common baseline that should be aligned between this text span and its parent text span, or, for the root text spans, with the line box.
-  /// * `height`: The height of this text span, as a multiplier of the font size.
+  /// * `height`: The height of this text span, as a multiplier of the font size. Omitting `height` will allow the text
+  ///   to take the height as defined by the font, which may not be exactly the height of the fontSize.
   /// * `locale`: The locale used to select region-specific glyphs.
   /// * `background`: The paint drawn as a background for the text.
   /// * `foreground`: The paint used to draw the text. If this is specified, `color` must be null.
@@ -776,9 +777,11 @@ class ParagraphStyle {
   /// * `fontSize`: The fallback size of glyphs (in logical pixels) to
   ///   use when painting the text. This is used when there is no [TextStyle].
   ///
-  /// * `height`: The height of the spans as a multiplier of the font size. The
-  ///   fallback height to use when no height is provided in through
-  ///   [TextStyle.height].
+  /// * `height`: The fallback height of the spans as a multiplier of the font
+  ///   size. The fallback height is used when no height is provided in through
+  ///   [TextStyle.height]. Omitting `height` here and in [TextStyle] will allow
+  ///   the text to take the height as defined by the font, which may not be
+  ///   exactly the height of the fontSize.
   ///
   /// * `fontWeight`: The typeface thickness to use when painting the text
   ///   (e.g., bold).
@@ -970,7 +973,9 @@ class StrutStyle {
   ///
   /// * `height`: The minimum height of the line boxes, as a multiplier of the
   ///   font size. The lines of the paragraph will be at least `(height + leading)
-  ///   * fontSize` tall when fontSize is not null. When fontSize is null, there
+  ///   * fontSize` tall when fontSize is not null. Omitting `height` will allow
+  ///   the minimum line height to take the height as defined by the font, which
+  ///   may not be exactly the height of the fontSize. When fontSize is null, there
   ///   is no minimum line height. Tall glyphs due to baseline alignment or large
   ///   [TextStyle.fontSize] may cause the actual line height after layout to be
   ///   taller than specified here. [fontSize] must be provided for this property

From 45f63c1498de34e56e8f693d41e6546f2476beee Mon Sep 17 00:00:00 2001
From: GaryQian <garyq@google.com>
Date: Wed, 22 May 2019 17:02:37 -0700
Subject: [PATCH 03/10] Strut implementation

---
 third_party/txt/src/txt/paragraph.cc | 32 +++++++++++++++++++++-------
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/third_party/txt/src/txt/paragraph.cc b/third_party/txt/src/txt/paragraph.cc
index 79dfb8c93f5ec..f4038563305ea 100644
--- a/third_party/txt/src/txt/paragraph.cc
+++ b/third_party/txt/src/txt/paragraph.cc
@@ -507,14 +507,30 @@ void Paragraph::ComputeStrut(StrutMetrics* strut, SkFont& font) {
     SkFontMetrics strut_metrics;
     font.getMetrics(&strut_metrics);
 
-    strut->ascent = paragraph_style_.strut_height * -strut_metrics.fAscent;
-    strut->descent = paragraph_style_.strut_height * strut_metrics.fDescent;
-    strut->leading =
-        // Use font's leading if there is no user specified strut leading.
-        paragraph_style_.strut_leading < 0
-            ? strut_metrics.fLeading
-            : (paragraph_style_.strut_leading *
-               (strut_metrics.fDescent - strut_metrics.fAscent));
+    if (paragraph_style_.strut_has_height_override) {
+      double metrics_height = -strut_metrics.fAscent + strut_metrics.fDescent;
+      strut->ascent = (-strut_metrics.fAscent / metrics_height) *
+                      paragraph_style_.strut_height *
+                      paragraph_style_.strut_font_size;
+      strut->descent = (strut_metrics.fDescent / metrics_height) *
+                       paragraph_style_.strut_height *
+                       paragraph_style_.strut_font_size;
+      strut->leading =
+          // Zero leading if there is no user specified strut leading.
+          paragraph_style_.strut_leading < 0
+              ? 0
+              : (paragraph_style_.strut_leading *
+                 paragraph_style_.strut_font_size);
+    } else {
+      strut->ascent = paragraph_style_.strut_height * -strut_metrics.fAscent;
+      strut->descent = paragraph_style_.strut_height * strut_metrics.fDescent;
+      strut->leading =
+          // Use font's leading if there is no user specified strut leading.
+          paragraph_style_.strut_leading < 0
+              ? strut_metrics.fLeading
+              : (paragraph_style_.strut_leading *
+                 paragraph_style_.strut_font_size);
+    }
     strut->half_leading = strut->leading / 2;
     strut->line_height = strut->ascent + strut->descent + strut->leading;
   }

From 767d9a91d7a19450c8e42b44012a68eadd8e78de Mon Sep 17 00:00:00 2001
From: GaryQian <garyq@google.com>
Date: Wed, 22 May 2019 17:37:07 -0700
Subject: [PATCH 04/10] Add tests, fix strut impl

---
 third_party/txt/src/txt/paragraph.cc         |  4 +-
 third_party/txt/tests/paragraph_unittests.cc | 85 ++++++++++++++++++++
 2 files changed, 87 insertions(+), 2 deletions(-)

diff --git a/third_party/txt/src/txt/paragraph.cc b/third_party/txt/src/txt/paragraph.cc
index f4038563305ea..c854f2d8355a3 100644
--- a/third_party/txt/src/txt/paragraph.cc
+++ b/third_party/txt/src/txt/paragraph.cc
@@ -522,8 +522,8 @@ void Paragraph::ComputeStrut(StrutMetrics* strut, SkFont& font) {
               : (paragraph_style_.strut_leading *
                  paragraph_style_.strut_font_size);
     } else {
-      strut->ascent = paragraph_style_.strut_height * -strut_metrics.fAscent;
-      strut->descent = paragraph_style_.strut_height * strut_metrics.fDescent;
+      strut->ascent = -strut_metrics.fAscent;
+      strut->descent = strut_metrics.fDescent;
       strut->leading =
           // Use font's leading if there is no user specified strut leading.
           paragraph_style_.strut_leading < 0
diff --git a/third_party/txt/tests/paragraph_unittests.cc b/third_party/txt/tests/paragraph_unittests.cc
index 7503bf7d1baf5..26f350e84cf7f 100644
--- a/third_party/txt/tests/paragraph_unittests.cc
+++ b/third_party/txt/tests/paragraph_unittests.cc
@@ -4188,6 +4188,7 @@ TEST_F(ParagraphTest, DISABLE_ON_WINDOWS(StrutParagraph1)) {
   paragraph_style.strut_font_families.push_back("ahem");
   paragraph_style.strut_font_size = 50;
   paragraph_style.strut_height = 1.8;
+  paragraph_style.strut_has_height_override = true;
   paragraph_style.strut_leading = 0.1;
   paragraph_style.strut_enabled = true;
 
@@ -4315,6 +4316,7 @@ TEST_F(ParagraphTest, DISABLE_ON_WINDOWS(StrutParagraph2)) {
   paragraph_style.strut_font_families = std::vector<std::string>(1, "ahem");
   paragraph_style.strut_font_size = 50;
   paragraph_style.strut_height = 1.6;
+  paragraph_style.strut_has_height_override = true;
   paragraph_style.strut_enabled = true;
   txt::ParagraphBuilder builder(paragraph_style, GetTestFontCollection());
 
@@ -4440,6 +4442,7 @@ TEST_F(ParagraphTest, DISABLE_ON_WINDOWS(StrutParagraph3)) {
   paragraph_style.strut_font_families = std::vector<std::string>(1, "ahem");
   paragraph_style.strut_font_size = 50;
   paragraph_style.strut_height = 1.2;
+  paragraph_style.strut_has_height_override = true;
   paragraph_style.strut_enabled = true;
   txt::ParagraphBuilder builder(paragraph_style, GetTestFontCollection());
 
@@ -4565,6 +4568,7 @@ TEST_F(ParagraphTest, DISABLE_ON_WINDOWS(StrutForceParagraph)) {
   paragraph_style.strut_font_families = std::vector<std::string>(1, "ahem");
   paragraph_style.strut_font_size = 50;
   paragraph_style.strut_height = 1.5;
+  paragraph_style.strut_has_height_override = true;
   paragraph_style.strut_leading = 0.1;
   paragraph_style.force_strut_height = true;
   paragraph_style.strut_enabled = true;
@@ -4729,4 +4733,85 @@ TEST_F(ParagraphTest, FontFeaturesParagraph) {
   ASSERT_TRUE(Snapshot());
 }
 
+// The height override is disabled for this test. DIrect metrics from the font.
+TEST_F(ParagraphTest, DISABLE_ON_WINDOWS(StrutDefaultParagraph)) {
+  const char* text = "01234満毎冠行来昼本可\nabcd\n満毎冠行来昼本可";
+  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.max_lines = 10;
+  paragraph_style.strut_font_families = std::vector<std::string>(1, "ahem");
+  paragraph_style.strut_font_size = 50;
+  paragraph_style.strut_height = 1.5;
+  paragraph_style.strut_leading = 0.1;
+  paragraph_style.force_strut_height = false;
+  paragraph_style.strut_enabled = true;
+  txt::ParagraphBuilder builder(paragraph_style, GetTestFontCollection());
+
+  txt::TextStyle text_style;
+  text_style.font_families = std::vector<std::string>(1, "ahem");
+  text_style.font_families.push_back("ahem");
+  text_style.font_size = 20;
+  text_style.letter_spacing = 0;
+  text_style.word_spacing = 0;
+  text_style.color = SK_ColorBLACK;
+  text_style.height = 1;
+  builder.PushStyle(text_style);
+
+  builder.AddText(u16_text);
+
+  builder.Pop();
+
+  auto paragraph = builder.Build();
+  paragraph->Layout(550);
+
+  paragraph->Paint(GetCanvas(), 0, 0);
+
+  SkPaint paint;
+  paint.setStyle(SkPaint::kStroke_Style);
+  paint.setAntiAlias(true);
+  paint.setStrokeWidth(1);
+
+  // Tests for GetRectsForRange()
+  Paragraph::RectHeightStyle rect_height_style =
+      Paragraph::RectHeightStyle::kTight;
+  Paragraph::RectHeightStyle rect_height_strut_style =
+      Paragraph::RectHeightStyle::kStrut;
+  Paragraph::RectWidthStyle rect_width_style =
+      Paragraph::RectWidthStyle::kTight;
+  paint.setColor(SK_ColorRED);
+  std::vector<txt::Paragraph::TextBox> 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);
+
+  boxes =
+      paragraph->GetRectsForRange(0, 1, 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_NEAR(boxes[0].rect.top(), 26.5, 0.0001);
+  EXPECT_FLOAT_EQ(boxes[0].rect.right(), 20);
+  EXPECT_NEAR(boxes[0].rect.bottom(), 46.5, 0.0001);
+
+  boxes =
+      paragraph->GetRectsForRange(0, 2, rect_height_strut_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_NEAR(boxes[0].rect.top(), 2.5, 0.0001);
+  EXPECT_FLOAT_EQ(boxes[0].rect.right(), 40);
+  EXPECT_NEAR(boxes[0].rect.bottom(), 52.5, 0.0001);
+
+  ASSERT_TRUE(Snapshot());
+}
+
 }  // namespace txt

From fe71568c474aed8497a80ba8ec3d6f10a0cab62a Mon Sep 17 00:00:00 2001
From: GaryQian <garyq@google.com>
Date: Thu, 23 May 2019 10:54:59 -0700
Subject: [PATCH 05/10] Additional tests for height override

---
 third_party/txt/tests/paragraph_unittests.cc | 167 +++++++++++++------
 1 file changed, 114 insertions(+), 53 deletions(-)

diff --git a/third_party/txt/tests/paragraph_unittests.cc b/third_party/txt/tests/paragraph_unittests.cc
index 26f350e84cf7f..33201971003e9 100644
--- a/third_party/txt/tests/paragraph_unittests.cc
+++ b/third_party/txt/tests/paragraph_unittests.cc
@@ -1322,6 +1322,67 @@ TEST_F(ParagraphTest, BoldParagraph) {
                    boxes[boxes.size() - 1].rect.right() - boxes[0].rect.left());
 }
 
+TEST_F(ParagraphTest, DISABLE_ON_WINDOWS(HeightOverrideParagraph)) {
+  const char* text = "01234満毎冠行来昼本可\nabcd\n満毎冠行来昼本可";
+  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.max_lines = 10;
+  txt::ParagraphBuilder builder(paragraph_style, GetTestFontCollection());
+
+  txt::TextStyle text_style;
+  text_style.font_families = std::vector<std::string>(1, "Roboto");
+  text_style.font_size = 20;
+  text_style.letter_spacing = 0;
+  text_style.word_spacing = 0;
+  text_style.color = SK_ColorBLACK;
+  text_style.height = 3.6345;
+  text_style.has_height_override = true;
+  builder.PushStyle(text_style);
+
+  builder.AddText(u16_text);
+
+  builder.Pop();
+
+  auto paragraph = builder.Build();
+  paragraph->Layout(550);
+
+  paragraph->Paint(GetCanvas(), 0, 0);
+
+  SkPaint paint;
+  paint.setStyle(SkPaint::kStroke_Style);
+  paint.setAntiAlias(true);
+  paint.setStrokeWidth(1);
+
+  // Tests for GetRectsForRange()
+  Paragraph::RectHeightStyle rect_height_style =
+      Paragraph::RectHeightStyle::kIncludeLineSpacingMiddle;
+  Paragraph::RectWidthStyle rect_width_style =
+      Paragraph::RectWidthStyle::kTight;
+  paint.setColor(SK_ColorRED);
+  std::vector<txt::Paragraph::TextBox> 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);
+
+  boxes =
+      paragraph->GetRectsForRange(0, 40, 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(), 3ull);
+  EXPECT_FLOAT_EQ(boxes[1].rect.left(), 0);
+  EXPECT_NEAR(boxes[1].rect.top(), 92.805778503417969, 0.0001);
+  EXPECT_FLOAT_EQ(boxes[1].rect.right(), 43.84375);
+  EXPECT_NEAR(boxes[1].rect.bottom(), 165.49578857421875, 0.0001);
+
+  ASSERT_TRUE(Snapshot());
+}
+
 TEST_F(ParagraphTest, DISABLE_ON_WINDOWS(LeftAlignParagraph)) {
   const char* text =
       "This is a very long sentence to test if the text will properly wrap "
@@ -4683,57 +4744,7 @@ TEST_F(ParagraphTest, DISABLE_ON_WINDOWS(StrutForceParagraph)) {
   ASSERT_TRUE(Snapshot());
 }
 
-TEST_F(ParagraphTest, FontFeaturesParagraph) {
-  const char* text = "12ab\n";
-  auto icu_text = icu::UnicodeString::fromUTF8(text);
-  std::u16string u16_text(icu_text.getBuffer(),
-                          icu_text.getBuffer() + icu_text.length());
-
-  txt::ParagraphStyle paragraph_style;
-  txt::ParagraphBuilder builder(paragraph_style, GetTestFontCollection());
-
-  txt::TextStyle text_style;
-  text_style.font_families = std::vector<std::string>(1, "Roboto");
-  text_style.color = SK_ColorBLACK;
-  text_style.font_features.SetFeature("tnum", 1);
-  builder.PushStyle(text_style);
-  builder.AddText(u16_text);
-
-  text_style.font_features.SetFeature("tnum", 0);
-  text_style.font_features.SetFeature("pnum", 1);
-  builder.PushStyle(text_style);
-  builder.AddText(u16_text);
-
-  builder.Pop();
-  builder.Pop();
-
-  auto paragraph = builder.Build();
-  paragraph->Layout(GetTestCanvasWidth());
-
-  paragraph->Paint(GetCanvas(), 10.0, 15.0);
-
-  ASSERT_EQ(paragraph->glyph_lines_.size(), 3ull);
-
-  // Tabular numbers should have equal widths.
-  const txt::Paragraph::GlyphLine& tnum_line = paragraph->glyph_lines_[0];
-  ASSERT_EQ(tnum_line.positions.size(), 4ull);
-  EXPECT_FLOAT_EQ(tnum_line.positions[0].x_pos.width(),
-                  tnum_line.positions[1].x_pos.width());
-
-  // Proportional numbers should have variable widths.
-  const txt::Paragraph::GlyphLine& pnum_line = paragraph->glyph_lines_[1];
-  ASSERT_EQ(pnum_line.positions.size(), 4ull);
-  EXPECT_NE(pnum_line.positions[0].x_pos.width(),
-            pnum_line.positions[1].x_pos.width());
-
-  // Alphabetic characters should be unaffected.
-  EXPECT_FLOAT_EQ(tnum_line.positions[2].x_pos.width(),
-                  pnum_line.positions[2].x_pos.width());
-
-  ASSERT_TRUE(Snapshot());
-}
-
-// The height override is disabled for this test. DIrect metrics from the font.
+// The height override is disabled for this test. Direct metrics from the font.
 TEST_F(ParagraphTest, DISABLE_ON_WINDOWS(StrutDefaultParagraph)) {
   const char* text = "01234満毎冠行来昼本可\nabcd\n満毎冠行来昼本可";
   auto icu_text = icu::UnicodeString::fromUTF8(text);
@@ -4800,8 +4811,8 @@ TEST_F(ParagraphTest, DISABLE_ON_WINDOWS(StrutDefaultParagraph)) {
   EXPECT_FLOAT_EQ(boxes[0].rect.right(), 20);
   EXPECT_NEAR(boxes[0].rect.bottom(), 46.5, 0.0001);
 
-  boxes =
-      paragraph->GetRectsForRange(0, 2, rect_height_strut_style, rect_width_style);
+  boxes = paragraph->GetRectsForRange(0, 2, rect_height_strut_style,
+                                      rect_width_style);
   for (size_t i = 0; i < boxes.size(); ++i) {
     GetCanvas()->drawRect(boxes[i].rect, paint);
   }
@@ -4814,4 +4825,54 @@ TEST_F(ParagraphTest, DISABLE_ON_WINDOWS(StrutDefaultParagraph)) {
   ASSERT_TRUE(Snapshot());
 }
 
+TEST_F(ParagraphTest, FontFeaturesParagraph) {
+  const char* text = "12ab\n";
+  auto icu_text = icu::UnicodeString::fromUTF8(text);
+  std::u16string u16_text(icu_text.getBuffer(),
+                          icu_text.getBuffer() + icu_text.length());
+
+  txt::ParagraphStyle paragraph_style;
+  txt::ParagraphBuilder builder(paragraph_style, GetTestFontCollection());
+
+  txt::TextStyle text_style;
+  text_style.font_families = std::vector<std::string>(1, "Roboto");
+  text_style.color = SK_ColorBLACK;
+  text_style.font_features.SetFeature("tnum", 1);
+  builder.PushStyle(text_style);
+  builder.AddText(u16_text);
+
+  text_style.font_features.SetFeature("tnum", 0);
+  text_style.font_features.SetFeature("pnum", 1);
+  builder.PushStyle(text_style);
+  builder.AddText(u16_text);
+
+  builder.Pop();
+  builder.Pop();
+
+  auto paragraph = builder.Build();
+  paragraph->Layout(GetTestCanvasWidth());
+
+  paragraph->Paint(GetCanvas(), 10.0, 15.0);
+
+  ASSERT_EQ(paragraph->glyph_lines_.size(), 3ull);
+
+  // Tabular numbers should have equal widths.
+  const txt::Paragraph::GlyphLine& tnum_line = paragraph->glyph_lines_[0];
+  ASSERT_EQ(tnum_line.positions.size(), 4ull);
+  EXPECT_FLOAT_EQ(tnum_line.positions[0].x_pos.width(),
+                  tnum_line.positions[1].x_pos.width());
+
+  // Proportional numbers should have variable widths.
+  const txt::Paragraph::GlyphLine& pnum_line = paragraph->glyph_lines_[1];
+  ASSERT_EQ(pnum_line.positions.size(), 4ull);
+  EXPECT_NE(pnum_line.positions[0].x_pos.width(),
+            pnum_line.positions[1].x_pos.width());
+
+  // Alphabetic characters should be unaffected.
+  EXPECT_FLOAT_EQ(tnum_line.positions[2].x_pos.width(),
+                  pnum_line.positions[2].x_pos.width());
+
+  ASSERT_TRUE(Snapshot());
+}
+
 }  // namespace txt

From 080363e1df319a0627083a8d3e62598fc7d288cf Mon Sep 17 00:00:00 2001
From: GaryQian <garyq@google.com>
Date: Thu, 23 May 2019 11:12:44 -0700
Subject: [PATCH 06/10] Better wording

---
 lib/stub_ui/lib/src/ui/text.dart | 6 +++---
 lib/ui/text.dart                 | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/stub_ui/lib/src/ui/text.dart b/lib/stub_ui/lib/src/ui/text.dart
index 21ccd5e218d54..1c3b66c0caefb 100644
--- a/lib/stub_ui/lib/src/ui/text.dart
+++ b/lib/stub_ui/lib/src/ui/text.dart
@@ -298,7 +298,7 @@ class TextStyle {
   /// * `letterSpacing`: The amount of space (in logical pixels) to add between each letter.
   /// * `wordSpacing`: The amount of space (in logical pixels) to add at each sequence of white-space (i.e. between each word).
   /// * `textBaseline`: The common baseline that should be aligned between this text span and its parent text span, or, for the root text spans, with the line box.
-  /// * `height`: The height of this text span, as a multiplier of the font size. Omitting `height` will allow the text
+  /// * `height`: The height of this text span, as a multiplier of the font size. Omitting `height` will allow the line height
   ///   to take the height as defined by the font, which may not be exactly the height of the fontSize.
   /// * `locale`: The locale used to select region-specific glyphs.
   /// * `background`: The paint drawn as a background for the text.
@@ -627,8 +627,8 @@ class ParagraphStyle {
   /// * `height`: The fallback height of the spans as a multiplier of the font
   ///   size. The fallback height is used when no height is provided in through
   ///   [TextStyle.height]. Omitting `height` here and in [TextStyle] will allow
-  ///   the text to take the height as defined by the font, which may not be
-  ///   exactly the height of the fontSize.
+  ///   the line height to take the height as defined by the font, which may not
+  ///   be exactly the height of the fontSize.
   ///
   /// * `fontWeight`: The typeface thickness to use when painting the text
   ///   (e.g., bold).
diff --git a/lib/ui/text.dart b/lib/ui/text.dart
index 6d98610e799a5..1386598bfc35f 100644
--- a/lib/ui/text.dart
+++ b/lib/ui/text.dart
@@ -527,7 +527,7 @@ class TextStyle {
   /// * `letterSpacing`: The amount of space (in logical pixels) to add between each letter.
   /// * `wordSpacing`: The amount of space (in logical pixels) to add at each sequence of white-space (i.e. between each word).
   /// * `textBaseline`: The common baseline that should be aligned between this text span and its parent text span, or, for the root text spans, with the line box.
-  /// * `height`: The height of this text span, as a multiplier of the font size. Omitting `height` will allow the text
+  /// * `height`: The height of this text span, as a multiplier of the font size. Omitting `height` will allow the line height
   ///   to take the height as defined by the font, which may not be exactly the height of the fontSize.
   /// * `locale`: The locale used to select region-specific glyphs.
   /// * `background`: The paint drawn as a background for the text.
@@ -780,8 +780,8 @@ class ParagraphStyle {
   /// * `height`: The fallback height of the spans as a multiplier of the font
   ///   size. The fallback height is used when no height is provided in through
   ///   [TextStyle.height]. Omitting `height` here and in [TextStyle] will allow
-  ///   the text to take the height as defined by the font, which may not be
-  ///   exactly the height of the fontSize.
+  ///   the line height to take the height as defined by the font, which may not
+  ///   be exactly the height of the fontSize.
   ///
   /// * `fontWeight`: The typeface thickness to use when painting the text
   ///   (e.g., bold).

From d2b680978cbc3132da885240bee563fa9e9e7a14 Mon Sep 17 00:00:00 2001
From: GaryQian <garyq@google.com>
Date: Fri, 31 May 2019 16:51:22 -0700
Subject: [PATCH 07/10] Fix docs

---
 lib/stub_ui/lib/src/ui/text.dart | 20 ++++++++++----------
 lib/ui/text.dart                 | 20 ++++++++++----------
 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/lib/stub_ui/lib/src/ui/text.dart b/lib/stub_ui/lib/src/ui/text.dart
index 1c3b66c0caefb..f9207f6cf7bc8 100644
--- a/lib/stub_ui/lib/src/ui/text.dart
+++ b/lib/stub_ui/lib/src/ui/text.dart
@@ -625,10 +625,10 @@ class ParagraphStyle {
   ///   the text.
   ///
   /// * `height`: The fallback height of the spans as a multiplier of the font
-  ///   size. The fallback height is used when no height is provided in through
+  ///   size. The fallback height is used when no height is provided through
   ///   [TextStyle.height]. Omitting `height` here and in [TextStyle] will allow
   ///   the line height to take the height as defined by the font, which may not
-  ///   be exactly the height of the fontSize.
+  ///   be exactly the height of the `fontSize`.
   ///
   /// * `fontWeight`: The typeface thickness to use when painting the text
   ///   (e.g., bold).
@@ -770,14 +770,14 @@ class StrutStyle {
   ///   the text.
   ///
   /// * `height`: The minimum height of the line boxes, as a multiplier of the
-  ///   font size. The lines of the paragraph will be at least `(height + leading)
-  ///   * fontSize` tall when fontSize is not null. Omitting `height` will allow
-  ///   the minimum line height to take the height as defined by the font, which
-  ///   may not be exactly the height of the fontSize. When fontSize is null, there
-  ///   is no minimum line height. Tall glyphs due to baseline alignment or large
-  ///   [TextStyle.fontSize] may cause the actual line height after layout to be
-  ///   taller than specified here. [fontSize] must be provided for this property
-  ///   to take effect.
+  ///   font size. The lines of the paragraph will be at least
+  ///   `(height + leading) * fontSize` tall when fontSize is not null. Omitting
+  ///   `height` will allow the minimum line height to take the height as defined
+  ///   by the font, which may not be exactly the height of the [fontSize]. When
+  ///   [fontSize] is null, there is no minimum line height. Tall glyphs due to
+  ///   baseline alignment or large [TextStyle.fontSize] may cause the actual line
+  ///   height after layout to be taller than specified here. The [fontSize] must
+  ///   be provided for this property to take effect.
   ///
   /// * `leading`: The minimum amount of leading between lines as a multiple of
   ///   the font size. [fontSize] must be provided for this property to take effect.
diff --git a/lib/ui/text.dart b/lib/ui/text.dart
index 1386598bfc35f..ff47981b0571a 100644
--- a/lib/ui/text.dart
+++ b/lib/ui/text.dart
@@ -778,10 +778,10 @@ class ParagraphStyle {
   ///   use when painting the text. This is used when there is no [TextStyle].
   ///
   /// * `height`: The fallback height of the spans as a multiplier of the font
-  ///   size. The fallback height is used when no height is provided in through
+  ///   size. The fallback height is used when no height is provided through
   ///   [TextStyle.height]. Omitting `height` here and in [TextStyle] will allow
   ///   the line height to take the height as defined by the font, which may not
-  ///   be exactly the height of the fontSize.
+  ///   be exactly the height of the `fontSize`.
   ///
   /// * `fontWeight`: The typeface thickness to use when painting the text
   ///   (e.g., bold).
@@ -972,14 +972,14 @@ class StrutStyle {
   ///   the text.
   ///
   /// * `height`: The minimum height of the line boxes, as a multiplier of the
-  ///   font size. The lines of the paragraph will be at least `(height + leading)
-  ///   * fontSize` tall when fontSize is not null. Omitting `height` will allow
-  ///   the minimum line height to take the height as defined by the font, which
-  ///   may not be exactly the height of the fontSize. When fontSize is null, there
-  ///   is no minimum line height. Tall glyphs due to baseline alignment or large
-  ///   [TextStyle.fontSize] may cause the actual line height after layout to be
-  ///   taller than specified here. [fontSize] must be provided for this property
-  ///   to take effect.
+  ///   font size. The lines of the paragraph will be at least
+  ///   `(height + leading) * fontSize` tall when fontSize is not null. Omitting
+  ///   `height` will allow the minimum line height to take the height as defined
+  ///   by the font, which may not be exactly the height of the [fontSize]. When
+  ///   [fontSize] is null, there is no minimum line height. Tall glyphs due to
+  ///   baseline alignment or large [TextStyle.fontSize] may cause the actual line
+  ///   height after layout to be taller than specified here. The [fontSize] must
+  ///   be provided for this property to take effect.
   ///
   /// * `leading`: The minimum amount of leading between lines as a multiple of
   ///   the font size. [fontSize] must be provided for this property to take effect.

From ea3f4a99004d5b6ed494871c490ad18d70f5b22e Mon Sep 17 00:00:00 2001
From: GaryQian <garyq@google.com>
Date: Fri, 31 May 2019 16:55:26 -0700
Subject: [PATCH 08/10] []->

---
 lib/stub_ui/lib/src/ui/text.dart | 20 ++++++++++----------
 lib/ui/text.dart                 | 18 +++++++++---------
 2 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/lib/stub_ui/lib/src/ui/text.dart b/lib/stub_ui/lib/src/ui/text.dart
index f9207f6cf7bc8..64cc94600c4e7 100644
--- a/lib/stub_ui/lib/src/ui/text.dart
+++ b/lib/stub_ui/lib/src/ui/text.dart
@@ -763,24 +763,24 @@ class StrutStyle {
   /// * `fontFamily`: The name of the font to use when painting the text (e.g.,
   ///   Roboto).
   ///
-  /// * `fontFamilyFallback`: An ordered list of font family names that will be searched for when
-  ///    the font in `fontFamily` cannot be found.
+  /// * `fontFamilyFallback`: An ordered list of font family names that will be
+  ///    searched for when the font in `fontFamily` cannot be found.
   ///
   /// * `fontSize`: The size of glyphs (in logical pixels) to use when painting
   ///   the text.
   ///
   /// * `height`: The minimum height of the line boxes, as a multiplier of the
   ///   font size. The lines of the paragraph will be at least
-  ///   `(height + leading) * fontSize` tall when fontSize is not null. Omitting
+  ///   `(height + leading) * fontSize` tall when `fontSize` is not null. Omitting
   ///   `height` will allow the minimum line height to take the height as defined
-  ///   by the font, which may not be exactly the height of the [fontSize]. When
-  ///   [fontSize] is null, there is no minimum line height. Tall glyphs due to
+  ///   by the font, which may not be exactly the height of the `fontSize`. When
+  ///   `fontSize` is null, there is no minimum line height. Tall glyphs due to
   ///   baseline alignment or large [TextStyle.fontSize] may cause the actual line
-  ///   height after layout to be taller than specified here. The [fontSize] must
+  ///   height after layout to be taller than specified here. The `fontSize` must
   ///   be provided for this property to take effect.
   ///
   /// * `leading`: The minimum amount of leading between lines as a multiple of
-  ///   the font size. [fontSize] must be provided for this property to take effect.
+  ///   the font size. `fontSize` must be provided for this property to take effect.
   ///
   /// * `fontWeight`: The typeface thickness to use when painting the text
   ///   (e.g., bold).
@@ -789,11 +789,11 @@ class StrutStyle {
   ///   italics).
   ///
   /// * `forceStrutHeight`: When true, the paragraph will force all lines to be exactly
-  ///   `(lineHeight + leading) * fontSize` tall from baseline to baseline.
+  ///   `(height + leading) * fontSize` tall from baseline to baseline.
   ///   [TextStyle] is no longer able to influence the line height, and any tall
-  ///   glyphs may overlap with lines above. If a [fontFamily] is specified, the
+  ///   glyphs may overlap with lines above. If a `fontFamily` is specified, the
   ///   total ascent of the first line will be the min of the `Ascent + half-leading`
-  ///   of the [fontFamily] and `(lineHeight + leading) * fontSize`. Otherwise, it
+  ///   of the `fontFamily` and `(height + leading) * fontSize`. Otherwise, it
   ///   will be determined by the Ascent + half-leading of the first text.
   StrutStyle({
     String fontFamily,
diff --git a/lib/ui/text.dart b/lib/ui/text.dart
index ff47981b0571a..bea3cbf616e22 100644
--- a/lib/ui/text.dart
+++ b/lib/ui/text.dart
@@ -965,24 +965,24 @@ class StrutStyle {
   /// * `fontFamily`: The name of the font to use when painting the text (e.g.,
   ///   Roboto).
   ///
-  /// * `fontFamilyFallback`: An ordered list of font family names that will be searched for when
-  ///    the font in `fontFamily` cannot be found.
+  /// * `fontFamilyFallback`: An ordered list of font family names that will be
+  ///    searched for when the font in `fontFamily` cannot be found.
   ///
   /// * `fontSize`: The size of glyphs (in logical pixels) to use when painting
   ///   the text.
   ///
   /// * `height`: The minimum height of the line boxes, as a multiplier of the
   ///   font size. The lines of the paragraph will be at least
-  ///   `(height + leading) * fontSize` tall when fontSize is not null. Omitting
+  ///   `(height + leading) * fontSize` tall when `fontSize` is not null. Omitting
   ///   `height` will allow the minimum line height to take the height as defined
-  ///   by the font, which may not be exactly the height of the [fontSize]. When
-  ///   [fontSize] is null, there is no minimum line height. Tall glyphs due to
+  ///   by the font, which may not be exactly the height of the `fontSize`. When
+  ///   `fontSize` is null, there is no minimum line height. Tall glyphs due to
   ///   baseline alignment or large [TextStyle.fontSize] may cause the actual line
-  ///   height after layout to be taller than specified here. The [fontSize] must
+  ///   height after layout to be taller than specified here. The `fontSize` must
   ///   be provided for this property to take effect.
   ///
   /// * `leading`: The minimum amount of leading between lines as a multiple of
-  ///   the font size. [fontSize] must be provided for this property to take effect.
+  ///   the font size. `fontSize` must be provided for this property to take effect.
   ///
   /// * `fontWeight`: The typeface thickness to use when painting the text
   ///   (e.g., bold).
@@ -993,9 +993,9 @@ class StrutStyle {
   /// * `forceStrutHeight`: When true, the paragraph will force all lines to be exactly
   ///   `(height + leading) * fontSize` tall from baseline to baseline.
   ///   [TextStyle] is no longer able to influence the line height, and any tall
-  ///   glyphs may overlap with lines above. If a [fontFamily] is specified, the
+  ///   glyphs may overlap with lines above. If a `fontFamily` is specified, the
   ///   total ascent of the first line will be the min of the `Ascent + half-leading`
-  ///   of the [fontFamily] and `(height + leading) * fontSize`. Otherwise, it
+  ///   of the `fontFamily` and `(height + leading) * fontSize`. Otherwise, it
   ///   will be determined by the Ascent + half-leading of the first text.
   StrutStyle({
     String fontFamily,

From 70fe7c4a0720bd94822785c19f65cfb4f36d2486 Mon Sep 17 00:00:00 2001
From: GaryQian <garyq@google.com>
Date: Fri, 21 Jun 2019 11:04:52 -0700
Subject: [PATCH 09/10] Correct order of placeholder metrics calcuation

---
 third_party/txt/src/txt/paragraph.cc | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/third_party/txt/src/txt/paragraph.cc b/third_party/txt/src/txt/paragraph.cc
index c854f2d8355a3..aafe8bddfe6ef 100644
--- a/third_party/txt/src/txt/paragraph.cc
+++ b/third_party/txt/src/txt/paragraph.cc
@@ -1073,10 +1073,10 @@ void Paragraph::Layout(double width, bool force) {
           ascent = (-metrics.fAscent + metrics.fLeading / 2);
           descent = (metrics.fDescent + metrics.fLeading / 2);
         }
+        ComputePlaceholder(placeholder_run, ascent, descent);
+
         max_ascent = std::max(ascent, max_ascent);
         max_descent = std::max(descent, max_descent);
-
-        ComputePlaceholder(placeholder_run, ascent, descent);
       }
 
       max_unscaled_ascent = std::max(placeholder_run == nullptr
@@ -1496,6 +1496,8 @@ std::vector<Paragraph::TextBox> Paragraph::GetRectsForRange(
       top = baseline - run.placeholder_run->baseline_offset;
       bottom = baseline + run.placeholder_run->height -
                run.placeholder_run->baseline_offset;
+      FML_DLOG(ERROR) << baseline << " " << run.placeholder_run->baseline_offset
+                      << " " << run.placeholder_run->height;
     }
 
     max_line = std::max(run.line_number, max_line);

From 28b17a601cac4dad461ec23846ff0041d63d054b Mon Sep 17 00:00:00 2001
From: GaryQian <garyq@google.com>
Date: Fri, 21 Jun 2019 11:14:40 -0700
Subject: [PATCH 10/10] Remove debug prints

---
 third_party/txt/src/txt/paragraph.cc | 2 --
 1 file changed, 2 deletions(-)

diff --git a/third_party/txt/src/txt/paragraph.cc b/third_party/txt/src/txt/paragraph.cc
index aafe8bddfe6ef..6acdc160bd30e 100644
--- a/third_party/txt/src/txt/paragraph.cc
+++ b/third_party/txt/src/txt/paragraph.cc
@@ -1496,8 +1496,6 @@ std::vector<Paragraph::TextBox> Paragraph::GetRectsForRange(
       top = baseline - run.placeholder_run->baseline_offset;
       bottom = baseline + run.placeholder_run->height -
                run.placeholder_run->baseline_offset;
-      FML_DLOG(ERROR) << baseline << " " << run.placeholder_run->baseline_offset
-                      << " " << run.placeholder_run->height;
     }
 
     max_line = std::max(run.line_number, max_line);