libtxt: miscellaneous cleanup (#4679)

* use TextDecoration enums
* replace GetBlobLength with a function that returns runs based on typeface
* a few other minor style fixes

third_party/txt/benchmarks/paragraph_benchmarks.cc

@@ -175,6 +175,7 @@ static void BM_ParagraphTextBigO(benchmark::State& state) {
   std::u16string u16_text(text.data(), text.data() + text.size());
 
   txt::ParagraphStyle paragraph_style;
+  paragraph_style.font_family = "Roboto";
 
   txt::TextStyle text_style;
   text_style.font_family = "Roboto";
@@ -315,7 +316,9 @@ static void BM_ParagraphPaintDecoration(benchmark::State& state) {
 
   txt::TextStyle text_style;
   text_style.font_family = "Roboto";
-  text_style.decoration = TextDecoration(0x1 | 0x2 | 0x4);
+  text_style.decoration = TextDecoration::kUnderline |
+                          TextDecoration::kOverline |
+                          TextDecoration::kLineThrough;
   text_style.decoration_style = TextDecorationStyle(kSolid);
   text_style.color = SK_ColorBLACK;
 

third_party/txt/src/txt/paragraph.cc

@@ -49,15 +49,24 @@ const sk_sp<SkTypeface>& GetTypefaceForGlyph(const minikin::Layout& layout,
   return font->GetSkTypeface();
 }
 
-// Return the number of glyphs until the typeface changes.
-size_t GetBlobLength(const minikin::Layout& layout, size_t blob_start) {
-  const size_t glyph_count = layout.nGlyphs();
-  const sk_sp<SkTypeface>& typeface = GetTypefaceForGlyph(layout, blob_start);
-  for (size_t blob_end = blob_start + 1; blob_end < glyph_count; ++blob_end) {
-    if (GetTypefaceForGlyph(layout, blob_end).get() != typeface.get())
-      return blob_end - blob_start;
+// Return ranges of text that have the same typeface in the layout.
+std::vector<Paragraph::Range<size_t>> GetLayoutTypefaceRuns(
+    const minikin::Layout& layout) {
+  std::vector<Paragraph::Range<size_t>> result;
+  if (layout.nGlyphs() == 0)
+    return result;
+  size_t run_start = 0;
+  const SkTypeface* run_typeface = GetTypefaceForGlyph(layout, run_start).get();
+  for (size_t i = 1; i < layout.nGlyphs(); ++i) {
+    const SkTypeface* typeface = GetTypefaceForGlyph(layout, i).get();
+    if (typeface != run_typeface) {
+      result.emplace_back(run_start, i);
+      run_start = i;
+      run_typeface = typeface;
+    }
   }
-  return glyph_count - blob_start;
+  result.emplace_back(run_start, layout.nGlyphs());
+  return result;
 }
 
 int GetWeight(const FontWeight weight) {
@@ -498,12 +507,7 @@ void Paragraph::Layout(double width, bool force) {
         continue;
 
       // Break the layout into blobs that share the same SkPaint parameters.
-      std::vector<Range<size_t>> glyph_blobs;
-      for (size_t blob_start = 0; blob_start < layout.nGlyphs();) {
-        size_t blob_len = GetBlobLength(layout, blob_start);
-        glyph_blobs.emplace_back(blob_start, blob_start + blob_len);
-        blob_start += blob_len;
-      }
+      std::vector<Range<size_t>> glyph_blobs = GetLayoutTypefaceRuns(layout);
 
       grapheme_breaker_->setText(
           icu::UnicodeString(false, text_ptr + text_start, text_count));
@@ -765,20 +769,15 @@ void Paragraph::Paint(SkCanvas* canvas, double x, double y) {
   SkAutoCanvasRestore canvas_restore(canvas, true);
   canvas->translate(x, y);
   SkPaint paint;
-  for (size_t index = 0; index < records_.size(); ++index) {
-    PaintRecord& record = records_[index];
+  for (const PaintRecord& record : records_) {
     paint.setColor(record.style().color);
     SkPoint offset = record.offset();
     canvas->drawTextBlob(record.text(), offset.x(), offset.y(), paint);
-    PaintDecorations(canvas, offset.x(), offset.y(), index);
+    PaintDecorations(canvas, record);
   }
 }
 
-void Paragraph::PaintDecorations(SkCanvas* canvas,
-                                 double x,
-                                 double y,
-                                 size_t record_index) {
-  PaintRecord& record = records_[record_index];
+void Paragraph::PaintDecorations(SkCanvas* canvas, const PaintRecord& record) {
   if (record.style().decoration == TextDecoration::kNone)
     return;
 
@@ -819,6 +818,9 @@ void Paragraph::PaintDecorations(SkCanvas* canvas,
   paint.setStrokeWidth(underline_thickness *
                        record.style().decoration_thickness_multiplier);
 
+  SkScalar x = record.offset().x();
+  SkScalar y = record.offset().y();
+
   // Setup the decorations.
   switch (record.style().decoration_style) {
     case TextDecorationStyle::kSolid: {
@@ -878,7 +880,7 @@ void Paragraph::PaintDecorations(SkCanvas* canvas,
     double y_offset = i * underline_thickness * kDoubleDecorationSpacing;
     double y_offset_original = y_offset;
     // Underline
-    if (record.style().decoration & 0x1) {
+    if (record.style().decoration & TextDecoration::kUnderline) {
       y_offset += (metrics.fFlags & SkPaint::FontMetrics::FontMetricsFlags::
                                         kUnderlinePositionIsValid_Flag)
                       ? metrics.fUnderlinePosition
@@ -893,7 +895,7 @@ void Paragraph::PaintDecorations(SkCanvas* canvas,
       y_offset = y_offset_original;
     }
     // Overline
-    if (record.style().decoration & 0x2) {
+    if (record.style().decoration & TextDecoration::kOverline) {
       // We subtract fAscent here because for double overlines, we want the
       // second line to be above, not below the first.
       y_offset -= metrics.fAscent;
@@ -907,7 +909,7 @@ void Paragraph::PaintDecorations(SkCanvas* canvas,
       y_offset = y_offset_original;
     }
     // Strikethrough
-    if (record.style().decoration & 0x4) {
+    if (record.style().decoration & TextDecoration::kLineThrough) {
       if (metrics.fFlags & SkPaint::FontMetrics::FontMetricsFlags::
                                kStrikeoutThicknessIsValid_Flag)
         paint.setStrokeWidth(metrics.fStrikeoutThickness *

third_party/txt/src/txt/paragraph.h

@@ -300,10 +300,7 @@ class Paragraph {
   double GetLineXOffset(size_t line);
 
   // Creates and draws the decorations onto the canvas.
-  void PaintDecorations(SkCanvas* canvas,
-                        double x,
-                        double y,
-                        size_t record_index);
+  void PaintDecorations(SkCanvas* canvas, const PaintRecord& record);
 
   FXL_DISALLOW_COPY_AND_ASSIGN(Paragraph);
 };

third_party/txt/src/txt/text_style.h

@@ -30,7 +30,7 @@ namespace txt {
 class TextStyle {
  public:
   SkColor color = SK_ColorWHITE;
-  TextDecoration decoration = TextDecoration::kNone;
+  int decoration = TextDecoration::kNone;
   // Does not make sense to draw a transparent object, so we use it as a default
   // value to indicate no decoration color was set.
   SkColor decoration_color = SK_ColorTRANSPARENT;

third_party/txt/tests/paragraph_unittests.cc

@@ -140,7 +140,9 @@ TEST_F(ParagraphTest, RainbowParagraph) {
   text_style2.font_weight = txt::FontWeight::w600;
   text_style2.color = SK_ColorGREEN;
   text_style2.font_family = "Roboto";
-  text_style2.decoration = txt::TextDecoration(0x1 | 0x2 | 0x4);
+  text_style2.decoration = TextDecoration::kUnderline |
+                           TextDecoration::kOverline |
+                           TextDecoration::kLineThrough;
   text_style2.decoration_color = SK_ColorBLACK;
   builder.PushStyle(text_style2);
 
@@ -156,7 +158,9 @@ TEST_F(ParagraphTest, RainbowParagraph) {
   text_style4.font_size = 14;
   text_style4.color = SK_ColorBLUE;
   text_style4.font_family = "Roboto";
-  text_style4.decoration = txt::TextDecoration(0x1 | 0x2 | 0x4);
+  text_style4.decoration = TextDecoration::kUnderline |
+                           TextDecoration::kOverline |
+                           TextDecoration::kLineThrough;
   text_style4.decoration_color = SK_ColorBLACK;
   builder.PushStyle(text_style4);
 
@@ -291,7 +295,7 @@ TEST_F(ParagraphTest, DISABLE_ON_WINDOWS(LeftAlignParagraph)) {
   text_style.word_spacing = 5;
   text_style.color = SK_ColorBLACK;
   text_style.height = 1;
-  text_style.decoration = txt::TextDecoration(0x1);
+  text_style.decoration = TextDecoration::kUnderline;
   text_style.decoration_color = SK_ColorBLACK;
   builder.PushStyle(text_style);
 
@@ -388,7 +392,7 @@ TEST_F(ParagraphTest, DISABLE_ON_WINDOWS(RightAlignParagraph)) {
   text_style.word_spacing = 5;
   text_style.color = SK_ColorBLACK;
   text_style.height = 1;
-  text_style.decoration = txt::TextDecoration(0x1);
+  text_style.decoration = TextDecoration::kUnderline;
   text_style.decoration_color = SK_ColorBLACK;
   builder.PushStyle(text_style);
 
@@ -492,7 +496,7 @@ TEST_F(ParagraphTest, DISABLE_ON_WINDOWS(CenterAlignParagraph)) {
   text_style.word_spacing = 5;
   text_style.color = SK_ColorBLACK;
   text_style.height = 1;
-  text_style.decoration = txt::TextDecoration(0x1);
+  text_style.decoration = TextDecoration::kUnderline;
   text_style.decoration_color = SK_ColorBLACK;
   builder.PushStyle(text_style);
 
@@ -598,7 +602,7 @@ TEST_F(ParagraphTest, DISABLE_ON_WINDOWS(JustifyAlignParagraph)) {
   text_style.word_spacing = 5;
   text_style.color = SK_ColorBLACK;
   text_style.height = 1;
-  text_style.decoration = txt::TextDecoration(0x1);
+  text_style.decoration = TextDecoration::kUnderline;
   text_style.decoration_color = SK_ColorBLACK;
   builder.PushStyle(text_style);
 
@@ -665,7 +669,9 @@ TEST_F(ParagraphTest, DecorationsParagraph) {
   text_style.word_spacing = 5;
   text_style.color = SK_ColorBLACK;
   text_style.height = 2;
-  text_style.decoration = txt::TextDecoration(0x1 | 0x2 | 0x4);
+  text_style.decoration = TextDecoration::kUnderline |
+                          TextDecoration::kOverline |
+                          TextDecoration::kLineThrough;
   text_style.decoration_style = txt::TextDecorationStyle::kSolid;
   text_style.decoration_color = SK_ColorBLACK;
   builder.PushStyle(text_style);
@@ -705,7 +711,8 @@ TEST_F(ParagraphTest, DecorationsParagraph) {
 
   for (size_t i = 0; i < 6; ++i) {
     ASSERT_EQ(paragraph->records_[i].style().decoration,
-              txt::TextDecoration(0x1 | 0x2 | 0x4));
+              TextDecoration::kUnderline | TextDecoration::kOverline |
+                  TextDecoration::kLineThrough);
   }
 
   ASSERT_EQ(paragraph->records_[0].style().decoration_style,
@@ -782,7 +789,9 @@ TEST_F(ParagraphTest, ChineseParagraph) {
   text_style.font_size = 35;
   text_style.letter_spacing = 2;
   text_style.font_family = "Source Han Serif CN";
-  text_style.decoration = txt::TextDecoration(0x1 | 0x2 | 0x4);
+  text_style.decoration = TextDecoration::kUnderline |
+                          TextDecoration::kOverline |
+                          TextDecoration::kLineThrough;
   text_style.decoration_style = txt::TextDecorationStyle::kSolid;
   text_style.decoration_color = SK_ColorBLACK;
   builder.PushStyle(text_style);
@@ -825,7 +834,9 @@ TEST_F(ParagraphTest, DISABLED_ArabicParagraph) {
   text_style.font_size = 35;
   text_style.letter_spacing = 2;
   text_style.font_family = "Katibeh";
-  text_style.decoration = txt::TextDecoration(0x1 | 0x2 | 0x4);
+  text_style.decoration = TextDecoration::kUnderline |
+                          TextDecoration::kOverline |
+                          TextDecoration::kLineThrough;
   text_style.decoration_style = txt::TextDecorationStyle::kSolid;
   text_style.decoration_color = SK_ColorBLACK;
   builder.PushStyle(text_style);