Incorrect clippping with overflow:scroll inside oveflow:hidden with border-radius
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 26 Jun 2019 19:12:40 +0000 (19:12 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 26 Jun 2019 19:12:40 +0000 (19:12 +0000)
https://bugs.webkit.org/show_bug.cgi?id=199135
rdar://problem/51882383

Reviewed by Zalan Bujtas.
Source/WebCore:

In some cases the geometry of the shape mask layer added to m_childContainmentLayer, for
border-radius, was incorrect. GraphicsLayerCA::updateClippingStrategy() treated
the rounded rect as if it were in renderer coordinates, but to match the other geometry
on GraphicsLayer, it should in GraphicsLayer coordinates, so we need to offset by
clipLayer->offsetFromRenderer().

In addition, RenderLayerBacking::updateChildClippingStrategy() is called from
the updateConfiguration(), which is before we've set offsetFromRenderer() on the clipLayer.
This first call is really to find out whether the platform supports this rounded rect
as a shape mask.

So we need to call setMasksToBoundsRect() a second time in RenderLayerBacking::updateGeometry()
after clipLayers's offsetFromRenderer() has been computed.

Test: compositing/scrolling/async-overflow-scrolling/border-radius-on-scroll-container.html

* platform/graphics/ca/GraphicsLayerCA.cpp:
(WebCore::GraphicsLayerCA::updateClippingStrategy):
* rendering/RenderLayerBacking.cpp:
(WebCore::RenderLayerBacking::createPrimaryGraphicsLayer):
(WebCore::RenderLayerBacking::updateDescendantClippingLayer):
(WebCore::RenderLayerBacking::updateChildClippingStrategy):

LayoutTests:

* compositing/scrolling/async-overflow-scrolling/border-radius-on-scroll-container-expected.html: Added.
* compositing/scrolling/async-overflow-scrolling/border-radius-on-scroll-container.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@246845 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/compositing/scrolling/async-overflow-scrolling/border-radius-on-scroll-container-expected.html [new file with mode: 0644]
LayoutTests/compositing/scrolling/async-overflow-scrolling/border-radius-on-scroll-container.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp
Source/WebCore/rendering/RenderLayerBacking.cpp

index f90083b..c0941a9 100644 (file)
@@ -1,3 +1,14 @@
+2019-06-26  Simon Fraser  <simon.fraser@apple.com>
+
+        Incorrect clippping with overflow:scroll inside oveflow:hidden with border-radius
+        https://bugs.webkit.org/show_bug.cgi?id=199135
+        rdar://problem/51882383
+
+        Reviewed by Zalan Bujtas.
+
+        * compositing/scrolling/async-overflow-scrolling/border-radius-on-scroll-container-expected.html: Added.
+        * compositing/scrolling/async-overflow-scrolling/border-radius-on-scroll-container.html: Added.
+
 2019-06-26  Antoine Quint  <graouts@apple.com>
 
         [Pointer Events] Respect pointer capture when dispatching mouse boundary events and updating :hover
diff --git a/LayoutTests/compositing/scrolling/async-overflow-scrolling/border-radius-on-scroll-container-expected.html b/LayoutTests/compositing/scrolling/async-overflow-scrolling/border-radius-on-scroll-container-expected.html
new file mode 100644 (file)
index 0000000..760bb29
--- /dev/null
@@ -0,0 +1,63 @@
+<style>
+body {
+    font-family: monospace;
+}
+
+.absolute {
+       position: absolute;
+    margin: 50px;
+       overflow: hidden;
+       border: 3px solid blue;
+       border-radius: 10px;
+       box-shadow: 0px 0px 26px transparent;
+}
+
+.scroll {
+    overflow: scroll;
+       height: 300px;
+}
+
+.contents {
+    background-color: green;
+    margin: 0;
+    height: 400px;
+    width: 227px;
+}
+
+.hider {
+    position: absolute;
+    background-color: gray;
+}
+
+.edge.hider {
+    top: 20px;
+    left: 260px;
+    width: 80px;
+    height: 400px;
+}
+
+.topleft.corner.hider {
+    top: 40px;
+    left: 44px;
+    width: 40px;
+    height: 40px;
+}
+
+.bottomleft.corner.hider {
+    top: 340px;
+    left: 45px;
+    width: 40px;
+    height: 40px;
+}
+</style>
+<body>
+    <div class=absolute>
+        <div class=scroll>
+            <div class="contents">
+            </div>
+        </div>
+    </div>
+    <div class="edge hider"></div>
+    <div class="topleft corner hider"></div>
+    <div class="bottomleft corner hider"></div>
+</body>
diff --git a/LayoutTests/compositing/scrolling/async-overflow-scrolling/border-radius-on-scroll-container.html b/LayoutTests/compositing/scrolling/async-overflow-scrolling/border-radius-on-scroll-container.html
new file mode 100644 (file)
index 0000000..275415d
--- /dev/null
@@ -0,0 +1,68 @@
+<style>
+body {
+    font-family: monospace;
+}
+
+.absolute {
+       position: absolute;
+    margin: 50px;
+       overflow: hidden;
+       border: 3px solid blue;
+       border-radius: 10px;
+       box-shadow: 0px 0px 26px transparent;
+}
+
+.scroll {
+    overflow: scroll;
+       height: 300px;
+}
+
+.contents {
+    background-color: green;
+    margin: 0;
+    height: 400px;
+}
+
+p {
+    margin: 0;
+    color: green;
+}
+
+.hider {
+    position: absolute;
+    background-color: gray;
+}
+
+.edge.hider {
+    top: 20px;
+    left: 260px;
+    width: 80px;
+    height: 400px;
+}
+
+.topleft.corner.hider {
+    top: 40px;
+    left: 44px;
+    width: 40px;
+    height: 40px;
+}
+
+.bottomleft.corner.hider {
+    top: 340px;
+    left: 45px;
+    width: 40px;
+    height: 40px;
+}
+</style>
+<body>
+    <div class=absolute>
+        <div class=scroll>
+            <div class="contents">
+                <p>some text here some text here</p>
+            </div>
+        </div>
+    </div>
+    <div class="edge hider"></div>
+    <div class="topleft corner hider"></div>
+    <div class="bottomleft corner hider"></div>    
+</body>
index 6b0d732..9aa5e2d 100644 (file)
@@ -1,3 +1,34 @@
+2019-06-26  Simon Fraser  <simon.fraser@apple.com>
+
+        Incorrect clippping with overflow:scroll inside oveflow:hidden with border-radius
+        https://bugs.webkit.org/show_bug.cgi?id=199135
+        rdar://problem/51882383
+
+        Reviewed by Zalan Bujtas.
+        
+        In some cases the geometry of the shape mask layer added to m_childContainmentLayer, for
+        border-radius, was incorrect. GraphicsLayerCA::updateClippingStrategy() treated
+        the rounded rect as if it were in renderer coordinates, but to match the other geometry
+        on GraphicsLayer, it should in GraphicsLayer coordinates, so we need to offset by
+        clipLayer->offsetFromRenderer().
+        
+        In addition, RenderLayerBacking::updateChildClippingStrategy() is called from
+        the updateConfiguration(), which is before we've set offsetFromRenderer() on the clipLayer.
+        This first call is really to find out whether the platform supports this rounded rect
+        as a shape mask.
+        
+        So we need to call setMasksToBoundsRect() a second time in RenderLayerBacking::updateGeometry()
+        after clipLayers's offsetFromRenderer() has been computed.
+
+        Test: compositing/scrolling/async-overflow-scrolling/border-radius-on-scroll-container.html
+
+        * platform/graphics/ca/GraphicsLayerCA.cpp:
+        (WebCore::GraphicsLayerCA::updateClippingStrategy):
+        * rendering/RenderLayerBacking.cpp:
+        (WebCore::RenderLayerBacking::createPrimaryGraphicsLayer):
+        (WebCore::RenderLayerBacking::updateDescendantClippingLayer):
+        (WebCore::RenderLayerBacking::updateChildClippingStrategy):
+
 2019-06-26  Antoine Quint  <graouts@apple.com>
 
         [Pointer Events] Respect pointer capture when dispatching mouse boundary events and updating :hover
index 08dd591..5d8658a 100644 (file)
@@ -2602,7 +2602,7 @@ void GraphicsLayerCA::updateClippingStrategy(PlatformCALayer& clippingLayer, Ref
         shapeMaskLayer->setName("shape mask");
     }
     
-    shapeMaskLayer->setPosition(roundedRect.rect().location() - offsetFromRenderer());
+    shapeMaskLayer->setPosition(roundedRect.rect().location());
     FloatRect shapeBounds({ }, roundedRect.rect().size());
     shapeMaskLayer->setBounds(shapeBounds);
     FloatRoundedRect offsetRoundedRect(shapeBounds, roundedRect.radii());
index b78a4c6..7ecac43 100644 (file)
@@ -1172,6 +1172,13 @@ void RenderLayerBacking::updateGeometry()
         clipLayer->setSize(snappedClippingGraphicsLayer.m_snappedRect.size());
         clipLayer->setOffsetFromRenderer(toLayoutSize(clippingBox.location() - snappedClippingGraphicsLayer.m_snapDelta));
 
+        if ((renderer().style().clipPath() || renderer().style().hasBorderRadius()) && !m_childClippingMaskLayer) {
+            LayoutRect boxRect(LayoutPoint(), downcast<RenderBox>(renderer()).size());
+            FloatRoundedRect contentsClippingRect = renderer().style().getRoundedInnerBorderFor(boxRect).pixelSnappedRoundedRectForPainting(deviceScaleFactor());
+            contentsClippingRect.move(LayoutSize(-clipLayer->offsetFromRenderer()));
+            clipLayer->setMasksToBoundsRect(contentsClippingRect);
+        }
+
         if (m_childClippingMaskLayer && !m_scrollContainerLayer) {
             m_childClippingMaskLayer->setSize(clipLayer->size());
             m_childClippingMaskLayer->setPosition(FloatPoint());
@@ -1868,11 +1875,13 @@ void RenderLayerBacking::updateChildClippingStrategy(bool needsDescendantsClippi
 {
     if (hasClippingLayer() && needsDescendantsClippingLayer) {
         if (is<RenderBox>(renderer()) && (renderer().style().clipPath() || renderer().style().hasBorderRadius())) {
+            auto* clipLayer = clippingLayer();
             LayoutRect boxRect(LayoutPoint(), downcast<RenderBox>(renderer()).size());
-            boxRect.move(contentOffsetInCompositingLayer());
             FloatRoundedRect contentsClippingRect = renderer().style().getRoundedInnerBorderFor(boxRect).pixelSnappedRoundedRectForPainting(deviceScaleFactor());
-            if (clippingLayer()->setMasksToBoundsRect(contentsClippingRect)) {
-                clippingLayer()->setMaskLayer(nullptr);
+            contentsClippingRect.move(LayoutSize(clipLayer->offsetFromRenderer()));
+            // Note that we have to set this rounded rect again during the geometry update (clipLayer->offsetFromRenderer() may be stale here).
+            if (clipLayer->setMasksToBoundsRect(contentsClippingRect)) {
+                clipLayer->setMaskLayer(nullptr);
                 GraphicsLayer::clear(m_childClippingMaskLayer);
                 return;
             }
@@ -1891,7 +1900,7 @@ void RenderLayerBacking::updateChildClippingStrategy(bool needsDescendantsClippi
             GraphicsLayer::clear(m_childClippingMaskLayer);
         } else 
             if (hasClippingLayer())
-                clippingLayer()->setMasksToBoundsRect(FloatRoundedRect(FloatRect(FloatPoint(), clippingLayer()->size())));
+                clippingLayer()->setMasksToBoundsRect(FloatRoundedRect(FloatRect({ }, clippingLayer()->size())));
     }
 }