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

ui/gfx/render_text_harfbuzz.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/gfx/render_text_harfbuzz.h"
 
 #include <limits>
 #include <set>
 
 #include "base/i18n/bidi_line_iterator.h"
@@ skipping 72 matching lines @@
     const bool block_break = current_block != first_block &&
         (first_block_unusual || IsUnusualBlockCode(current_block));
     if (block_break || current_char == '\n' ||
         first_bracket != IsBracket(current_char)) {
       return run_start + iter.array_pos();
     }
   }
   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;
 }
 
 // Writes the script and the script extensions of the character with the
 // Unicode |codepoint|. Returns the number of written scripts.
 int GetScriptExtensions(UChar32 codepoint, UScriptCode* scripts) {
   UErrorCode icu_error = U_ZERO_ERROR;
   // ICU documentation incorrectly states that the result of
   // |uscript_getScriptExtensions| will contain the regular script property.
@@ skipping 48 matching lines @@
   DCHECK_GT(length, 0U);
 
   UScriptCode scripts[kMaxScripts] = { USCRIPT_INVALID_CODE };
 
   base::i18n::UTF16CharIterator char_iterator(text.c_str() + start, length);
   size_t scripts_size = GetScriptExtensions(char_iterator.get(), scripts);
   *script = scripts[0];
 
   while (char_iterator.Advance()) {
     ScriptSetIntersect(char_iterator.get(), scripts, &scripts_size);
+    // Special handling for white space. White space should be merged into the

[msw, 2015/04/23 00:00:38]

nit: "Special handling to merge white space into the previous run."

[xdai1, 2015/04/23 22:59:42]

Done.

+    // previous run.
+    if (u_isUWhiteSpace(char_iterator.get())) {

[msw, 2015/04/23 00:00:38]

Just "continue;" instead of faking out |scripts| and |scripts_size|?

[xdai1, 2015/04/23 22:59:42]

Actually this is depending on how we want to break the text into runs. For
example, suppose we have text "abc defg". If we use "continue;" here, then we'll
get two runs: "abc " and "defg", but if we fake out |scripts| and
|scripts_size|, then we'll get one single run: "abc defg", which is same with
the behavior before this change. I think these two approaches are both fine. If
we decide to use the former one (continue;), we'll need to modify some
corresponding tests. What do you think? 

+      scripts[0] = *script;
+      scripts_size = 1U;
+    }
     if (scripts_size == 0U)
       return char_iterator.array_pos();
     *script = scripts[0];
   }
 
   return length;
 }
 
 // A port of hb_icu_script_to_script because harfbuzz on CrOS is built without
 // hb-icu. See http://crbug.com/356929
@@ skipping 131 matching lines @@
                        size_t start_char,
                        SkScalar* width,
                        size_t* end_char,
                        size_t* next_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;
+    SkScalar word_width =

[msw, 2015/04/23 00:00:38]

Is this necessary for the run breaking change? Can you split this out?

[xdai1, 2015/04/23 22:59:42]

Yes, it has to be modified as described in the CL description.

+        start_char == 0 ? 0 : GetGlyphWidth(word->first, start_char - 1);
     *width = 0;
 
     Range char_range;
     SkScalar truncated_width = 0;
     for (size_t i = start_char; i < run.range.end(); i += char_range.length()) {
       // |word| holds the word boundary at or before |i|, and |next_word| holds
       // 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) {
@@ skipping 16 matching lines @@
       SkScalar char_width = ((glyph_range.end() >= run.glyph_count)
                                  ? SkFloatToScalar(run.width)
                                  : run.positions[glyph_range.end()].x()) -
                             run.positions[glyph_range.start()].x();
 
       *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;
           return true;
         } else if (word_wrap_behavior_ == WRAP_LONG_WORDS) {
           if (char_width < *width) {
             // Roll back one character.
             *width -= char_width;
             *next_char = i;
@@ skipping 94 matching lines @@
           SegmentHandle(lines_.size() - 1, line->segments.size() - 1));
       // If this is the last segment of an RTL run, reprocess the text-space x
       // ranges of all segments from the run.
       if (char_range.end() == run.range.end())
         UpdateRTLSegmentRanges();
     }
     text_x_ += SkFloatToScalar(width);
     line_x_ += SkFloatToScalar(width);
   }
 
+  // Returns the index for the run that contains text_[pos] using binary search.
+  size_t GetRunIndexAt(size_t pos) const {
+    pos = (pos < text_.size()) ? pos : text_.size() - 1;
+    size_t low = 0, high = run_list_.size() - 1;
+    while (low <= high) {
+      size_t mid = low + (high - low) / 2;
+      if (run_list_.runs()[mid]->range.start() <= pos &&
+          run_list_.runs()[mid]->range.end() > pos)
+        return mid;
+      else if (run_list_.runs()[mid]->range.start() > pos)
+        high = mid - 1;
+      else
+        low = mid + 1;
+    }
+    return 0;
+  }
+
+  // Returns the glyph width for |text_| between [start_pos, end_pos].
+  SkScalar GetGlyphWidth(size_t start_pos, size_t end_pos) const {
+    end_pos = (end_pos < text_.size()) ? end_pos : text_.size() - 1;
+    if (start_pos > end_pos)
+      return 0;
+
+    // Finding the runs that containing |start_char| and |end_char|.
+    size_t start_run_idx = GetRunIndexAt(start_pos);
+    size_t end_run_idx = GetRunIndexAt(end_pos);
+    const internal::TextRunHarfBuzz* start_run =
+        run_list_.runs()[start_run_idx];
+    const internal::TextRunHarfBuzz* end_run = run_list_.runs()[end_run_idx];
+
+    SkScalar width = 0;
+    for (size_t i = start_run_idx; i <= end_run_idx; ++i) {
+      width += run_list_.runs()[i]->width;
+    }
+
+    Range char_range;
+    Range glyph_range;
+    for (size_t pos = start_run->range.start(); pos < start_pos; ++pos) {
+      start_run->GetClusterAt(pos, &char_range, &glyph_range);
+      SkScalar char_width =
+          ((glyph_range.end() >= start_run->glyph_count)
+               ? SkFloatToScalar(start_run->width)
+               : start_run->positions[glyph_range.end()].x()) -
+          start_run->positions[glyph_range.start()].x();
+      width -= char_width;
+    }
+
+    for (size_t pos = end_pos + 1; pos < end_run->range.end(); ++pos) {
+      end_run->GetClusterAt(pos, &char_range, &glyph_range);
+      SkScalar char_width = ((glyph_range.end() >= end_run->glyph_count)
+                                 ? SkFloatToScalar(end_run->width)
+                                 : end_run->positions[glyph_range.end()].x()) -
+                            end_run->positions[glyph_range.start()].x();
+      width -= char_width;
+    }
+
+    return width;
+  }
+
   const SkScalar max_width_;
   const int min_baseline_;
   const float min_height_;
   const bool multiline_;
   const WordWrapBehavior word_wrap_behavior_;
   const base::string16& text_;
   const BreakList<size_t>* const words_;
   const internal::TextRunList& run_list_;
 
   // Stores the resulting lines.
@@ skipping 996 matching lines @@
   DCHECK(!update_layout_run_list_);
   DCHECK(!update_display_run_list_);
   return text_elided() ? display_run_list_.get() : &layout_run_list_;
 }
 
 const internal::TextRunList* RenderTextHarfBuzz::GetRunList() const {
   return const_cast<RenderTextHarfBuzz*>(this)->GetRunList();
 }
 
 }  // namespace gfx