Windows implementation of multiline RenderText

ui/gfx/render_text_win.cc

 // Copyright (c) 2012 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_win.h"
 
 #include <algorithm>
 
 #include "base/i18n/break_iterator.h"
 #include "base/i18n/rtl.h"
@@ skipping 136 matching lines @@
   } else {
     result = ui::Range(run.logical_clusters[run_range.start()],
         run_range.end() < run.range.length() ?
             run.logical_clusters[run_range.end()] : run.glyph_count);
   }
   DCHECK(!result.is_reversed());
   DCHECK(ui::Range(0, run.glyph_count).Contains(result));
   return result;
 }
 
+struct BreakRunResults {
+  // Whether an overflow occurred.
+  bool overflowed;
+
+  // If |overflowed|, holds the width of [start_char, overflow_pos].
+  // Otherwise, holds the width of [start_char, run.range.end()).
+  int width;
+
+  // Index of the first character that doesn't fit the given width.
+  size_t overflow_pos;
+
+  // Width of the character at |overflow_pos|.
+  int char_rollback_width;
+
+  // Index of the last word break before |overflow_pos|.
+  size_t word_rollback_pos;
+
+  // Width of [word_rollback_pos, overflow_pos].
+  int word_rollback_width;
+};
+
+// Starting from |start_char|, finds the first character that doesn't fit the
+// given |width_cap|. If |width_cap| is reached, returns true and fills all the
+// fields in the result. Otherwise returns false and only fills |width|.
+BreakRunResults BreakRunAtWidth(const internal::TextRun& run,

[Alexei Svitkine (slow), 2013/08/12 20:42:33]

Returning the whole struct by value is a bit more expensive than I'd like.

How about making the function return a bool indicating whether an overflow
occurred (i.e. same as your current |overflowed| member) and taking a pointer to
an OverflowInfo* struct that has the other variables.

[ckocagil, 2013/08/13 20:22:28]

Done.

+                                const BreakList<size_t>& breaks,
+                                size_t start_char,
+                                int width_cap) {

[Alexei Svitkine (slow), 2013/08/12 20:42:33]

Nit: width_cap -> available_width

[ckocagil, 2013/08/13 20:22:28]

Done.

+  DCHECK(run.range.Contains(ui::Range(start_char)));
+  BreakRunResults results;

[Alexei Svitkine (slow), 2013/08/12 20:42:33]

Either add a default ctor that initializes the members or memset the struct to 0
at the start of the function.

[ckocagil, 2013/08/13 20:22:28]

Done, added memset.

+  size_t current_word = start_char;
+  // x distance from |current_word|.
+  int current_word_x = 0;
+  int x = 0;
+

[Alexei Svitkine (slow), 2013/08/12 20:42:33]

I'd like like to see this code try the whole run first, before resorting to
per-word iteration.

Or at the very least, add a TODO about it.

[ckocagil, 2013/08/13 20:22:28]

I added run width checking to LineBreaker::AddRun. Adding that check here would
require new parameters and more code here, and I would like to keep this
function simple. lgty?

+  for (size_t i = start_char; i < run.range.end(); ++i) {
+    size_t next_word = std::max(breaks.GetBreak(i)->first, start_char);

[Alexei Svitkine (slow), 2013/08/12 20:42:33]

You only need to do this if i < previous next_word value, right? If so, move
next_word outside the loop and put this in an if statement.

[ckocagil, 2013/08/13 20:22:28]

I did something different, this now stores two BreakList iterators instead of
indices. It avoids the GetBreak call in the for loop.

+    if (next_word != current_word) {
+      current_word = next_word;
+      current_word_x = 0;
+    }
+
+    ui::Range glyphs = CharRangeToGlyphRange(run, ui::Range(i, i + 1));
+    int char_width = 0;
+    for (size_t j = glyphs.start(); j < glyphs.end(); ++j)
+      char_width += run.advance_widths[j];
+
+    x += char_width;
+    current_word_x += char_width;
+
+    if (x > width_cap) {
+      results.overflowed = true;
+      results.overflow_pos = i;
+      results.width = x;
+      results.char_rollback_width = char_width;
+      results.word_rollback_pos = current_word;
+      results.word_rollback_width = current_word_x;
+      return results;
+    }
+  }
+
+  results.overflowed = false;
+  results.width = x;
+  return results;
+}
+
 }  // namespace
 
 namespace internal {
 
 TextRun::TextRun()
   : font_style(0),
     strike(false),
     diagonal_strike(false),
     underline(false),
     width(0),
@@ skipping 23 matching lines @@
       run->glyph_count,
       run->logical_clusters.get(),
       run->visible_attributes.get(),
       run->advance_widths.get(),
       &run->script_analysis,
       &x);
   DCHECK(SUCCEEDED(hr));
   return run->preceding_run_widths + x;
 }
 
