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

Issues Search
    My Issues | Starred     Open | Closed | All

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: 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
« ui/views/controls/label.h ('K') | « ui/views/controls/label.h ('k') | no next file » | 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..7d9c8c7491074fe1d1bfa89fce50ad193f056338 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();
}
@@ -114,6 +114,7 @@ void Label::SetShadows(const gfx::ShadowValues& shadows) {
is_first_paint_text_ = true;
shadows_ = shadows;
text_size_valid_ = false;
+ cached_draw_params_.reset();
msw 2015/01/07 20:35:56 Maybe just ResetLayoutCache(); here.
ckocagil 2015/01/07 22:55:08 Done.
}
void Label::SetSubpixelRenderingEnabled(bool subpixel_rendering_enabled) {
@@ -148,7 +149,7 @@ void Label::SetLineHeight(int height) {
is_first_paint_text_ = true;
if (height != line_height_) {
line_height_ = height;
- ResetCachedSize();
+ ResetLayoutCache();
PreferredSizeChanged();
SchedulePaint();
}
@@ -160,7 +161,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 +179,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 +191,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();
}
@@ -396,6 +397,7 @@ gfx::Size Label::GetTextSize() const {
void Label::OnBoundsChanged(const gfx::Rect& previous_bounds) {
text_size_valid_ &= !multi_line_;
+ cached_draw_params_.reset();
}
void Label::OnPaint(gfx::Canvas* canvas) {
@@ -405,23 +407,22 @@ void Label::OnPaint(gfx::Canvas* canvas) {
// interfere with that.
OnPaintBorder(canvas);
- base::string16 paint_text;
- gfx::Rect text_bounds;
- int flags = 0;
- CalculateDrawStringParams(&paint_text, &text_bounds, &flags);
+ const DrawStringParams* paint_params = CalculateDrawStringParams();
msw 2015/01/07 20:35:56 nit: |draw_params| or just |params|?
ckocagil 2015/01/07 22:55:08 Done, it's |params| now.
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, paint_params->text, paint_params->bounds,
+ paint_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, paint_params->text, paint_params->bounds,
+ paint_params->flags);
}
}
@@ -444,7 +445,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 +542,30 @@ 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_) {
+ DrawStringParams* paint_params = new DrawStringParams();
msw 2015/01/07 20:35:56 Can you just do cached_draw_params_.reset(new Draw
ckocagil 2015/01/07 22:55:08 It's |params| now. |cached_draw_params_| is longer
- 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_);
+ const bool forbid_ellipsis = elide_behavior_ == gfx::NO_ELIDE ||
+ elide_behavior_ == gfx::FADE_TAIL;
msw 2015/01/07 20:35:56 nit: retain relative indent.
ckocagil 2015/01/07 22:55:08 Done.
+ if (multi_line_ || forbid_ellipsis) {
+ paint_params->text = layout_text_;
+ }
+ else {
msw 2015/01/07 20:35:56 nit: else on line above.
ckocagil 2015/01/07 22:55:08 Done.
+ paint_params->text = gfx::ElideText(layout_text_, font_list_,
+ GetAvailableRect().width(), elide_behavior_);
+ }
+
+ paint_params->bounds = GetTextBounds();
+ paint_params->flags = ComputeDrawStringFlags();
+ // TODO(msw): Elide multi-line text with ElideRectangleText instead.
+ if (!multi_line_ || forbid_ellipsis)
+ paint_params->flags |= gfx::Canvas::NO_ELLIPSIS;
+
+ cached_draw_params_.reset(paint_params);
}
- *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_.get();
}
void Label::UpdateColorsFromTheme(const ui::NativeTheme* theme) {
@@ -578,7 +584,8 @@ void Label::UpdateColorsFromTheme(const ui::NativeTheme* theme) {
RecalculateColors();
}
-void Label::ResetCachedSize() {
+void Label::ResetLayoutCache() {
+ cached_draw_params_.reset();
text_size_valid_ = false;
cached_heights_cursor_ = 0;
for (int i = 0; i < kCachedSizeLimit; ++i)
« ui/views/controls/label.h ('K') | « ui/views/controls/label.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698