Reduce the number of text reshaping in RenderText

ui/gfx/render_text.cc

Index: ui/gfx/render_text.cc
diff --git a/ui/gfx/render_text.cc b/ui/gfx/render_text.cc
index 50d29e83fe14585dc6002bc9b2a502b49d5c0b31..efdec68981327e3d4d934e47f1e408fcbb716a9e 100644
--- a/ui/gfx/render_text.cc
+++ b/ui/gfx/render_text.cc
@@ -13,6 +13,7 @@
 #include "base/stl_util.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/trace_event/trace_event.h"
 #include "third_party/icu/source/common/unicode/rbbi.h"
 #include "third_party/icu/source/common/unicode/utf16.h"
 #include "third_party/skia/include/core/SkTypeface.h"
@@ -463,7 +464,7 @@ void RenderText::SetFontList(const FontList& font_list) {
   SetStyle(UNDERLINE, (font_style & gfx::Font::UNDERLINE) != 0);
   baseline_ = kInvalidBaseline;
   cached_bounds_and_offset_valid_ = false;
-  ResetLayout();
+  OnLayoutTextShapeChanged(false);
 }
 
 void RenderText::SetCursorEnabled(bool cursor_enabled) {
@@ -505,6 +506,7 @@ void RenderText::SetMultiline(bool multiline) {
     multiline_ = multiline;
     cached_bounds_and_offset_valid_ = false;
     lines_.clear();
+    OnElidedTextShapeChanged();
   }
 }
 
@@ -514,13 +516,14 @@ void RenderText::SetMinLineHeight(int line_height) {
   min_line_height_ = line_height;
   cached_bounds_and_offset_valid_ = false;
   lines_.clear();
+  OnElidedTextShapeChanged();
 }
 
 void RenderText::SetElideBehavior(ElideBehavior elide_behavior) {
   // TODO(skanuj) : Add a test for triggering layout change.
   if (elide_behavior_ != elide_behavior) {
     elide_behavior_ = elide_behavior;
-    UpdateLayoutText();
+    OnElidedTextShapeChanged();
   }
 }
 
@@ -530,8 +533,10 @@ void RenderText::SetDisplayRect(const Rect& r) {
     baseline_ = kInvalidBaseline;
     cached_bounds_and_offset_valid_ = false;
     lines_.clear();
-    if (elide_behavior_ != NO_ELIDE)
-      UpdateLayoutText();
+    if (elide_behavior_ != NO_ELIDE &&
+        elide_behavior_ != FADE_TAIL) {
+      OnElidedTextShapeChanged();
+    }
   }
 }
 
@@ -657,7 +662,7 @@ void RenderText::SetCompositionRange(const Range& composition_range) {
         Range(0, text_.length()).Contains(composition_range));
   composition_range_.set_end(composition_range.end());
   composition_range_.set_start(composition_range.start());
