Commit 8faba2d2 authored by Taylor Bergquist's avatar Taylor Bergquist Committed by Commit Bot

Skip subpixel rendering opacity check for tab labels.

Adds a mechanism for skipping the opacity check to
Label and applies it to the tab title label.

Bug: 1139395
Change-Id: I1c182b735964856be971d59af1c28ab29011d99c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2518277
Commit-Queue: Taylor Bergquist <tbergquist@chromium.org>
Reviewed-by: default avatarCollin Baker <collinbaker@chromium.org>
Reviewed-by: default avatarPeter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#824677}
parent 56fc5c68
...@@ -205,6 +205,13 @@ Tab::Tab(TabController* controller) ...@@ -205,6 +205,13 @@ Tab::Tab(TabController* controller)
title_->SetAutoColorReadabilityEnabled(false); title_->SetAutoColorReadabilityEnabled(false);
title_->SetText(CoreTabHelper::GetDefaultTitle()); title_->SetText(CoreTabHelper::GetDefaultTitle());
title_->SetFontList(tab_style_->GetFontList()); title_->SetFontList(tab_style_->GetFontList());
// |title_| paints on top of an opaque region (the tab background) of a
// non-opaque layer (the tabstrip's layer), which cannot currently be detected
// by the subpixel-rendering opacity check.
// TODO(https://crbug.com/1139395): Improve the check so that this case doen't
// need a manual suppression by detecting cases where the text is painted onto
// onto opaque parts of a not-entirely-opaque layer.
title_->SetSkipSubpixelRenderingOpacityCheck(true);
AddChildView(title_); AddChildView(title_);
SetEventTargeter(std::make_unique<views::ViewTargeter>(this)); SetEventTargeter(std::make_unique<views::ViewTargeter>(this));
......
...@@ -225,6 +225,21 @@ void Label::SetSubpixelRenderingEnabled(bool subpixel_rendering_enabled) { ...@@ -225,6 +225,21 @@ void Label::SetSubpixelRenderingEnabled(bool subpixel_rendering_enabled) {
OnPropertyChanged(&subpixel_rendering_enabled_, kPropertyEffectsPaint); OnPropertyChanged(&subpixel_rendering_enabled_, kPropertyEffectsPaint);
} }
bool Label::GetSkipSubpixelRenderingOpacityCheck() const {
return skip_subpixel_rendering_opacity_check_;
}
void Label::SetSkipSubpixelRenderingOpacityCheck(
bool skip_subpixel_rendering_opacity_check) {
if (skip_subpixel_rendering_opacity_check_ ==
skip_subpixel_rendering_opacity_check)
return;
skip_subpixel_rendering_opacity_check_ =
skip_subpixel_rendering_opacity_check;
OnPropertyChanged(&skip_subpixel_rendering_opacity_check_,
kPropertyEffectsNone);
}
gfx::HorizontalAlignment Label::GetHorizontalAlignment() const { gfx::HorizontalAlignment Label::GetHorizontalAlignment() const {
return full_text_->horizontal_alignment(); return full_text_->horizontal_alignment();
} }
...@@ -674,15 +689,16 @@ void Label::PaintText(gfx::Canvas* canvas) { ...@@ -674,15 +689,16 @@ void Label::PaintText(gfx::Canvas* canvas) {
// fixing either this check (to correctly idenfify more paints-on-opaque // fixing either this check (to correctly idenfify more paints-on-opaque
// cases), refactoring parents to use background() or by fixing // cases), refactoring parents to use background() or by fixing
// subpixel-rendering issues that the DCHECK detects. // subpixel-rendering issues that the DCHECK detects.
if (!display_text_ || display_text_->subpixel_rendering_suppressed()) if (!display_text_ || display_text_->subpixel_rendering_suppressed() ||
skip_subpixel_rendering_opacity_check_)
return; return;
// Ensure that, if we're using subpixel rendering, we're painted to an opaque // Ensure that, if we're using subpixel rendering, we're painted to an opaque
// region. Subpixel rendering will sample from the r,g,b color channels of the // region. Subpixel rendering will sample from the r,g,b color channels of the
// canvas. These values are incorrect when sampling from transparent pixels. // canvas. These values are incorrect when sampling from transparent pixels.
// Note that these checks may need to be amended for other methods of painting // Note that these checks may need to be amended for other methods of painting
// opaquely underneath the Label or we might need to allow individual cases to // opaquely underneath the Label. For now, individual cases can skip this
// skip this DCHECK. // DCHECK by calling Label::SetSkipSubpixelRenderingOpacityCheck().
for (View* view = this; view; view = view->parent()) { for (View* view = this; view; view = view->parent()) {
// This is our approximation of being painted on an opaque region. If any // This is our approximation of being painted on an opaque region. If any
// parent has an opaque background we assume that that background covers the // parent has an opaque background we assume that that background covers the
...@@ -1176,6 +1192,7 @@ ADD_PROPERTY_METADATA(SkColor, BackgroundColor) ...@@ -1176,6 +1192,7 @@ ADD_PROPERTY_METADATA(SkColor, BackgroundColor)
ADD_PROPERTY_METADATA(SkColor, SelectionTextColor) ADD_PROPERTY_METADATA(SkColor, SelectionTextColor)
ADD_PROPERTY_METADATA(SkColor, SelectionBackgroundColor) ADD_PROPERTY_METADATA(SkColor, SelectionBackgroundColor)
ADD_PROPERTY_METADATA(bool, SubpixelRenderingEnabled) ADD_PROPERTY_METADATA(bool, SubpixelRenderingEnabled)
ADD_PROPERTY_METADATA(bool, SkipSubpixelRenderingOpacityCheck)
ADD_PROPERTY_METADATA(gfx::ShadowValues, Shadows) ADD_PROPERTY_METADATA(gfx::ShadowValues, Shadows)
ADD_PROPERTY_METADATA(gfx::HorizontalAlignment, HorizontalAlignment) ADD_PROPERTY_METADATA(gfx::HorizontalAlignment, HorizontalAlignment)
ADD_PROPERTY_METADATA(gfx::VerticalAlignment, VerticalAlignment) ADD_PROPERTY_METADATA(gfx::VerticalAlignment, VerticalAlignment)
......
...@@ -140,6 +140,16 @@ class VIEWS_EXPORT Label : public View, ...@@ -140,6 +140,16 @@ class VIEWS_EXPORT Label : public View,
bool GetSubpixelRenderingEnabled() const; bool GetSubpixelRenderingEnabled() const;
void SetSubpixelRenderingEnabled(bool subpixel_rendering_enabled); void SetSubpixelRenderingEnabled(bool subpixel_rendering_enabled);
// Gets/Sets whether the DCHECK() checking that subpixel-rendered text is
// only drawn onto opaque layers is skipped. Use this to suppress false
// positives - for example, if the label is drawn onto an opaque region of a
// non-opaque layer. If possible, prefer making the layer opaque or painting
// onto an opaque views::Background, as those cases are detected and
// excluded by this DCHECK automatically.
bool GetSkipSubpixelRenderingOpacityCheck() const;
void SetSkipSubpixelRenderingOpacityCheck(
bool skip_subpixel_rendering_opacity_check);
// Gets/Sets the horizontal alignment; the argument value is mirrored in RTL // Gets/Sets the horizontal alignment; the argument value is mirrored in RTL
// UI. // UI.
gfx::HorizontalAlignment GetHorizontalAlignment() const; gfx::HorizontalAlignment GetHorizontalAlignment() const;
...@@ -430,6 +440,7 @@ class VIEWS_EXPORT Label : public View, ...@@ -430,6 +440,7 @@ class VIEWS_EXPORT Label : public View,
gfx::ElideBehavior elide_behavior_ = gfx::ELIDE_TAIL; gfx::ElideBehavior elide_behavior_ = gfx::ELIDE_TAIL;
bool subpixel_rendering_enabled_ = true; bool subpixel_rendering_enabled_ = true;
bool skip_subpixel_rendering_opacity_check_ = false;
bool auto_color_readability_enabled_ = true; bool auto_color_readability_enabled_ = true;
// TODO(mukai): remove |multi_line_| when all RenderText can render multiline. // TODO(mukai): remove |multi_line_| when all RenderText can render multiline.
bool multi_line_ = false; bool multi_line_ = false;
...@@ -462,6 +473,7 @@ VIEW_BUILDER_PROPERTY(SkColor, SelectionTextColor) ...@@ -462,6 +473,7 @@ VIEW_BUILDER_PROPERTY(SkColor, SelectionTextColor)
VIEW_BUILDER_PROPERTY(SkColor, SelectionBackgroundColor) VIEW_BUILDER_PROPERTY(SkColor, SelectionBackgroundColor)
VIEW_BUILDER_PROPERTY(const gfx::ShadowValues&, Shadows) VIEW_BUILDER_PROPERTY(const gfx::ShadowValues&, Shadows)
VIEW_BUILDER_PROPERTY(bool, SubpixelRenderingEnabled) VIEW_BUILDER_PROPERTY(bool, SubpixelRenderingEnabled)
VIEW_BUILDER_PROPERTY(bool, SkipSubpixelRenderingOpacityCheck)
VIEW_BUILDER_PROPERTY(gfx::HorizontalAlignment, HorizontalAlignment) VIEW_BUILDER_PROPERTY(gfx::HorizontalAlignment, HorizontalAlignment)
VIEW_BUILDER_PROPERTY(gfx::VerticalAlignment, VerticalAlignment) VIEW_BUILDER_PROPERTY(gfx::VerticalAlignment, VerticalAlignment)
VIEW_BUILDER_PROPERTY(int, LineHeight) VIEW_BUILDER_PROPERTY(int, LineHeight)
......
...@@ -1105,6 +1105,11 @@ TEST_F(LabelTest, ChecksSubpixelRenderingOntoOpaqueSurface) { ...@@ -1105,6 +1105,11 @@ TEST_F(LabelTest, ChecksSubpixelRenderingOntoOpaqueSurface) {
// Painting on a transparent layer should DCHECK. // Painting on a transparent layer should DCHECK.
EXPECT_DCHECK_DEATH(label->OnPaint(&canvas)); EXPECT_DCHECK_DEATH(label->OnPaint(&canvas));
// We should not DCHECK if the check is skipped.
label->SetSkipSubpixelRenderingOpacityCheck(true);
label->OnPaint(&canvas);
label->SetSkipSubpixelRenderingOpacityCheck(false);
// Painting onto a transparent layer should not DCHECK if there's an opaque // Painting onto a transparent layer should not DCHECK if there's an opaque
// background in a parent of the Label. // background in a parent of the Label.
view.SetBackground(CreateSolidBackground(SK_ColorWHITE)); view.SetBackground(CreateSolidBackground(SK_ColorWHITE));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment