Reduce the number of text reshaping in 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>
@@ skipping 118 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);
 };
 
 // Line segments are slices of the layout text to be rendered on a single line.

[msw, 2015/02/13 05:53:34]

nit: s/layout/display/

[oshima, 2015/02/13 21:03:09]

Done.

 struct LineSegment {
   LineSegment();
   ~LineSegment();
 
   // X coordinates of this line segment in text space.
   Range x_range;
 
   // The character range this segment corresponds to.
   Range char_range;
 
   // The width of this line segment in text space. This could be slightly
   // different from x_range.length().
   // TODO(mukai): Fix Range to support float values and merge it into x_range.
   float width;
 
   // Index of the text run that generated this segment.
   size_t run;
 };
 
 // A line of layout text, comprised of a line segment list and some metrics.

[msw, 2015/02/13 05:53:34]

nit: s/layout/display/

[oshima, 2015/02/13 21:03:09]

Done.

 struct Line {
   Line();
   ~Line();
 
   // Segments that make up this line in visual order.
   std::vector<LineSegment> segments;
 
   // The sum of segment widths and the maximum of segment heights.
   SizeF size;
 
@@ skipping 92 matching lines @@
   // TODO(ckocagil): Add vertical alignment and line spacing support instead.
   int min_line_height() const { return min_line_height_; }
   void SetMinLineHeight(int line_height);
 
   // Set the maximum length of the displayed layout text, not the actual text.
   // A |length| of 0 forgoes a hard limit, but does not guarantee proper
   // functionality of very long strings. Applies to subsequent SetText calls.
   // WARNING: Only use this for system limits, it lacks complex text support.
   void set_truncate_length(size_t length) { truncate_length_ = length; }
 
   // The layout text will be elided to fit |display_rect| using this behavior.

[msw, 2015/02/13 05:53:35]

nit: s/layout/display/

[oshima, 2015/02/13 21:03:09]

This should layout text because the display text is the result of eliding.

[msw, 2015/02/13 21:49:23]

That's fair, the comment is ambiguous about where the resulting elided text is
stored. Perhaps it should just say something like: "// Get or set the elide
behavior used to generate the display text."

   // The layout text may be shortened further by the truncate length.

[oshima, 2015/02/13 21:03:09]

I remove this line because it's not correct.

[msw, 2015/02/13 21:49:22]

It could say something like "Note that eliding is performed after truncation."

   void SetElideBehavior(ElideBehavior elide_behavior);
   ElideBehavior elide_behavior() const { return elide_behavior_; }
 
-  const base::string16& layout_text() const { return layout_text_; }
-
   const Rect& display_rect() const { return display_rect_; }
   void SetDisplayRect(const Rect& r);
 
   bool background_is_transparent() const { return background_is_transparent_; }
   void set_background_is_transparent(bool transparent) {
     background_is_transparent_ = transparent;
   }
 
   const SelectionModel& selection_model() const { return selection_model_; }
 
@@ skipping 62 matching lines @@
   DirectionalityMode directionality_mode() const {
       return directionality_mode_;
   }
   base::i18n::TextDirection GetTextDirection();
 
   // Returns the visual movement direction corresponding to the logical end
   // of the text, considering only the dominant direction returned by
   // |GetTextDirection()|, not the direction of a particular run.
   VisualCursorDirection GetVisualDirectionOfLogicalEnd();
 
+  // Returns the text used for layout, which may be obscured,

[msw, 2015/02/13 05:53:35]

nit: s/layout/display

[oshima, 2015/02/13 21:03:10]

Done.

+  // truncated or elided. The subclass may compute elided text on the fly,
+  // or use precomputed the elided text.
+  virtual const base::string16& GetLayoutText() = 0;

[msw, 2015/02/13 05:53:35]

I think this should be renamed GetDisplayText().

[oshima, 2015/02/13 21:03:09]

Done.

+
   // Returns the size required to display the current string (which is the
   // wrapped size in multiline mode). The returned size does not include space
   // reserved for the cursor or the offset text shadows.
   virtual Size GetStringSize() = 0;
 
   // This is same as GetStringSize except that fractional size is returned.
   // The default implementation is same as GetStringSize. Certain platforms that
   // compute the text size as floating-point values, like Mac, will override
   // this method.
   // See comment in Canvas::GetStringWidthF for its usage.
@@ skipping 70 matching lines @@
   // Range will have is_reversed() true.  (This does not return a Rect because a
   // Rect can't have a negative width.)
   virtual Range GetGlyphBounds(size_t index) = 0;
 
   const Vector2d& GetUpdatedDisplayOffset();
   void SetDisplayOffset(int horizontal_offset);
 
  protected:
   RenderText();
 
+  const base::string16& layout_text() const { return layout_text_; }
+  const base::string16& display_text() const { return display_text_; }
+  bool text_elided() const { return text_elided_; }
+
   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); }
 
   // Returns the baseline of the current text.  The return value depends on
   // the text and its layout while the return value of GetBaseline() doesn't.
   // GetAlignmentOffset() takes into account the difference between them.
   //
   // We'd like a RenderText to show the text always on the same baseline
   // regardless of the text, so the text does not jump up or down depending
   // on the text.  However, underlying layout engines return different baselines
   // depending on the text.  In general, layout engines determine the minimum
   // bounding box for the text and return the baseline from the top of the
   // bounding box.  So the baseline changes depending on font metrics used to
   // layout the text.
   //
   // For example, suppose there are FontA and FontB and the baseline of FontA
   // is smaller than the one of FontB.  If the text is laid out only with FontA,
   // then the baseline of FontA may be returned.  If the text includes some
   // characters which are laid out with FontB, then the baseline of FontB may
   // be returned.
   //
   // GetBaseline() returns the fixed baseline regardless of the text.
   // GetLayoutTextBaseline() returns the baseline determined by the underlying
   // layout engine, and it changes depending on the text.  GetAlignmentOffset()
   // returns the difference between them.
   virtual int GetLayoutTextBaseline() = 0;