-  ResetLayout();
+  OnLayoutTextShapeChanged(false);
 }
 
 void RenderText::SetColor(SkColor value) {
@@ -672,7 +677,7 @@ void RenderText::SetStyle(TextStyle style, bool value) {
   styles_[style].SetValue(value);
 
   cached_bounds_and_offset_valid_ = false;
-  ResetLayout();
+  OnLayoutTextShapeChanged(false);
 }
 
 void RenderText::ApplyStyle(TextStyle style, bool value, const Range& range) {
@@ -684,7 +689,7 @@ void RenderText::ApplyStyle(TextStyle style, bool value, const Range& range) {
   styles_[style].ApplyValue(value, Range(start, end));
 
   cached_bounds_and_offset_valid_ = false;
-  ResetLayout();
+  OnLayoutTextShapeChanged(false);
 }
 
 bool RenderText::GetStyle(TextStyle style) const {
@@ -699,7 +704,7 @@ void RenderText::SetDirectionalityMode(DirectionalityMode mode) {
   directionality_mode_ = mode;
   text_direction_ = base::i18n::UNKNOWN_DIRECTION;
   cached_bounds_and_offset_valid_ = false;
-  ResetLayout();
+  OnLayoutTextShapeChanged(false);
 }
 
 base::i18n::TextDirection RenderText::GetTextDirection() {
@@ -931,6 +936,7 @@ RenderText::RenderText()
       obscured_reveal_index_(-1),
       truncate_length_(0),
       elide_behavior_(NO_ELIDE),
+      text_elided_(false),
       replace_newline_chars_with_symbols_(true),
       min_line_height_(0),
       multiline_(false),
@@ -967,8 +973,27 @@ void RenderText::SetSelectionModel(const SelectionModel& model) {
   cached_bounds_and_offset_valid_ = false;
 }
 
-const base::string16& RenderText::GetLayoutText() const {
-  return layout_text_;
+void RenderText::UpdateElidedText(float visual_width) {
+  if (multiline_ ||
+      elide_behavior() == NO_ELIDE ||
+      elide_behavior() == FADE_TAIL ||
+      visual_width < display_rect_.width() ||
+      layout_text_.empty()) {
+    text_elided_ = false;
+    elided_text_.clear();
+    return;
+  }
+
+  // This doesn't trim styles so ellipsis may get rendered as a different
+  // style than the preceding text. See crbug.com/327850.
+  elided_text_.assign(Elide(layout_text_,
+                            visual_width,
+                            static_cast<float>(display_rect_.width()),
+                            elide_behavior_));
+
+  text_elided_ = elided_text_ != layout_text_;
+  if (!text_elided_)
+    elided_text_.clear();
 }
 
 const BreakList<size_t>& RenderText::GetLineBreaks() {
@@ -1176,6 +1201,7 @@ void RenderText::MoveCursorTo(size_t position, bool select) {
 
 void RenderText::UpdateLayoutText() {
   layout_text_.clear();
+  elided_text_.clear();
   line_breaks_.SetMax(0);
 
   if (obscured_) {
@@ -1224,33 +1250,24 @@ void RenderText::UpdateLayoutText() {
     }
   }
 
-  if (elide_behavior_ != NO_ELIDE &&
-      elide_behavior_ != FADE_TAIL &&
-      !layout_text_.empty() &&
-      GetContentWidth() > display_rect_.width()) {
-    // This doesn't trim styles so ellipsis may get rendered as a different
-    // style than the preceding text. See crbug.com/327850.
-    layout_text_.assign(Elide(layout_text_,
-                              static_cast<float>(display_rect_.width()),
-                              elide_behavior_));
-  }
-
   // Replace the newline character with a newline symbol in single line mode.
   static const base::char16 kNewline[] = { '\n', 0 };
   static const base::char16 kNewlineSymbol[] = { 0x2424, 0 };
   if (!multiline_ && replace_newline_chars_with_symbols_)
     base::ReplaceChars(layout_text_, kNewline, kNewlineSymbol, &layout_text_);
 
-  ResetLayout();
+  OnLayoutTextShapeChanged(true);
 }
 
 base::string16 RenderText::Elide(const base::string16& text,
+                                 float visual_width,

[msw, 2015/02/12 22:45:06]

nit: match the argument name with the declaration's |text_width|

[oshima, 2015/02/13 00:29:43]

Done.

                                  float available_width,
                                  ElideBehavior behavior) {
   if (available_width <= 0 || text.empty())
     return base::string16();
   if (behavior == ELIDE_EMAIL)
     return ElideEmail(text, available_width);
+  TRACE_EVENT0("ui", "RenderText::Elide");

[Jun Mukai, 2015/02/12 21:56:46]

Is this intentionally added?

[oshima, 2015/02/12 22:09:17]

Yes. This is very expensive operation and I think it's worth showing this in
chrome://tracing.

 
   // Create a RenderText copy with attributes that affect the rendering width.
   scoped_ptr<RenderText> render_text = CreateInstanceOfSameType();
@@ -1260,21 +1277,26 @@ base::string16 RenderText::Elide(const base::string16& text,
   render_text->set_truncate_length(truncate_length_);
   render_text->styles_ = styles_;
   render_text->colors_ = colors_;
-  render_text->SetText(text);
-  if (render_text->GetContentWidthF() <= available_width)
+  if (visual_width == 0) {
+    render_text->SetText(text);
+    visual_width = render_text->GetContentWidthF();
+  }
+  if (visual_width <= available_width)

[msw, 2015/02/12 22:45:06]

nit: return at the top if a non-zero visual_width <= available_width

[oshima, 2015/02/13 00:29:43]

Done.

     return text;
 
   const base::string16 ellipsis = base::string16(kEllipsisUTF16);
   const bool insert_ellipsis = (behavior != TRUNCATE);
   const bool elide_in_middle = (behavior == ELIDE_MIDDLE);
   const bool elide_at_beginning = (behavior == ELIDE_HEAD);
-  StringSlicer slicer(text, ellipsis, elide_in_middle, elide_at_beginning);
 
-  render_text->SetText(ellipsis);
-  const float ellipsis_width = render_text->GetContentWidthF();
+  if (insert_ellipsis) {
+    render_text->SetText(ellipsis);
+    const float ellipsis_width = render_text->GetContentWidthF();

[msw, 2015/02/12 22:45:06]

I doubt it, but maybe it'd be helpful to cache the ellipses width?
(I doubt this function is called with repeated layout attributes)

[oshima, 2015/02/13 00:29:43]

The ellipse size depends on the font list, so we can't cache it easily.

+    if (ellipsis_width > available_width)
+      return base::string16();
+  }
 
-  if (insert_ellipsis && (ellipsis_width > available_width))
-    return base::string16();
+  StringSlicer slicer(text, ellipsis, elide_in_middle, elide_at_beginning);
 
   // Use binary search to compute the elided text.
   size_t lo = 0;
@@ -1384,7 +1406,7 @@ base::string16 RenderText::ElideEmail(const base::string16& email,
         std::min<float>(available_domain_width,
             std::max<float>(available_width - full_username_width,
                             available_width / 2));
-    domain = Elide(domain, desired_domain_width, ELIDE_MIDDLE);
+    domain = Elide(domain, 0, desired_domain_width, ELIDE_MIDDLE);
     // Failing to elide the domain such that at least one character remains
     // (other than the ellipsis itself) remains: return a single ellipsis.
     if (domain.length() <= 1U)
@@ -1395,7 +1417,7 @@ base::string16 RenderText::ElideEmail(const base::string16& email,
   // is guaranteed to fit with at least one character remaining given all the
   // precautions taken earlier).
   available_width -= GetStringWidthF(domain, font_list());
-  username = Elide(username, available_width, ELIDE_TAIL);
+  username = Elide(username, 0, available_width, ELIDE_TAIL);
   return username + kAtSignUTF16 + domain;
 }
 
@@ -1426,9 +1448,9 @@ void RenderText::UpdateCachedBoundsAndOffset() {
 }
 
 void RenderText::DrawSelection(Canvas* canvas) {
-  const std::vector<Rect> sel = GetSubstringBounds(selection());
-  for (std::vector<Rect>::const_iterator i = sel.begin(); i < sel.end(); ++i)
-    canvas->FillRect(*i, selection_background_focused_color_);
+  const std::vector<Rect> selections = GetSubstringBounds(selection());

[msw, 2015/02/12 22:45:06]

nit: I think you can inline this in the range-based for loop below without
re-evaluating GetSubStringBounds on each iteration.

[oshima, 2015/02/13 00:29:43]

Done.

+  for (const Rect& s : selections)
+    canvas->FillRect(s, selection_background_focused_color_);
 }
 
 }  // namespace gfx