Stop combining text runs which are connected by 'COMMON' blocks.

ui/gfx/render_text_harfbuzz.cc

Index: ui/gfx/render_text_harfbuzz.cc
diff --git a/ui/gfx/render_text_harfbuzz.cc b/ui/gfx/render_text_harfbuzz.cc
index 32a9e554f60b47127cef9d426aef71a9f410adfa..3a9381636a6ee6ab03dd1ecc8d3115931a70b7cc 100644
--- a/ui/gfx/render_text_harfbuzz.cc
+++ b/ui/gfx/render_text_harfbuzz.cc
@@ -90,15 +90,12 @@ size_t FindRunBreakingCharacter(const base::string16& text,
   return run_break;
 }
 
-// If the given scripts match, returns the one that isn't USCRIPT_COMMON or
-// USCRIPT_INHERITED, i.e. the more specific one. Otherwise returns
-// USCRIPT_INVALID_CODE.
+// If the given scripts match, returns the one that isn't USCRIPT_INHERITED,
+// i.e. the more specific one. Otherwise returns USCRIPT_INVALID_CODE.
 UScriptCode ScriptIntersect(UScriptCode first, UScriptCode second) {
-  if (first == second ||
-      (second > USCRIPT_INVALID_CODE && second <= USCRIPT_INHERITED)) {
+  if (first == second || second == USCRIPT_INHERITED)
     return first;
-  }
-  if (first > USCRIPT_INVALID_CODE && first <= USCRIPT_INHERITED)
+  if (first == USCRIPT_INHERITED)
     return second;
   return USCRIPT_INVALID_CODE;
 }
