Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโ€™ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

border-radius clipping of composited layers doesn't work #3883

Conversation

smfr
Copy link
Contributor

@smfr smfr commented Aug 31, 2022

9791054

border-radius clipping of composited layers doesn't work
https://bugs.webkit.org/show_bug.cgi?id=68196
<rdar://10133719>

Reviewed by Alan Bujtas.

In CSS it's possible for clipping (via overflow:scroll, overflow:hidden and the `clip`
property) to apply to elements that are descendants in containing-block order, but
siblings in z-order, for example:
    <div style="position: relative; overflow: hidden">
      <div style="position: absolute"></div
    </div>
If a non-z-order descendant here has a composited layer, we'd fail to take border-radius
into account when clipping it.

In this layer configuration we fall into the "AncestorClippingStack" code path: the
descendant layer owns a stack of clips that represent clipping by non-paint-order
ancestors. Previously, all overflow:hidden clips were simply intersected and represented
as a single rectangular clipping GraphicsLayer, but when border-radius is present on
some or all of them, we need to maintain a layer for each, since you can't trivially
generate a path that is the intersection of rounded rects. Code was added to
RenderLayerCompositor::computeAncestorClippingStack() to do this.

When an intermediate clip with border radius represents a scrollable elements (i.e.
scrollable overflow), its corresponding entry in the ancestor clipping stack needs two
layers; one to apply the clipping, which doesn't scroll with the content, and one that
applies the scroll offset, so ClippingStackEntry gains a scrollingLayer, which is the
one handed off to the scrolling tree, and the rounded clip is applied to the clipping
layer. Logic is added to build the hierarchy of these layers.

When style changes we have to ensure that layers are updated with the new rounded rects;
notably, style can change on a non-composited RenderLayer (e.g. an overflow:hidden layer
with border-radius) which requires that non-z-order descendants need to update their
composited layer geometry; new code in RenderLayerCompositor::layerStyleChanged() takes
care of this. Tested by
compositing/clipping/border-radius-with-composited-descendant-dynamic.html

