Windows implementation of multiline RenderText

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 34277400b7369e28c41e120950e357555c8c9415..4671e7752095f363c1757f6d29706d213d34d116 100644
--- a/ui/gfx/render_text.h
+++ b/ui/gfx/render_text.h
@@ -13,6 +13,7 @@
 
 #include "base/gtest_prod_util.h"
 #include "base/i18n/rtl.h"
+#include "base/memory/scoped_vector.h"
 #include "base/strings/string16.h"
 #include "skia/ext/refptr.h"
 #include "third_party/skia/include/core/SkColor.h"
@@ -111,6 +112,43 @@ class StyleIterator {
   DISALLOW_COPY_AND_ASSIGN(StyleIterator);
 };
 
+// Line segments are slices of the layout text to be rendered on a single line.
+// This structure holds the logical and visual position information for a line
+// segment.
+struct LineSegment {
+  // X coordinates of this line segment in text coordinate space.
+  ui::Range x_pos;
+
+  // The character range this segment corresponds to.
+  ui::Range char_pos;
+};
+
+// Holds a list of line segments and metrics for a line.
+struct Line {
+  Line();
+
+  // Destructive. Empties the |segments| field of the given Line.

[msw, 2013/08/15 02:44:17]

As per my comment on the old patch set, I hope we can use ScopedVector instead
of an std::vector of Line structs and then dissalow copy and assign here.

[Alexei Svitkine (slow), 2013/08/15 14:51:05]

Hmm, I'm not sure I like your proposed approach - then every line would have to
be be alloc'd from the heap, which will add even more overhead and memory
trashing.

[msw, 2013/08/15 15:42:09]

Do you have qualms with the ScopedVector<LineSegment> already used here? There's
always fewer lines than LineSegments, so I don't quite buy the argument. I'd
like DISALLOW_COPY_AND_ASSIGN to remove the destructive copy ctor and assignment
op, since I'm always reluctant to do premature performance optimization that
adds code or complexity, unless it's super obvious. I don't offhand suspect that
heap allocations have much perf impact compared to Layout overall, but since we
Layout for each Canvas::*Text call, I'd hear out your argument if you had data
showing a significant perf win of std::vector over ScopedVector in common cases.

[Alexei Svitkine (slow), 2013/08/15 17:27:42]

Yes, I do. I suggested merging LineSegmentWin to LineSegment, so that it's a
common struct and can be used in a regular std::vector. I proposed storing
run_index instead of a run pointer. But Cem pointed outed that this may not be
enough for other platforms than RenderTextWin, which is true since a run_index
is not useful to RenderTextLinux which uses Pango's linked list of runs. Which
is why I suggested that perhaps we should have a typedef for NativeTextRun, so
that LineSegment can have a member of that type and not need platform-specific
subclasses, so that we can use a std::vector for LineSegments.

[msw, 2013/08/15 17:59:14]

Okay, after more thought and talking with Alexei, I agree that std::vector
likely has measurably better performance for both Line and LineSegment lists.
Cem, ignore my ScopedVector suggestion and instead use a run index instead of a
specialized LineSegmentWin with a internal::TextRun* member so the LineSegment
ScopedVector can become an std::vector and we can trash the destructive copy
ctor and assignment operator for the Line struct. Run indicies should be okay
for now since RenderTextLinux will likely able to use built-in Pango
functionality to support multi-line layout instead of porting our RenderTextWin
multi-line solution. But if a cross-platform base class/struct for TextRun makes
more sense, then a std::vector<TextRun*> works for me too. Let me/us know if any
of that direction is unclear; thanks!

[ckocagil, 2013/08/20 21:25:07]

Done; using std::vector and a run index now.

+  Line(Line& other);
+
+  // Destructive. Empties the |segments| field of the Line on the rhs.
+  Line& operator=(Line& other);
+
+  // Segments that make up this line in visual order.
+  ScopedVector<LineSegment> segments;
+
+  // Sum of the widths of all segments on this line.
+  int width;

[msw, 2013/08/15 02:44:17]

optional nit: combine |width| and |height| as a gfx::Size.

[ckocagil, 2013/08/20 21:25:07]

Done.

+
+  // Maximum text height of all segments on this line.
+  int height;
+
+  // Sum of the |height| fields of preceding lines.
+  int preceding_heights;
+
+  // Maximum baseline of all runs on this line.
+  int baseline;
+};
+
 }  // namespace internal
 
 // RenderText represents an abstract model of styled text and its corresponding
