From b3dedd0e1dfa9b78a420d840eecce2fce9e4f1e2 Mon Sep 17 00:00:00 2001 From: Simon Fraser Date: Fri, 20 May 2022 05:27:37 +0000 Subject: [PATCH] REGRESSION (r293126): Gmail formatting menu/panel in compose view becomes blank/empty while scrolling https://bugs.webkit.org/show_bug.cgi?id=240625 Reviewed by Alan Bujtas. Gmail uses the css `clip` property with negative offsets, which is surprising, and revealed a bug in the compositing code for clipping. When a stacking-context layer has overflow:hidden or clip:, we assume that the clip encloses all the descendants, so make a GraphicsLayer with masksToBounds as a parent of the child layers. When the layer has border-radius with uneven corners, we implement that with a shape layer which acts as a mask on that clipping GraphicsLayer. The content in question had CSS clip with negative offsets and border-radius, so the shape layer mask would clip out any content outside the border shape. So if the clip rect extends beyond the border, we need to follow a different code path, which pushes the clipping layers onto descendants; this code path is used for non-stacking context clipping, and for mix-blend-mode which needs to avoid the creation of the intermediate clipping layer. So generalize the `isolatesCompositedBlending()` code path to also be used in the scenario of CSS clip which extends outside the border box. Tests: compositing/clipping/css-clip-and-border-radius.html compositing/clipping/css-clip-non-stacking.html * Source/WebCore/rendering/RenderLayerCompositor.cpp: (WebCore::canUseDescendantClippingLayer): (WebCore::RenderLayerCompositor::clippedByAncestor const): (WebCore::RenderLayerCompositor::computeAncestorClippingStack const): (WebCore::RenderLayerCompositor::clipsCompositingDescendants): * LayoutTests/compositing/clipping/css-clip-and-border-radius-expected.html: Added. * LayoutTests/compositing/clipping/css-clip-and-border-radius.html: Added. * LayoutTests/compositing/clipping/css-clip-non-stacking-expected.html: Added. * LayoutTests/compositing/clipping/css-clip-non-stacking.html: Added. Canonical link: https://commits.webkit.org/250785@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294530 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- .../css-clip-and-border-radius-expected.html | 57 ++++++++++++++++ .../clipping/css-clip-and-border-radius.html | 55 ++++++++++++++++ .../css-clip-non-stacking-expected.html | 65 +++++++++++++++++++ .../clipping/css-clip-non-stacking.html | 65 +++++++++++++++++++ .../rendering/RenderLayerCompositor.cpp | 36 ++++++++-- 5 files changed, 273 insertions(+), 5 deletions(-) create mode 100644 LayoutTests/compositing/clipping/css-clip-and-border-radius-expected.html create mode 100644 LayoutTests/compositing/clipping/css-clip-and-border-radius.html create mode 100644 LayoutTests/compositing/clipping/css-clip-non-stacking-expected.html create mode 100644 LayoutTests/compositing/clipping/css-clip-non-stacking.html diff --git a/LayoutTests/compositing/clipping/css-clip-and-border-radius-expected.html b/LayoutTests/compositing/clipping/css-clip-and-border-radius-expected.html new file mode 100644 index 000000000000..dfd3864b707b --- /dev/null +++ b/LayoutTests/compositing/clipping/css-clip-and-border-radius-expected.html @@ -0,0 +1,57 @@ + + + + + + +
+
+ this element should not be clipped +
+
+ +
+   +
+ this element should not be clipped +
+
+ +
+
+ this element should be clipped +
+
+ +
+   +
+ this element should be clipped +
+
+ + diff --git a/LayoutTests/compositing/clipping/css-clip-and-border-radius.html b/LayoutTests/compositing/clipping/css-clip-and-border-radius.html new file mode 100644 index 000000000000..d3c37e7ab172 --- /dev/null +++ b/LayoutTests/compositing/clipping/css-clip-and-border-radius.html @@ -0,0 +1,55 @@ + + + + + + +
+
+ this element should not be clipped +
+
+ +
+
+ this element should not be clipped +
+
+ +
+
+ this element should be clipped +
+
+ +
+
+ this element should be clipped +
+
+ + diff --git a/LayoutTests/compositing/clipping/css-clip-non-stacking-expected.html b/LayoutTests/compositing/clipping/css-clip-non-stacking-expected.html new file mode 100644 index 000000000000..41c89f70026d --- /dev/null +++ b/LayoutTests/compositing/clipping/css-clip-non-stacking-expected.html @@ -0,0 +1,65 @@ + + + + + + +
+ +
+
+ this element should not be clipped +
+
+ +
+
+ this element should not be clipped +
+
+ +
+
+ this element should be clipped +
+
+ +
+
+ this element should be clipped +
+
+ + diff --git a/LayoutTests/compositing/clipping/css-clip-non-stacking.html b/LayoutTests/compositing/clipping/css-clip-non-stacking.html new file mode 100644 index 000000000000..78ade3fbc792 --- /dev/null +++ b/LayoutTests/compositing/clipping/css-clip-non-stacking.html @@ -0,0 +1,65 @@ + + + + + + +
+ +
+
+ this element should not be clipped +
+
+ +
+
+ this element should not be clipped +
+
+ +
+
+ this element should be clipped +
+
+ +
+
+ this element should be clipped +
+
+ + diff --git a/Source/WebCore/rendering/RenderLayerCompositor.cpp b/Source/WebCore/rendering/RenderLayerCompositor.cpp index c3889f8146c0..3a640a4f6e88 100644 --- a/Source/WebCore/rendering/RenderLayerCompositor.cpp +++ b/Source/WebCore/rendering/RenderLayerCompositor.cpp @@ -2839,6 +2839,25 @@ const char* RenderLayerCompositor::logOneReasonForCompositing(const RenderLayer& } #endif + +static bool canUseDescendantClippingLayer(const RenderLayer& layer) +{ + if (layer.isolatesCompositedBlending()) + return false; + + // We can only use the "descendant clipping layer" strategy when the clip rect is entirely within + // the border box, because of interactions with border-radius clipping and compositing. + if (auto* renderer = layer.renderBox(); renderer && renderer->hasClip()) { + auto borderBoxRect = renderer->borderBoxRect(); + auto clipRect = renderer->clipRect({ }, nullptr); + + bool clipRectInsideBorderRect = intersection(borderBoxRect, clipRect) == clipRect; + return clipRectInsideBorderRect; + } + + return true; +} + // Return true if the given layer has some ancestor in the RenderLayer hierarchy that clips, // up to the enclosing compositing ancestor. This is required because compositing layers are parented // according to the z-order hierarchy, yet clipping goes down the renderer hierarchy. @@ -2857,7 +2876,7 @@ bool RenderLayerCompositor::clippedByAncestor(RenderLayer& layer, const RenderLa // in this case it is not allowed to clipsCompositingDescendants() and each of its children // will be clippedByAncestor()s, including the compositingAncestor. auto* computeClipRoot = compositingAncestor; - if (!compositingAncestor->isolatesCompositedBlending()) { + if (canUseDescendantClippingLayer(*compositingAncestor)) { computeClipRoot = nullptr; auto* parent = &layer; while (parent) { @@ -2873,7 +2892,8 @@ bool RenderLayerCompositor::clippedByAncestor(RenderLayer& layer, const RenderLa return false; } - return !layer.backgroundClipRect(RenderLayer::ClipRectsContext(computeClipRoot, TemporaryClipRects)).isInfinite(); // FIXME: Incorrect for CSS regions. + auto backgroundClipRect = layer.backgroundClipRect(RenderLayer::ClipRectsContext(computeClipRoot, TemporaryClipRects)); + return !backgroundClipRect.isInfinite(); // FIXME: Incorrect for CSS regions. } bool RenderLayerCompositor::updateAncestorClippingStack(const RenderLayer& layer, const RenderLayer* compositingAncestor) const @@ -2913,8 +2933,8 @@ Vector RenderLayerCompositor::computeAncestorClippingStack(c if (&ancestorLayer == compositingAncestor) { if (haveNonScrollableClippingIntermediateLayer) - pushNonScrollableClip(*currentClippedLayer, ancestorLayer, ancestorLayer.isolatesCompositedBlending() ? RespectOverflowClip : IgnoreOverflowClip); - else if (ancestorLayer.isolatesCompositedBlending() && newStack.isEmpty()) + pushNonScrollableClip(*currentClippedLayer, ancestorLayer, !canUseDescendantClippingLayer(ancestorLayer) ? RespectOverflowClip : IgnoreOverflowClip); + else if (!canUseDescendantClippingLayer(ancestorLayer) && newStack.isEmpty()) pushNonScrollableClip(*currentClippedLayer, ancestorLayer, RespectOverflowClip); return AncestorTraversal::Stop; @@ -2988,7 +3008,13 @@ bool RenderLayerCompositor::isCompositedSubframeRenderer(const RenderObject& ren // into the hierarchy between this layer and its children in the z-order hierarchy. bool RenderLayerCompositor::clipsCompositingDescendants(const RenderLayer& layer) { - return layer.hasCompositingDescendant() && layer.renderer().hasClipOrNonVisibleOverflow() && !layer.isolatesCompositedBlending() && !layer.hasCompositedNonContainedDescendants(); + if (!(layer.hasCompositingDescendant() && layer.renderer().hasClipOrNonVisibleOverflow())) + return false; + + if (layer.hasCompositedNonContainedDescendants()) + return false; + + return canUseDescendantClippingLayer(layer); } bool RenderLayerCompositor::requiresCompositingForAnimation(RenderLayerModelObject& renderer) const