Replace StyleRange with BreakList; update RenderText, etc.

ui/gfx/render_text.h

Index: ui/gfx/render_text.h
diff --git a/ui/gfx/render_text.h b/ui/gfx/render_text.h
index dd86fc53dc40bf0d94621bbfe37adcb3b1371045..e175aa2ce3666783cfa62631f8cd359ba47d860a 100644
--- a/ui/gfx/render_text.h
+++ b/ui/gfx/render_text.h
@@ -37,7 +37,6 @@ namespace gfx {
 class Canvas;
 class Font;
 class RenderTextTest;
-struct StyleRange;
 
 namespace internal {
 
@@ -62,7 +61,14 @@ class SkiaTextRenderer {
   void DrawPosText(const SkPoint* pos,
                    const uint16* glyphs,
                    size_t glyph_count);
-  void DrawDecorations(int x, int y, int width, const StyleRange& style);
+  // Draw underline and strike-through text decorations.
+  // Based on |SkCanvas::DrawTextDecorations()| and constants from:
+  //   third_party/skia/src/core/SkTextFormatParams.h
+  void DrawDecorations(int x, int y, int width, bool underline, bool strike,
+                       bool diagonal_strike);
+  void DrawUnderline(int x, int y, int width);
+  void DrawStrike(int x, int y, int width) const;
+  void DrawDiagonalStrike(int x, int y, int width) const;
 
  private:
   SkCanvas* canvas_skia_;
@@ -78,20 +84,12 @@ class SkiaTextRenderer {
 
 }  // namespace internal
 
-// A visual style applicable to a range of text.
-struct UI_EXPORT StyleRange {
-  StyleRange();
-
-  SkColor foreground;
-  // A gfx::Font::FontStyle flag to specify bold and italic styles.
-  int font_style;
-  bool strike;
-  bool diagonal_strike;
-  bool underline;
-  ui::Range range;
-};
-
-typedef std::vector<StyleRange> StyleRanges;
+// Ordered ColorBreak/StyleBreak lists split text into ranged colors and styles.
+// Each |break| applies to logical text range: [break.first, (break+1).first)

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

The comment isn't clear about what the last entry applies to. Is there always a
terminator entry or is the range of the last break assumed to be the end of the
logical text. Please document.

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

Done.

+typedef std::pair<size_t, SkColor> ColorBreak;
+typedef std::vector<ColorBreak> ColorBreaks;
+typedef std::pair<size_t, bool> StyleBreak;
+typedef std::vector<StyleBreak> StyleBreaks;

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

Do we really need StyleBreaks and ColorBreaks typedefs?

I find using std::vector<ColorBreak> more readable.

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

Done.

 
 // RenderText represents an abstract model of styled text and its corresponding
 // visual layout. Support is built in for a cursor, a selection, simple styling,
@@ -157,9 +155,6 @@ class UI_EXPORT RenderText {
   bool clip_to_display_rect() const { return clip_to_display_rect_; }
   void set_clip_to_display_rect(bool clip) { clip_to_display_rect_ = clip; }
 
-  const StyleRange& default_style() const { return default_style_; }
-  void set_default_style(const StyleRange& style) { default_style_ = style; }
-
   // In an obscured (password) field, all text is drawn as asterisks or bullets.
   bool obscured() const { return obscured_; }
   void SetObscured(bool obscured);
@@ -227,11 +222,13 @@ class UI_EXPORT RenderText {
   const ui::Range& GetCompositionRange() const;
   void SetCompositionRange(const ui::Range& composition_range);
 
-  // Apply |style_range| to the internal style model.
-  void ApplyStyleRange(const StyleRange& style_range);
+  // Set the text color over the entire text or a logical character range.
+  void SetColor(SkColor value);
+  void ApplyColor(SkColor value, const ui::Range& range);
 
-  // Apply |default_style_| over the entire text range.
-  void ApplyDefaultStyle();
+  // Set various text styles over the entire text or a logical character range.

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

Same comment as textfield, please document |value| so that it's obvious how this
works.

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

Done; but I hope usage is obvious, even ApplyStyle(UNDERLINE, true, range).

+  void SetStyle(TextStyle style, bool value);
+  void ApplyStyle(TextStyle style, bool value, const ui::Range& range);
 
   // Set the text directionality mode and get the text direction yielded.
   void SetDirectionalityMode(DirectionalityMode mode);
@@ -295,14 +292,23 @@ class UI_EXPORT RenderText {
  protected:
   RenderText();
 
+  const ColorBreaks& colors() { return colors_; }
+  const StyleBreaks& styles(TextStyle style) { return styles_[style]; }
+
+  // Get the end of the supplied break (the next break's start or text length).

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

From reading the comment, I still have no idea what this does. Please document
this better.

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

Done.

+  template <class T>
+  size_t GetBreakEnd(const std::vector<std::pair<size_t, T> >& breaks,

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

Can you make the template param the vector itself?

i.e.

size_t GetBreakEnd(const T& breaks, const typename T::const_iteratio& i) {
  ...
}

This will make it more readable, I think.

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

Done.

+      const typename std::vector<std::pair<size_t, T> >::
+          const_iterator& i) const {
+    return (i + 1) == breaks.end() ? text().length() : (i + 1)->first;
+  }
+
   const Vector2d& GetUpdatedDisplayOffset();
 
   void set_cached_bounds_and_offset_valid(bool valid) {
     cached_bounds_and_offset_valid_ = valid;
   }
 
-  const StyleRanges& style_ranges() const { return style_ranges_; }
-
   // Get the selection model that visually neighbors |position| by |break_type|.
   // The returned value represents a cursor/caret position without a selection.
   SelectionModel GetAdjacentSelectionModel(const SelectionModel& current,
@@ -364,9 +370,10 @@ class UI_EXPORT RenderText {
   // Returns the text used for layout, which may be |obscured_text_|.
   const string16& GetLayoutText() const;
 
-  // Apply composition style (underline) to composition range and selection
-  // style (foreground) to selection range.
-  void ApplyCompositionAndSelectionStyles(StyleRanges* style_ranges);
+  // Apply underline styling to mark the composition range(s).
+  void ApplyCompositionStyle(StyleBreaks* underlines);

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

Is this specific to underline? If not, change the comments / names.

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

It is specific to underlines, left as-is.

+  // Apply the selection text color to mark the selected text range.
+  void ApplySelectionColor(ColorBreaks* colors);
 
   // Returns the text offset from the origin after applying text alignment and
   // display offset.
@@ -404,14 +411,22 @@ class UI_EXPORT RenderText {
   friend class RenderTextTest;
 
   FRIEND_TEST_ALL_PREFIXES(RenderTextTest, DefaultStyle);
-  FRIEND_TEST_ALL_PREFIXES(RenderTextTest, CustomDefaultStyle);
-  FRIEND_TEST_ALL_PREFIXES(RenderTextTest, ApplyStyleRange);
-  FRIEND_TEST_ALL_PREFIXES(RenderTextTest, StyleRangesAdjust);
+  FRIEND_TEST_ALL_PREFIXES(RenderTextTest, SetColorAndStyle);
+  FRIEND_TEST_ALL_PREFIXES(RenderTextTest, ApplyInvalidOrEmptyRange);
+  FRIEND_TEST_ALL_PREFIXES(RenderTextTest, ApplyStyle);
+  FRIEND_TEST_ALL_PREFIXES(RenderTextTest, ResizeStyle);
+  FRIEND_TEST_ALL_PREFIXES(RenderTextTest, ApplyColor);
   FRIEND_TEST_ALL_PREFIXES(RenderTextTest, ObscuredText);
   FRIEND_TEST_ALL_PREFIXES(RenderTextTest, GraphemePositions);
   FRIEND_TEST_ALL_PREFIXES(RenderTextTest, EdgeSelectionModels);
   FRIEND_TEST_ALL_PREFIXES(RenderTextTest, OriginForDrawing);
 
+  // Adjust the existing |breaks| to apply |value| over the supplied |range|.
+  template <class T>
+  void SetBreaks(std::vector<std::pair<size_t, T> >* breaks,

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

Is there a better name for this than SetBreaks()?

ApplyBreakOverRange()? or just ApplyBreak()?

Since you're modifying |breaks|, it should be the last parameter.

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

Done.

+                 T value,
+                 const ui::Range& range);
+
   // Set the cursor to |position|, with the caret trailing the previous
   // grapheme, or if there is no previous grapheme, leading the cursor position.
   // If |select| is false, the selection start is moved to the same position.
@@ -478,10 +493,10 @@ class UI_EXPORT RenderText {
   // Composition text range.
   ui::Range composition_range_;
 
-  // List of style ranges. Elements in the list never overlap each other.
-  StyleRanges style_ranges_;
-  // The default text style.
-  StyleRange default_style_;
+  // Color and style breaks, used to color and stylize ranges of text.
+  // TODO(msw): Expand to support cursor, selection, background, etc. colors.
+  ColorBreaks colors_;
+  StyleBreaks styles_[NUM_TEXT_STYLES];
 
   // A flag and the text to display for obscured (password) fields.
   // Asterisks are used instead of the actual text glyphs when true.