Move allow_character_break property to RenderText.

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 206 matching lines @@
 // the given runs. |min_baseline| and |min_height| are the minimum baseline and
 // height for each line.
 // TODO(ckocagil): Expose the interface of this class in the header and test
 //                 this class directly.
 class HarfBuzzLineBreaker {
  public:
   HarfBuzzLineBreaker(size_t max_width,
                       int min_baseline,
                       float min_height,
                       bool multiline,
+                      WordWrapBehavior word_wrap_behavior,
                       const base::string16& text,
                       const BreakList<size_t>* words,
                       const internal::TextRunList& run_list)
       : max_width_((max_width == 0) ? SK_ScalarMax : SkIntToScalar(max_width)),
         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) {
     DCHECK_EQ(multiline_, (words_ != nullptr));
     AdvanceLine();
   }
@@ skipping 34 matching lines @@
   // 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) {
     const internal::TextRunHarfBuzz& run = *(run_list_.runs()[run_index]);
     SkScalar width = 0;
     size_t next_char = run.range.start();
 
     // 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;
       const bool skip_line =
-          BreakRunAtWidth(run, current_char, &width, &next_char);
-      AddSegment(run_index, Range(current_char, next_char),
+          BreakRunAtWidth(run, current_char, &width, &end_char, &next_char);
+      AddSegment(run_index, Range(current_char, end_char),
                  SkScalarToFloat(width));
       if (skip_line)
         AdvanceLine();
     }
   }
 
   // Starting from |start_char|, finds a suitable line break position at or
   // before available width using word break. If the current position is at the
   // beginning of a line, this function will not roll back to |start_char| and
   // |*next_char| will be greater than |start_char| (to avoid constructing empty
-  // lines).
+  // lines). It stores the end of the segment range to |end_char|, which can be
+  // smaller than |*next_char| if word_wrap_behavior is TRUNCATE_LONG_WORDS.

[msw, 2015/03/26 19:36:29]

nit: "smaller than |*next_char| for certain word wrapping behaviors."
(that should eventually be the case for elide too, right?)

[Jun Mukai, 2015/03/26 22:47:38]

Done.

   // Returns whether to skip the line before |*next_char|.
   // TODO(ckocagil): We might have to reshape after breaking at ligatures.
   //                 See whether resolving the TODO above resolves this too.
   // TODO(ckocagil): Do not reserve width for whitespace at the end of 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;
     *width = 0;
 
     Range char_range;
+    bool truncated = false;
     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(word_wrap_behavior_ == IGNORE_LONG_WORDS ||

[msw, 2015/03/26 19:36:30]

nit: maybe just DCHECK_NE WRAP_LONG_WORDS?

[Jun Mukai, 2015/03/26 22:47:38]

Done.

+                 word_wrap_behavior_ == TRUNCATE_LONG_WORDS);
+          *next_char = i;
+          if (!truncated)
+            *end_char = *next_char;
+          return true;
+        }
         word = next_word++;
         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) {
           // Roll back one word.
           *width -= word_width;
           *next_char = std::max(word->first, start_char);
-        } else 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) {
+          *end_char = i;

[msw, 2015/03/26 19:36:30]

Shouldn't end_char be set as soon as the width exceeds the available width, and
not afterward? Otherwise, doesn't this extend end_char for each overflowing
char, so that it'll match next_char? ie. shouldn't the condition above be:
        } else if (word_wrap_behavior_ == TRUNCATE_LONG_WORDS && !truncated) {

Actually, I wonder if you can simplify the code a bit:
1) Remove this "else if" block and |truncated|.
2) After "if (*width > available_width)" add "else { *end_char = i; }".
3) Change 335 and 382 to check "if (word_wrap_behavior_ == IGNORE_LONG_WORDS)".

[Jun Mukai, 2015/03/26 22:47:38]

You are right, "!truncated" is necessary in the condition. My test case didn't
catch that. Thanks!

however I'm afraid the latter approach doesn't work, anyways truncated flag is
necessary to truncate in middle of a word, we want to record the first character
which exceeds the available width (when the behavior is TRUNCATE).

[msw, 2015/03/26 23:05:54]

Hmm, if we only set "*end_char = i;" if (*width <= available_width), as I
suggested in #2 above, and when (word_wrap_behavior_ != TRUNCATE_LONG_WORDS) for
lines 335 and 383, as I suggested in #3, wouldn't that correctly capture the
first character that exceeds the available width?

[Jun Mukai, 2015/03/27 00:31:58]

Aagh, I see. Sorry for my poor understanding, now I got it.
Fixed (to be more precise, *end_char is not i, it's char_range.end(), otherwise
it's truncated a bit more aggressively).

+          truncated = 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;
         }
-        return true;
       }
     }
 
     *next_char = run.range.end();
+    if (!truncated)
+      *end_char = *next_char;
     return false;
   }
 
   // RTL runs are broken in logical order but displayed in visual order. To find
   // the text-space coordinate (where it would fall in a single-line text)
   // |x_range| of RTL segments, segment widths are applied in reverse order.
   // e.g. {[5, 10], [10, 40]} will become {[35, 40], [5, 35]}.
   void UpdateRTLSegmentRanges() {
     if (rtl_segments_.empty())
       return;
@@ skipping 72 matching lines @@
         UpdateRTLSegmentRanges();
     }
     text_x_ += SkFloatToScalar(width);
     line_x_ += SkFloatToScalar(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_;
@@ skipping 529 matching lines @@
   if (lines().empty()) {
     // TODO(vadimt): Remove ScopedTracker below once crbug.com/431326 is fixed.
     tracked_objects::ScopedTracker tracking_profile2(
         FROM_HERE_WITH_EXPLICIT_FUNCTION(
             "431326 RenderTextHarfBuzz::EnsureLayout2"));
 
     internal::TextRunList* run_list = GetRunList();
     HarfBuzzLineBreaker line_breaker(
         display_rect().width(), font_list().GetBaseline(),
         std::max(font_list().GetHeight(), min_line_height()), multiline(),
-        GetDisplayText(), multiline() ? &GetLineBreaks() : nullptr, *run_list);
+        word_wrap_behavior(), GetDisplayText(),
+        multiline() ? &GetLineBreaks() : nullptr, *run_list);
 
     // TODO(vadimt): Remove ScopedTracker below once crbug.com/431326 is fixed.
     tracked_objects::ScopedTracker tracking_profile3(
         FROM_HERE_WITH_EXPLICIT_FUNCTION(
             "431326 RenderTextHarfBuzz::EnsureLayout3"));
 
     for (size_t i = 0; i < run_list->size(); ++i)
       line_breaker.AddRun(i);
     std::vector<internal::Line> lines;
     line_breaker.Finalize(&lines, &total_size_);
@@ skipping 514 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