[msw, 2015/02/13 05:53:34]

nit: Rename this as GetDisplayTextBaseLine or maybe just GetCurrentTextBaseLine?
(update any mentions of the function name in comments too)

[oshima, 2015/02/13 21:03:09]

Chagned to GetDisplayTextBaseLine.

 
   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,
                                            VisualCursorDirection direction);
@@ skipping 18 matching lines @@
   virtual void SetSelectionModel(const SelectionModel& model);
 
   // Get the visual bounds containing the logical substring within the |range|.
   // If |range| is empty, the result is empty. These bounds could be visually
   // discontinuous if the substring is split by a LTR/RTL level change.
   // These bounds are in local coordinates, but may be outside the visible
   // region if the text is longer than the textfield. Subsequent text, cursor,
   // or bounds changes may invalidate returned values.
   virtual std::vector<Rect> GetSubstringBounds(const Range& range) = 0;
 
-  // Convert between indices into |text_| and indices into |obscured_text_|,
-  // which differ when the text is obscured. Regardless of whether or not the
-  // text is obscured, the character (code point) offsets always match.
-  virtual size_t TextIndexToLayoutIndex(size_t index) const = 0;
-  virtual size_t LayoutIndexToTextIndex(size_t index) const = 0;
+  // Convert between indices into |text_| and indices into
+  // GetLayoutText(), which differ when the text is obscured,

[msw, 2015/02/13 05:53:34]

nit: GetDisplayText()

[oshima, 2015/02/13 21:03:09]

Done.

+  // truncated or elided. Regardless of whether or not the text is
+  // obscured, the character (code point) offsets always match.
+  virtual size_t TextIndexToLayoutIndex(size_t index) = 0;

