libtxt: exclude trailing whitespace from right-justified lines (#5190)

If a line is right justified, then remove any trailing whitespace from the text range given to Minikin. Right justification shifts the line's glyphs by the layout advance computed by Minikin, and this advance should exclude whitespace so that the last visible character will be flush with the right margin. See #16333
flutter · May 8, 2018 · f6359e4 · f6359e4
1 parent 435028f
commit f6359e4
Show file tree

Hide file tree

Showing 6 changed files with 53 additions and 25 deletions.
diff --git a/third_party/txt/src/minikin/LineBreaker.cpp b/third_party/txt/src/minikin/LineBreaker.cpp
@@ -107,7 +107,7 @@ void LineBreaker::setIndents(const std::vector<float>& indents) {
 // end of line. It is the Unicode set:
 // [[:General_Category=Space_Separator:]-[:Line_Break=Glue:]], plus '\n'. Note:
 // all such characters are in the BMP, so it's ok to use code units for this.
-static bool isLineEndSpace(uint16_t c) {
+bool isLineEndSpace(uint16_t c) {
   return c == '\n' || c == ' ' || c == 0x1680 ||
          (0x2000 <= c && c <= 0x200A && c != 0x2007) || c == 0x205F ||
          c == 0x3000;

diff --git a/third_party/txt/src/minikin/LineBreaker.h b/third_party/txt/src/minikin/LineBreaker.h
@@ -49,6 +49,8 @@ enum HyphenationFrequency {
   kHyphenationFrequency_Full = 2
 };
 
+bool isLineEndSpace(uint16_t c);
+
 // TODO: want to generalize to be able to handle array of line widths
 class LineWidths {
  public:

diff --git a/third_party/txt/src/txt/paragraph.cc b/third_party/txt/src/txt/paragraph.cc
@@ -258,7 +258,8 @@ bool Paragraph::ComputeLineBreaks() {
     size_t block_size = block_end - block_start;
 
     if (block_size == 0) {
-      line_ranges_.emplace_back(block_start, block_end, block_end + 1, true);
+      line_ranges_.emplace_back(block_start, block_end, block_end,
+                                block_end + 1, true);
       line_widths_.push_back(0);
       continue;
     }
@@ -311,7 +312,14 @@ bool Paragraph::ComputeLineBreaks() {
       bool hard_break = i == breaks_count - 1;
       size_t line_end_including_newline =
           (hard_break && line_end < text_.size()) ? line_end + 1 : line_end;
+      size_t line_end_excluding_whitespace = line_end;
+      while (
+          line_end_excluding_whitespace > 0 &&
+          minikin::isLineEndSpace(text_[line_end_excluding_whitespace - 1])) {
+        line_end_excluding_whitespace--;
+      }
       line_ranges_.emplace_back(line_start, line_end,
+                                line_end_excluding_whitespace,
                                 line_end_including_newline, hard_break);
       line_widths_.push_back(breaker_.getWidths()[i]);
     }
@@ -462,13 +470,20 @@ void Paragraph::Layout(double width, bool force) {
       }
     }
 
+    // Exclude trailing whitespace from right-justified lines so the last
+    // visible character in the line will be flush with the right margin.
+    size_t line_end_index =
+        (paragraph_style_.effective_align() == TextAlign::right)
+            ? line_range.end_excluding_whitespace
+            : line_range.end;
+
     // Find the runs comprising this line.
     std::vector<BidiRun> line_runs;
     for (const BidiRun& bidi_run : bidi_runs) {
-      if (bidi_run.start() < line_range.end &&
+      if (bidi_run.start() < line_end_index &&
           bidi_run.end() > line_range.start) {
         line_runs.emplace_back(std::max(bidi_run.start(), line_range.start),
-                               std::min(bidi_run.end(), line_range.end),
+                               std::min(bidi_run.end(), line_end_index),
                                bidi_run.direction(), bidi_run.style());
       }
     }
@@ -687,7 +702,7 @@ void Paragraph::Layout(double width, bool force) {
     }
 
     // Adjust the glyph positions based on the alignment of the line.
-    double line_x_offset = GetLineXOffset(line_number, run_x_offset);
+    double line_x_offset = GetLineXOffset(run_x_offset);
     if (line_x_offset) {
       for (CodeUnitRun& code_unit_run : line_code_unit_runs) {
         for (GlyphPosition& position : code_unit_run.positions) {
@@ -780,25 +795,16 @@ void Paragraph::Layout(double width, bool force) {
             });
 }
 
-double Paragraph::GetLineXOffset(size_t line_number,
-                                 double line_total_advance) {
-  if (line_number >= line_widths_.size() || isinf(width_))
+double Paragraph::GetLineXOffset(double line_total_advance) {
+  if (isinf(width_))
     return 0;
 
-  TextAlign align = paragraph_style_.text_align;
-  TextDirection direction = paragraph_style_.text_direction;
-
-  if (align == TextAlign::right ||
-      (align == TextAlign::start && direction == TextDirection::rtl) ||
-      (align == TextAlign::end && direction == TextDirection::ltr)) {
-    // If this line has a soft break, then use the line width calculated by the
-    // line breaker because that width excludes the soft break's whitespace.
-    double text_width = line_ranges_[line_number].hard_break
-                            ? line_total_advance
-                            : line_widths_[line_number];
-    return width_ - text_width;
-  } else if (paragraph_style_.text_align == TextAlign::center) {
-    return (width_ - line_widths_[line_number]) / 2;
+  TextAlign align = paragraph_style_.effective_align();
+
+  if (align == TextAlign::right) {
+    return width_ - line_total_advance;
+  } else if (align == TextAlign::center) {
+    return (width_ - line_total_advance) / 2;
   } else {
     return 0;
   }

diff --git a/third_party/txt/src/txt/paragraph.h b/third_party/txt/src/txt/paragraph.h
@@ -187,9 +187,14 @@ class Paragraph {
   mutable std::unique_ptr<icu::BreakIterator> word_breaker_;
 
   struct LineRange {
-    LineRange(size_t s, size_t e, size_t ewn, bool h)
-        : start(s), end(e), end_including_newline(ewn), hard_break(h) {}
+    LineRange(size_t s, size_t e, size_t eew, size_t ein, bool h)
+        : start(s),
+          end(e),
+          end_excluding_whitespace(eew),
+          end_including_newline(ein),
+          hard_break(h) {}
     size_t start, end;
+    size_t end_excluding_whitespace;
     size_t end_including_newline;
     bool hard_break;
   };
@@ -300,7 +305,7 @@ class Paragraph {
 
   // Calculate the starting X offset of a line based on the line's width and
   // alignment.
-  double GetLineXOffset(size_t line_number, double line_total_advance);
+  double GetLineXOffset(double line_total_advance);
 
   // Creates and draws the decorations onto the canvas.
   void PaintDecorations(SkCanvas* canvas, const PaintRecord& record);

diff --git a/third_party/txt/src/txt/paragraph_style.cc b/third_party/txt/src/txt/paragraph_style.cc
@@ -37,4 +37,16 @@ bool ParagraphStyle::ellipsized() const {
   return !ellipsis.empty();
 }
 
+TextAlign ParagraphStyle::effective_align() const {
+  if (text_align == TextAlign::start) {
+    return (text_direction == TextDirection::ltr) ? TextAlign::left
+                                                  : TextAlign::right;
+  } else if (text_align == TextAlign::end) {
+    return (text_direction == TextDirection::ltr) ? TextAlign::right
+                                                  : TextAlign::left;
+  } else {
+    return text_align;
+  }
+}
+
 }  // namespace txt
diff --git a/third_party/txt/src/txt/paragraph_style.h b/third_party/txt/src/txt/paragraph_style.h
@@ -66,6 +66,9 @@ class ParagraphStyle {
 
   bool unlimited_lines() const;
   bool ellipsized() const;
+
+  // Return a text alignment value that is not dependent on the text direction.
+  TextAlign effective_align() const;
 };
 
 }  // namespace txt