Attachment 372904 Details for Bug 199135 – Patch

[patch] Patch

bug-199135-20190625213331.patch (text/plain), 9.69 KB, created by Simon Fraser (smfr) on 2019-06-25 21:33:32 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Simon Fraser (smfr)

Created: 2019-06-25 21:33:32 PDT

Size: 9.69 KB

patch

obsolete

>Subversion Revision: 246828
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 958f299fa742c7e359d0ccb7eb0433abe682084a..6edd48303911decb8ad73954fe5cd7859099e303 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,34 @@
>+2019-06-23  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 NOBODY (OOPS!).
>+        
>+        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-25  Timothy Hatcher  <timothy@apple.com>
> 
>         Clicking the up/down spin buttons for a number input increments/decrements by 2.
>diff --git a/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp b/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp
>index 08dd5910eac4548b6a95e27d07ea1a0665ce1bc6..5d8658a65cd9b19b376cf593243c14e4922cd005 100644
>--- a/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp
>+++ b/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp
>@@ -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());
>diff --git a/Source/WebCore/rendering/RenderLayerBacking.cpp b/Source/WebCore/rendering/RenderLayerBacking.cpp
>index b78a4c61c235c6d433c6167f5cf5c8ab6891568b..7ecac435e090a32d3e6415d4e06d129b79e35523 100644
>--- a/Source/WebCore/rendering/RenderLayerBacking.cpp
>+++ b/Source/WebCore/rendering/RenderLayerBacking.cpp
>@@ -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())));
>     }
> }
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 4885271c3bf0fcfb76df23e0c4d8a3c468faf150..6b05028a0a06e5845c157182b3be2882e5be8d68 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2019-06-23  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 NOBODY (OOPS!).
>+
>+        * 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-25  Justin Fan  <justin_fan@apple.com>
> 
>         [WHLSL] Make whlsl-test-harness actually generate WHLSL shaders by default
>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
>index 0000000000000000000000000000000000000000..760bb29bbd9ca3207deeab33409471bc72d66767
>--- /dev/null
>+++ b/LayoutTests/compositing/scrolling/async-overflow-scrolling/border-radius-on-scroll-container-expected.html
>@@ -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
>index 0000000000000000000000000000000000000000..275415df36cb706e8bcf510a35205fcbc169c2f5
>--- /dev/null
>+++ b/LayoutTests/compositing/scrolling/async-overflow-scrolling/border-radius-on-scroll-container.html
>@@ -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>

Actions: View | Formatted Diff | Diff

Attachments on bug 199135: 372904