libtxt: some cleanup (#4268)

* rename glyph_position_x to glyph_lines
* use round instead of roundf
* return a range start/end struct in Paragraph::GetWordBoundary

lib/ui/text/paragraph_impl_txt.cc

@@ -83,10 +83,10 @@ Dart_Handle ParagraphImplTxt::getPositionForOffset(double dx, double dy) {
 }
 
 Dart_Handle ParagraphImplTxt::getWordBoundary(unsigned offset) {
-  SkIPoint point = m_paragraph->GetWordBoundary(offset);
+  txt::Paragraph::Range point = m_paragraph->GetWordBoundary(offset);
   Dart_Handle result = Dart_NewList(2);
-  Dart_ListSetAt(result, 0, ToDart(point.x()));
-  Dart_ListSetAt(result, 1, ToDart(point.y()));
+  Dart_ListSetAt(result, 0, ToDart(point.start));
+  Dart_ListSetAt(result, 1, ToDart(point.end));
   return result;
 }
 

third_party/txt/src/txt/paragraph.cc

@@ -20,7 +20,6 @@
 #include <algorithm>
 #include <limits>
 #include <numeric>
-#include <tuple>
 #include <utility>
 #include <vector>
 
@@ -42,11 +41,6 @@
 namespace txt {
 namespace {
 
-struct Range {
-  Range(size_t s, size_t e) : start(s), end(e) {}
-  size_t start, end;
-};
-
 const sk_sp<SkTypeface>& GetTypefaceForGlyph(const minikin::Layout& layout,
                                              size_t index) {
   const FontSkia* font = static_cast<const FontSkia*>(layout.getFont(index));
@@ -123,7 +117,7 @@ void GetPaint(const TextStyle& style, SkPaint* paint) {
 void FindWords(const std::vector<uint16_t>& text,
                size_t start,
                size_t end,
-               std::vector<Range>* words) {
+               std::vector<Paragraph::Range>* words) {
   bool in_word = false;
   size_t word_start;
   for (size_t i = start; i < end; ++i) {
@@ -274,7 +268,7 @@ void Paragraph::Layout(double width, bool force) {
 
   records_.clear();
   line_heights_.clear();
-  glyph_position_x_.clear();
+  glyph_lines_.clear();
 
   minikin::Layout layout;
   SkTextBlobBuilder builder;
@@ -316,7 +310,7 @@ void Paragraph::Layout(double width, bool force) {
       run_index++;
     }
 
-    std::vector<GlyphPosition> glyph_single_line_position_x;
+    std::vector<GlyphPosition> glyph_positions;
     double run_x_offset = GetLineXOffset(line_number);
     double justify_x_offset = 0;
     std::vector<PaintRecord> paint_records;
@@ -436,14 +430,14 @@ void Paragraph::Layout(double width, bool force) {
           }
           float subglyph_advance =
               glyph_advance / subglyph_code_unit_counts.size();
-          glyph_single_line_position_x.emplace_back(
-              run_x_offset + glyph_x_offset, subglyph_advance,
-              subglyph_code_unit_counts[0]);
+          glyph_positions.emplace_back(run_x_offset + glyph_x_offset,
+                                       subglyph_advance,
+                                       subglyph_code_unit_counts[0]);
 
           // Compute positions for the additional characters in the ligature.
           for (size_t i = 1; i < subglyph_code_unit_counts.size(); ++i) {
-            glyph_single_line_position_x.emplace_back(
-                glyph_single_line_position_x.back().start + subglyph_advance,
+            glyph_positions.emplace_back(
+                glyph_positions.back().start + subglyph_advance,
                 subglyph_advance, subglyph_code_unit_counts[i]);
           }
 
@@ -455,8 +449,7 @@ void Paragraph::Layout(double width, bool force) {
 
             if (!isnan(word_start_position)) {
               double word_width =
-                  glyph_single_line_position_x.back().glyph_end() -
-                  word_start_position;
+                  glyph_positions.back().glyph_end() - word_start_position;
               max_word_width = std::max(word_width, max_word_width);
               word_start_position = std::numeric_limits<double>::quiet_NaN();
             }
@@ -498,8 +491,8 @@ void Paragraph::Layout(double width, bool force) {
     }
 
     line_heights_.push_back((line_heights_.empty() ? 0 : line_heights_.back()) +
-                            roundf(max_line_spacing + max_descent));
-    y_offset += roundf(max_line_spacing + prev_max_descent);
+                            round(max_line_spacing + max_descent));
+    y_offset += round(max_line_spacing + prev_max_descent);
     prev_max_descent = max_descent;
 
     for (PaintRecord& paint_record : paint_records) {
@@ -511,8 +504,8 @@ void Paragraph::Layout(double width, bool force) {
     size_t next_line_start = (line_number < line_ranges_.size() - 1)
                                  ? line_ranges_[line_number + 1].start
                                  : text_.size();
-    glyph_position_x_.emplace_back(std::move(glyph_single_line_position_x),
-                                   next_line_start - line_range.start);
+    glyph_lines_.emplace_back(std::move(glyph_positions),
+                              next_line_start - line_range.start);
   }
 
   max_intrinsic_width_ = 0;
@@ -767,34 +760,34 @@ std::vector<SkRect> Paragraph::GetRectsForRange(size_t start,
 
   size_t pos = 0;
   size_t line;
-  for (line = 0; line < glyph_position_x_.size(); ++line) {
-    if (start < pos + glyph_position_x_[line].total_code_units)
+  for (line = 0; line < glyph_lines_.size(); ++line) {
+    if (start < pos + glyph_lines_[line].total_code_units)
       break;
-    pos += glyph_position_x_[line].total_code_units;
+    pos += glyph_lines_[line].total_code_units;
   }
-  if (line == glyph_position_x_.size())
+  if (line == glyph_lines_.size())
     return rects;
 
-  if (end <= pos + glyph_position_x_[line].total_code_units) {
+  if (end <= pos + glyph_lines_[line].total_code_units) {
     rects.push_back(GetRectForLineRange(line, start - pos, end - pos));
     return rects;
   }
 
-  rects.push_back(GetRectForLineRange(
-      line, start - pos, glyph_position_x_[line].total_code_units));
+  rects.push_back(GetRectForLineRange(line, start - pos,
+                                      glyph_lines_[line].total_code_units));
 
   while (true) {
-    pos += glyph_position_x_[line].total_code_units;
+    pos += glyph_lines_[line].total_code_units;
     line++;
-    if (line == glyph_position_x_.size())
+    if (line == glyph_lines_.size())
       break;
 
-    if (end <= pos + glyph_position_x_[line].total_code_units) {
+    if (end <= pos + glyph_lines_[line].total_code_units) {
       rects.push_back(GetRectForLineRange(line, 0, end - pos));
       break;
     } else {
-      rects.push_back(GetRectForLineRange(
-          line, 0, glyph_position_x_[line].total_code_units));
+      rects.push_back(
+          GetRectForLineRange(line, 0, glyph_lines_[line].total_code_units));
     }
   }
 
@@ -804,8 +797,8 @@ std::vector<SkRect> Paragraph::GetRectsForRange(size_t start,
 SkRect Paragraph::GetRectForLineRange(size_t line,
                                       size_t start,
                                       size_t end) const {
-  FXL_DCHECK(line < glyph_position_x_.size());
-  const GlyphLine& glyph_line = glyph_position_x_[line];
+  FXL_DCHECK(line < glyph_lines_.size());
+  const GlyphLine& glyph_line = glyph_lines_[line];
   if (glyph_line.positions.empty())
     return SkRect::MakeEmpty();
 
@@ -830,11 +823,11 @@ Paragraph::PositionWithAffinity Paragraph::GetGlyphPositionAtCoordinate(
   for (y_index = 0; y_index < line_heights_.size() - 1; ++y_index) {
     if (dy < line_heights_[y_index])
       break;
-    offset += glyph_position_x_[y_index].total_code_units;
+    offset += glyph_lines_[y_index].total_code_units;
   }
 
   const std::vector<GlyphPosition>& line_glyph_position =
-      glyph_position_x_[y_index].positions;
+      glyph_lines_[y_index].positions;
   size_t x_index;
   for (x_index = 0; x_index < line_glyph_position.size(); ++x_index) {
     double glyph_end = (x_index < line_glyph_position.size() - 1)
@@ -855,11 +848,11 @@ Paragraph::PositionWithAffinity Paragraph::GetGlyphPositionAtCoordinate(
   return PositionWithAffinity(offset, x_index > 0 ? UPSTREAM : DOWNSTREAM);
 }
 
-SkIPoint Paragraph::GetWordBoundary(size_t offset) const {
+Paragraph::Range Paragraph::GetWordBoundary(size_t offset) const {
   // TODO(garyq): Consider punctuation as separate words.
   if (text_.size() == 0)
-    return SkIPoint::Make(0, 0);
-  return SkIPoint::Make(
+    return Range(0, 0);
+  return Range(
       minikin::getPrevWordBreakForCache(text_.data(), offset + 1, text_.size()),
       minikin::getNextWordBreakForCache(text_.data(), offset, text_.size()));
 }

third_party/txt/src/txt/paragraph.h

@@ -18,7 +18,7 @@
 #define LIB_TXT_SRC_PARAGRAPH_H_
 
 #include <set>
-#include <tuple>
+#include <utility>
 #include <vector>
 
 #include "font_collection.h"
@@ -63,6 +63,14 @@ class Paragraph {
     PositionWithAffinity(size_t p, Affinity a) : position(p), affinity(a) {}
   };
 
+  struct Range {
+    Range(size_t s, size_t e) : start(s), end(e) {}
+    size_t start, end;
+    bool operator==(const Range& other) const {
+      return start == other.start && end == other.end;
+    }
+  };
+
   // Minikin Layout doLayout() and LineBreaker addStyleRun() has an
   // O(N^2) (according to benchmarks) time complexity where N is the total
   // number of characters. However, this is not significant for reasonably sized
@@ -133,7 +141,7 @@ class Paragraph {
 
   // Finds the first and last glyphs that define a word containing the glyph at
   // index offset.
-  SkIPoint GetWordBoundary(size_t offset) const;
+  Range GetWordBoundary(size_t offset) const;
 
   // Returns the number of lines the paragraph takes up. If the text exceeds the
   // amount width and maxlines provides, Layout() truncates the extra text from
@@ -218,7 +226,7 @@ class Paragraph {
   };
 
   // Holds the laid out x positions of each glyph.
-  std::vector<GlyphLine> glyph_position_x_;
+  std::vector<GlyphLine> glyph_lines_;
 
   // The max width of the paragraph as provided in the most recent Layout()
   // call.

third_party/txt/tests/paragraph_unittests.cc

@@ -1075,35 +1075,35 @@ TEST_F(ParagraphTest, GetWordBoundaryParagraph) {
   SkRect rect = GetCoordinatesForGlyphPosition(*paragraph, 0);
   GetCanvas()->drawLine(rect.fLeft, rect.fTop, rect.fLeft, rect.fBottom, paint);
 
-  EXPECT_EQ(paragraph->GetWordBoundary(0), SkIPoint::Make(0, 5));
-  EXPECT_EQ(paragraph->GetWordBoundary(1), SkIPoint::Make(0, 5));
-  EXPECT_EQ(paragraph->GetWordBoundary(2), SkIPoint::Make(0, 5));
-  EXPECT_EQ(paragraph->GetWordBoundary(3), SkIPoint::Make(0, 5));
-  EXPECT_EQ(paragraph->GetWordBoundary(4), SkIPoint::Make(0, 5));
+  EXPECT_EQ(paragraph->GetWordBoundary(0), txt::Paragraph::Range(0, 5));
+  EXPECT_EQ(paragraph->GetWordBoundary(1), txt::Paragraph::Range(0, 5));
+  EXPECT_EQ(paragraph->GetWordBoundary(2), txt::Paragraph::Range(0, 5));
+  EXPECT_EQ(paragraph->GetWordBoundary(3), txt::Paragraph::Range(0, 5));
+  EXPECT_EQ(paragraph->GetWordBoundary(4), txt::Paragraph::Range(0, 5));
   rect = GetCoordinatesForGlyphPosition(*paragraph, 5);
   GetCanvas()->drawLine(rect.fLeft, rect.fTop, rect.fLeft, rect.fBottom, paint);
 
-  EXPECT_EQ(paragraph->GetWordBoundary(5), SkIPoint::Make(5, 6));
+  EXPECT_EQ(paragraph->GetWordBoundary(5), txt::Paragraph::Range(5, 6));
   rect = GetCoordinatesForGlyphPosition(*paragraph, 6);
   GetCanvas()->drawLine(rect.fLeft, rect.fTop, rect.fLeft, rect.fBottom, paint);
 
-  EXPECT_EQ(paragraph->GetWordBoundary(6), SkIPoint::Make(6, 7));
+  EXPECT_EQ(paragraph->GetWordBoundary(6), txt::Paragraph::Range(6, 7));
   rect = GetCoordinatesForGlyphPosition(*paragraph, 7);
   GetCanvas()->drawLine(rect.fLeft, rect.fTop, rect.fLeft, rect.fBottom, paint);
 
-  EXPECT_EQ(paragraph->GetWordBoundary(7), SkIPoint::Make(7, 12));
-  EXPECT_EQ(paragraph->GetWordBoundary(8), SkIPoint::Make(7, 12));
-  EXPECT_EQ(paragraph->GetWordBoundary(9), SkIPoint::Make(7, 12));
-  EXPECT_EQ(paragraph->GetWordBoundary(10), SkIPoint::Make(7, 12));
-  EXPECT_EQ(paragraph->GetWordBoundary(11), SkIPoint::Make(7, 12));
+  EXPECT_EQ(paragraph->GetWordBoundary(7), txt::Paragraph::Range(7, 12));
+  EXPECT_EQ(paragraph->GetWordBoundary(8), txt::Paragraph::Range(7, 12));
+  EXPECT_EQ(paragraph->GetWordBoundary(9), txt::Paragraph::Range(7, 12));
+  EXPECT_EQ(paragraph->GetWordBoundary(10), txt::Paragraph::Range(7, 12));
+  EXPECT_EQ(paragraph->GetWordBoundary(11), txt::Paragraph::Range(7, 12));
   rect = GetCoordinatesForGlyphPosition(*paragraph, 12);
   GetCanvas()->drawLine(rect.fLeft, rect.fTop, rect.fLeft, rect.fBottom, paint);
 
-  EXPECT_EQ(paragraph->GetWordBoundary(12), SkIPoint::Make(12, 13));
+  EXPECT_EQ(paragraph->GetWordBoundary(12), txt::Paragraph::Range(12, 13));
   rect = GetCoordinatesForGlyphPosition(*paragraph, 13);
   GetCanvas()->drawLine(rect.fLeft, rect.fTop, rect.fLeft, rect.fBottom, paint);
 
-  EXPECT_EQ(paragraph->GetWordBoundary(13), SkIPoint::Make(13, 18));
+  EXPECT_EQ(paragraph->GetWordBoundary(13), txt::Paragraph::Range(13, 18));
   rect = GetCoordinatesForGlyphPosition(*paragraph, 18);
   GetCanvas()->drawLine(rect.fLeft, rect.fTop, rect.fLeft, rect.fBottom, paint);
 
@@ -1119,7 +1119,7 @@ TEST_F(ParagraphTest, GetWordBoundaryParagraph) {
   rect = GetCoordinatesForGlyphPosition(*paragraph, 30);
   GetCanvas()->drawLine(rect.fLeft, rect.fTop, rect.fLeft, rect.fBottom, paint);
 
-  EXPECT_EQ(paragraph->GetWordBoundary(30), SkIPoint::Make(30, 31));
+  EXPECT_EQ(paragraph->GetWordBoundary(30), txt::Paragraph::Range(30, 31));
   rect = GetCoordinatesForGlyphPosition(*paragraph, 31);
   GetCanvas()->drawLine(rect.fLeft, rect.fTop, rect.fLeft, rect.fBottom, paint);
 
@@ -1127,7 +1127,7 @@ TEST_F(ParagraphTest, GetWordBoundaryParagraph) {
   GetCanvas()->drawLine(rect.fLeft, rect.fTop, rect.fLeft, rect.fBottom, paint);
 
   EXPECT_EQ(paragraph->GetWordBoundary(icu_text.length() - 1),
-            SkIPoint::Make(icu_text.length() - 5, icu_text.length()));
+            txt::Paragraph::Range(icu_text.length() - 5, icu_text.length()));
   rect = GetCoordinatesForGlyphPosition(*paragraph, icu_text.length());
   GetCanvas()->drawLine(rect.fLeft, rect.fTop, rect.fLeft, rect.fBottom, paint);