* LayoutTests/compositing/clipping/border-radius-async-overflow-non-stacking.html:
* LayoutTests/compositing/clipping/border-radius-with-composited-descendant-dynamic-expected.html: Added.
* LayoutTests/compositing/clipping/border-radius-with-composited-descendant-dynamic.html: Added.
* LayoutTests/compositing/clipping/border-radius-with-composited-descendant-expected.html: Added.
* LayoutTests/compositing/clipping/border-radius-with-composited-descendant-nested-expected.html: Added.
* LayoutTests/compositing/clipping/border-radius-with-composited-descendant-nested.html: Added.
* LayoutTests/compositing/clipping/border-radius-with-composited-descendant.html: Added.
* LayoutTests/compositing/layer-creation/clipping-scope/nested-scroller-overlap-expected.txt:
* LayoutTests/compositing/layer-creation/clipping-scope/overlap-constrained-inside-scroller-expected.txt:
* LayoutTests/compositing/layer-creation/clipping-scope/scroller-with-negative-z-children-expected.txt:
* LayoutTests/compositing/overflow/scrolling-content-clip-to-viewport-expected.txt:
* LayoutTests/compositing/rtl/rtl-scrolling-with-transformed-descendants-expected.txt:
* LayoutTests/compositing/scrolling/async-overflow-scrolling/clipped-layer-in-overflow-clipped-by-scroll-expected.txt:
* LayoutTests/compositing/scrolling/async-overflow-scrolling/clipped-layer-in-overflow-expected.txt:
* LayoutTests/compositing/scrolling/async-overflow-scrolling/clipped-layer-in-overflow-nested-expected.txt:
* LayoutTests/compositing/scrolling/async-overflow-scrolling/layer-for-negative-z-in-scroller-expected.txt:
* LayoutTests/compositing/scrolling/async-overflow-scrolling/layer-in-overflow-clip-to-hidden-expected.txt:
* LayoutTests/compositing/scrolling/async-overflow-scrolling/layer-in-overflow-clip-to-visible-expected.txt:
* LayoutTests/compositing/scrolling/async-overflow-scrolling/layer-in-overflow-expected.txt:
* LayoutTests/compositing/scrolling/async-overflow-scrolling/layer-in-overflow-gain-clipping-layer-expected.txt:
* LayoutTests/compositing/scrolling/async-overflow-scrolling/layer-in-overflow-in-clipped-expected.txt:
* LayoutTests/compositing/scrolling/async-overflow-scrolling/layer-in-overflow-lose-clipping-layer-expected.txt:
* LayoutTests/compositing/scrolling/async-overflow-scrolling/nested-scrollers-backing-attachment-expected.txt:
* LayoutTests/compositing/scrolling/async-overflow-scrolling/overlapped-overlay-scrollbar-expected.txt:
* LayoutTests/compositing/scrolling/async-overflow-scrolling/overlapped-overlay-scrollbar-inside-hidden-expected.txt:
* LayoutTests/compositing/scrolling/async-overflow-scrolling/overlapped-overlay-scrollbar-nested-expected.txt:
* LayoutTests/compositing/shared-backing/overflow-scroll/composited-absolute-in-absolute-in-relative-in-scroller-expected.txt:
* LayoutTests/compositing/shared-backing/overflow-scroll/previous-sibling-prevents-inclusiveness-expected.txt:
* LayoutTests/platform/ios-wk2/compositing/layer-creation/clipping-scope/nested-scroller-overlap-expected.txt:
* LayoutTests/platform/ios-wk2/compositing/layer-creation/clipping-scope/overlap-constrained-inside-scroller-expected.txt:
* LayoutTests/platform/ios-wk2/compositing/layer-creation/clipping-scope/scroller-with-negative-z-children-expected.txt:
* LayoutTests/platform/ios-wk2/compositing/scrolling/async-overflow-scrolling/clipped-layer-in-overflow-clipped-by-scroll-expected.txt:
* LayoutTests/platform/ios-wk2/compositing/scrolling/async-overflow-scrolling/clipped-layer-in-overflow-expected.txt:
* LayoutTests/platform/ios-wk2/compositing/scrolling/async-overflow-scrolling/clipped-layer-in-overflow-nested-expected.txt:
* LayoutTests/platform/ios-wk2/compositing/scrolling/async-overflow-scrolling/layer-for-negative-z-in-scroller-expected.txt:
* LayoutTests/platform/ios-wk2/compositing/scrolling/async-overflow-scrolling/layer-in-overflow-clip-to-hidden-expected.txt:
* LayoutTests/platform/ios-wk2/compositing/scrolling/async-overflow-scrolling/layer-in-overflow-clip-to-visible-expected.txt:
* LayoutTests/platform/ios-wk2/compositing/scrolling/async-overflow-scrolling/layer-in-overflow-expected.txt:
* LayoutTests/platform/ios-wk2/compositing/scrolling/async-overflow-scrolling/layer-in-overflow-gain-clipping-layer-expected.txt:
* LayoutTests/platform/ios-wk2/compositing/scrolling/async-overflow-scrolling/layer-in-overflow-in-clipped-expected.txt:
* LayoutTests/platform/ios-wk2/compositing/scrolling/async-overflow-scrolling/layer-in-overflow-lose-clipping-layer-expected.txt:
* LayoutTests/platform/ios-wk2/compositing/scrolling/async-overflow-scrolling/nested-scrollers-backing-attachment-expected.txt:
* LayoutTests/platform/ios-wk2/compositing/shared-backing/overflow-scroll/composited-absolute-in-absolute-in-relative-in-scroller-expected.txt:
* LayoutTests/platform/ios-wk2/compositing/shared-backing/overflow-scroll/previous-sibling-prevents-inclusiveness-expected.txt:
* Source/WebCore/platform/graphics/RoundedRect.cpp:
(WebCore::operator<<):
* Source/WebCore/platform/graphics/RoundedRect.h:
* Source/WebCore/rendering/LayerAncestorClippingStack.cpp:
(WebCore::LayerAncestorClippingStack::LayerAncestorClippingStack):
(WebCore::LayerAncestorClippingStack::firstLayer const):
(WebCore::LayerAncestorClippingStack::lastLayer const):
(WebCore::LayerAncestorClippingStack::updateScrollingNodeLayers):
(WebCore::LayerAncestorClippingStack::updateWithClipData):
(WebCore::operator<<):
(WebCore::LayerAncestorClippingStack::firstClippingLayer const): Deleted.
(WebCore::LayerAncestorClippingStack::lastClippingLayer const): Deleted.
* Source/WebCore/rendering/LayerAncestorClippingStack.h:
(WebCore::CompositedClipData::CompositedClipData):
(WebCore::LayerAncestorClippingStack::ClippingStackEntry::parentForSublayers const):
(WebCore::LayerAncestorClippingStack::ClippingStackEntry::childForSuperlayers const):
* Source/WebCore/rendering/RenderLayer.h:
* Source/WebCore/rendering/RenderLayerBacking.cpp:
(WebCore::RenderLayerBacking::updateInternalHierarchy):
(WebCore::RenderLayerBacking::ensureClippingStackLayers):
(WebCore::RenderLayerBacking::removeClippingStackLayers):
(WebCore::RenderLayerBacking::connectClippingStackLayers):
(WebCore::RenderLayerBacking::updateClippingStackLayerGeometry):
(WebCore::RenderLayerBacking::childForSuperlayers const):
* Source/WebCore/rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::adjustOverflowScrollbarContainerLayers):
(WebCore::RenderLayerCompositor::layerStyleChanged):
(WebCore::RenderLayerCompositor::computeAncestorClippingStack const):
(WebCore::RenderLayerCompositor::parentRelativeScrollableRect const):
(WebCore::RenderLayerCompositor::updateScrollingNodeForScrollingProxyRole):
* Source/WebCore/rendering/RenderLayerCompositor.h:

