From a7e14ea474aff4e8258befea8d22ef404562e6eb Mon Sep 17 00:00:00 2001 From: Jason Simmons Date: Fri, 22 Sep 2017 15:15:59 -0700 Subject: [PATCH] libtxt: refactor glyph position calculation (#4134) * Remove padding values in line_heights and glyph_position_x. Each value in glyph_position_x now represents an actual glyph in the layout. * Remove code intended to handle extra characters beyond the end of the last line. The LineBreaker should ensure that the end of the last run matches the end of the last line. * Return the upstream/downstream affinity of the cursor position in GetGlyphPositionAtCoordinate. * Account for the space at the end of a word wrapped line in GetGlyphPositionAtCoordinate / GetCoordinatesForGlyphPosition --- lib/ui/text/paragraph_impl_txt.cc | 8 +- third_party/txt/src/txt/paragraph.cc | 127 ++++++++----------- third_party/txt/src/txt/paragraph.h | 26 +++- third_party/txt/tests/paragraph_unittests.cc | 60 +++++---- 4 files changed, 115 insertions(+), 106 deletions(-) diff --git a/lib/ui/text/paragraph_impl_txt.cc b/lib/ui/text/paragraph_impl_txt.cc index d2eb877d690..f51921c85fd 100644 --- a/lib/ui/text/paragraph_impl_txt.cc +++ b/lib/ui/text/paragraph_impl_txt.cc @@ -75,10 +75,10 @@ std::vector ParagraphImplTxt::getRectsForRange(unsigned start, Dart_Handle ParagraphImplTxt::getPositionForOffset(double dx, double dy) { Dart_Handle result = Dart_NewList(2); - Dart_ListSetAt( - result, 0, - ToDart(m_paragraph->GetGlyphPositionAtCoordinate(dx, dy, true))); - Dart_ListSetAt(result, 1, ToDart(static_cast(EAffinity::DOWNSTREAM))); + txt::Paragraph::PositionWithAffinity pos = + m_paragraph->GetGlyphPositionAtCoordinate(dx, dy, true); + Dart_ListSetAt(result, 0, ToDart(pos.position)); + Dart_ListSetAt(result, 1, ToDart(static_cast(pos.affinity))); return result; } diff --git a/third_party/txt/src/txt/paragraph.cc b/third_party/txt/src/txt/paragraph.cc index 55bf8104ae9..ee8af2f6423 100644 --- a/third_party/txt/src/txt/paragraph.cc +++ b/third_party/txt/src/txt/paragraph.cc @@ -223,14 +223,10 @@ void Paragraph::Layout(double width, bool force) { lines_ = 0; line_widths_ = std::vector(); line_heights_ = std::vector(); - line_heights_.push_back(0); records_ = std::vector(); - // Set padding elements to have a minimum point. - glyph_position_x_ = std::vector>(); - glyph_position_x_.push_back(std::vector()); - std::vector glyph_single_line_position_x; - glyph_single_line_position_x.push_back(0); + glyph_position_x_ = std::vector>(); + std::vector glyph_single_line_position_x; // Track the x of the previous run to maintain accurate xposition when // multiple SkTextBlobs make up a single line. double previous_run_x_position = 0.0f; @@ -290,14 +286,14 @@ void Paragraph::Layout(double width, bool force) { break; case TextAlign::right: { for (size_t i = 0; i < glyph_position_x_[y_index].size(); ++i) { - glyph_position_x_[y_index][i] += + glyph_position_x_[y_index][i].start += width_ - breaker_.getWidths()[y_index]; } break; } case TextAlign::center: { for (size_t i = 0; i < glyph_position_x_[y_index].size(); ++i) { - glyph_position_x_[y_index][i] += + glyph_position_x_[y_index][i].start += (width_ - breaker_.getWidths()[y_index]) / 2; } break; @@ -433,12 +429,9 @@ void Paragraph::Layout(double width, bool force) { ? 0 : layout.getX(glyph_index) - current_x_position; buffers.back()->pos[pos_index] = current_x_position + letter_spacing; - glyph_single_line_position_x.push_back( - current_x_position + previous_run_x_position + letter_spacing); buffers.back()->pos[pos_index + 1] = layout.getY(glyph_index); float glyph_advance = layout.getCharAdvance(character_index); - current_x_position += glyph_advance; // The glyph may be a ligature. Determine how many input characters // are joined into this glyph. @@ -448,13 +441,21 @@ void Paragraph::Layout(double width, bool force) { break; } size_t ligature_width = ligature_end - character_index; - prev_char_advance = glyph_advance / ligature_width; + float subglyph_advance = glyph_advance / ligature_width; + glyph_single_line_position_x.emplace_back( + current_x_position + previous_run_x_position + letter_spacing, + subglyph_advance); + // Compute positions for the additional characters in the ligature. for (size_t i = 1; i < ligature_width; ++i) { - glyph_single_line_position_x.push_back( - glyph_single_line_position_x.back() + prev_char_advance); + glyph_single_line_position_x.emplace_back( + glyph_single_line_position_x.back().start + subglyph_advance, + subglyph_advance); } + + current_x_position += glyph_advance; character_index += ligature_width; + prev_char_advance = subglyph_advance; // Check if the current Glyph is a whitespace and handle multiple // whitespaces in a row. @@ -514,9 +515,6 @@ void Paragraph::Layout(double width, bool force) { line_heights_.push_back( (line_heights_.empty() ? 0 : line_heights_.back()) + roundf(max_line_spacing + max_descent)); - glyph_single_line_position_x.push_back( - glyph_single_line_position_x.back() + prev_char_advance); - glyph_single_line_position_x.push_back(FLT_MAX); glyph_position_x_.push_back(glyph_single_line_position_x); prev_max_descent = max_descent; line_widths_.push_back(line_width); @@ -533,8 +531,7 @@ void Paragraph::Layout(double width, bool force) { line_width = 0.0f; break_index += 1; lines_++; - glyph_single_line_position_x = std::vector(); - glyph_single_line_position_x.push_back(0); + glyph_single_line_position_x.clear(); } else { x += layout.getAdvance(); } @@ -542,19 +539,6 @@ void Paragraph::Layout(double width, bool force) { layout_start = layout_end; } } - // Handle last line tasks. - y += roundf(max_line_spacing + prev_max_descent); - postprocess_line(); - if (line_width != 0) - line_widths_.push_back(line_width); - - // Finalize measurements - line_heights_.push_back((line_heights_.empty() ? 0 : line_heights_.back()) + - roundf(max_line_spacing + max_descent)); - glyph_single_line_position_x.push_back(glyph_single_line_position_x.back() + - prev_char_advance); - glyph_single_line_position_x.push_back(FLT_MAX); - glyph_position_x_.push_back(glyph_single_line_position_x); // Remove justification on the last line. if (paragraph_style_.text_align == TextAlign::justify && @@ -650,7 +634,7 @@ size_t Paragraph::TextSize() const { } double Paragraph::GetHeight() const { - return line_heights_[line_heights_.size() - 2]; + return line_heights_.size() ? line_heights_.back() : 0; } double Paragraph::GetLayoutWidth() const { @@ -878,59 +862,60 @@ std::vector Paragraph::GetRectsForRange(size_t start, } SkRect Paragraph::GetCoordinatesForGlyphPosition(size_t pos) const { - size_t remainder = fmin(pos, text_.size()); - remainder++; - size_t line = 1; - for (line = 1; line < line_heights_.size() - 1; ++line) { - if (remainder > glyph_position_x_[line].size() - 3) { - remainder -= glyph_position_x_[line].size() - 3; + size_t remainder = fmin(pos, text_.size() - 1); + size_t line; + for (line = 0; line < glyph_position_x_.size(); ++line) { + if (remainder >= glyph_position_x_[line].size()) { + remainder -= glyph_position_x_[line].size(); } else { break; } } - return SkRect::MakeLTRB(glyph_position_x_[line][remainder], - line_heights_[line - 1], - remainder < glyph_position_x_[line].size() - 2 - ? glyph_position_x_[line][remainder + 1] - : line_widths_[line - 1], + const std::vector& line_glyph_position = + glyph_position_x_[line]; + double glyph_end = (remainder < line_glyph_position.size() - 1) + ? line_glyph_position[remainder + 1].start + : line_glyph_position[remainder].glyph_end(); + return SkRect::MakeLTRB(line_glyph_position[remainder].start, + line > 0 ? line_heights_[line - 1] : 0, glyph_end, line_heights_[line]); } -size_t Paragraph::GetGlyphPositionAtCoordinate( +Paragraph::PositionWithAffinity Paragraph::GetGlyphPositionAtCoordinate( double dx, double dy, bool using_glyph_center_as_boundary) const { + if (line_heights_.empty()) + return PositionWithAffinity(0, DOWNSTREAM); + size_t offset = 0; - size_t y_index = 1; - size_t prev_count = 0; - for (y_index = 1; y_index < line_heights_.size() - 2; ++y_index) { - if (dy < line_heights_[y_index]) { - offset += prev_count; - prev_count = glyph_position_x_[y_index - 1].size() - 3; + size_t y_index; + for (y_index = 0; y_index < line_heights_.size() - 1; ++y_index) { + if (dy < line_heights_[y_index]) break; - } else { - offset += prev_count; - prev_count = glyph_position_x_[y_index].size() - 3; - } + offset += glyph_position_x_[y_index].size(); } - if (y_index == line_heights_.size() - 2) - offset += prev_count; - prev_count = 0; - for (size_t x_index = 1; x_index < glyph_position_x_[y_index].size() - 1; - ++x_index) { - if (dx < glyph_position_x_[y_index][x_index] - - (using_glyph_center_as_boundary - ? (glyph_position_x_[y_index][x_index] - - glyph_position_x_[y_index][x_index - 1]) / - 2.0f - : 0)) { + + const std::vector& line_glyph_position = + glyph_position_x_[y_index]; + 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) + ? line_glyph_position[x_index + 1].start + : line_glyph_position[x_index].glyph_end(); + double boundary; + if (using_glyph_center_as_boundary) { + boundary = (line_glyph_position[x_index].start + glyph_end) / 2.0f; + } else { + boundary = glyph_end; + } + + if (dx < boundary) break; - } else { - offset += prev_count; - prev_count = 1; - } + + offset++; } - return offset; + return PositionWithAffinity(offset, x_index > 0 ? UPSTREAM : DOWNSTREAM); } SkIPoint Paragraph::GetWordBoundary(size_t offset) const { diff --git a/third_party/txt/src/txt/paragraph.h b/third_party/txt/src/txt/paragraph.h index ac30484382c..d188cf8d20c 100644 --- a/third_party/txt/src/txt/paragraph.h +++ b/third_party/txt/src/txt/paragraph.h @@ -54,6 +54,15 @@ class Paragraph { ~Paragraph(); + enum Affinity { UPSTREAM, DOWNSTREAM }; + + struct PositionWithAffinity { + const size_t position; + const Affinity affinity; + + PositionWithAffinity(size_t p, Affinity a) : position(p), affinity(a) {} + }; + // 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 @@ -116,7 +125,7 @@ class Paragraph { // typical use-case for this is when the cursor is meant to be on either side // of any given character. This allows the transition border to be middle of // each character. - size_t GetGlyphPositionAtCoordinate( + PositionWithAffinity GetGlyphPositionAtCoordinate( double dx, double dy, bool using_glyph_center_as_boundary = false) const; @@ -180,9 +189,18 @@ class Paragraph { // TODO(garyq): Can we access this info without redundantly storing it here? std::vector line_widths_; std::vector line_heights_; - // Holds the laid out x positions of each glyph, as well as padding to make - // math on it simpler. - std::vector> glyph_position_x_; + + struct GlyphPosition { + double start; + double advance; + + GlyphPosition(double s, double a) : start(s), advance(a) {} + + double glyph_end() const { return start + advance; } + }; + + // Holds the laid out x positions of each glyph. + std::vector> glyph_position_x_; // Set of glyph IDs that correspond to whitespace. std::set whitespace_set_; diff --git a/third_party/txt/tests/paragraph_unittests.cc b/third_party/txt/tests/paragraph_unittests.cc index ee5c02b1a7f..723fd673efc 100644 --- a/third_party/txt/tests/paragraph_unittests.cc +++ b/third_party/txt/tests/paragraph_unittests.cc @@ -341,11 +341,11 @@ TEST_F(ParagraphTest, LeftAlignParagraph) { paragraph->GetParagraphStyle().text_align); // Tests for GetGlyphPositionAtCoordinate() - ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(0, 0), 0ull); - ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(1, 1), 0ull); - ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(1, 35), 68ull); - ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(1, 70), 134ull); - ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(2000, 35), 134ull); + ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(0, 0).position, 0ull); + ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(1, 1).position, 0ull); + ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(1, 35).position, 68ull); + ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(1, 70).position, 134ull); + ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(2000, 35).position, 134ull); ASSERT_TRUE(Snapshot()); } @@ -884,28 +884,34 @@ TEST_F(ParagraphTest, GetGlyphPositionAtCoordinateParagraph) { // the original text because the final trailing whitespaces are sometimes not // drawn (namely, when using "justify" alignment) and therefore are not active // glyphs. - ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(-10000, -10000), 0ull); - ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(-1, -1), 0ull); - ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(0, 0), 0ull); - ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(3, 3), 0ull); - ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(35, 1), 1ull); - ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(300, 2), 10ull); - ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(301, 2.2), 10ull); - ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(302, 2.6), 10ull); - ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(301, 2.1), 10ull); - ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(100000, 20), 18ull); - ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(450, 20), 16ull); - ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(100000, 90), 36ull); - ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(-100000, 90), 18ull); - ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(20, -80), 0ull); - ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(1, 90), 18ull); - ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(1, 180), 36ull); - ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(10000, 180), 54ull); - ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(70, 180), 38ull); - ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(1, 270), 54ull); - ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(35, 90), 19ull); - ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(10000, 10000), 77ull); - ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(85, 10000), 74ull); + ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(-10000, -10000).position, + 0ull); + ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(-1, -1).position, 0ull); + ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(0, 0).position, 0ull); + ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(3, 3).position, 0ull); + ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(35, 1).position, 1ull); + ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(300, 2).position, 10ull); + ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(301, 2.2).position, 10ull); + ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(302, 2.6).position, 10ull); + ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(301, 2.1).position, 10ull); + ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(100000, 20).position, + 18ull); + ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(450, 20).position, 16ull); + ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(100000, 90).position, + 36ull); + ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(-100000, 90).position, + 18ull); + ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(20, -80).position, 0ull); + ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(1, 90).position, 18ull); + ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(1, 180).position, 36ull); + ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(10000, 180).position, + 54ull); + ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(70, 180).position, 38ull); + ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(1, 270).position, 54ull); + ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(35, 90).position, 19ull); + ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(10000, 10000).position, + 77ull); + ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(85, 10000).position, 74ull); } TEST_F(ParagraphTest, GetRectsForRangeParagraph) {