Commit 11674f6e authored by Manuel Rego Casasnovas's avatar Manuel Rego Casasnovas Committed by Commit Bot

Avoid border radius clipping when painting scrolling background

This patch fixes an under paint invalidation issue related
to border-radius on a scrolling element.
Before this change we were applying border radius clipping twice,
on the container and on the scrolling background.
This was producing an unnecessary change of background painting
when border radius changes, thus the under-invalidation problem.

To fix this we're avoiding to apply the border radius clipping
when painting the scrolling background.
The main change is at BoxPainterBase::FillLayerInfo::FillLayerInfo()
which gets a new parameter is_painting_scrolling_background,
we use it to avoid setting is_rounded_fill to true.

The test expectation is modified so it also uses composition
by setting "will-change: transform" on the "scroller".
Also the "fixed" elements is moved out of the "scroller"
as the "scroller" is now its containing block,
otherwise it wouldn't be visible,

BUG=1025019
TEST=compositing/overflow/scroller-with-border-radius.html

Change-Id: Ib9b6e2e8b4e8ee42dec413c69b853a3f06336774
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1984312
Commit-Queue: Manuel Rego <rego@igalia.com>
Reviewed-by: default avatarXianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#729517}
parent cf8bf011
...@@ -124,14 +124,24 @@ LayoutRectOutsets BoxModelObjectPainter::ComputePadding() const { ...@@ -124,14 +124,24 @@ LayoutRectOutsets BoxModelObjectPainter::ComputePadding() const {
BoxPainterBase::FillLayerInfo BoxModelObjectPainter::GetFillLayerInfo( BoxPainterBase::FillLayerInfo BoxModelObjectPainter::GetFillLayerInfo(
const Color& color, const Color& color,
const FillLayer& bg_layer, const FillLayer& bg_layer,
BackgroundBleedAvoidance bleed_avoidance) const { BackgroundBleedAvoidance bleed_avoidance,
bool is_painting_scrolling_background) const {
return BoxPainterBase::FillLayerInfo( return BoxPainterBase::FillLayerInfo(
box_model_.GetDocument(), box_model_.StyleRef(), box_model_.GetDocument(), box_model_.StyleRef(),
box_model_.HasOverflowClip(), color, bg_layer, bleed_avoidance, box_model_.HasOverflowClip(), color, bg_layer, bleed_avoidance,
LayoutObject::ShouldRespectImageOrientation(&box_model_), LayoutObject::ShouldRespectImageOrientation(&box_model_),
(flow_box_ ? flow_box_->IncludeLogicalLeftEdge() : true), (flow_box_ ? flow_box_->IncludeLogicalLeftEdge() : true),
(flow_box_ ? flow_box_->IncludeLogicalRightEdge() : true), (flow_box_ ? flow_box_->IncludeLogicalRightEdge() : true),
box_model_.IsInline()); box_model_.IsInline(), is_painting_scrolling_background);
}
bool BoxModelObjectPainter::IsPaintingScrollingBackground(
const PaintInfo& paint_info) const {
if (!box_model_.IsBox())
return false;
const auto& this_box = ToLayoutBox(box_model_);
return BoxDecorationData::IsPaintingScrollingBackground(paint_info, this_box);
} }
} // namespace blink } // namespace blink
...@@ -33,7 +33,9 @@ class BoxModelObjectPainter : public BoxPainterBase { ...@@ -33,7 +33,9 @@ class BoxModelObjectPainter : public BoxPainterBase {
BoxPainterBase::FillLayerInfo GetFillLayerInfo( BoxPainterBase::FillLayerInfo GetFillLayerInfo(
const Color&, const Color&,
const FillLayer&, const FillLayer&,
BackgroundBleedAvoidance) const override; BackgroundBleedAvoidance,
bool is_painting_scrolling_background) const override;
bool IsPaintingScrollingBackground(const PaintInfo&) const override;
void PaintTextClipMask(GraphicsContext&, void PaintTextClipMask(GraphicsContext&,
const IntRect& mask_rect, const IntRect& mask_rect,
......
...@@ -278,7 +278,8 @@ BoxPainterBase::FillLayerInfo::FillLayerInfo( ...@@ -278,7 +278,8 @@ BoxPainterBase::FillLayerInfo::FillLayerInfo(
RespectImageOrientationEnum respect_image_orientation, RespectImageOrientationEnum respect_image_orientation,
bool include_left, bool include_left,
bool include_right, bool include_right,
bool is_inline) bool is_inline,
bool is_painting_scrolling_background)
: image(layer.GetImage()), : image(layer.GetImage()),
color(bg_color), color(bg_color),
respect_image_orientation(respect_image_orientation), respect_image_orientation(respect_image_orientation),
...@@ -318,7 +319,7 @@ BoxPainterBase::FillLayerInfo::FillLayerInfo( ...@@ -318,7 +319,7 @@ BoxPainterBase::FillLayerInfo::FillLayerInfo(
// BorderFillBox radius clipping is taken care of by // BorderFillBox radius clipping is taken care of by
// BackgroundBleedClip{Only,Layer} // BackgroundBleedClip{Only,Layer}
is_rounded_fill = is_rounded_fill =
has_rounded_border && has_rounded_border && !is_painting_scrolling_background &&
!(is_border_fill && BleedAvoidanceIsClipping(bleed_avoidance)); !(is_border_fill && BleedAvoidanceIsClipping(bleed_avoidance));
should_paint_image = image && image->CanRender(); should_paint_image = image && image->CanRender();
...@@ -784,7 +785,9 @@ void BoxPainterBase::PaintFillLayer(const PaintInfo& paint_info, ...@@ -784,7 +785,9 @@ void BoxPainterBase::PaintFillLayer(const PaintInfo& paint_info,
if (rect.IsEmpty()) if (rect.IsEmpty())
return; return;
const FillLayerInfo info = GetFillLayerInfo(color, bg_layer, bleed_avoidance); const FillLayerInfo info =
GetFillLayerInfo(color, bg_layer, bleed_avoidance,
IsPaintingScrollingBackground(paint_info));
// If we're not actually going to paint anything, abort early. // If we're not actually going to paint anything, abort early.
if (!info.should_paint_image && !info.should_paint_color) if (!info.should_paint_image && !info.should_paint_color)
return; return;
......
...@@ -116,7 +116,8 @@ class BoxPainterBase { ...@@ -116,7 +116,8 @@ class BoxPainterBase {
RespectImageOrientationEnum, RespectImageOrientationEnum,
bool include_left_edge, bool include_left_edge,
bool include_right_edge, bool include_right_edge,
bool is_inline); bool is_inline,
bool is_painting_scrolling_background);
// FillLayerInfo is a temporary, stack-allocated container which cannot // FillLayerInfo is a temporary, stack-allocated container which cannot
// outlive the StyleImage. This would normally be a raw pointer, if not for // outlive the StyleImage. This would normally be a raw pointer, if not for
...@@ -155,9 +156,12 @@ class BoxPainterBase { ...@@ -155,9 +156,12 @@ class BoxPainterBase {
virtual PhysicalRect AdjustRectForScrolledContent(const PaintInfo&, virtual PhysicalRect AdjustRectForScrolledContent(const PaintInfo&,
const FillLayerInfo&, const FillLayerInfo&,
const PhysicalRect&) = 0; const PhysicalRect&) = 0;
virtual FillLayerInfo GetFillLayerInfo(const Color&, virtual FillLayerInfo GetFillLayerInfo(
const FillLayer&, const Color&,
BackgroundBleedAvoidance) const = 0; const FillLayer&,
BackgroundBleedAvoidance,
bool is_painting_scrolling_background) const = 0;
virtual bool IsPaintingScrollingBackground(const PaintInfo&) const = 0;
static void PaintInsetBoxShadow(const PaintInfo&, static void PaintInsetBoxShadow(const PaintInfo&,
const FloatRoundedRect&, const FloatRoundedRect&,
const ComputedStyle&, const ComputedStyle&,
......
...@@ -1320,7 +1320,7 @@ NGBoxFragmentPainter::MoveTo NGBoxFragmentPainter::PaintBoxItem( ...@@ -1320,7 +1320,7 @@ NGBoxFragmentPainter::MoveTo NGBoxFragmentPainter::PaintBoxItem(
} }
bool NGBoxFragmentPainter::IsPaintingScrollingBackground( bool NGBoxFragmentPainter::IsPaintingScrollingBackground(
const PaintInfo& paint_info) { const PaintInfo& paint_info) const {
if (RuntimeEnabledFeatures::CompositeAfterPaintEnabled()) if (RuntimeEnabledFeatures::CompositeAfterPaintEnabled())
return paint_info.IsPaintingScrollingBackground(); return paint_info.IsPaintingScrollingBackground();
...@@ -1414,7 +1414,8 @@ LayoutRectOutsets NGBoxFragmentPainter::ComputePadding() const { ...@@ -1414,7 +1414,8 @@ LayoutRectOutsets NGBoxFragmentPainter::ComputePadding() const {
BoxPainterBase::FillLayerInfo NGBoxFragmentPainter::GetFillLayerInfo( BoxPainterBase::FillLayerInfo NGBoxFragmentPainter::GetFillLayerInfo(
const Color& color, const Color& color,
const FillLayer& bg_layer, const FillLayer& bg_layer,
BackgroundBleedAvoidance bleed_avoidance) const { BackgroundBleedAvoidance bleed_avoidance,
bool is_painting_scrolling_background) const {
const NGBorderEdges& border_edges = BorderEdges(); const NGBorderEdges& border_edges = BorderEdges();
const NGPhysicalBoxFragment& fragment = PhysicalFragment(); const NGPhysicalBoxFragment& fragment = PhysicalFragment();
return BoxPainterBase::FillLayerInfo( return BoxPainterBase::FillLayerInfo(
...@@ -1422,7 +1423,7 @@ BoxPainterBase::FillLayerInfo NGBoxFragmentPainter::GetFillLayerInfo( ...@@ -1422,7 +1423,7 @@ BoxPainterBase::FillLayerInfo NGBoxFragmentPainter::GetFillLayerInfo(
fragment.HasOverflowClip(), color, bg_layer, bleed_avoidance, fragment.HasOverflowClip(), color, bg_layer, bleed_avoidance,
LayoutObject::ShouldRespectImageOrientation(fragment.GetLayoutObject()), LayoutObject::ShouldRespectImageOrientation(fragment.GetLayoutObject()),
border_edges.line_left, border_edges.line_right, border_edges.line_left, border_edges.line_right,
fragment.GetLayoutObject()->IsInline()); fragment.GetLayoutObject()->IsInline(), is_painting_scrolling_background);
} }
bool NGBoxFragmentPainter::IsInSelfHitTestingPhase(HitTestAction action) const { bool NGBoxFragmentPainter::IsInSelfHitTestingPhase(HitTestAction action) const {
......
...@@ -65,7 +65,9 @@ class NGBoxFragmentPainter : public BoxPainterBase { ...@@ -65,7 +65,9 @@ class NGBoxFragmentPainter : public BoxPainterBase {
BoxPainterBase::FillLayerInfo GetFillLayerInfo( BoxPainterBase::FillLayerInfo GetFillLayerInfo(
const Color&, const Color&,
const FillLayer&, const FillLayer&,
BackgroundBleedAvoidance) const override; BackgroundBleedAvoidance,
bool is_painting_scrolling_background) const override;
bool IsPaintingScrollingBackground(const PaintInfo&) const override;
void PaintTextClipMask(GraphicsContext&, void PaintTextClipMask(GraphicsContext&,
const IntRect& mask_rect, const IntRect& mask_rect,
...@@ -83,7 +85,6 @@ class NGBoxFragmentPainter : public BoxPainterBase { ...@@ -83,7 +85,6 @@ class NGBoxFragmentPainter : public BoxPainterBase {
NGInlineCursor* descendants = nullptr); NGInlineCursor* descendants = nullptr);
enum MoveTo { kDontSkipChildren, kSkipChildren }; enum MoveTo { kDontSkipChildren, kSkipChildren };
bool IsPaintingScrollingBackground(const PaintInfo&);
bool ShouldPaint(const ScopedPaintState&) const; bool ShouldPaint(const ScopedPaintState&) const;
void PaintBoxDecorationBackground(const PaintInfo&, void PaintBoxDecorationBackground(const PaintInfo&,
......
...@@ -356,9 +356,6 @@ crbug.com/1018273 [ Mac ] paint/invalidation/compositing/should-not-repaint-scro ...@@ -356,9 +356,6 @@ crbug.com/1018273 [ Mac ] paint/invalidation/compositing/should-not-repaint-scro
crbug.com/1018273 [ Mac ] paint/invalidation/compositing/stop-painting-onto-scrolling-contents.html [ Failure ] crbug.com/1018273 [ Mac ] paint/invalidation/compositing/stop-painting-onto-scrolling-contents.html [ Failure ]
crbug.com/1018273 [ Mac ] paint/overflow/composited-scroll-vertical-rl.html [ Failure ] crbug.com/1018273 [ Mac ] paint/overflow/composited-scroll-vertical-rl.html [ Failure ]
crbug.com/1025019 compositing/overflow/scroller-with-border-radius.html [ Crash ]
crbug.com/1025019 virtual/prefer_compositing_to_lcd_text/compositing/overflow/scroller-with-border-radius.html [ Crash ]
# Fail due to lack of fuzzy matching on Win and Mac platforms for WPT. The pixels in the pre-rotated reference # Fail due to lack of fuzzy matching on Win and Mac platforms for WPT. The pixels in the pre-rotated reference
# images do not exactly match the exif rotated images, probably due to jpeg decoding/encoding issues. Maybe # images do not exactly match the exif rotated images, probably due to jpeg decoding/encoding issues. Maybe
# adjusting the image size to a multiple of 8 would fix this (so all jpeg blocks are solid color). # adjusting the image size to a multiple of 8 would fix this (so all jpeg blocks are solid color).
......
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
width: 300px; width: 300px;
background-color: red; background-color: red;
border-radius: 5px; border-radius: 5px;
will-change: transform;
} }
#scrolled { #scrolled {
...@@ -25,5 +26,5 @@ ...@@ -25,5 +26,5 @@
</style> </style>
<div id="scroller"> <div id="scroller">
<div id="scrolled"></div> <div id="scrolled"></div>
<div id="fixed"></div>
</div> </div>
<div id="fixed"></div>
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