Canonical link: https://commits.webkit.org/254253@main

ee7713b

Misc iOS, tvOS & watchOS macOS Linux Windows
โœ… ๐Ÿงช style โœ… ๐Ÿ›  ios โœ… ๐Ÿ›  mac โœ… ๐Ÿ›  wpe โœ… ๐Ÿ›  ๐Ÿงช win
โœ… ๐Ÿ›  ios-sim โœ… ๐Ÿ›  mac-debug โœ… ๐Ÿ›  gtk โœ… ๐Ÿ›  wincairo
โœ… ๐Ÿงช webkitperl โœ… ๐Ÿงช ios-wk2 โœ… ๐Ÿ›  mac-AS-debug โœ… ๐Ÿงช gtk-wk2
โœ… ๐Ÿงช api-ios โ€ƒ ๐Ÿงช api-mac โœ… ๐Ÿงช api-gtk
โœ… ๐Ÿ›  tv โ€ƒ ๐Ÿงช mac-wk1
โ€ƒ ๐Ÿ›  tv-sim โœ… ๐Ÿงช mac-wk2
โœ… ๐Ÿ›  ๐Ÿงช merge โœ… ๐Ÿ›  watch โœ… ๐Ÿงช mac-AS-debug-wk2
โœ… ๐Ÿ›  watch-sim โœ… ๐Ÿงช mac-wk2-stress

@smfr smfr self-assigned this Aug 31, 2022
@smfr smfr added 528+ (Nightly build) Layout and Rendering For bugs with layout and rendering of Web pages. labels Aug 31, 2022
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Sep 1, 2022
@smfr smfr removed the merging-blocked Applied to prevent a change from being merged label Sep 7, 2022
@smfr smfr force-pushed the eng/border-radius-clipping-of-composited-layers-doesnt-work branch from 2aa3a45 to ec76312 Compare September 7, 2022 01:05
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Sep 7, 2022
@smfr smfr removed the merging-blocked Applied to prevent a change from being merged label Sep 7, 2022
@smfr smfr force-pushed the eng/border-radius-clipping-of-composited-layers-doesnt-work branch from ec76312 to ee7713b Compare September 7, 2022 17:36
@smfr smfr added the merge-queue Applied to send a pull request to merge-queue label Sep 7, 2022
https://bugs.webkit.org/show_bug.cgi?id=68196
<rdar://10133719>

Reviewed by Alan Bujtas.

In CSS it's possible for clipping (via overflow:scroll, overflow:hidden and the `clip`
property) to apply to elements that are descendants in containing-block order, but
siblings in z-order, for example:
    <div style="position: relative; overflow: hidden">
      <div style="position: absolute"></div
    </div>
If a non-z-order descendant here has a composited layer, we'd fail to take border-radius
into account when clipping it.

In this layer configuration we fall into the "AncestorClippingStack" code path: the
descendant layer owns a stack of clips that represent clipping by non-paint-order
ancestors. Previously, all overflow:hidden clips were simply intersected and represented
as a single rectangular clipping GraphicsLayer, but when border-radius is present on
some or all of them, we need to maintain a layer for each, since you can't trivially
generate a path that is the intersection of rounded rects. Code was added to
RenderLayerCompositor::computeAncestorClippingStack() to do this.

When an intermediate clip with border radius represents a scrollable elements (i.e.
scrollable overflow), its corresponding entry in the ancestor clipping stack needs two
layers; one to apply the clipping, which doesn't scroll with the content, and one that
applies the scroll offset, so ClippingStackEntry gains a scrollingLayer, which is the
one handed off to the scrolling tree, and the rounded clip is applied to the clipping
layer. Logic is added to build the hierarchy of these layers.