@@ -166,6 +163,9 @@ int ScriptInterval(const base::string16& text,
   *script = scripts[0];
 
   while (char_iterator.Advance()) {
+    // Special handling to merge white space into the previous run.
+    if (u_isUWhiteSpace(char_iterator.get()))
+      continue;
     ScriptSetIntersect(char_iterator.get(), scripts, &scripts_size);
     if (scripts_size == 0U)
       return char_iterator.array_pos();
@@ -239,7 +239,9 @@ class HarfBuzzLineBreaker {
         text_x_(0),
         line_x_(0),
         max_descent_(0),
-        max_ascent_(0) {
+        max_ascent_(0),
+        next_char_pos_(0),
+        word_width_(0) {
     DCHECK_EQ(multiline_, (words_ != nullptr));
     AdvanceLine();
   }
@@ -250,11 +252,56 @@ class HarfBuzzLineBreaker {
     base::char16 first_char = text_[run->range.start()];
     if (multiline_ && first_char == '\n') {
       AdvanceLine();
-    } else if (multiline_ && (line_x_ + SkFloatToScalar(run->width)) >
-               max_width_) {
-      BreakRun(run_index);
+      return;
+    }
+
+    int ignored_char_offset = 0;
+    SkScalar ignored_width = 0;
+    if (multiline_ && word_wrap_behavior_ == TRUNCATE_LONG_WORDS ) {

[Jun Mukai, 2015/05/11 05:14:38]

This logic looks fairly complicated.  I don't get why.  Why don't you integrate
this into BreakRunAtWidth?

I think your logic here is to skip runs when its width is totally hidden by
available_width. You can do that with modifying BreakRunAtWidth().  This way,
you'll have to have a minor fix on BreakRun() to avoid adding segments if
BreakRunAtWidth()'s result is an empty segment.

[xdai1, 2015/05/11 21:55:22]

Done.

+      // Check if the run needs to be fully or partially ignored.
+      if (run->range.start() < next_char_pos_) {
+        BreakList<size_t>::const_iterator first_word_in_run =
+            words_->GetBreak(run->range.start());
+        BreakList<size_t>::const_iterator last_word_in_run =
+            words_->GetBreak(run->range.end() - 1);
+        if (first_word_in_run == last_word_in_run) {
+          // The run's range is fully covered by the current word.
+          return;
+        } else {
+          // The run contains multiple words, calculate |ignored_char_offset|
+          // and |ignored_width|.
+          BreakList<size_t>::const_iterator next_word = first_word_in_run + 1;
+          ignored_char_offset = next_word->first - run->range.start();
+          Range ignored_char_range(run->range.start(), next_word->first);
+          Range ignored_glyph_range =
+              run->CharRangeToGlyphRange(ignored_char_range);
+          ignored_width =
+              ((ignored_glyph_range.end() >= run->glyph_count)
+                  ? SkFloatToScalar(run->width)
+                  : run->positions[ignored_glyph_range.end()].x()) -
+              run->positions[ignored_glyph_range.start()].x();
+        }
+      }
+    }
+
+    if (multiline_ && line_x_ != 0) {
+      // If the following word is not the first word in the current line and
+      //  can't be fit into the current line, advance a new line.
+      BreakList<size_t>::const_iterator word =
+          words_->GetBreak(run->range.start());
+      if (run->range.start() == word->first &&
+          line_x_ + GetWordWidth(word) - ignored_width > max_width_) {
+        AdvanceLine();
+      }
+    }
+
+    if (multiline_ &&
+        (line_x_ + SkFloatToScalar(run->width) - ignored_width) > max_width_) {
+      BreakRun(run_index, ignored_char_offset);
     } else {
-      AddSegment(run_index, run->range, run->width);
+      Range char_range(run->range.start() + ignored_char_offset,
+                       run->range.end());
+      AddSegment(run_index, char_range, run->width - ignored_width);
     }
   }
 
@@ -276,20 +323,44 @@ class HarfBuzzLineBreaker {
     return &lines_[handle.first].segments[handle.second];
   }
 
+  SkScalar GetWordWidth(BreakList<size_t>::const_iterator word) {
+    if (word == words_->breaks().end())
+      return 0;
+    size_t word_st = word->first;
+    size_t word_en = (word + 1 == words_->breaks().end())
+                         ? text_.size()
+                         : (word + 1)->first;
+    internal::TextRunList::TextRunHarfBuzzIter run_st =
+        run_list_.GetRunAt(word_st);
+    internal::TextRunList::TextRunHarfBuzzIter run_en =
+        run_list_.GetRunAt(word_en);
+    SkScalar width = 0;
+    for (auto iter = run_st; iter != run_en; iter++) {
+      Range char_range = (*iter)->range.Intersect(Range(word_st, word_en));
+      Range glyph_range = (*iter)->CharRangeToGlyphRange(char_range);
+      SkScalar char_width = ((glyph_range.end() >= (*iter)->glyph_count)
+                                 ? SkFloatToScalar((*iter)->width)
+                                 : (*iter)->positions[glyph_range.end()].x()) -
+                            (*iter)->positions[glyph_range.start()].x();
+      width += char_width;
+    }
+    return width;
+  }
+
   // Breaks a run into segments that fit in the last line in |lines_| and adds
   // them. Adds a new Line to the back of |lines_| whenever a new segment can't
   // be added without the Line's width exceeding |max_width_|.
-  void BreakRun(int run_index) {
+  void BreakRun(int run_index, int ignored_offset) {
     const internal::TextRunHarfBuzz& run = *(run_list_.runs()[run_index]);
     SkScalar width = 0;
-    size_t next_char = run.range.start();
+    next_char_pos_ = run.range.start() + ignored_offset;
 
     // Break the run until it fits the current line.
-    while (next_char < run.range.end()) {
-      const size_t current_char = next_char;
-      size_t end_char = next_char;
+    while (next_char_pos_ < run.range.end()) {
+      const size_t current_char = next_char_pos_;
+      size_t end_char = next_char_pos_;
       const bool skip_line =
-          BreakRunAtWidth(run, current_char, &width, &end_char, &next_char);
+          BreakRunAtWidth(run, current_char, &width, &end_char);
       AddSegment(run_index, Range(current_char, end_char),
                  SkScalarToFloat(width));
       if (skip_line)
@@ -310,15 +381,15 @@ class HarfBuzzLineBreaker {
   bool BreakRunAtWidth(const internal::TextRunHarfBuzz& run,
                        size_t start_char,
                        SkScalar* width,
-                       size_t* end_char,
-                       size_t* next_char) {
+                       size_t* end_char) {
     DCHECK(words_);
     DCHECK(run.range.Contains(Range(start_char, start_char + 1)));
     SkScalar available_width = max_width_ - line_x_;
     BreakList<size_t>::const_iterator word = words_->GetBreak(start_char);
     BreakList<size_t>::const_iterator next_word = word + 1;
-    // Width from |std::max(word->first, start_char)| to the current character.
-    SkScalar word_width = 0;
+    size_t word_end_pos =
+        (next_word == words_->breaks().end()) ? text_.size() : next_word->first;
+    word_width_ = (start_char == word->first) ? 0 : word_width_;
     *width = 0;
 
     Range char_range;
@@ -328,17 +399,11 @@ class HarfBuzzLineBreaker {
       // the word boundary right after |i|. Advance both |word| and |next_word|
       // when |i| reaches |next_word|.
       if (next_word != words_->breaks().end() && i >= next_word->first) {
-        if (*width > available_width) {

[Jun Mukai, 2015/05/11 05:14:38]

And then, you will have to keep this logic right?
What will happen on the example of "aaaaa'bb ccc" I previously commented?

[xdai1, 2015/05/11 21:55:22]

As we discussed offline, "aaaaa'bb ccc" has two words: "aaaaa'bb " and "ccc",
three runs: "aaaaa", "'", and "bb ccc", the first two runs are fine, when it
comes to the third run, because of "bb " still belongs to the first word, "bb "
will expand to the first line (because of logic line 423 in Patch 10) but "ccc"
will be put into the second line (also because of logic 423 in Patch 10).

-          DCHECK_NE(WRAP_LONG_WORDS, word_wrap_behavior_);
-          *next_char = i;
-          if (word_wrap_behavior_ != TRUNCATE_LONG_WORDS)
-            *end_char = *next_char;
-          else
-            *width = truncated_width;
-          return true;
-        }
         word = next_word++;
-        word_width = 0;
+        word_end_pos = (next_word == words_->breaks().end())
+                            ? text_.size()
+                            : next_word->first;
+        word_width_ = 0;
       }
 
       Range glyph_range;
@@ -351,26 +416,35 @@ class HarfBuzzLineBreaker {
                             run.positions[glyph_range.start()].x();
 
       *width += char_width;
-      word_width += char_width;
+      word_width_ += char_width;
 
       // TODO(mukai): implement ELIDE_LONG_WORDS.
       if (*width > available_width) {
-        if (line_x_ != 0 || word_width < *width) {
+        if ((line_x_ != 0 && word_width_ <= *width) ||
+            (line_x_ == 0 && word_width_ < *width)) {
           // Roll back one word.
-          *width -= word_width;
-          *next_char = std::max(word->first, start_char);
-          *end_char = *next_char;
+          *width -= word_width_;
+          next_char_pos_ = std::max(word->first, start_char);;
+          *end_char = next_char_pos_;
+          word_width_ = 0;
           return true;
         } else if (word_wrap_behavior_ == WRAP_LONG_WORDS) {
-          if (char_width < *width) {
+          if (char_width < *width ||
+              (word_width_ > *width && char_width <= *width)) {
             // Roll back one character.
             *width -= char_width;
-            *next_char = i;
+            next_char_pos_ = i;
           } else {
             // Continue from the next character.
-            *next_char = i + char_range.length();
+            next_char_pos_ = i + char_range.length();
           }
-          *end_char = *next_char;
+          *end_char = next_char_pos_;
+          word_width_ = 0;
+          return true;
+        } else if (word_wrap_behavior_ == TRUNCATE_LONG_WORDS) {
+          *width = truncated_width;
+          next_char_pos_ = word_end_pos;
+          word_width_ = 0;
           return true;
         }
       } else {
@@ -381,7 +455,7 @@ class HarfBuzzLineBreaker {
 
     if (word_wrap_behavior_ == TRUNCATE_LONG_WORDS)
       *width = truncated_width;
-    *end_char = *next_char = run.range.end();
+    *end_char = next_char_pos_ = run.range.end();
     return false;
   }
 
@@ -423,6 +497,7 @@ class HarfBuzzLineBreaker {
     max_descent_ = 0;
     max_ascent_ = 0;
     line_x_ = 0;
+    word_width_ = 0;
     lines_.push_back(internal::Line());
   }
 
@@ -468,6 +543,31 @@ class HarfBuzzLineBreaker {
     }
     text_x_ += SkFloatToScalar(width);
     line_x_ += SkFloatToScalar(width);
+
+    // Calculate |word_width_|.
+    if (words_) {
+      BreakList<size_t>::const_iterator word_for_range_start =
+          words_->GetBreak(char_range.start());
+      BreakList<size_t>::const_iterator word_for_range_end =
+          (char_range.end() == 0)
+              ? words_->breaks().begin()
+              : words_->GetBreak(char_range.end() - 1);
+
+      if (word_for_range_start == word_for_range_end) {
+        // Current word covers the entire char_range.
+        if (word_width_ == 0) {
+          Range glyph_range = run.CharRangeToGlyphRange(char_range);
+          SkScalar char_width = ((glyph_range.end() >= run.glyph_count)
+                                     ? SkFloatToScalar(run.width)
+                                     : run.positions[glyph_range.end()].x()) -
+                                run.positions[glyph_range.start()].x();
+          word_width_ += char_width;
+        }
+      } else {
+        // Current char_range intersects with more than one word.
+        word_width_ = char_range.end() - word_for_range_end->first;
+      }
+    }
   }
 
   const SkScalar max_width_;
@@ -489,6 +589,14 @@ class HarfBuzzLineBreaker {
   float max_descent_;
   float max_ascent_;
 
+  // Stores the position of the character that need to be processed when
+  // breaking runs into multiple lines.
+  size_t next_char_pos_;
+  // Stores the glyph width between the current position and the start position
+  // of the current word. If the current position is pointing to the first
+  // character of the current word, |word_width_| is set to 0.
+  SkScalar word_width_;
+
   // Size of the multiline text, not including the currently processed line.
   SizeF total_size_;
 
@@ -660,6 +768,15 @@ void TextRunList::ComputePrecedingRunWidths() {
   }
 }
 
+TextRunList::TextRunHarfBuzzIter TextRunList::GetRunAt(size_t position) const {
+  for (TextRunList::TextRunHarfBuzzIter iter = runs_.begin();
+      iter != runs_.end(); iter++) {
+    if ((*iter)->range.start() <= position && (*iter)->range.end() > position)
+      return iter;
+  }
+  return runs_.end();
+}
+
 }  // namespace internal
 
 RenderTextHarfBuzz::RenderTextHarfBuzz()