@@ -179,6 +217,11 @@ class UI_EXPORT RenderText {
   bool obscured() const { return obscured_; }
   void SetObscured(bool obscured);
 
+  // TODO(ckocagil): Multiline text rendering is currently only supported on
+  // Windows. Support other platforms.
+  bool multiline() const { return multiline_; }

[msw, 2013/08/15 02:44:17]

Order these after SetObscuredRevealIndex, ditto for the .cc def.

[ckocagil, 2013/08/20 21:25:07]

Done.

+  void SetMultiline(bool multiline);
+
   // Makes a char in obscured text at |index| to be revealed. |index| should be
   // a UTF16 text index. If there is a previous revealed index, the previous one
   // is cleared and only the last set index will be revealed. If |index| is -1
@@ -287,6 +330,11 @@ class UI_EXPORT RenderText {
   // the margin area of text shadows.
   virtual Size GetStringSize() = 0;
 
+  // Returns the size required to display the current multiline text, which is
+  // horizontally capped by the width of |display_rect()|.
+  // TODO(ckocagil): Merge with GetContentWidth.
+  virtual Size GetMultilineTextSize() = 0;
+
   // Returns the width of content, which reserves room for the cursor if
   // |cursor_enabled_| is true.
   int GetContentWidth();
@@ -348,6 +396,9 @@ class UI_EXPORT RenderText {
   const BreakList<SkColor>& colors() const { return colors_; }
   const std::vector<BreakList<bool> >& styles() const { return styles_; }
 
+  const std::vector<internal::Line>& lines() const { return lines_; }
+  void set_lines(std::vector<internal::Line>* lines) { lines_.swap(*lines); }

[msw, 2013/08/15 02:44:17]

Hmm, I'm not sure if vector ops are allowed for inlining.

[ckocagil, 2013/08/20 21:25:07]

This is okay as we talked on IM.

+
   const Vector2d& GetUpdatedDisplayOffset();
 
   void set_cached_bounds_and_offset_valid(bool valid) {
@@ -406,7 +457,7 @@ class UI_EXPORT RenderText {
   // Reset the layout to be invalid.
   virtual void ResetLayout() = 0;
 
-  // Ensure the text is laid out.
+  // Ensure the text is laid out, lines are computed, and |lines_| is valid.
   virtual void EnsureLayout() = 0;
 
   // Draw the text.
@@ -415,22 +466,30 @@ class UI_EXPORT RenderText {
   // Returns the text used for layout, which may be obscured or truncated.
   const base::string16& GetLayoutText() const;
 
+  // Returns layout text positions that are suitable for breaking lines at.
+  const BreakList<size_t>& GetLineBreaks();
+
   // Apply (and undo) temporary composition underlines and selection colors.
   void ApplyCompositionAndSelectionStyles();
   void UndoCompositionAndSelectionStyles();
 
-  // Returns the text offset from the origin after applying text alignment and
+  // Returns the line offset from the origin after applying text alignment and
   // display offset.
-  Vector2d GetTextOffset();
+  Vector2d GetLineOffset(size_t line_number);
 
   // Convert points from the text space to the view space and back.
-  // Handles the display area, display offset, and the application LTR/RTL mode.
+  // Handles the display area, display offset, application LTR/RTL mode and

[msw, 2013/08/15 02:44:17]

nit: wrap this sentence above for a two-line comment.

[ckocagil, 2013/08/20 21:25:07]

Done.

+  // multi-line.
   Point ToTextPoint(const Point& point);
   Point ToViewPoint(const Point& point);
 
-  // Returns the text offset from the origin, taking into account text alignment
+  // Convert the range in text coordinates to rects in view coordinates that
+  // cover the given range.
+  std::vector<Rect> TextBoundsToViewBounds(const ui::Range& x);
+
+  // Returns the line offset from the origin, taking into account text alignment
   // only.
-  Vector2d GetAlignmentOffset();
+  Vector2d GetAlignmentOffset(size_t line_number);
 
   // Applies fade effects to |renderer|.
   void ApplyFadeEffects(internal::SkiaTextRenderer* renderer);
@@ -547,6 +606,10 @@ class UI_EXPORT RenderText {
   // The obscured and/or truncated text that will be displayed.
   base::string16 layout_text_;
 
+  // Whether the text should be broken into multiple lines. Uses the width of
+  // |display_rect_| as the width cap.
+  bool multiline_;
+
   // Fade text head and/or tail, if text doesn't fit into |display_rect_|.
   bool fade_head_;
   bool fade_tail_;
@@ -574,6 +637,13 @@ class UI_EXPORT RenderText {
   // Text shadows to be drawn.
   ShadowValues text_shadows_;
 
+  // BreakList to find valid positions to break the line at.
+  BreakList<size_t> line_breaks_;
+
+  // Lines computed by EnsureLayout. Should be emptied whenever the text, styles
+  // or |display_rect_| changes.
+  std::vector<internal::Line> lines_;
+
   DISALLOW_COPY_AND_ASSIGN(RenderText);
 };