libtxt: fix the paragraph level text direction passed to the ICU bidi…

… API (flutter#4455) Also includes some cleanup of the BidiRun struct
voyagewu · Dec 14, 2017 · 1e669f0 · 1e669f0
1 parent e0d19e2
commit 1e669f0
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 23 deletions.
diff --git a/third_party/txt/src/txt/paragraph.cc b/third_party/txt/src/txt/paragraph.cc
@@ -257,18 +257,16 @@ bool Paragraph::ComputeLineBreaks() {
   return true;
 }
 
-bool Paragraph::ComputeBidiRuns() {
-  bidi_runs_.clear();
-
+bool Paragraph::ComputeBidiRuns(std::vector<BidiRun>* result) {
   auto ubidi_closer = [](UBiDi* b) { ubidi_close(b); };
   std::unique_ptr<UBiDi, decltype(ubidi_closer)> bidi(ubidi_open(),
                                                       ubidi_closer);
   if (!bidi)
     return false;
 
   UBiDiLevel paraLevel = (paragraph_style_.text_direction == TextDirection::rtl)
-                             ? UBIDI_DEFAULT_RTL
-                             : UBIDI_DEFAULT_LTR;
+                             ? UBIDI_RTL
+                             : UBIDI_LTR;
   UErrorCode status = U_ZERO_ERROR;
   ubidi_setPara(bidi.get(), reinterpret_cast<const UChar*>(text_.data()),
                 text_.size(), paraLevel, nullptr, &status);
@@ -326,8 +324,8 @@ bool Paragraph::ComputeBidiRuns() {
       styled_run_iter--;
       const StyledRuns::Run& styled_run = styled_run_iter->second;
       size_t chunk_end = std::min(bidi_run_end, styled_run.end);
-      bidi_runs_.emplace_back(chunk_start, chunk_end, text_direction,
-                              styled_run.style);
+      result->emplace_back(chunk_start, chunk_end, text_direction,
+                           styled_run.style);
       chunk_start = chunk_end;
     }
   }
@@ -344,10 +342,12 @@ void Paragraph::Layout(double width, bool force) {
 
   width_ = width;
 
-  if (!ComputeLineBreaks()) {
+  if (!ComputeLineBreaks())
+    return;
+
+  std::vector<BidiRun> bidi_runs;
+  if (!ComputeBidiRuns(&bidi_runs))
     return;
-  }
-  ComputeBidiRuns();
 
   SkPaint paint;
   paint.setAntiAlias(true);
@@ -389,7 +389,7 @@ void Paragraph::Layout(double width, bool force) {
 
     // Find the runs comprising this line.
     std::vector<BidiRun> line_runs;
-    for (const BidiRun& bidi_run : bidi_runs_) {
+    for (const BidiRun& bidi_run : bidi_runs) {
       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_range.end),

diff --git a/third_party/txt/src/txt/paragraph.h b/third_party/txt/src/txt/paragraph.h
@@ -78,16 +78,6 @@ class Paragraph {
     }
   };
 
-  struct BidiRun {
-    BidiRun(size_t s, size_t e, TextDirection d, const TextStyle& st)
-        : start(s), end(e), direction(d), style(st) {}
-    size_t start, end;
-    TextDirection direction;
-    const TextStyle& style;
-
-    bool is_rtl() const { return direction == TextDirection::rtl; }
-  };
-
   // Minikin Layout doLayout() and LineBreaker addStyleRun() has an
   // O(N^2) (according to benchmarks) time complexity where N is the total
   // number of characters. However, this is not significant for reasonably sized
@@ -188,7 +178,6 @@ class Paragraph {
   // Starting data to layout.
   std::vector<uint16_t> text_;
   StyledRuns runs_;
-  std::vector<BidiRun> bidi_runs_;
   ParagraphStyle paragraph_style_;
   std::shared_ptr<FontCollection> font_collection_;
 
@@ -210,6 +199,17 @@ class Paragraph {
   std::vector<double> line_baselines_;
   bool did_exceed_max_lines_;
 
+  struct BidiRun {
+    BidiRun(size_t s, size_t e, TextDirection d, const TextStyle& st)
+        : start(s), end(e), direction(d), style(st) {}
+
+    const size_t start, end;
+    const TextDirection direction;
+    const TextStyle& style;
+
+    bool is_rtl() const { return direction == TextDirection::rtl; }
+  };
+
   struct GlyphPosition {
     Range<size_t> code_units;
     Range<double> x_pos;
@@ -284,7 +284,7 @@ class Paragraph {
   bool ComputeLineBreaks();
 
   // Break the text into runs based on LTR/RTL text direction.
-  bool ComputeBidiRuns();
+  bool ComputeBidiRuns(std::vector<BidiRun>* result);
 
   // Calculate the starting X offset of a line based on the line's width and
   // alignment.