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 to merge white space into the previous run.
+    if (u_isUWhiteSpace(char_iterator.get())) {

[Jun Mukai, 2015/04/28 01:57:36]

I am not so sure about other whitespace variations supports.  Did you confirm
that the Thai font can draw fullwidth space (U+3000) or some other spaces?
Otherwise directly comparing with ' ' would be safer.

[xdai1, 2015/04/29 18:23:27]

I added a test case for fullwidth space in
RenderTextTest.Multiline_LineBreakerBehavior. Based on the observation, it works
exactly the same as the normal space. I think it should be fine?

+      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 52 matching lines @@
         min_baseline_(min_baseline),
         min_height_(min_height),
         multiline_(multiline),
         word_wrap_behavior_(word_wrap_behavior),
         text_(text),
         words_(words),
         run_list_(run_list),
         text_x_(0),
         line_x_(0),
         max_descent_(0),
-        max_ascent_(0) {
+        max_ascent_(0),
+        next_word_start_pos_(0) {
     DCHECK_EQ(multiline_, (words_ != nullptr));
     AdvanceLine();
   }
 
   // Breaks the run at given |run_index| into Line structs.
   void AddRun(int run_index) {
     const internal::TextRunHarfBuzz* run = run_list_.runs()[run_index];
     base::char16 first_char = text_[run->range.start()];
+
+    if (word_wrap_behavior_ == TRUNCATE_LONG_WORDS && words_) {
+      // If the run has been processed, skip it.
+      size_t current_word_start_pos =
+          words_->GetBreak(run->range.start())->first;
+      if (current_word_start_pos < next_word_start_pos_)
+        return;
+    }
+
     if (multiline_ && first_char == '\n') {
       AdvanceLine();
     } else if (multiline_ && (line_x_ + SkFloatToScalar(run->width)) >
                max_width_) {
       BreakRun(run_index);
     } else {
       AddSegment(run_index, run->range, run->width);
     }
   }
 
@@ skipping 49 matching lines @@
   bool BreakRunAtWidth(const internal::TextRunHarfBuzz& run,
                        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;
+    next_word_start_pos_ =
+        (next_word == words_->breaks().end()) ? text_.size() : next_word->first;
+
+    // Glyph width of text_[word->first, start_char - 1].
+    SkScalar word_width =
+        start_char == 0 ? 0 : GetGlyphWidth(word->first, start_char - 1);

[Jun Mukai, 2015/04/28 01:57:36]

I think you don't have to add a method like 'GetGlyphWidth', the width in the
past runs (but in a same word) are already added and computed, you can just
remember them.

GetGlyphWidth() name and interface sounds too generic and I'm afraid it could
include tricky corner cases like mixture of RTL and LTR, in the middle of runs
and/or words. Those are not our expected cases I think. 

[xdai1, 2015/04/29 18:23:27]

Done.

     *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) {
-          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++;
+        next_word_start_pos_ = (next_word == words_->breaks().end())
+                                   ? text_.size()
+                                   : next_word->first;
         word_width = 0;
       }
 
       Range glyph_range;
       run.GetClusterAt(i, &char_range, &glyph_range);
       DCHECK_LT(0U, char_range.length());
 
       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)) {

[Jun Mukai, 2015/04/28 01:57:36]

I don't get these conditions.  If line_x_ is 0 (i.e. this run appears at the
beginning of a line), it should never wrap the word unnecessary because it
causes to leave an empty line. Am I wrong?

[xdai1, 2015/04/29 18:23:27]

For example, "I am a student", suppose the line width is 10 (which means it
should break before "d" character). When we come to "d", line_x_ equals 0, the
word_width_ equals 4, and the *width equals 11, then we know we should roll back
one word and advance a new line. 

           // 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;
           } else {
             // Continue from the next character.
             *next_char = i + char_range.length();
           }
           *end_char = *next_char;
           return true;
+        } else if (word_wrap_behavior_ == TRUNCATE_LONG_WORDS) {
+          *width = truncated_width;
+          *next_char = next_word_start_pos_;
+          return true;
         }
       } else {
         *end_char = char_range.end();
         truncated_width = *width;
       }
     }
 
     if (word_wrap_behavior_ == TRUNCATE_LONG_WORDS)
       *width = truncated_width;
     *end_char = *next_char = run.range.end();
@@ skipping 78 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.
   std::vector<internal::Line> lines_;
 
   // Text space and line space x coordinates of the next segment to be added.
   SkScalar text_x_;
   SkScalar line_x_;
 
   float max_descent_;
   float max_ascent_;
 
+  // Stores the start position of next word that need to be processed.
+  size_t next_word_start_pos_;
+
   // Size of the multiline text, not including the currently processed line.
   SizeF total_size_;
 
   // The current RTL run segments, to be applied by |UpdateRTLSegmentRanges()|.
   std::vector<SegmentHandle> rtl_segments_;
 
   DISALLOW_COPY_AND_ASSIGN(HarfBuzzLineBreaker);
 };
 
 // Function object for case insensitive string comparison.
@@ skipping 977 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