Revert f6359e4 (flutter#5233)

In introduced a regression in text layout: flutter#17502
rmistry · May 11, 2018 · 9ae10ef · 9ae10ef
1 parent b856303
commit 9ae10ef
Show file tree

Hide file tree

Showing 6 changed files with 25 additions and 53 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.
-bool isLineEndSpace(uint16_t c) {
+static 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,8 +49,6 @@ 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,8 +258,7 @@ 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,
-                                block_end + 1, true);
+      line_ranges_.emplace_back(block_start, block_end, block_end + 1, true);
       line_widths_.push_back(0);
       continue;
     }
@@ -312,14 +311,7 @@ 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]);
     }
@@ -470,20 +462,13 @@ 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_end_index &&
+      if (bidi_run.start() < line_range.end &&
           bidi_run.end() > line_range.start) {
         line_runs.emplace_back(std::max(bidi_run.start(), line_range.start),
-                               std::min(bidi_run.end(), line_end_index),
+                               std::min(bidi_run.end(), line_range.end),
                                bidi_run.direction(), bidi_run.style());
       }
     }
@@ -702,7 +687,7 @@ void Paragraph::Layout(double width, bool force) {
     }
 
     // Adjust the glyph positions based on the alignment of the line.
-    double line_x_offset = GetLineXOffset(run_x_offset);
+    double line_x_offset = GetLineXOffset(line_number, run_x_offset);
     if (line_x_offset) {
       for (CodeUnitRun& code_unit_run : line_code_unit_runs) {
         for (GlyphPosition& position : code_unit_run.positions) {
@@ -795,16 +780,25 @@ void Paragraph::Layout(double width, bool force) {
             });
 }
 
-double Paragraph::GetLineXOffset(double line_total_advance) {
-  if (isinf(width_))
+double Paragraph::GetLineXOffset(size_t line_number,
+                                 double line_total_advance) {
+  if (line_number >= line_widths_.size() || isinf(width_))
     return 0;
 
-  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;
+  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;
   } else {
     return 0;
   }

diff --git a/third_party/txt/src/txt/paragraph.h b/third_party/txt/src/txt/paragraph.h
@@ -187,14 +187,9 @@ class Paragraph {
   mutable std::unique_ptr<icu::BreakIterator> word_breaker_;
 
   struct LineRange {
-    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) {}
+    LineRange(size_t s, size_t e, size_t ewn, bool h)
+        : start(s), end(e), end_including_newline(ewn), hard_break(h) {}
     size_t start, end;
-    size_t end_excluding_whitespace;
     size_t end_including_newline;
     bool hard_break;
   };
@@ -305,7 +300,7 @@ class Paragraph {
 
   // Calculate the starting X offset of a line based on the line's width and
   // alignment.
-  double GetLineXOffset(double line_total_advance);
+  double GetLineXOffset(size_t line_number, 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,16 +37,4 @@ 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,9 +66,6 @@ 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