From 3c76b90b2528c36166f274ffa878e4debe0dfd09 Mon Sep 17 00:00:00 2001 From: Gary Qian Date: Tue, 23 Jul 2019 16:44:30 -0700 Subject: [PATCH] Track clusters and return cluster boundaries in getGlyphPositionForCoordinates (emoji fix) (#10063) --- third_party/txt/src/txt/paragraph_txt.cc | 37 ++++-- third_party/txt/src/txt/paragraph_txt.h | 9 +- third_party/txt/tests/paragraph_unittests.cc | 117 +++++++++++++++++++ 3 files changed, 154 insertions(+), 9 deletions(-) diff --git a/third_party/txt/src/txt/paragraph_txt.cc b/third_party/txt/src/txt/paragraph_txt.cc index 5e62ea8dab9..8109dd770d8 100644 --- a/third_party/txt/src/txt/paragraph_txt.cc +++ b/third_party/txt/src/txt/paragraph_txt.cc @@ -193,9 +193,11 @@ static const float kDoubleDecorationSpacing = 3.0f; ParagraphTxt::GlyphPosition::GlyphPosition(double x_start, double x_advance, size_t code_unit_index, - size_t code_unit_width) + size_t code_unit_width, + size_t cluster) : code_units(code_unit_index, code_unit_index + code_unit_width), - x_pos(x_start, x_start + x_advance) {} + x_pos(x_start, x_start + x_advance), + cluster(cluster) {} void ParagraphTxt::GlyphPosition::Shift(double delta) { x_pos.Shift(delta); @@ -881,13 +883,11 @@ void ParagraphTxt::Layout(double width) { builder.allocRunPos(font, glyph_blob.end - glyph_blob.start); double justify_x_offset_delta = 0; - for (size_t glyph_index = glyph_blob.start; glyph_index < glyph_blob.end;) { size_t cluster_start_glyph_index = glyph_index; uint32_t cluster = layout.getGlyphCluster(cluster_start_glyph_index); double glyph_x_offset; - // Add all the glyphs in this cluster to the text blob. do { size_t blob_index = glyph_index - glyph_blob.start; @@ -946,7 +946,7 @@ void ParagraphTxt::Layout(double width) { glyph_positions.emplace_back(run_x_offset + glyph_x_offset, grapheme_advance, run.start() + glyph_code_units.start, - grapheme_code_unit_counts[0]); + grapheme_code_unit_counts[0], cluster); // Compute positions for the additional graphemes in the ligature. for (size_t i = 1; i < grapheme_code_unit_counts.size(); ++i) { @@ -954,7 +954,7 @@ void ParagraphTxt::Layout(double width) { glyph_positions.back().x_pos.end, grapheme_advance, glyph_positions.back().code_units.start + grapheme_code_unit_counts[i - 1], - grapheme_code_unit_counts[i]); + grapheme_code_unit_counts[i], cluster); } bool at_word_start = false; @@ -1772,12 +1772,28 @@ Paragraph::PositionWithAffinity ParagraphTxt::GetGlyphPositionAtCoordinate( size_t x_index; const GlyphPosition* gp = nullptr; + const GlyphPosition* gp_cluster = nullptr; + bool is_cluster_corection = false; 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].x_pos.start : line_glyph_position[x_index].x_pos.end; + if (gp_cluster == nullptr || + gp_cluster->cluster != line_glyph_position[x_index].cluster) { + gp_cluster = &line_glyph_position[x_index]; + } if (dx < glyph_end) { - gp = &line_glyph_position[x_index]; + // Check if the glyph position is part of a cluster. If it is, + // we assign the cluster's root GlyphPosition to represent it. + if (gp_cluster->cluster == line_glyph_position[x_index].cluster) { + gp = gp_cluster; + // Detect if the matching GlyphPosition was non-root for the cluster. + if (gp_cluster != &line_glyph_position[x_index]) { + is_cluster_corection = true; + } + } else { + gp = &line_glyph_position[x_index]; + } break; } } @@ -1798,8 +1814,13 @@ Paragraph::PositionWithAffinity ParagraphTxt::GetGlyphPositionAtCoordinate( } double glyph_center = (gp->x_pos.start + gp->x_pos.end) / 2; + // We want to use the root cluster's start when the cluster + // was corrected. + // TODO(garyq): Detect if the position is in the middle of the cluster + // and properly assign the start/end positions. if ((direction == TextDirection::ltr && dx < glyph_center) || - (direction == TextDirection::rtl && dx >= glyph_center)) { + (direction == TextDirection::rtl && dx >= glyph_center) || + is_cluster_corection) { return PositionWithAffinity(gp->code_units.start, DOWNSTREAM); } else { return PositionWithAffinity(gp->code_units.end, UPSTREAM); diff --git a/third_party/txt/src/txt/paragraph_txt.h b/third_party/txt/src/txt/paragraph_txt.h index d1b68ab6f95..c023f0d0762 100644 --- a/third_party/txt/src/txt/paragraph_txt.h +++ b/third_party/txt/src/txt/paragraph_txt.h @@ -139,6 +139,7 @@ class ParagraphTxt : public Paragraph { FRIEND_TEST(ParagraphTest, KernScaleParagraph); FRIEND_TEST_WINDOWS_DISABLED(ParagraphTest, NewlineParagraph); FRIEND_TEST_WINDOWS_DISABLED(ParagraphTest, EmojiParagraph); + FRIEND_TEST_WINDOWS_DISABLED(ParagraphTest, EmojiMultiLineRectsParagraph); FRIEND_TEST(ParagraphTest, HyphenBreakParagraph); FRIEND_TEST(ParagraphTest, RepeatLayoutParagraph); FRIEND_TEST(ParagraphTest, Ellipsize); @@ -262,11 +263,17 @@ class ParagraphTxt : public Paragraph { struct GlyphPosition { Range code_units; Range x_pos; + // Tracks the cluster that this glyph position belongs to. For example, in + // extended emojis, multiple glyph positions will have the same cluster. The + // cluster can be used as a key to distinguish between codepoints that + // contribute to the drawing of a single glyph. + size_t cluster; GlyphPosition(double x_start, double x_advance, size_t code_unit_index, - size_t code_unit_width); + size_t code_unit_width, + size_t cluster); void Shift(double delta); }; diff --git a/third_party/txt/tests/paragraph_unittests.cc b/third_party/txt/tests/paragraph_unittests.cc index 06f636e620d..30e7c076a58 100644 --- a/third_party/txt/tests/paragraph_unittests.cc +++ b/third_party/txt/tests/paragraph_unittests.cc @@ -4042,6 +4042,123 @@ TEST_F(ParagraphTest, DISABLE_ON_WINDOWS(EmojiParagraph)) { EXPECT_EQ(paragraph->records_[7].line(), 7ull); } +TEST_F(ParagraphTest, DISABLE_ON_WINDOWS(EmojiMultiLineRectsParagraph)) { + // clang-format off + const char* text = + "๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘ฆ๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ง๐Ÿ‡บ๐Ÿ‡ธ๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘ฆ๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘งi๐Ÿ‡บ๐Ÿ‡ธ๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘ฆ๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ง๐Ÿ‡บ๐Ÿ‡ธ๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘ฆ๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ง๐Ÿ‡บ๐Ÿ‡ธ" + "๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘ฆ๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ง๐Ÿ‡บ๐Ÿ‡ธ๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘ฆ๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ง๐Ÿ‡บ๐Ÿ‡ธ๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘ฆ๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ง๐Ÿ‡บ๐Ÿ‡ธ๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘ฆ๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ง๐Ÿ‡บ๐Ÿ‡ธ" + "๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘ฆ๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ง๐Ÿ‡บ๐Ÿ‡ธ๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘ฆ๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ง๐Ÿ‡บ๐Ÿ‡ธ๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘ฆ๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ง๐Ÿ‡บ๐Ÿ‡ธ๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘ฆ๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ง๐Ÿ‡บ๐Ÿ‡ธ" + "๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘ฆ๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ง๐Ÿ‡บ๐Ÿ‡ธ๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘ฆ๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ง๐Ÿ‡บ๐Ÿ‡ธ๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘ฆ๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ง๐Ÿ‡บ๐Ÿ‡ธ๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘ฆ๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ง๐Ÿ‡บ๐Ÿ‡ธ" + "โ„๐Ÿ•๐Ÿ”๐ŸŸ๐Ÿฅ๐Ÿฑ๐Ÿ•ถ๐ŸŽฉ๐Ÿˆโšฝ๐Ÿšดโ€โ™€๏ธ๐ŸŽป๐ŸŽผ๐ŸŽน๐Ÿšจ๐ŸšŽ๐Ÿšโš“๐Ÿ›ณ๐Ÿš€๐Ÿš๐Ÿช๐Ÿข๐Ÿ–ฑโฐ๐Ÿ“ฑ๐Ÿ’พ๐Ÿ’‰๐Ÿ“‰๐Ÿ›๐Ÿ”‘๐Ÿ”“" + "๐Ÿ“๐Ÿ—“๐Ÿ“Šโค๐Ÿ’ฏ๐Ÿšซ๐Ÿ”ปโ™ โ™ฃ๐Ÿ•“โ—๐Ÿณ๐Ÿ๐Ÿณ๏ธโ€๐ŸŒˆ๐Ÿ‡ฎ๐Ÿ‡น๐Ÿ‡ฑ๐Ÿ‡ท๐Ÿ‡บ๐Ÿ‡ธ๐Ÿ‡ฌ๐Ÿ‡ง๐Ÿ‡จ๐Ÿ‡ณ๐Ÿ‡ง๐Ÿ‡ด"; + // clang-format on + 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::ParagraphBuilderTxt builder(paragraph_style, GetTestFontCollection()); + + txt::TextStyle text_style; + text_style.color = SK_ColorBLACK; + text_style.font_families = std::vector(1, "Noto Color Emoji"); + text_style.font_size = 50; + builder.PushStyle(text_style); + builder.AddText(u16_text); + + builder.Pop(); + + auto paragraph = BuildParagraph(builder); + paragraph->Layout(GetTestCanvasWidth() - 300); + + paragraph->Paint(GetCanvas(), 0, 0); + + for (size_t i = 0; i < u16_text.length(); i++) { + ASSERT_EQ(paragraph->text_[i], u16_text[i]); + } + + ASSERT_EQ(paragraph->records_.size(), 10ull); + + 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::RectWidthStyle rect_width_style = + Paragraph::RectWidthStyle::kTight; + paint.setColor(SK_ColorRED); + std::vector 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); + + // Ligature style indexing. + boxes = + paragraph->GetRectsForRange(0, 119, 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(), 2ull); + EXPECT_FLOAT_EQ(boxes[1].rect.left(), 0); + EXPECT_FLOAT_EQ(boxes[1].rect.right(), 334.61475); + + boxes = paragraph->GetRectsForRange(122, 132, rect_height_style, + rect_width_style); + paint.setColor(SK_ColorBLUE); + 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(), 357.95996); + EXPECT_FLOAT_EQ(boxes[0].rect.right(), 418.79901); + + // GetPositionForCoordinates should not return inter-emoji positions. + boxes = paragraph->GetRectsForRange( + 0, paragraph->GetGlyphPositionAtCoordinate(610, 100).position, + rect_height_style, rect_width_style); + paint.setColor(SK_ColorGREEN); + for (size_t i = 0; i < boxes.size(); ++i) { + GetCanvas()->drawRect(boxes[i].rect, paint); + } + EXPECT_EQ(boxes.size(), 2ull); + EXPECT_FLOAT_EQ(boxes[1].rect.left(), 0); + // The following is expected to change to a higher value when + // rounding up is added to getGlyphPositionForCoordinate. + EXPECT_FLOAT_EQ(boxes[1].rect.right(), 560.28516); + + boxes = paragraph->GetRectsForRange( + 0, paragraph->GetGlyphPositionAtCoordinate(580, 100).position, + rect_height_style, rect_width_style); + paint.setColor(SK_ColorGREEN); + for (size_t i = 0; i < boxes.size(); ++i) { + GetCanvas()->drawRect(boxes[i].rect, paint); + } + EXPECT_EQ(boxes.size(), 2ull); + EXPECT_FLOAT_EQ(boxes[1].rect.left(), 0); + EXPECT_FLOAT_EQ(boxes[1].rect.right(), 560.28516); + + boxes = paragraph->GetRectsForRange( + 0, paragraph->GetGlyphPositionAtCoordinate(560, 100).position, + rect_height_style, rect_width_style); + paint.setColor(SK_ColorGREEN); + for (size_t i = 0; i < boxes.size(); ++i) { + GetCanvas()->drawRect(boxes[i].rect, paint); + } + EXPECT_EQ(boxes.size(), 2ull); + EXPECT_FLOAT_EQ(boxes[1].rect.left(), 0); + // The following is expected to change to the 560.28516 value when + // rounding up is added to getGlyphPositionForCoordinate. + EXPECT_FLOAT_EQ(boxes[1].rect.right(), 498.03125); + + ASSERT_TRUE(Snapshot()); +} + TEST_F(ParagraphTest, HyphenBreakParagraph) { const char* text = "A "