+struct LineSegmentWin : LineSegment {
+  const internal::TextRun* run;
+};
+
+// Internal class to help break a text into lines.
+class LineBreaker {
+ public:
+  LineBreaker(int max_width, bool multiline, const BreakList<size_t>* words)
+      : max_width_(max_width),
+        multiline_(multiline),
+        words_(words),
+        preceding_line_heights_(0),
+        pos_(0),
+        text_x_(0),
+        line_x_(0),
+        current_pos_(0),
+        current_delta_(0) {
+    SkipLine();
+  }
+
+  // Breaks the given |runs| into |lines|. Should be called at most once for
+  // each instance. If |multiline| is false, doesn't do any breaking and fills
+  // |lines| with a single Line.
+  void AddRun(const internal::TextRun& run) {
+    ResetPos(run.range.start());
+    int width = multiline_ ? BreakRun(run) : run.width;
+    // Remaining part of the run fits the line, add it as well.
+    Advance(run.range.end(), width);
+    CommitSegment(run);
+  }
+
+  // Finishes line breaking and outputs the results. Can be called at most once.
+  void Finalize(std::vector<internal::Line>* lines) {
+    lines->swap(lines_);

[Alexei Svitkine (slow), 2013/08/12 20:42:33]

Add a DCHECK() that !lines_.empty().

[ckocagil, 2013/08/13 20:22:28]

Done.

+  }
+
+ private:
+  // 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_pos| of RTL segments, text-space coordinate of the run beginning is
+  // saved and the segment widths are applied in reverse order.
+  // e.g. {[5, 10], [10, 40]} will become {[35, 40], [5, 35]}.
+  struct RtlData {
+    int run_x_; // Beginning of the RTL run in text-space.

[Alexei Svitkine (slow), 2013/08/12 20:42:33]

It's a struct, so no need for trailing _'s.

Although, see my comments below about getting rid of this struct.

[ckocagil, 2013/08/13 20:22:28]

Got rid of this struct.

+    std::vector<LineSegment*> segments_;
+  };
+
+  // Breaks a run into segments of at most |max_width| width, adds the segments,
+  // returns the width of the final segment that fits the current line.
+  int BreakRun(const internal::TextRun& run) {
+    BreakRunResults results =
+        BreakRunAtWidth(run, *words_, pos_, max_width_ - line_x_);
+
+    // Break the run until it fits the current line.
+    while (results.overflowed) {
+      DCHECK(pos_ < run.range.end());
+      Advance(results.overflow_pos + 1, results.width);
+      // First, try rolling back one word to move it to the next line. If
+      // it's not possible, roll back one character. If neither of these
+      // could be done, don't do any rollback since each line must have at
+      // least one character.
+      if (line_x_ > 0 || results.word_rollback_width < results.width)

[Alexei Svitkine (slow), 2013/08/12 20:42:33]

I don't understand this logic. Why do we Rollback() if line_x_ > 0 or even
results.word_rollback_width < results.width. If the word is smaller than the
width without it? It's not clear to me.

Can you clarify how this works? Thanks.

[ckocagil, 2013/08/13 20:22:28]

This condition is to avoid rolling back the entire text on the line, since that
would cause an infinite loop and output infinitely many lines (rollback
everything, skip line, rollback everything, skip line, ...).

|line_x_ > 0| means there is some text before the current run, so we won't end
up with an empty line even if we rollback the entire word. If this condition
isn't satisfied, we then check |word_rollback_width < width|, which tells us
that the current segment has text other than the last word, so rolling back the
word wouldn't leave us with an empty line.

The char rollback condition below is similar.

[Alexei Svitkine (slow), 2013/08/13 20:56:32]

I see. I think I understand how this works now. You call BreakRunAtWidth(),
which returns as soon as it finds a character that doesn't fit. Yet, the issue
is you want to break by word, rather than character. So to make that work, you
have to rollback to the previous word that you got info about from
BreakRunAtWidth().

If my understanding is correct, I think it should be possible to simplify this
by moving this logic into BreakRunAtWidth().

That is, when BreakRunAtWidth() finds it can't fit any more chars, then it will
rollback to the last word internally, returning the new info from there. This
way, you can avoid passing back the extra state information about the word
rollback position and width (keep this info only in BreakRunAtWidth) and remove
the extra Rollback() functions on this class.

It may even make sense to make BreakRunAtWidth() a member function on this class
so that it can operate on |current_pos_| and such directly - once everything is
simplified. Though, this one I'm not 100% sure about.

WDYT?

[ckocagil, 2013/08/14 11:40:25]

Good call. Done.
I think there is value in having this as a separate, self-contained helper
function. It makes the job of LineBreaker much easier without making the class
any more complicated. It also doesn't have much to do with lines (it operates on
a single run), so I don't think it belongs to LineBreaker.

+        Rollback(results.word_rollback_pos, results.word_rollback_width);
+      else if (line_x_ > 0 || results.char_rollback_width < results.width)
+        Rollback(results.overflow_pos, results.char_rollback_width);
+      CommitSegment(run);
+      SkipLine();
+
+      results = BreakRunAtWidth(run, *words_, pos_, max_width_ - line_x_);
+    }
+
+    return results.width;
+  }
+
+  // Applies |rtl_data_|.
+  void PopRtl() {
+    int x = rtl_data_.run_x_;
+    while (!rtl_data_.segments_.empty()) {

[Alexei Svitkine (slow), 2013/08/12 20:42:33]

I think it would be cleaner to just iterate over them by index in reverse order
and then just .clear() them.

Although, see my comment below in CommitSegment() about not needing rtl segments
vector.

[ckocagil, 2013/08/13 20:22:28]

Done, it now iterates and .clear()s.

+      LineSegment* segment = rtl_data_.segments_.back();
+      rtl_data_.segments_.pop_back();
+      segment->x_pos = ui::Range(x, x + segment->x_pos.length());
+      x += segment->x_pos.length();
+    }
+  }
+
+  void ResetPos(size_t new_pos) {
+    pos_ = new_pos;
+    current_pos_ = new_pos;
+    current_delta_ = 0;
+  }
+
+  void Advance(size_t new_pos, int amount) {
+    current_pos_ = new_pos;
+    current_delta_ += amount;
+  }
+
+  void Rollback(size_t new_pos, int amount) {
+    Advance(new_pos, -amount);
+  }
+
+  void SkipLine() {
+    if (!lines_.empty())
+      preceding_line_heights_ += lines_.back().height;
+    line_x_ = 0;
+    lines_.push_back(internal::Line());
+  }
+
+  void CommitSegment(const internal::TextRun& run) {
+    if (pos_ == current_pos_) {
+      DCHECK(current_delta_ == 0);
+      return;
+    }
+    internal::LineSegmentWin* segment = new internal::LineSegmentWin;
+    segment->run = &run;
+    segment->char_pos = ui::Range(pos_, current_pos_);
+    segment->x_pos = ui::Range(text_x_, text_x_ + current_delta_);
+    lines_.back().segments.push_back(segment);
+    lines_.back().width += segment->x_pos.length();
+    lines_.back().height = std::max(lines_.back().height,
+                                    segment->run->font.GetHeight());
+    lines_.back().baseline = std::max(lines_.back().baseline,
+                                      segment->run->font.GetBaseline());
+    lines_.back().preceding_heights = preceding_line_heights_;
+    if (run.script_analysis.fRTL) {
+      if (pos_ == run.range.start())
+        rtl_data_.run_x_ = text_x_;
+      rtl_data_.segments_.push_back(segment);

[Alexei Svitkine (slow), 2013/08/12 20:42:33]

It seems rtl_data_.segments_ is just a list of segments for this run, in the
same order as they are in lines_.back().segments.

So I think it's not actually necessary to store them in a separate vector.
Instead, you just need to store the index of the first segment for the current
run.

Then, in PopRtl() you can simply iterate via:

for (size_t i = lines_.back().segments.size() - 1; i >= first_run_segment_; i--)

To process them.

[ckocagil, 2013/08/13 20:22:28]

I realized that |rtl_data_.run_x_| isn't really needed since the first segment's
x_pos.start() gives the same value. I'm getting rid of RtlData and moving the
vector to LineBreaker.

The reason I'm keeping a vector is because the line segments can be on different
lines. So if I were to get rid of the vector, I would have to reverse iterate
over the lines too. wdyt?

+      if (current_pos_ == run.range.end())
+        PopRtl();
+    }
+    pos_ = current_pos_;
+    text_x_ += current_delta_;
+    line_x_ += current_delta_;
+    current_delta_ = 0;
+  }
+
+  std::vector<internal::Line> lines_;
+  int max_width_;
+  bool multiline_;
+  const BreakList<size_t>* words_;
+  int preceding_line_heights_;
+  size_t pos_;