[msw, 2015/02/13 05:53:35]

nit: Rename these TextIndexToDisplayIndex and DisplayIndexToTextIndex.

[oshima, 2015/02/13 21:03:09]

Done.

+  virtual size_t LayoutIndexToTextIndex(size_t index) = 0;
 
-  // Reset the layout to be invalid.
-  virtual void ResetLayout() = 0;
+  // Notifies that layout text, or attributes that affect the layout text
+  // shape have changed. |text_changed| is true if the content of the
+  // |layout_text_| has changed, not just attributes.
+  virtual void OnLayoutTextAttributeChanged(bool text_changed) = 0;
+
+  // Notifies that attributes that affect the display text shape have changed.
+  virtual void OnDisplayTextAttributeChanged() = 0;
 
   // Ensure the text is laid out, lines are computed, and |lines_| is valid.
   virtual void EnsureLayout() = 0;
 
   // Draw the text.
   virtual void DrawVisualText(Canvas* canvas) = 0;
 
-  // Returns the text used for layout, which may be obscured or truncated.
-  const base::string16& GetLayoutText() const;
+  // Update the display text.
+  void UpdateDisplayText(float text_width);
 
   // Returns layout text positions that are suitable for breaking lines.

[msw, 2015/02/13 05:53:34]

nit: s/layout/display/

[oshima, 2015/02/13 21:03:09]

Done.

   const BreakList<size_t>& GetLineBreaks();
 
   // Apply (and undo) temporary composition underlines and selection colors.
   void ApplyCompositionAndSelectionStyles();
   void UndoCompositionAndSelectionStyles();
 
   // Returns the line offset from the origin after applying the text alignment
   // and the display offset.
   Vector2d GetLineOffset(size_t line_number);
 
@@ skipping 54 matching lines @@
   FRIEND_TEST_ALL_PREFIXES(RenderTextTest, PangoAttributes);
   FRIEND_TEST_ALL_PREFIXES(RenderTextTest, StringFitsOwnWidth);
 
   // 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.
   // If the |position| is not a cursorable position (not on grapheme boundary),
   // it is a NO-OP.
   void MoveCursorTo(size_t position, bool select);
 
-  // Updates |layout_text_| if the text is obscured or truncated.
-  void UpdateLayoutText();
+  // Updates |layout_text_| and |display_text_| as needed.

[msw, 2015/02/13 05:53:34]

nit: maybe this should say "as needed (or marks them dirty)."... Since calling
this for RTHB doesn't actually update |display_text_|, so using that or
display_text() immediately afterwards is incorrect.

[oshima, 2015/02/13 21:03:09]

Done.

+  void OnTextAttributeChanged();
 
   // Elides |text| as needed to fit in the |available_width| using |behavior|.
+  // |text_width| is the pre-calculated width of the text shaped by this render
+  // text, or pass 0 if the width is unknown.
   base::string16 Elide(const base::string16& text,
+                       float text_width,
                        float available_width,
                        ElideBehavior behavior);
 
   // Elides |email| as needed to fit the |available_width|.
   base::string16 ElideEmail(const base::string16& email, float available_width);
 
   // Update the cached bounds and display offset to ensure that the current
   // cursor is within the visible display area.
   void UpdateCachedBoundsAndOffset();
 
@@ skipping 40 matching lines @@
   // The background color used for drawing the selection when focused.
   SkColor selection_background_focused_color_;
 
   // The focus state of the text.
   bool focused_;
 
   // Composition text range.
   Range composition_range_;
 
   // Color and style breaks, used to color and stylize ranges of text.
   // BreakList positions are stored with text indices, not layout indices.

[msw, 2015/02/13 05:53:34]

nit: s/layout/display/

[oshima, 2015/02/13 21:03:10]

