Windows implementation of multiline RenderText

ui/gfx/render_text.h

 // 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.
 
 #ifndef UI_GFX_RENDER_TEXT_H_
 #define UI_GFX_RENDER_TEXT_H_
 
 #include <algorithm>
 #include <cstring>
 #include <string>
 #include <utility>
 #include <vector>
 
 #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"
 #include "third_party/skia/include/core/SkPaint.h"
 #include "third_party/skia/include/core/SkRect.h"
 #include "ui/base/range/range.h"
 #include "ui/gfx/break_list.h"
 #include "ui/gfx/font_list.h"
 #include "ui/gfx/point.h"
 #include "ui/gfx/rect.h"
@@ skipping 78 matching lines @@
  private:
   BreakList<SkColor> colors_;
   std::vector<BreakList<bool> > styles_;
 
   BreakList<SkColor>::const_iterator color_;
   std::vector<BreakList<bool>::const_iterator> style_;
 
   DISALLOW_COPY_AND_ASSIGN(StyleIterator);
 };
 
+struct LineSegment {

[Alexei Svitkine (slow), 2013/07/09 15:35:13]

This should live in the class unless you're planning to expose this externally.

I suggest in the protected section.

[msw, 2013/07/10 04:01:56]

Add a struct comment to describe what this is.
(a reader might wonder how this differs from a run)

[msw, 2013/07/10 04:01:56]

I disagree; SkiaTextRenderer and StyleIterator aren't used elsewhere either, but
we shouldn't make all of these protected to RenderText. I think the internal
namespace is fine for now, and I'd rather eventually see these components moved
to other files, instead of cramming everything in the RenderText class.

[ckocagil, 2013/07/13 16:05:10]

Keeping it in internal namespace for now.

+  ui::Range x_pos; // x coord, in text coordinates
+  ui::Range char_pos;
+};
+
 }  // namespace internal
 
 // RenderText represents an abstract model of styled text and its corresponding
 // visual layout. Support is built in for a cursor, a selection, simple styling,
 // complex scripts, and bi-directional text. Implementations provide mechanisms
 // for rendering and translation between logical and visual data.
 class UI_EXPORT RenderText {
  public:
   virtual ~RenderText();
 
@@ skipping 55 matching lines @@
   bool focused() const { return focused_; }
   void set_focused(bool focused) { focused_ = focused; }
 
   bool clip_to_display_rect() const { return clip_to_display_rect_; }
   void set_clip_to_display_rect(bool clip) { clip_to_display_rect_ = clip; }
 
   // In an obscured (password) field, all text is drawn as asterisks or bullets.
   bool obscured() const { return obscured_; }
   void SetObscured(bool obscured);
 
+  bool multiline() const { return multiline_; }
+  void set_multiline(bool multiline) { multiline_ = 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
   // or out of range, no char will be revealed. The revealed index is also
   // cleared when SetText or SetObscured is called.
   void SetObscuredRevealIndex(int index);
 
   const Rect& display_rect() const { return display_rect_; }
   void SetDisplayRect(const Rect& r);
 
@@ skipping 82 matching lines @@
   // of the text, considering only the dominant direction returned by
   // |GetTextDirection()|, not the direction of a particular run.
   VisualCursorDirection GetVisualDirectionOfLogicalEnd();
 
   // Returns the size in pixels of the entire string. For the height, this will
   // return the maximum height among the different fonts in the text runs.
   // Note that this returns the raw size of the string, which does not include
   // the margin area of text shadows.
   virtual Size GetStringSize() = 0;
 
+  virtual Size RenderText::GetMultilineTextSize() = 0;

[msw, 2013/07/10 04:01:56]

Remove "RenderText::" and add a comment (explain the width cap).

[ckocagil, 2013/07/13 16:05:10]

Done.

+
   // Returns the width of content, which reserves room for the cursor if
   // |cursor_enabled_| is true.
   int GetContentWidth();
 
   // Returns the common baseline of the text. The returned value is the vertical
   // offset from the top of |display_rect| to the text baseline, in pixels.
   virtual int GetBaseline() = 0;
 
   void Draw(Canvas* canvas);
 
@@ skipping 40 matching lines @@
   // specifies the character range for which the corresponding font has been
   // chosen.
   virtual std::vector<FontSpan> GetFontSpansForTesting() = 0;
 
  protected:
   RenderText();
 
   const BreakList<SkColor>& colors() const { return colors_; }
   const std::vector<BreakList<bool> >& styles() const { return styles_; }
 
+  const std::vector<size_t>& line_breaks() const { return line_breaks_; }
+
+  const std::vector<ScopedVector<internal::LineSegment> >& lines() const {
+    return lines_;
+  }
+  void set_lines(std::vector<ScopedVector<internal::LineSegment> >* lines) {
+    lines_.swap(*lines);
+  }
+
+  bool lines_valid() const { return lines_valid_; }
+  void set_lines_valid(bool lines_valid) {

[msw, 2013/07/10 04:01:56]

nit: rename the param to |valid| to fit this on one line.

[ckocagil, 2013/07/13 16:05:10]

Actually it fits without renaming.

+    lines_valid_ = lines_valid;
+  }
+
+  int LineWidth(int line) {

[Alexei Svitkine (slow), 2013/07/09 15:35:13]

This is not a trivial method so the implementation should live in the .cc file.

Also, should be named GetLineWidth() and have a comment.

[ckocagil, 2013/07/13 16:05:10]

Removed since we now have a Line struct that caches width.

+    int width = 0;
+    for (size_t i = 0; i < lines()[line].size(); ++i)

[Alexei Svitkine (slow), 2013/07/09 15:35:13]

Add a DCHECK() that |line| is within a valid range.

[ckocagil, 2013/07/13 16:05:10]

This method no longer exists.

+      width += lines()[line][i]->x_pos.length();
+    return width;
+  }
+
   const Vector2d& GetUpdatedDisplayOffset();
 
   void set_cached_bounds_and_offset_valid(bool valid) {
     cached_bounds_and_offset_valid_ = valid;
   }
 
   // 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,
                                            BreakType break_type,
@@ skipping 44 matching lines @@
 
   // Reset the layout to be invalid.
   virtual void ResetLayout() = 0;
 
   // Ensure the text is laid out.
   virtual void EnsureLayout() = 0;
 
   // Draw the text.
   virtual void DrawVisualText(Canvas* canvas) = 0;
 
+  virtual void ComputeLines() = 0;

[msw, 2013/07/10 04:01:56]

Add a comment, move this above DrawVisualText (it's more like Layout).

[ckocagil, 2013/07/13 16:05:10]

I moved this to render_text_win since we now call EnsureLayout to compute the
lines.

+
   // Returns the text used for layout, which may be |obscured_text_|.
   const base::string16& GetLayoutText() const;
 
   // 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
   // display offset.
-  Vector2d GetTextOffset();
+  Vector2d GetTextOffset(int line_width);

[msw, 2013/07/10 04:01:56]

Comment on what |line_width| is used for here. It's needed to get the offsets of
individually centered lines, right? But it's also used to get the more general
offset for a whole block of left/right-aligned text with GetContentWidth, right?
Rename this and the param for GetAlignmentOffset |text_width| or simply |width|.
I would really suggest that you leave these functions as-is and make per-line
offsets a separate matter dealt with elsewhere (like each Line storing it's
bounds, etc.).

[ckocagil, 2013/07/13 16:05:10]

I renamed this to |GetLineOffset| and it now takes a line number.
|GetAlignmentOffset| now takes a line number as well. These should make things
easier since I aim for making everything line-oriented and make the single-line
mode use mostly the same code paths as multi-line. I would rather not have each
Line store its own view bounds to keep the struct simple. Sounds good?

[msw, 2013/07/17 06:47:18]

Sounds good.

 
   // Convert points from the text space to the view space and back.
   // Handles the display area, display offset, and the application LTR/RTL mode.
   Point ToTextPoint(const Point& point);
   Point ToViewPoint(const Point& point);
 
+  std::vector<Rect> RangeToViewRects(const ui::Range& x, int y);

[msw, 2013/07/10 04:01:56]

nit: Comment on what this does and what the args are.
|y| is only ever 0 so far, is it needed?

[ckocagil, 2013/07/13 16:05:10]

Comment added, |y| removed.

+
   // Returns the text offset from the origin, taking into account text alignment
   // only.
-  Vector2d GetAlignmentOffset();
+  Vector2d GetAlignmentOffset(int line_width);

[msw, 2013/07/10 04:01:56]

Ditto comment from GetTextOffset: Comment and rename.

[ckocagil, 2013/07/13 16:05:10]

This method now takes a line number.

 
   // Applies fade effects to |renderer|.
   void ApplyFadeEffects(internal::SkiaTextRenderer* renderer);
 
   // Applies text shadows to |renderer|.
   void ApplyTextShadows(internal::SkiaTextRenderer* renderer);
 
   // A convenience function to check whether the glyph attached to the caret
   // is within the given range.
   static bool RangeContainsCaret(const ui::Range& range,
@@ skipping 94 matching lines @@
   bool composition_and_selection_styles_applied_;
 
   // A flag and the text to display for obscured (password) fields.
   // Asterisks are used instead of the actual text glyphs when true.
   bool obscured_;
   base::string16 obscured_text_;
 
   // The index at which the char should be revealed in the obscured text.
   int obscured_reveal_index_;
 
+  // Whether we should break the text to multiple lines.

[msw, 2013/07/10 04:01:56]

optional nit: I prefer avoiding "we" in comments.
Consider "Whether the text should be broken into multiple lines."
Constider mentioning the use of the display rect's width.

[ckocagil, 2013/07/13 16:05:10]

Done.

+  bool multiline_;
+
   // Fade text head and/or tail, if text doesn't fit into |display_rect_|.
   bool fade_head_;
   bool fade_tail_;
 
   // Is the background transparent (either partially or fully)?
   bool background_is_transparent_;
 
   // The local display area for rendering the text.
   Rect display_rect_;
 
   // Flag to work around a Skia bug with the PDF path (http://crbug.com/133548)
   // that results in incorrect clipping when drawing to the document margins.
   // This field allows disabling clipping to work around the issue.
   // TODO(asvitkine): Remove this when the underlying Skia bug is fixed.
   bool clip_to_display_rect_;
 
   // The offset for the text to be drawn, relative to the display area.
   // Get this point with GetUpdatedDisplayOffset (or risk using a stale value).
   Vector2d display_offset_;
 
   // The cached bounds and offset are invalidated by changes to the cursor,
   // selection, font, and other operations that adjust the visible text bounds.
   bool cached_bounds_and_offset_valid_;
 
   // Text shadows to be drawn.
   ShadowValues text_shadows_;
 
+  // Positions at which we can break the line.
+  std::vector<size_t> line_breaks_;
+
+  // Lines computed by ComputeLines().
+  std::vector<ScopedVector<internal::LineSegment> > lines_;

[Alexei Svitkine (slow), 2013/07/09 15:35:13]

Can you use a regular vector instead of a ScopedVector to represent a line? (I
guess you can't because there's a platform specific subclass of LineSegment).

Also, consider making a typedef Line.

[msw, 2013/07/10 04:01:56]

I'm wondering if it might help in the long run to use a BreakList for
merging/splitting existing lines rather than frequently reconstructing the
Lines/LineSegments. I don't have a solid recommendation but maybe you already
thought about this.

[ckocagil, 2013/07/13 16:05:10]

I can't, we need pointers as long as we have LineSegmentWin.
We now have a Line struct.

[ckocagil, 2013/07/13 16:05:10]

I'm not sure how this would work but I'm adding a TODO to the document in Google
Docs and I'll give it some thought later.

+
+  bool lines_valid_;

[Alexei Svitkine (slow), 2013/07/09 15:35:13]

Add a comment.

[ckocagil, 2013/07/13 16:05:10]

Done.

+
   DISALLOW_COPY_AND_ASSIGN(RenderText);
 };
 
 }  // namespace gfx
 
 #endif  // UI_GFX_RENDER_TEXT_H_