[Alexei Svitkine (slow), 2013/08/12 20:42:33]

Can you document these vars? I'd like to e.g. understand what pos_,
current_pos_, text_x_, line_x_ current_delta_ refers to, for example. If there
were clear comments about them, it would be much easier to understand.

Alternatively or additionally, better names might make this clearer. For
example, current_delta_ -> current_width_ (assuming that's what it means) and
other such changes.

[ckocagil, 2013/08/13 20:22:28]

Done both.

+  int text_x_;
+  int line_x_;
+  size_t current_pos_;
+  int current_delta_;
+  RtlData rtl_data_;

[Alexei Svitkine (slow), 2013/08/12 20:42:33]

Hmm, I'm not convinced this needs to be its own struct, if its just one entry
here.

Just inline the members - i.e. rtl_run_x_ and rtl_segments_, with appropriate
comments.

Although, see my comment above in CommitSegment() about not needing rtl segments
vector.

[ckocagil, 2013/08/13 20:22:28]

Got rid of the struct.

+
+  DISALLOW_COPY_AND_ASSIGN(LineBreaker);
+};
+
 }  // namespace internal
 
 // static
 HDC RenderTextWin::cached_hdc_ = NULL;
 
 // static
 std::map<std::string, Font> RenderTextWin::successful_substitute_fonts_;
 
 RenderTextWin::RenderTextWin()
     : RenderText(),
       common_baseline_(0),
       needs_layout_(false) {
   set_truncate_length(kMaxUniscribeTextLength);
 
   memset(&script_control_, 0, sizeof(script_control_));
   memset(&script_state_, 0, sizeof(script_state_));
 
   MoveCursorTo(EdgeSelectionModel(CURSOR_LEFT));
 }
 
 RenderTextWin::~RenderTextWin() {
 }
 
 Size RenderTextWin::GetStringSize() {
   EnsureLayout();
   return string_size_;
 }
 
