From 967e80b96c64f06df79305fd058a8225681bf4cf Mon Sep 17 00:00:00 2001 From: Gary Qian Date: Tue, 11 Jul 2017 18:38:14 -0700 Subject: [PATCH] Fix and smooth justifications to properly handle ligatures with whitespace mapping. Change-Id: I15bc093ed61f0540644ce31110769ee419dd7ac2 --- .../benchmarks/paragraph_benchmarks.cc | 44 +++++++++- engine/src/flutter/src/paragraph.cc | 85 ++++++++++++------- engine/src/flutter/src/paragraph.h | 10 ++- .../flutter/tests/txt/paragraph_unittests.cc | 2 +- 4 files changed, 108 insertions(+), 33 deletions(-) diff --git a/engine/src/flutter/benchmarks/paragraph_benchmarks.cc b/engine/src/flutter/benchmarks/paragraph_benchmarks.cc index 7bbfaacba50..e92715ff1a0 100644 --- a/engine/src/flutter/benchmarks/paragraph_benchmarks.cc +++ b/engine/src/flutter/benchmarks/paragraph_benchmarks.cc @@ -98,6 +98,48 @@ static void BM_ParagraphLongLayout(benchmark::State& state) { } BENCHMARK(BM_ParagraphLongLayout); +static void BM_ParagraphJustifyLayout(benchmark::State& state) { + const char* text = + "This is a very long sentence to test if the text will properly wrap " + "around and go to the next line. Sometimes, short sentence. Longer " + "sentences are okay too because they are necessary. Very short. " + "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod " + "tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim " + "veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea " + "commodo consequat. Duis aute irure dolor in reprehenderit in voluptate " + "velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint " + "occaecat cupidatat non proident, sunt in culpa qui officia deserunt " + "mollit anim id est laborum. " + "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod " + "tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim " + "veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea " + "commodo consequat. Duis aute irure dolor in reprehenderit in voluptate " + "velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint " + "occaecat cupidatat non proident, sunt in culpa qui officia deserunt " + "mollit anim id est laborum."; + 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.text_align = TextAlign::justify; + + txt::TextStyle text_style; + text_style.color = SK_ColorBLACK; + auto font_collection = FontCollection::GetFontCollection(txt::GetFontDir()); + while (state.KeepRunning()) { + txt::ParagraphBuilder builder(paragraph_style, &font_collection); + + builder.PushStyle(text_style); + builder.AddText(u16_text); + builder.Pop(); + auto paragraph = builder.Build(); + + paragraph->Layout(300, true); + } +} +BENCHMARK(BM_ParagraphJustifyLayout); + static void BM_ParagraphManyStylesLayout(benchmark::State& state) { const char* text = "A short sentence. "; auto icu_text = icu::UnicodeString::fromUTF8(text); @@ -145,7 +187,7 @@ static void BM_ParagraphTextBigO(benchmark::State& state) { state.SetComplexityN(state.range(0)); } BENCHMARK(BM_ParagraphTextBigO) - ->RangeMultiplier(20) + ->RangeMultiplier(10) ->Range(1 << 6, 1 << 14) ->Complexity(benchmark::oN); diff --git a/engine/src/flutter/src/paragraph.cc b/engine/src/flutter/src/paragraph.cc index bf07472d9f9..1fcad9ee75c 100644 --- a/engine/src/flutter/src/paragraph.cc +++ b/engine/src/flutter/src/paragraph.cc @@ -16,6 +16,7 @@ #include "lib/txt/src/paragraph.h" +#include #include #include #include @@ -24,6 +25,7 @@ #include #include "lib/ftl/logging.h" +#include "lib/txt/libs/minikin/HbFontCache.h" #include "lib/txt/libs/minikin/LayoutUtils.h" #include "lib/txt/src/font_collection.h" #include "lib/txt/src/font_skia.h" @@ -140,6 +142,18 @@ void Paragraph::AddRunsToLineBreaker( } } +void Paragraph::FillWhitespaceSet(size_t start, + size_t end, + hb_font_t* hb_font) { + uint32_t unusedGlyph; + for (size_t i = start; i < end; ++i) { + if (minikin::isWordSpace(text_[i])) { + hb_font_get_glyph(hb_font, text_[i], 0, &unusedGlyph); + whitespace_set_.insert(unusedGlyph); + } + } +} + void Paragraph::Layout(double width, bool force) { // Do not allow calling layout multiple times without changing anything. if (!needs_layout_ && !force) @@ -189,7 +203,12 @@ void Paragraph::Layout(double width, bool force) { double prev_max_descent = 0.0f; double line_width = 0.0f; std::vector x_queue; - size_t character_index = 0; + double justify_spacing = 0; + + // Each blob. + std::vector buffers; + std::vector buffer_sizes; + int word_count = 0; auto postprocess_line = [this, &x_queue, &y]() -> void { size_t record_index = 0; @@ -271,12 +290,17 @@ void Paragraph::Layout(double width, bool force) { text_.size(), bidiFlags, font, minikin_paint, font_collection_->GetMinikinFontCollectionForFamily( run.style.font_family)); + + FillWhitespaceSet(run.start, run.end, + minikin::getHbFontLocked(layout.getFont(run_index))); + const size_t glyph_count = layout.nGlyphs(); size_t blob_start = 0; + // Each blob. - std::vector buffers; - std::vector buffer_sizes; - int word_count = 0; + buffers = std::vector(); + buffer_sizes = std::vector(); + word_count = 0; while (blob_start < glyph_count) { const size_t blob_length = GetBlobLength(layout, blob_start); buffer_sizes.push_back(blob_length); @@ -285,11 +309,10 @@ void Paragraph::Layout(double width, bool force) { // Check if we should remove trailing whitespace of blobs. size_t trailing_length = 0; - while ( - paragraph_style_.text_align == TextAlign::justify && - minikin::isWordSpace( - text_[character_index + blob_length - trailing_length - 1]) && - layout_end == next_break) { + while (paragraph_style_.text_align == TextAlign::justify && + minikin::isWordSpace( + text_[blob_start + blob_length - trailing_length - 1]) && + layout_end == next_break) { ++trailing_length; } @@ -307,7 +330,7 @@ void Paragraph::Layout(double width, bool force) { buffers.back()->glyphs[blob_index] = layout.getGlyphId(glyph_index); // Check if the current Glyph is a whitespace and handle multiple // whitespaces in a row. - if (minikin::isWordSpace(text_[character_index])) { + if (whitespace_set_.count(layout.getGlyphId(glyph_index)) > 0) { // Only increment word_count if it is the first in a series of // whitespaces. if (whitespace_ended) @@ -316,7 +339,7 @@ void Paragraph::Layout(double width, bool force) { } else { whitespace_ended = true; } - ++character_index; + const size_t pos_index = 2 * blob_index; buffers.back()->pos[pos_index] = @@ -328,7 +351,6 @@ void Paragraph::Layout(double width, bool force) { letter_spacing_offset += run.style.letter_spacing; } blob_start += blob_length; - character_index += trailing_length; // Subtract letter offset to avoid big gap at end of run. This my be // removed depending on the specifications for letter spacing. @@ -344,8 +366,8 @@ void Paragraph::Layout(double width, bool force) { paint.getFontMetrics(&metrics); // Apply additional word spacing if the text is justified. if (paragraph_style_.text_align == TextAlign::justify && - buffer_sizes.size() > 0 && character_index != text_.size()) { - JustifyLine(buffers, buffer_sizes, word_count, character_index); + buffer_sizes.size() > 0) { + JustifyLine(buffers, buffer_sizes, word_count, justify_spacing); } records_.push_back( PaintRecord{run.style, builder.make(), metrics, lines_}); @@ -390,9 +412,8 @@ void Paragraph::Layout(double width, bool force) { max_descent = 0.0f; x = 0.0f; letter_spacing_offset = 0.0f; - word_count = 0; + // word_count = 0; line_width = 0.0f; - character_index = layout_end; break_index += 1; lines_++; glyph_single_line_position_x = std::vector(); @@ -413,6 +434,12 @@ void Paragraph::Layout(double width, bool force) { line_heights_.push_back(FLT_MAX); 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 && + buffer_sizes.size() > 0) { + JustifyLine(buffers, buffer_sizes, word_count, justify_spacing, -1); + } } // Amends the buffers to incorporate justification. @@ -420,36 +447,34 @@ void Paragraph::JustifyLine( std::vector& buffers, std::vector& buffer_sizes, int word_count, - size_t character_index) { - // TODO(garyq): Add letter_spacing_offset back in. It is Temporarily - // removed. - double justify_spacing = - (width_ - breaker_.getWidths()[lines_]) / (word_count - 1); + double& justify_spacing, + double multiplier) { + // We will use the previous justification spacing when undoing justification. + if (multiplier > 0) { + justify_spacing = + (width_ - breaker_.getWidths()[lines_]) / (word_count - 1); + } word_count = 0; - // Set up index to properly access text_ because minikin::isWordSpace() - // takes uint_16 instead of GlyphIDs. - size_t line_character_index = character_index; - for (size_t i = 0; i < buffers.size(); ++i) - line_character_index -= buffer_sizes[i]; bool whitespace_ended = true; for (size_t i = 0; i < buffers.size(); ++i) { for (size_t glyph_index = 0; glyph_index < buffer_sizes[i]; ++glyph_index) { // Check if the current Glyph is a whitespace and handle multiple // whitespaces in a row. - if (minikin::isWordSpace(text_[line_character_index])) { + if (whitespace_set_.count(buffers[i]->glyphs[glyph_index]) > 0) { // Only increment word_count and add justification spacing to // whitespace if it is the first in a series of whitespaces. if (whitespace_ended) { ++word_count; - buffers[i]->pos[glyph_index * 2] += justify_spacing * word_count; + buffers[i]->pos[glyph_index * 2] += + justify_spacing * multiplier * word_count; } whitespace_ended = false; } else { // Add justification spacing for all non-whitespace glyphs. - buffers[i]->pos[glyph_index * 2] += justify_spacing * word_count; + buffers[i]->pos[glyph_index * 2] += + justify_spacing * multiplier * word_count; whitespace_ended = true; } - ++line_character_index; } } } diff --git a/engine/src/flutter/src/paragraph.h b/engine/src/flutter/src/paragraph.h index 7b5278fc919..87398f126de 100644 --- a/engine/src/flutter/src/paragraph.h +++ b/engine/src/flutter/src/paragraph.h @@ -35,6 +35,8 @@ class SkCanvas; namespace txt { +using GlyphID = uint32_t; + class Paragraph { public: Paragraph(); @@ -97,6 +99,9 @@ class Paragraph { std::vector line_heights_; std::vector> glyph_position_x_; + // Set of glyph IDs that correspond to whitespace. + std::set whitespace_set_; + ParagraphStyle paragraph_style_; FontCollection* font_collection_; SkScalar height_ = 0.0f; @@ -128,10 +133,13 @@ class Paragraph { std::unordered_map>& collection_map); + void FillWhitespaceSet(size_t start, size_t end, hb_font_t* hb_font); + void JustifyLine(std::vector& buffers, std::vector& buffer_sizes, int word_count, - size_t character_index); + double& justify_spacing, + double multiplier = 1); void PaintDecorations(SkCanvas* canvas, double x, diff --git a/engine/src/flutter/tests/txt/paragraph_unittests.cc b/engine/src/flutter/tests/txt/paragraph_unittests.cc index 3d72c9199c4..3b9a5f5c7ed 100644 --- a/engine/src/flutter/tests/txt/paragraph_unittests.cc +++ b/engine/src/flutter/tests/txt/paragraph_unittests.cc @@ -588,7 +588,7 @@ TEST_F(RenderTest, JustifyAlignParagraph) { txt::TextStyle text_style; text_style.font_size = 26; - text_style.letter_spacing = 1; + text_style.letter_spacing = 0; text_style.word_spacing = 5; text_style.color = SK_ColorBLACK; text_style.height = 1;