Done.

   // TODO(msw): Expand to support cursor, selection, background, etc. colors.
   BreakList<SkColor> colors_;
   std::vector<BreakList<bool> > styles_;
 
   // Breaks saved without temporary composition and selection styling.
   BreakList<SkColor> saved_colors_;
   BreakList<bool> saved_underlines_;
   bool composition_and_selection_styles_applied_;
 
   // A flag to obscure actual text with asterisks for password fields.
   bool obscured_;
   // The index at which the char should be revealed in the obscured text.
   int obscured_reveal_index_;
 
   // The maximum length of text to display, 0 forgoes a hard limit.
   size_t truncate_length_;
 
+  // The obscured and/or truncated text used to layout the text to display.
+  base::string16 layout_text_;
+
+  // The elided text displayed visually. This is empty if the text
+  // does not have to be elided, or became empty as a result of eliding.
+  base::string16 display_text_;

[msw, 2015/02/13 05:53:34]

It's a little unfortunate that we need to keep an additional copy of the text
when it's elided (especially if we're space-conscious enough to the point of
using bit-field bools), but I see how this makes the layout passes more
piecemeal and allows more efficient handling of display attributes changes.

[oshima, 2015/02/13 21:03:10]

Because just drawing text requires only display info, not layout info,
it's possible to clear layout side upon paint. Added TODO.

[msw, 2015/02/13 22:19:05]

True, but then any display attribute changes would require not just updates to
the display text, but a full layout pass too, right? (I'm not sure this would be
a good idea to implement)

[oshima, 2015/02/13 23:24:11]

It's rare to apply style change after text has been displayed. Many texts rarely
gets new content (menu, tray), and when you get the new text, you have to update
them all anyway.

We should definitely measure how many of usages can benefit from this though.

[msw, 2015/02/14 00:11:26]

Good point. Agreed that measurements would help.

+
   // The behavior for eliding, fading, or truncating.
   ElideBehavior elide_behavior_;
 
-  // The obscured and/or truncated text that will be displayed.
-  base::string16 layout_text_;
+  // True if the text is elided given the current behavior and display area.
+  bool text_elided_;

[msw, 2015/02/13 05:53:34]

This is nearly equivalent to display_text_.empty(), but I guess we can't simply
replace uses of this with !display_text_.empty(), for the case where eliding the
text causes it to vanish completely (not even an ellipsis can fit).

[oshima, 2015/02/13 21:03:09]

Yep, I tried it and reverted thanks to broken test.

 
   // Whether newline characters should be replaced with newline symbols.
   bool replace_newline_chars_with_symbols_;

[msw, 2015/02/13 05:53:34]

You can remove this, SetReplaceNewlineCharsWithSymbols is never called.

[oshima, 2015/02/13 21:03:09]

Done.

 
   // The minimum height a line should have.
   int min_line_height_;
 
   // Whether the text should be broken into multiple lines. Uses the width of
   // |display_rect_| as the width cap.
   bool multiline_;
 
   // Is the background transparent (either partially or fully)?
   bool background_is_transparent_;
@@ skipping 16 matching lines @@
   // centered.
   int baseline_;
 
   // 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 shadows_;
 
   // A list of valid layout text line break positions.

[msw, 2015/02/13 05:53:34]

nit: s/layout/display/

[oshima, 2015/02/13 21:03:10]

Done.

   BreakList<size_t> line_breaks_;
 
-  // Lines computed by EnsureLayout. These should be invalidated with
-  // ResetLayout and on |display_rect_| changes.
+  // Lines computed by EnsureLayout. These should be invalidated upon
+  // OnLayoutTextAttributeChanged and OnDisplayTextAttributeChanged call.

[msw, 2015/02/13 05:53:34]

nit: "calls"

[oshima, 2015/02/13 21:03:09]

Done.

   std::vector<internal::Line> lines_;
 
   DISALLOW_COPY_AND_ASSIGN(RenderText);
 };
 
 }  // namespace gfx
 
 #endif  // UI_GFX_RENDER_TEXT_H_