Fix and smooth justifications to properly handle ligatures with whitespace mapping.

Change-Id: I15bc093ed61f0540644ce31110769ee419dd7ac2

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);
 

engine/src/flutter/src/paragraph.cc

@@ -16,6 +16,7 @@
 
 #include "lib/txt/src/paragraph.h"
 
+#include <hb.h>
 #include <algorithm>
 #include <limits>
 #include <tuple>
@@ -24,6 +25,7 @@
 
 #include <minikin/Layout.h>
 #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<SkScalar> x_queue;
-  size_t character_index = 0;
+  double justify_spacing = 0;
+
+  // Each blob.
+  std::vector<const SkTextBlobBuilder::RunBuffer*> buffers;
+  std::vector<size_t> 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<const SkTextBlobBuilder::RunBuffer*> buffers;
-      std::vector<size_t> buffer_sizes;
-      int word_count = 0;
+      buffers = std::vector<const SkTextBlobBuilder::RunBuffer*>();
+      buffer_sizes = std::vector<size_t>();
+      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<double>();
@@ -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<const SkTextBlobBuilder::RunBuffer*>& buffers,
     std::vector<size_t>& 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;
     }
   }
 }

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<double> line_heights_;
   std::vector<std::vector<double>> glyph_position_x_;
 
+  // Set of glyph IDs that correspond to whitespace.
+  std::set<GlyphID> whitespace_set_;
+
   ParagraphStyle paragraph_style_;
   FontCollection* font_collection_;
   SkScalar height_ = 0.0f;
@@ -128,10 +133,13 @@ class Paragraph {
       std::unordered_map<std::string, std::shared_ptr<minikin::FontCollection>>&
           collection_map);
 
+  void FillWhitespaceSet(size_t start, size_t end, hb_font_t* hb_font);
+
   void JustifyLine(std::vector<const SkTextBlobBuilder::RunBuffer*>& buffers,
                    std::vector<size_t>& buffer_sizes,
                    int word_count,
-                   size_t character_index);
+                   double& justify_spacing,
+                   double multiplier = 1);
 
   void PaintDecorations(SkCanvas* canvas,
                         double x,

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;