From 76d250550b9d5a8139ea4bd6db78d610dfd99dc2 Mon Sep 17 00:00:00 2001 From: Gary Qian Date: Mon, 7 Aug 2017 16:18:52 -0700 Subject: [PATCH] Reset breaker after every layout to handle different widths. Change-Id: I786c4c42b4db9418f4f802b4cbb70a051b35925a --- engine/src/flutter/src/paragraph.cc | 6 +- engine/src/flutter/src/paragraph.h | 9 ++- engine/src/flutter/src/styled_runs.h | 1 + .../flutter/tests/txt/paragraph_unittests.cc | 61 +++++++++++++++++++ 4 files changed, 75 insertions(+), 2 deletions(-) diff --git a/engine/src/flutter/src/paragraph.cc b/engine/src/flutter/src/paragraph.cc index 606855cd6f..5bb0a41f0e 100644 --- a/engine/src/flutter/src/paragraph.cc +++ b/engine/src/flutter/src/paragraph.cc @@ -140,7 +140,9 @@ void Paragraph::SetText(std::vector text, StyledRuns runs) { return; text_ = std::move(text); runs_ = std::move(runs); +} +void Paragraph::InitBreaker() { breaker_.setLocale(icu::Locale(), nullptr); breaker_.resize(text_.size()); memcpy(breaker_.buffer(), text_.data(), text_.size() * sizeof(text_[0])); @@ -182,7 +184,7 @@ void Paragraph::FillWhitespaceSet(size_t start, void Paragraph::Layout(double width, bool force) { // Do not allow calling layout multiple times without changing anything. - if (!needs_layout_ && !force && width == width_) + if (!needs_layout_ && width == width_ && !force) return; needs_layout_ = false; @@ -199,6 +201,7 @@ void Paragraph::Layout(double width, bool force) { // minikin::Hyphenator::loadBinary(); // breaker_.setLocale(icu::Locale::getRoot(), &hyph); // + InitBreaker(); AddRunsToLineBreaker(collection_map); breaker_.setJustified(paragraph_style_.text_align == TextAlign::justify); breaker_.setStrategy(paragraph_style_.break_strategy); @@ -505,6 +508,7 @@ void Paragraph::Layout(double width, bool force) { line_widths_ = std::vector(breaker_.getWidths(), breaker_.getWidths() + lines_); CalculateIntrinsicWidths(); + breaker_.finish(); } // Amends the buffers to incorporate justification. diff --git a/engine/src/flutter/src/paragraph.h b/engine/src/flutter/src/paragraph.h index 08d90e7846..d4b7a491b3 100644 --- a/engine/src/flutter/src/paragraph.h +++ b/engine/src/flutter/src/paragraph.h @@ -163,6 +163,7 @@ class Paragraph { FRIEND_TEST(RenderTest, NewlineParagraph); FRIEND_TEST(RenderTest, EmojiParagraph); FRIEND_TEST(RenderTest, HyphenBreakParagraph); + FRIEND_TEST(RenderTest, RepeatLayoutParagraph); // Starting data to layout. std::vector text_; @@ -187,7 +188,7 @@ class Paragraph { // The max width of the paragraph as provided in the most recent Layout() // call. - double width_ = 0.0f; + double width_ = -1.0f; SkScalar height_ = 0.0f; size_t lines_ = 0; double max_intrinsic_width_ = 0; @@ -207,8 +208,14 @@ class Paragraph { : x_start(x_s), y_start(y_s), x_end(x_e), y_end(y_e) {} }; + // Passes in the text and Styled Runs. text_ and runs_ will later be passed + // into breaker_ in InitBreaker(), which is called in Layout(). void SetText(std::vector text, StyledRuns runs); + // Sets up breaker_ with the contents of text_ and runs_. This is called every + // Layout() call to allow for different widths to be used. + void InitBreaker(); + void SetParagraphStyle(const ParagraphStyle& style); void SetFontCollection(FontCollection* font_collection); diff --git a/engine/src/flutter/src/styled_runs.h b/engine/src/flutter/src/styled_runs.h index e731d61b16..ff3d58aaba 100644 --- a/engine/src/flutter/src/styled_runs.h +++ b/engine/src/flutter/src/styled_runs.h @@ -77,6 +77,7 @@ class StyledRuns { FRIEND_TEST(RenderTest, LongWordParagraph); FRIEND_TEST(RenderTest, KernParagraph); FRIEND_TEST(RenderTest, HyphenBreakParagraph); + FRIEND_TEST(RenderTest, RepeatLayoutParagraph); struct IndexedRun { size_t style_index = 0; diff --git a/engine/src/flutter/tests/txt/paragraph_unittests.cc b/engine/src/flutter/tests/txt/paragraph_unittests.cc index abdff311e7..103c49b7f9 100644 --- a/engine/src/flutter/tests/txt/paragraph_unittests.cc +++ b/engine/src/flutter/tests/txt/paragraph_unittests.cc @@ -1484,4 +1484,65 @@ TEST_F(RenderTest, HyphenBreakParagraph) { ASSERT_TRUE(Snapshot()); } +TEST_F(RenderTest, RepeatLayoutParagraph) { + const char* text = + "Sentence to layout at diff widths to get diff line counts. short words " + "short words short words short words short words short words short words " + "short words short words short words short words short words short words"; + auto icu_text = icu::UnicodeString::fromUTF8(text); + std::u16string u16_text(icu_text.getBuffer(), + icu_text.getBuffer() + icu_text.length()); + + txt::ParagraphStyle paragraph_style; + paragraph_style.break_strategy = minikin::kBreakStrategy_HighQuality; + auto font_collection = FontCollection::GetFontCollection(txt::GetFontDir()); + txt::ParagraphBuilder builder(paragraph_style, &font_collection); + + txt::TextStyle text_style; + text_style.font_family = "Roboto"; + text_style.font_size = 31; + text_style.letter_spacing = 0; + text_style.word_spacing = 0; + text_style.color = SK_ColorBLACK; + text_style.height = 1; + builder.PushStyle(text_style); + builder.AddText(u16_text); + + builder.Pop(); + + // First Layout. + auto paragraph = builder.Build(); + paragraph->Layout(300); + + paragraph->Paint(GetCanvas(), 0, 0); + + ASSERT_TRUE(Snapshot()); + ASSERT_EQ(paragraph->text_.size(), std::string{text}.length()); + for (size_t i = 0; i < u16_text.length(); i++) { + ASSERT_EQ(paragraph->text_[i], u16_text[i]); + } + ASSERT_EQ(paragraph->runs_.runs_.size(), 1ull); + ASSERT_EQ(paragraph->runs_.styles_.size(), 1ull); + ASSERT_TRUE(paragraph->runs_.styles_[0].equals(text_style)); + ASSERT_EQ(paragraph->records_[0].style().color, text_style.color); + ASSERT_EQ(paragraph->GetLineCount(), 12); + + // Second Layout. + SetUp(); + paragraph->Layout(600); + paragraph->Paint(GetCanvas(), 0, 0); + + ASSERT_TRUE(Snapshot()); + ASSERT_EQ(paragraph->text_.size(), std::string{text}.length()); + for (size_t i = 0; i < u16_text.length(); i++) { + ASSERT_EQ(paragraph->text_[i], u16_text[i]); + } + ASSERT_EQ(paragraph->runs_.runs_.size(), 1ull); + ASSERT_EQ(paragraph->runs_.styles_.size(), 1ull); + ASSERT_TRUE(paragraph->runs_.styles_[0].equals(text_style)); + ASSERT_EQ(paragraph->records_[0].style().color, text_style.color); + ASSERT_EQ(paragraph->GetLineCount(), 6); + ASSERT_TRUE(Snapshot()); +} + } // namespace txt