When style changes we have to ensure that layers are updated with the new rounded rects;
notably, style can change on a non-composited RenderLayer (e.g. an overflow:hidden layer
with border-radius) which requires that non-z-order descendants need to update their
composited layer geometry; new code in RenderLayerCompositor::layerStyleChanged() takes
care of this. Tested by
compositing/clipping/border-radius-with-composited-descendant-dynamic.html

* LayoutTests/compositing/clipping/border-radius-async-overflow-non-stacking.html:
* LayoutTests/compositing/clipping/border-radius-with-composited-descendant-dynamic-expected.html: Added.
* LayoutTests/compositing/clipping/border-radius-with-composited-descendant-dynamic.html: Added.
* LayoutTests/compositing/clipping/border-radius-with-composited-descendant-expected.html: Added.
* LayoutTests/compositing/clipping/border-radius-with-composited-descendant-nested-expected.html: Added.
* LayoutTests/compositing/clipping/border-radius-with-composited-descendant-nested.html: Added.
* LayoutTests/compositing/clipping/border-radius-with-composited-descendant.html: Added.
* LayoutTests/compositing/layer-creation/clipping-scope/nested-scroller-overlap-expected.txt:
* LayoutTests/compositing/layer-creation/clipping-scope/overlap-constrained-inside-scroller-expected.txt:
* LayoutTests/compositing/layer-creation/clipping-scope/scroller-with-negative-z-children-expected.txt:
* LayoutTests/compositing/overflow/scrolling-content-clip-to-viewport-expected.txt:
* LayoutTests/compositing/rtl/rtl-scrolling-with-transformed-descendants-expected.txt:
* LayoutTests/compositing/scrolling/async-overflow-scrolling/clipped-layer-in-overflow-clipped-by-scroll-expected.txt:
* LayoutTests/compositing/scrolling/async-overflow-scrolling/clipped-layer-in-overflow-expected.txt:
* LayoutTests/compositing/scrolling/async-overflow-scrolling/clipped-layer-in-overflow-nested-expected.txt:
* LayoutTests/compositing/scrolling/async-overflow-scrolling/layer-for-negative-z-in-scroller-expected.txt:
* LayoutTests/compositing/scrolling/async-overflow-scrolling/layer-in-overflow-clip-to-hidden-expected.txt:
* LayoutTests/compositing/scrolling/async-overflow-scrolling/layer-in-overflow-clip-to-visible-expected.txt:
* LayoutTests/compositing/scrolling/async-overflow-scrolling/layer-in-overflow-expected.txt:
* LayoutTests/compositing/scrolling/async-overflow-scrolling/layer-in-overflow-gain-clipping-layer-expected.txt:
* LayoutTests/compositing/scrolling/async-overflow-scrolling/layer-in-overflow-in-clipped-expected.txt:
* LayoutTests/compositing/scrolling/async-overflow-scrolling/layer-in-overflow-lose-clipping-layer-expected.txt:
* LayoutTests/compositing/scrolling/async-overflow-scrolling/nested-scrollers-backing-attachment-expected.txt:
* LayoutTests/compositing/scrolling/async-overflow-scrolling/overlapped-overlay-scrollbar-expected.txt:
* LayoutTests/compositing/scrolling/async-overflow-scrolling/overlapped-overlay-scrollbar-inside-hidden-expected.txt:
* LayoutTests/compositing/scrolling/async-overflow-scrolling/overlapped-overlay-scrollbar-nested-expected.txt:
* LayoutTests/compositing/shared-backing/overflow-scroll/composited-absolute-in-absolute-in-relative-in-scroller-expected.txt:
* LayoutTests/compositing/shared-backing/overflow-scroll/previous-sibling-prevents-inclusiveness-expected.txt:
* LayoutTests/platform/ios-wk2/compositing/layer-creation/clipping-scope/nested-scroller-overlap-expected.txt:
* LayoutTests/platform/ios-wk2/compositing/layer-creation/clipping-scope/overlap-constrained-inside-scroller-expected.txt:
* LayoutTests/platform/ios-wk2/compositing/layer-creation/clipping-scope/scroller-with-negative-z-children-expected.txt:
* LayoutTests/platform/ios-wk2/compositing/scrolling/async-overflow-scrolling/clipped-layer-in-overflow-clipped-by-scroll-expected.txt:
* LayoutTests/platform/ios-wk2/compositing/scrolling/async-overflow-scrolling/clipped-layer-in-overflow-expected.txt:
* LayoutTests/platform/ios-wk2/compositing/scrolling/async-overflow-scrolling/clipped-layer-in-overflow-nested-expected.txt:
* LayoutTests/platform/ios-wk2/compositing/scrolling/async-overflow-scrolling/layer-for-negative-z-in-scroller-expected.txt:
* LayoutTests/platform/ios-wk2/compositing/scrolling/async-overflow-scrolling/layer-in-overflow-clip-to-hidden-expected.txt:
* LayoutTests/platform/ios-wk2/compositing/scrolling/async-overflow-scrolling/layer-in-overflow-clip-to-visible-expected.txt:
* LayoutTests/platform/ios-wk2/compositing/scrolling/async-overflow-scrolling/layer-in-overflow-expected.txt:
* LayoutTests/platform/ios-wk2/compositing/scrolling/async-overflow-scrolling/layer-in-overflow-gain-clipping-layer-expected.txt:
* LayoutTests/platform/ios-wk2/compositing/scrolling/async-overflow-scrolling/layer-in-overflow-in-clipped-expected.txt:
* LayoutTests/platform/ios-wk2/compositing/scrolling/async-overflow-scrolling/layer-in-overflow-lose-clipping-layer-expected.txt:
* LayoutTests/platform/ios-wk2/compositing/scrolling/async-overflow-scrolling/nested-scrollers-backing-attachment-expected.txt:
* LayoutTests/platform/ios-wk2/compositing/shared-backing/overflow-scroll/composited-absolute-in-absolute-in-relative-in-scroller-expected.txt:
* LayoutTests/platform/ios-wk2/compositing/shared-backing/overflow-scroll/previous-sibling-prevents-inclusiveness-expected.txt:
* Source/WebCore/platform/graphics/RoundedRect.cpp:
(WebCore::operator<<):
* Source/WebCore/platform/graphics/RoundedRect.h:
* Source/WebCore/rendering/LayerAncestorClippingStack.cpp:
(WebCore::LayerAncestorClippingStack::LayerAncestorClippingStack):
(WebCore::LayerAncestorClippingStack::firstLayer const):
(WebCore::LayerAncestorClippingStack::lastLayer const):
(WebCore::LayerAncestorClippingStack::updateScrollingNodeLayers):
(WebCore::LayerAncestorClippingStack::updateWithClipData):
(WebCore::operator<<):
(WebCore::LayerAncestorClippingStack::firstClippingLayer const): Deleted.
(WebCore::LayerAncestorClippingStack::lastClippingLayer const): Deleted.
* Source/WebCore/rendering/LayerAncestorClippingStack.h:
(WebCore::CompositedClipData::CompositedClipData):
(WebCore::LayerAncestorClippingStack::ClippingStackEntry::parentForSublayers const):
(WebCore::LayerAncestorClippingStack::ClippingStackEntry::childForSuperlayers const):
* Source/WebCore/rendering/RenderLayer.h:
* Source/WebCore/rendering/RenderLayerBacking.cpp:
(WebCore::RenderLayerBacking::updateInternalHierarchy):
(WebCore::RenderLayerBacking::ensureClippingStackLayers):
(WebCore::RenderLayerBacking::removeClippingStackLayers):
(WebCore::RenderLayerBacking::connectClippingStackLayers):
(WebCore::RenderLayerBacking::updateClippingStackLayerGeometry):
(WebCore::RenderLayerBacking::childForSuperlayers const):
* Source/WebCore/rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::adjustOverflowScrollbarContainerLayers):
(WebCore::RenderLayerCompositor::layerStyleChanged):
(WebCore::RenderLayerCompositor::computeAncestorClippingStack const):
(WebCore::RenderLayerCompositor::parentRelativeScrollableRect const):
(WebCore::RenderLayerCompositor::updateScrollingNodeForScrollingProxyRole):
* Source/WebCore/rendering/RenderLayerCompositor.h:

Canonical link: https://commits.webkit.org/254253@main
@webkit-early-warning-system webkit-early-warning-system force-pushed the eng/border-radius-clipping-of-composited-layers-doesnt-work branch from ee7713b to 9791054 Compare September 7, 2022 23:10
@webkit-early-warning-system webkit-early-warning-system merged commit 9791054 into WebKit:main Sep 7, 2022
@webkit-commit-queue
Copy link
Collaborator

Committed 254253@main (9791054): https://commits.webkit.org/254253@main

Reviewed commits have been landed. Closing PR #3883 and removing active labels.

@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Sep 7, 2022
@smfr smfr deleted the eng/border-radius-clipping-of-composited-layers-doesnt-work branch November 15, 2022 05:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Layout and Rendering For bugs with layout and rendering of Web pages.
Projects
None yet
5 participants