+Size RenderTextWin::GetMultilineTextSize() {
+  EnsureLayout();
+  if (!multiline())
+    return Size(display_rect().width(), string_size_.height());
+  return Size(display_rect().width(),
+              lines().back().preceding_heights + lines().back().height);
+}
+
 int RenderTextWin::GetBaseline() {
   EnsureLayout();
   return common_baseline_;
 }
 
 SelectionModel RenderTextWin::FindCursorPosition(const Point& point) {
   if (text().empty())
     return SelectionModel();
 
   EnsureLayout();
@@ skipping 138 matching lines @@
                    GetGlyphXBoundary(run, layout_index, true));
 }
 
 std::vector<Rect> RenderTextWin::GetSubstringBounds(const ui::Range& range) {
   DCHECK(!needs_layout_);
   DCHECK(ui::Range(0, text().length()).Contains(range));
   ui::Range layout_range(TextIndexToLayoutIndex(range.start()),
                          TextIndexToLayoutIndex(range.end()));
   DCHECK(ui::Range(0, GetLayoutText().length()).Contains(layout_range));
 
-  std::vector<Rect> bounds;
+  std::vector<ui::Range> bounds;
+  std::vector<Rect> rects;
   if (layout_range.is_empty())
-    return bounds;
+    return rects;
 
-  // Add a Rect for each run/selection intersection.
+  // Add a Range for each run/selection intersection.
   // TODO(msw): The bounds should probably not always be leading the range ends.
   for (size_t i = 0; i < runs_.size(); ++i) {
     const internal::TextRun* run = runs_[visual_to_logical_[i]];
     ui::Range intersection = run->range.Intersect(layout_range);
     if (intersection.IsValid()) {
       DCHECK(!intersection.is_reversed());
       ui::Range range_x(GetGlyphXBoundary(run, intersection.start(), false),
                         GetGlyphXBoundary(run, intersection.end(), false));
-      Rect rect(range_x.GetMin(), 0, range_x.length(), run->font.GetHeight());
-      rect.set_origin(ToViewPoint(rect.origin()));
-      // Union this with the last rect if they're adjacent.
-      if (!bounds.empty() && rect.SharesEdgeWith(bounds.back())) {
-        rect.Union(bounds.back());
+      if (range_x.is_empty())
+        continue;
+      range_x = ui::Range(range_x.GetMin(), range_x.GetMax());
+      // Union this with the last range if they're adjacent.
+      DCHECK(bounds.empty() || bounds.back().GetMin() != range_x.GetMax());
+      if (!bounds.empty() && bounds.back().GetMax() == range_x.GetMin()) {
+        range_x = ui::Range(bounds.back().GetMin(), range_x.GetMax());
         bounds.pop_back();
       }
-      bounds.push_back(rect);
+      bounds.push_back(range_x);
     }
   }
-  return bounds;
+  for (size_t i = 0; i < bounds.size(); ++i) {
+    std::vector<Rect> current_rects = TextBoundsToViewBounds(bounds[i]);
+    rects.insert(rects.end(), current_rects.begin(), current_rects.end());
+  }
+  return rects;
 }
 
 size_t RenderTextWin::TextIndexToLayoutIndex(size_t index) const {
   DCHECK_LE(index, text().length());
   ptrdiff_t i = obscured() ? ui::UTF16IndexToOffset(text(), 0, index) : index;
   CHECK_GE(i, 0);
   // Clamp layout indices to the length of the text actually used for layout.
   return std::min<size_t>(GetLayoutText().length(), i);
 }
 
@@ skipping 21 matching lines @@
          position < LayoutIndexToTextIndex(GetLayoutText().length()) &&
          GetGlyphBounds(position) != GetGlyphBounds(position - 1);
 }
 
 void RenderTextWin::ResetLayout() {
   // Layout is performed lazily as needed for drawing/metrics.
   needs_layout_ = true;
 }
 
 void RenderTextWin::EnsureLayout() {
-  if (!needs_layout_)
-    return;
-  // TODO(msw): Skip complex processing if ScriptIsComplex returns false.
-  ItemizeLogicalText();
-  if (!runs_.empty())
-    LayoutVisualText();
-  needs_layout_ = false;
+  if (needs_layout_) {
+    // TODO(msw): Skip complex processing if ScriptIsComplex returns false.
+    ItemizeLogicalText();
+    if (!runs_.empty())
+      LayoutVisualText();
+    needs_layout_ = false;
+    std::vector<internal::Line> lines;
+    set_lines(&lines);
+  }
+  // Compute lines if they're not valid. This is separate from the layout steps
+  // above to avoid text layout and shaping when we resize |display_rect_|.
+  if (lines().empty())
+    ComputeLines();
+}
+
+void RenderTextWin::ComputeLines() {
+  DCHECK(!needs_layout_);
+  std::vector<internal::Line> lines;
+  internal::LineBreaker line_breaker(display_rect().width(), multiline(),
+      multiline() ? &GetLineBreaks() : 0);

[Alexei Svitkine (slow), 2013/08/12 20:42:33]

Nit: Align with previous' line's params.

[ckocagil, 2013/08/13 20:22:28]

Done.

+  for (size_t i = 0; i < runs_.size(); ++i)
+    line_breaker.AddRun(*runs_[visual_to_logical_[i]]);
+  line_breaker.Finalize(&lines);
+  DCHECK(!lines.empty());
+  set_lines(&lines);
 }
 
 void RenderTextWin::DrawVisualText(Canvas* canvas) {
   DCHECK(!needs_layout_);
-
-  // Skia will draw glyphs with respect to the baseline.
-  Vector2d offset(GetTextOffset() + Vector2d(0, common_baseline_));
-
-  SkScalar x = SkIntToScalar(offset.x());
-  SkScalar y = SkIntToScalar(offset.y());
+  DCHECK(!multiline() || !lines().empty());
 
   std::vector<SkPoint> pos;
 
   internal::SkiaTextRenderer renderer(canvas);
   ApplyFadeEffects(&renderer);
   ApplyTextShadows(&renderer);
 
   bool smoothing_enabled;
   bool cleartype_enabled;
   GetCachedFontSmoothingSettings(&smoothing_enabled, &cleartype_enabled);
   // Note that |cleartype_enabled| corresponds to Skia's |enable_lcd_text|.
   renderer.SetFontSmoothingSettings(
       smoothing_enabled, cleartype_enabled && !background_is_transparent());
 
   ApplyCompositionAndSelectionStyles();
 
-  for (size_t i = 0; i < runs_.size(); ++i) {
-    // Get the run specified by the visual-to-logical map.
-    internal::TextRun* run = runs_[visual_to_logical_[i]];
+  for (size_t i = 0; i < lines().size(); ++i) {
+    const internal::Line& line = lines()[i];
+    Vector2d line_offset = GetLineOffset(i);
+    Vector2d text_offset = line_offset + Vector2d(0, line.baseline);
+    int preceding_segment_widths = 0;
 
-    // Skip painting empty runs and runs outside the display rect area.
-    if ((run->glyph_count == 0) || (x >= display_rect().right()) ||
-        (x + run->width <= display_rect().x())) {
-      x += run->width;
+    // Skip painting empty lines or lines outside the display rect area.
+    if (!display_rect().Intersects(Rect(PointAtOffsetFromOrigin(line_offset),
+                                        Size(line.width, line.height))))
       continue;
+
+    for (size_t j = 0; j < line.segments.size(); ++j) {
+      const internal::LineSegmentWin* segment =
+          static_cast<internal::LineSegmentWin*>(line.segments[j]);
+      const int segment_width = segment->x_pos.length();
+      const internal::TextRun* run = segment->run;
+      DCHECK(!segment->char_pos.is_empty());
+      DCHECK(run->range.Contains(segment->char_pos));
+      ui::Range glyphs = CharRangeToGlyphRange(*run, segment->char_pos);
+      if (glyphs.is_empty()) {
+        DCHECK(segment_width == 0);
+        continue;
+      }
+      // Skip painting segments outside the display rect area.
+      Rect segment_bounds(PointAtOffsetFromOrigin(line_offset) +
+                              Vector2d(preceding_segment_widths, 0),
+                          Size(segment_width, line.height));
+      if (!display_rect().Intersects(segment_bounds)) {
+        preceding_segment_widths += segment_width;
+        continue;
+      }
+
+      int segment_x = 0;
+      pos.resize(glyphs.length());
+      for (size_t g = glyphs.start(); g < glyphs.end(); ++g) {
+        pos[g - glyphs.start()].set(
+            SkIntToScalar(text_offset.x() + preceding_segment_widths +
+                          segment_x + run->offsets[g].du),
+            SkIntToScalar(text_offset.y() + run->offsets[g].dv));
+        segment_x += run->advance_widths[g];
+      }
+
+      renderer.SetTextSize(run->font.GetFontSize());
+      renderer.SetFontFamilyWithStyle(run->font.GetFontName(), run->font_style);
+
+      for (BreakList<SkColor>::const_iterator it =
+               colors().GetBreak(run->range.start());
+           it != colors().breaks().end() && it->first < run->range.end();
+           ++it) {
+        ui::Range intersection = colors().GetRange(it).Intersect(
+            segment->char_pos);
+        ui::Range colored_glyphs = CharRangeToGlyphRange(*run, intersection);
+        DCHECK(glyphs.Contains(colored_glyphs));
+        if (colored_glyphs.is_empty())
+          continue;
+        renderer.SetForegroundColor(it->second);
+        renderer.DrawPosText(&pos[colored_glyphs.start() - glyphs.start()],
+                             &run->glyphs[colored_glyphs.start()],
+                             colored_glyphs.length());
+        SkScalar width = (colored_glyphs.end() < glyphs.end()
+                          ? pos[colored_glyphs.end() - glyphs.start()].x()
+                          : pos[0].x() + SkIntToScalar(segment_width))
+            - pos[colored_glyphs.start() - glyphs.start()].x();
+        renderer.DrawDecorations(
+            pos[colored_glyphs.start() - glyphs.start()].x(), text_offset.y(),
+            SkScalarCeilToInt(width), run->underline, run->strike,
+            run->diagonal_strike);
+      }
+
+      preceding_segment_widths += segment_width;
     }
-
-    // Based on WebCore::skiaDrawText. |pos| contains the positions of glyphs.
-    // An extra terminal |pos| entry is added to simplify width calculations.
-    pos.resize(run->glyph_count + 1);
-    SkScalar glyph_x = x;
-    for (int glyph = 0; glyph < run->glyph_count; glyph++) {
-      pos[glyph].set(glyph_x + run->offsets[glyph].du,
-                     y + run->offsets[glyph].dv);
-      glyph_x += SkIntToScalar(run->advance_widths[glyph]);
-    }
-    pos.back().set(glyph_x, y);
-
-    renderer.SetTextSize(run->font.GetFontSize());
-    renderer.SetFontFamilyWithStyle(run->font.GetFontName(), run->font_style);
-
-    for (BreakList<SkColor>::const_iterator it =
-             colors().GetBreak(run->range.start());
-         it != colors().breaks().end() && it->first < run->range.end();
-         ++it) {
-      const ui::Range glyph_range = CharRangeToGlyphRange(*run,
-          colors().GetRange(it).Intersect(run->range));
-      if (glyph_range.is_empty())
-        continue;
-      renderer.SetForegroundColor(it->second);
-      renderer.DrawPosText(&pos[glyph_range.start()],
-                           &run->glyphs[glyph_range.start()],
-                           glyph_range.length());
-      const SkScalar width = pos[glyph_range.end()].x() -
-          pos[glyph_range.start()].x();
-      renderer.DrawDecorations(pos[glyph_range.start()].x(), y,
-                               SkScalarCeilToInt(width), run->underline,
-                               run->strike, run->diagonal_strike);
-    }
-
-    DCHECK_EQ(glyph_x - x, run->width);
-    x = glyph_x;
   }
 
   UndoCompositionAndSelectionStyles();
 }
 
 void RenderTextWin::ItemizeLogicalText() {
   runs_.clear();
   // Make |string_size_|'s height and |common_baseline_| tall enough to draw
   // often-used characters which are rendered with fonts in the font list.
   string_size_ = Size(0, font_list().GetHeight());
@@ skipping 357 matching lines @@
   size_t position = LayoutIndexToTextIndex(run->range.end());
   position = IndexOfAdjacentGrapheme(position, CURSOR_BACKWARD);
   return SelectionModel(position, CURSOR_FORWARD);
 }
 
 RenderText* RenderText::CreateInstance() {
   return new RenderTextWin;
 }
 
 }  // namespace gfx