Skip to content

Commit

Permalink
REGRESSION (r293126): Gmail formatting menu/panel in compose view bec…
Browse files Browse the repository at this point in the history
…omes blank/empty while scrolling

https://bugs.webkit.org/show_bug.cgi?id=240625
<rdar://92984518>

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
  • Loading branch information
smfr committed May 20, 2022
1 parent c705d36 commit b3dedd0
Show file tree
Hide file tree
Showing 5 changed files with 273 additions and 5 deletions.
@@ -0,0 +1,57 @@
<!DOCTYPE html>
<html>
<head>
<style>
.container {
position: absolute;
z-index: 1;
left: 100px;
height: 50px;
width: 500px;
border-radius: 0 25px 25px 0;
background-color: silver;
box-sizing: border-box;
transform: translateZ(0);
}

.child {
position: absolute;
top: 20px;
left: 20px;
height: 50px;
width: 220px;
border: 4px solid green;
padding: 10px;
background-color: orange;
transform: translateZ(0);
}
</style>
</head>
<body>
<div class="container" style="top: 50px;">
<div class="child">
this element should not be clipped
</div>
</div>

<div class="container" style="top: 200px; border-radius: 0px">
&nbsp;
<div class="child">
this element should not be clipped
</div>
</div>

<div class="container" style="top: 350px; overflow: hidden;">
<div class="child">
this element should be clipped
</div>
</div>

<div class="container" style="top: 500px; overflow: hidden; border-radius: 0px">
&nbsp;
<div class="child">
this element should be clipped
</div>
</div>
</body>
</html>
55 changes: 55 additions & 0 deletions LayoutTests/compositing/clipping/css-clip-and-border-radius.html
@@ -0,0 +1,55 @@
<!DOCTYPE html>
<html>
<head>
<style>
.container {
position: absolute;
z-index: 1;
left: 100px;
height: 50px;
width: 500px;
border-radius: 0 25px 25px 0;
background-color: silver;
box-sizing: border-box;
transform: translateZ(0);
}

.child {
position: absolute;
top: 20px;
left: 20px;
height: 50px;
width: 220px;
border: 4px solid green;
padding: 10px;
background-color: orange;
transform: translateZ(0);
}
</style>
</head>
<body>
<div class="container" style="top: 50px; clip: rect(0px, 500px, 150px, 0px);">
<div class="child">
this element should not be clipped
</div>
</div>

<div class="container" style="top: 200px; clip: rect(0px, 500px, 150px, 0px); border-radius: 0px">
<div class="child">
this element should not be clipped
</div>
</div>

<div class="container" style="top: 350px; clip: rect(0px, 500px, 50px, 0px);">
<div class="child">
this element should be clipped
</div>
</div>

<div class="container" style="top: 500px; clip: rect(0px, 500px, 50px, 0px); border-radius: 0px">
<div class="child">
this element should be clipped
</div>
</div>
</body>
</html>
@@ -0,0 +1,65 @@
<!DOCTYPE html>
<html>
<head>
<style>
.container {
position: absolute;
left: 100px;
height: 50px;
width: 500px;
border-radius: 0 25px 25px 0;
background-color: silver;
box-sizing: border-box;
}

.child {
position: absolute;
top: 20px;
left: 20px;
height: 50px;
width: 220px;
border: 4px solid green;
padding: 10px;
background-color: orange;
transform: translateZ(0);
}

.compositing-trigger {
position: absolute;
top: 40px;
left: 110px;
width: 20px;
height: 520px;
background-color: gray;
transform: translateZ(0);
}
</style>
</head>
<body>
<div class="compositing-trigger"></div>

<div class="container" style="top: 50px;">
<div class="child">
this element should not be clipped
</div>
</div>

<div class="container" style="top: 200px; border-radius: 0px">
<div class="child">
this element should not be clipped
</div>
</div>

<div class="container" style="top: 350px; overflow: hidden">
<div class="child">
this element should be clipped
</div>
</div>

<div class="container" style="top: 500px; overflow: hidden; border-radius: 0px">
<div class="child">
this element should be clipped
</div>
</div>
</body>
</html>
65 changes: 65 additions & 0 deletions LayoutTests/compositing/clipping/css-clip-non-stacking.html
@@ -0,0 +1,65 @@
<!DOCTYPE html>
<html>
<head>
<style>
.container {
position: absolute;
left: 100px;
height: 50px;
width: 500px;
border-radius: 0 25px 25px 0;
background-color: silver;
box-sizing: border-box;
}

.child {
position: absolute;
top: 20px;
left: 20px;
height: 50px;
width: 220px;
border: 4px solid green;
padding: 10px;
background-color: orange;
transform: translateZ(0);
}

.compositing-trigger {
position: absolute;
top: 40px;
left: 110px;
width: 20px;
height: 520px;
background-color: gray;
transform: translateZ(0);
}
</style>
</head>
<body>
<div class="compositing-trigger"></div>

<div class="container" style="top: 50px; clip: rect(0px, 500px, 150px, 0px);">
<div class="child">
this element should not be clipped
</div>
</div>

<div class="container" style="top: 200px; clip: rect(0px, 500px, 150px, 0px); border-radius: 0px">
<div class="child">
this element should not be clipped
</div>
</div>

<div class="container" style="top: 350px; clip: rect(0px, 500px, 50px, 0px);">
<div class="child">
this element should be clipped
</div>
</div>

<div class="container" style="top: 500px; clip: rect(0px, 500px, 50px, 0px); border-radius: 0px">
<div class="child">
this element should be clipped
</div>
</div>
</body>
</html>
36 changes: 31 additions & 5 deletions Source/WebCore/rendering/RenderLayerCompositor.cpp
Expand Up @@ -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.
Expand All @@ -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) {
Expand All @@ -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
Expand Down Expand Up @@ -2913,8 +2933,8 @@ Vector<CompositedClipData> 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;
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit b3dedd0

Please sign in to comment.