Replace StyleRange with BreakList; update RenderText, etc.

ui/gfx/render_text_win.cc

Index: ui/gfx/render_text_win.cc
diff --git a/ui/gfx/render_text_win.cc b/ui/gfx/render_text_win.cc
index d17baee67cf4477b5a1c0c003b712674266e10a0..3f370abdb30fb7289378765f001c3d681673d28a 100644
--- a/ui/gfx/render_text_win.cc
+++ b/ui/gfx/render_text_win.cc
@@ -111,7 +111,7 @@ void DeriveFontIfNecessary(int font_size,
   const int current_style = (font->GetStyle() & kStyleMask);
   const int current_size = font->GetFontSize();
   if (current_style != target_style || current_size != font_size)
-    *font = font->DeriveFont(font_size - current_size, font_style);
+    *font = font->DeriveFont(font_size - current_size, target_style);
 }
 
 // Returns true if |c| is a Unicode BiDi control character.
@@ -334,11 +334,9 @@ SelectionModel RenderTextWin::AdjacentWordSelectionModel(
 
 void RenderTextWin::SetSelectionModel(const SelectionModel& model) {
   RenderText::SetSelectionModel(model);
-  // TODO(xji): The styles are applied to text inside ItemizeLogicalText(). So,
-  // we need to update layout here in order for the styles, such as selection
-  // foreground, to be picked up. Eventually, we should separate styles from
-  // layout by applying foreground, strike, and underline styles during
-  // DrawVisualText as what RenderTextLinux does.
+  // TODO(xji|msw): The text selection color is applied in ItemizeLogicalText().
+  // So, the layout must be updated in order to draw the proper selection range.
+  // Colors should be applied in DrawVisualText(), as done by RenderTextLinux.
   ResetLayout();
 }
 
@@ -487,15 +485,8 @@ void RenderTextWin::DrawVisualText(Canvas* canvas) {
     renderer.SetFontFamilyWithStyle(run->font.GetFontName(), run->font_style);
     renderer.SetForegroundColor(run->foreground);
     renderer.DrawPosText(&pos[0], run->glyphs.get(), run->glyph_count);
-    // TODO(oshima|msw): Consider refactoring StyleRange into Style
-    // class and StyleRange containing Style, and use Style class in
-    // TextRun class.  This may conflict with msw's comment in
-    // TextRun, so please consult with msw when refactoring.
-    StyleRange style;
-    style.strike = run->strike;
-    style.diagonal_strike = run->diagonal_strike;
-    style.underline = run->underline;
-    renderer.DrawDecorations(x, y, run->width, style);
+    renderer.DrawDecorations(x, y, run->width, run->underline, run->strike,
+                             run->diagonal_strike);
 
     x = glyph_x;
   }
@@ -516,7 +507,7 @@ void RenderTextWin::ItemizeLogicalText() {
   HRESULT hr = E_OUTOFMEMORY;
   int script_items_count = 0;
   std::vector<SCRIPT_ITEM> script_items;
-  const int text_length = GetLayoutText().length();
+  const size_t text_length = GetLayoutText().length();
   for (size_t n = kGuessItems; hr == E_OUTOFMEMORY && n < kMaxItems; n *= 2) {
     // Derive the array of Uniscribe script items from the logical text.
     // ScriptItemize always adds a terminal array item so that the length of the
@@ -535,33 +526,58 @@ void RenderTextWin::ItemizeLogicalText() {
   if (script_items_count <= 0)
     return;
 
-  // Build the list of runs, merge font/underline styles.
-  // TODO(msw): Only break for font changes, not color etc. See TextRun comment.
-  StyleRanges styles(style_ranges());
-  ApplyCompositionAndSelectionStyles(&styles);
-  StyleRanges::const_iterator style = styles.begin();
+  // Adjust the text colors to reflect the selection range.
+  ColorBreaks adjusted_colors(colors());
+  ApplySelectionColor(&adjusted_colors);
+
+  // Adjust the underline styling to reflect composition ranges.
+  const StyleBreaks* adjusted_styles[NUM_TEXT_STYLES];

[Alexei Svitkine (slow), 2013/01/22 19:20:21]

I don't get why you need this. Can you explain?

[msw, 2013/01/22 22:27:24]

Similar to the previous code, where |styles| was a local copy to which we
applied the transient composition and selection styles, I'm doing the same with
the underline and color break lists.

+  for (size_t i = 0; i < NUM_TEXT_STYLES; ++i)
+    adjusted_styles[i] = &styles(static_cast<TextStyle>(i));
+  StyleBreaks adjusted_underlines(styles(UNDERLINE));
+  ApplyCompositionStyle(&adjusted_underlines);
+  adjusted_styles[UNDERLINE] = &adjusted_underlines;

[Alexei Svitkine (slow), 2013/01/22 19:20:21]

The code in the diff doesn't seem to be special-casing underline, why is the new
code doing this?

[msw, 2013/01/22 22:27:24]

The prior code applied the transient composition underlines and selection colors
locally by copying/modifying all the style ranges together as |styles|. Here,
I'm only copying/modifying the |colors_| and |styles_[UNDERLINE]| vectors to
make their needed temporary local adjustments.

+
+  // Track the current color and style with iterators.
+  ColorBreaks::const_iterator color = adjusted_colors.begin();
+  StyleBreaks::const_iterator style[NUM_TEXT_STYLES];
+  for (size_t i = 0; i < NUM_TEXT_STYLES; ++i)
+    style[i] = adjusted_styles[i]->begin();
+  size_t style_end[NUM_TEXT_STYLES];
+
+  // Build the list of runs from the script items and ranged colors/styles.
+  // TODO(msw): Only break for bold/italic, not color etc. See TextRun comment.
   SCRIPT_ITEM* script_item = &script_items[0];
-  for (int run_break = 0; run_break < text_length;) {
+  for (size_t run_break = 0; run_break < text_length;) {
     internal::TextRun* run = new internal::TextRun();
     run->range.set_start(run_break);
     run->font = GetFont();
-    run->font_style = style->font_style;
+    run->font_style = (style[BOLD]->second ? Font::BOLD : 0) |
+                      (style[ITALIC]->second ? Font::ITALIC : 0);
     DeriveFontIfNecessary(run->font.GetFontSize(), run->font.GetHeight(),
                           run->font_style, &run->font);
-    run->foreground = style->foreground;
-    run->strike = style->strike;
-    run->diagonal_strike = style->diagonal_strike;
-    run->underline = style->underline;
+    run->foreground = color->second;
+    run->strike = style[STRIKE]->second;
+    run->diagonal_strike = style[DIAGONAL_STRIKE]->second;
+    run->underline = style[UNDERLINE]->second;
     run->script_analysis = script_item->a;
 
-    // Find the range end and advance the structures as needed.
-    const int script_item_end = (script_item + 1)->iCharPos;
-    const int style_range_end = TextIndexToLayoutIndex(style->range.end());
-    run_break = std::min(script_item_end, style_range_end);
-    if (script_item_end <= style_range_end)
-      script_item++;
-    if (script_item_end >= style_range_end)
-      style++;
+    // Find the range end and advance the color and style iterators as needed.
+    const size_t script_item_end = (script_item + 1)->iCharPos;
+    const size_t color_end = TextIndexToLayoutIndex(
+        GetBreakEnd(adjusted_colors, color));
+    run_break = std::min(script_item_end, color_end);
+    for (size_t i = 0; i < NUM_TEXT_STYLES; ++i) {
+      style_end[i] = TextIndexToLayoutIndex(
+          GetBreakEnd(*adjusted_styles[i], style[i]));
+      run_break = std::min(run_break, style_end[i]);
+    }
+
+    script_item += script_item_end == run_break ? 1 : 0;
+    color += color_end == run_break ? 1 : 0;
+    for (size_t i = 0; i < NUM_TEXT_STYLES; ++i)
+      style[i] += style_end[i] == run_break ? 1 : 0;
+
     run->range.set_end(run_break);
     runs_.push_back(run);
   }