Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(187)

Unified Diff: ui/views/controls/label.cc

Issue 843503003: Cache DrawStringParams in views::Label (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: add comments Created 5 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « ui/views/controls/label.h ('k') | ui/views/controls/label_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: ui/views/controls/label.cc
diff --git a/ui/views/controls/label.cc b/ui/views/controls/label.cc
index 05aaaab13fe9796c968b6b1e15e766976b4838ae..a90aec4042e2782125dbbe47bae88963b4d040d2 100644
--- a/ui/views/controls/label.cc
+++ b/ui/views/controls/label.cc
@@ -56,7 +56,7 @@ Label::~Label() {
void Label::SetFontList(const gfx::FontList& font_list) {
is_first_paint_text_ = true;
font_list_ = font_list;
- ResetCachedSize();
+ ResetLayoutCache();
PreferredSizeChanged();
SchedulePaint();
}
@@ -78,7 +78,7 @@ void Label::SetTextInternal(const base::string16& text) {
layout_text_ = text_;
}
- ResetCachedSize();
+ ResetLayoutCache();
PreferredSizeChanged();
SchedulePaint();
}
@@ -113,7 +113,7 @@ void Label::SetBackgroundColor(SkColor color) {
void Label::SetShadows(const gfx::ShadowValues& shadows) {
is_first_paint_text_ = true;
shadows_ = shadows;
- text_size_valid_ = false;
+ ResetLayoutCache();
}
void Label::SetSubpixelRenderingEnabled(bool subpixel_rendering_enabled) {
@@ -148,7 +148,7 @@ void Label::SetLineHeight(int height) {
is_first_paint_text_ = true;
if (height != line_height_) {
line_height_ = height;
- ResetCachedSize();
+ ResetLayoutCache();
PreferredSizeChanged();
SchedulePaint();
}
@@ -160,7 +160,7 @@ void Label::SetMultiLine(bool multi_line) {
elide_behavior_ == gfx::NO_ELIDE));
if (multi_line != multi_line_) {
multi_line_ = multi_line;
- ResetCachedSize();
+ ResetLayoutCache();
PreferredSizeChanged();
SchedulePaint();
}
@@ -178,7 +178,7 @@ void Label::SetAllowCharacterBreak(bool allow_character_break) {
is_first_paint_text_ = true;
if (allow_character_break != allow_character_break_) {
allow_character_break_ = allow_character_break;
- ResetCachedSize();
+ ResetLayoutCache();
PreferredSizeChanged();
SchedulePaint();
}
@@ -190,7 +190,7 @@ void Label::SetElideBehavior(gfx::ElideBehavior elide_behavior) {
elide_behavior_ == gfx::NO_ELIDE));
if (elide_behavior != elide_behavior_) {
elide_behavior_ = elide_behavior;
- ResetCachedSize();
+ ResetLayoutCache();
PreferredSizeChanged();
SchedulePaint();
}
@@ -291,6 +291,8 @@ int Label::GetHeightForWidth(int w) const {
int cache_width = w;
int h = font_list_.GetHeight();
+ // Flags returned in the cached |DrawStringParams| has a different value
+ // from the result of |ComputeDrawStringFlags()|. The latter is needed here.
const int flags = ComputeDrawStringFlags();
gfx::Canvas::SizeStringInt(
layout_text_, font_list_, &w, &h, line_height_, flags);
@@ -373,6 +375,8 @@ gfx::Size Label::GetTextSize() const {
int h = font_list_.GetHeight();
// For single-line strings, ignore the available width and calculate how
// wide the text wants to be.
+ // Call |ComputeDrawStringFlags()| instead of |CalculateDrawStringParams()|
+ // here since the latter calls this function and causes infinite recursion.
int flags = ComputeDrawStringFlags();
if (!multi_line_)
flags |= gfx::Canvas::NO_ELLIPSIS;
@@ -396,6 +400,7 @@ gfx::Size Label::GetTextSize() const {
void Label::OnBoundsChanged(const gfx::Rect& previous_bounds) {
text_size_valid_ &= !multi_line_;
+ cached_draw_params_.text.clear();
}
void Label::OnPaint(gfx::Canvas* canvas) {
@@ -404,24 +409,23 @@ void Label::OnPaint(gfx::Canvas* canvas) {
// some subclasses of Label. We do not want View's focus border painting to
// interfere with that.
OnPaintBorder(canvas);
+ if (layout_text_.empty())
+ return;
- base::string16 paint_text;
- gfx::Rect text_bounds;
- int flags = 0;
- CalculateDrawStringParams(&paint_text, &text_bounds, &flags);
+ const DrawStringParams* params = CalculateDrawStringParams();
if (is_first_paint_text_) {
// TODO(vadimt): Remove ScopedTracker below once crbug.com/431326 is fixed.
tracked_objects::ScopedTracker tracking_profile(
FROM_HERE_WITH_EXPLICIT_FUNCTION("431326 Label::PaintText first"));
is_first_paint_text_ = false;
- PaintText(canvas, paint_text, text_bounds, flags);
+ PaintText(canvas, params->text, params->bounds, params->flags);
} else {
// TODO(vadimt): Remove ScopedTracker below once crbug.com/431326 is fixed.
tracked_objects::ScopedTracker tracking_profile(
FROM_HERE_WITH_EXPLICIT_FUNCTION("431326 Label::PaintText not first"));
- PaintText(canvas, paint_text, text_bounds, flags);
+ PaintText(canvas, params->text, params->bounds, params->flags);
}
}
@@ -444,7 +448,7 @@ void Label::Init(const base::string16& text, const gfx::FontList& font_list) {
handles_tooltips_ = true;
collapse_when_hidden_ = false;
cached_heights_.resize(kCachedSizeLimit);
- ResetCachedSize();
+ ResetLayoutCache();
is_first_paint_text_ = true;
SetText(text);
@@ -541,25 +545,25 @@ gfx::Rect Label::GetAvailableRect() const {
return bounds;
}
-void Label::CalculateDrawStringParams(base::string16* paint_text,
- gfx::Rect* text_bounds,
- int* flags) const {
- DCHECK(paint_text && text_bounds && flags);
+const Label::DrawStringParams* Label::CalculateDrawStringParams() const {
+ if (cached_draw_params_.text.empty()) {
+ const bool forbid_ellipsis = elide_behavior_ == gfx::NO_ELIDE ||
+ elide_behavior_ == gfx::FADE_TAIL;
+ if (multi_line_ || forbid_ellipsis) {
+ cached_draw_params_.text = layout_text_;
+ } else {
+ cached_draw_params_.text = gfx::ElideText(layout_text_, font_list_,
+ GetAvailableRect().width(), elide_behavior_);
+ }
- const bool forbid_ellipsis = elide_behavior_ == gfx::NO_ELIDE ||
- elide_behavior_ == gfx::FADE_TAIL;
- if (multi_line_ || forbid_ellipsis) {
- *paint_text = layout_text_;
- } else {
- *paint_text = gfx::ElideText(layout_text_, font_list_,
- GetAvailableRect().width(), elide_behavior_);
+ cached_draw_params_.bounds = GetTextBounds();
+ cached_draw_params_.flags = ComputeDrawStringFlags();
+ // TODO(msw): Elide multi-line text with ElideRectangleText instead.
+ if (!multi_line_ || forbid_ellipsis)
+ cached_draw_params_.flags |= gfx::Canvas::NO_ELLIPSIS;
}
- *text_bounds = GetTextBounds();
- *flags = ComputeDrawStringFlags();
- // TODO(msw): Elide multi-line text with ElideRectangleText instead.
- if (!multi_line_ || forbid_ellipsis)
- *flags |= gfx::Canvas::NO_ELLIPSIS;
+ return &cached_draw_params_;
}
void Label::UpdateColorsFromTheme(const ui::NativeTheme* theme) {
@@ -578,7 +582,8 @@ void Label::UpdateColorsFromTheme(const ui::NativeTheme* theme) {
RecalculateColors();
}
-void Label::ResetCachedSize() {
+void Label::ResetLayoutCache() {
+ cached_draw_params_.text.clear();
text_size_valid_ = false;
cached_heights_cursor_ = 0;
for (int i = 0; i < kCachedSizeLimit; ++i)
« no previous file with comments | « ui/views/controls/label.h ('k') | ui/views/controls/label_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698