From 054a2cca35a8a1f307910e660bf6a304b7828a2d Mon Sep 17 00:00:00 2001 From: Jason Simmons Date: Mon, 23 Oct 2017 17:18:00 -0700 Subject: [PATCH] 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 | 6 +- third_party/txt/src/txt/paragraph.cc | 71 +++++++++----------- third_party/txt/src/txt/paragraph.h | 14 +++- third_party/txt/tests/paragraph_unittests.cc | 32 ++++----- 4 files changed, 62 insertions(+), 61 deletions(-) diff --git a/lib/ui/text/paragraph_impl_txt.cc b/lib/ui/text/paragraph_impl_txt.cc index df61016be57..939e53b7a7a 100644 --- a/lib/ui/text/paragraph_impl_txt.cc +++ b/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; } diff --git a/third_party/txt/src/txt/paragraph.cc b/third_party/txt/src/txt/paragraph.cc index e488a5c665a..a59d5d19a0e 100644 --- a/third_party/txt/src/txt/paragraph.cc +++ b/third_party/txt/src/txt/paragraph.cc @@ -20,7 +20,6 @@ #include #include #include -#include #include #include @@ -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& GetTypefaceForGlyph(const minikin::Layout& layout, size_t index) { const FontSkia* font = static_cast(layout.getFont(index)); @@ -123,7 +117,7 @@ void GetPaint(const TextStyle& style, SkPaint* paint) { void FindWords(const std::vector& text, size_t start, size_t end, - std::vector* words) { + std::vector* 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 glyph_single_line_position_x; + std::vector glyph_positions; double run_x_offset = GetLineXOffset(line_number); double justify_x_offset = 0; std::vector 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::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 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 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& 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())); } diff --git a/third_party/txt/src/txt/paragraph.h b/third_party/txt/src/txt/paragraph.h index f63bb238a7e..cd421d7a2ba 100644 --- a/third_party/txt/src/txt/paragraph.h +++ b/third_party/txt/src/txt/paragraph.h @@ -18,7 +18,7 @@ #define LIB_TXT_SRC_PARAGRAPH_H_ #include -#include +#include #include #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 glyph_position_x_; + std::vector glyph_lines_; // The max width of the paragraph as provided in the most recent Layout() // call. diff --git a/third_party/txt/tests/paragraph_unittests.cc b/third_party/txt/tests/paragraph_unittests.cc index 8724f081368..e6b636904d8 100644 --- a/third_party/txt/tests/paragraph_unittests.cc +++ b/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);