SVG clip-path is sometimes broken on stevejobsarchive.com

https://bugs.webkit.org/show_bug.cgi?id=255577 rdar://107885344 Reviewed by Said Abou-Hallawa. http://book.stevejobsarchive.com/ uses CSS clip-path with a reference to an SVG <clipPath> element which contains text. In this configuration, RenderSVGResourceClipper::applyClippingToContext() falls back to a code path that uses an ImageBuffer as a mask, and it caches the ImageBuffer between calls. This caused a problem when DOM Rendering in the GPU Process was enabled; this code is first hit for a "fake" paint with a NullGraphicsContext which is updating EventRegions, called out of `RenderLayerBacking::updateEventRegion()`. The NullGraphicsContext will make a local ImageBuffer. If we then hit this same code for actual painting with a painting GraphicsContext, we'll use that cached ImageBuffer, rather than creating a new one with appropriate GPU Process backing. Fix this by adding `isPaintingDisabled` to the criteria used to decide if the cached buffer can be re-used. * LayoutTests/svg/masking/masking-with-event-region-expected.html: Added. * LayoutTests/svg/masking/masking-with-event-region.html: Added. * Source/WebCore/rendering/svg/RenderSVGResourceClipper.cpp: (WebCore::RenderSVGResourceClipper::computeInputs): (WebCore::RenderSVGResourceClipper::applyClippingToContext): * Source/WebCore/rendering/svg/RenderSVGResourceClipper.h: (WebCore::ClipperData::Inputs::operator== const): Canonical link: https://commits.webkit.org/263087@main
WebKit · Apr 18, 2023 · e4d2a74 · e4d2a74
1 parent 25a06d7
commit e4d2a74
Show file tree

Hide file tree

Showing 4 changed files with 99 additions and 9 deletions.
diff --git a/LayoutTests/svg/masking/masking-with-event-region-expected.html b/LayoutTests/svg/masking/masking-with-event-region-expected.html
@@ -0,0 +1,35 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ AsyncOverflowScrollingEnabled=true ] -->
+<html>
+  <head>
+    <style>
+        #test-clip-text {
+            font-family: system-ui;
+            font-weight: 900;
+            font-size: 60px;
+        }
+        .clipped {
+            position: absolute;
+            width: 200px;
+            height: 100px;
+            z-index: 1;
+            top: 5px;
+            left: 50px;
+            clip-path: url("#test-clip-text");
+            background-color: blue;
+        }
+        .composited {
+            transform: translateZ(0);
+        }
+        </style>
+  </head>
+  <body>
+    <svg xmlns="http://www.w3.org/2000/svg" width="0" height="0">
+      <defs>
+        <clipPath id="test-clip-text">
+          <text x="5" y="50">TEST</text>
+        </clipPath>
+      </defs>
+    </svg>
+    <div class="composited clipped"></div>
+  </body>
+</html>
diff --git a/LayoutTests/svg/masking/masking-with-event-region.html b/LayoutTests/svg/masking/masking-with-event-region.html
@@ -0,0 +1,58 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ AsyncOverflowScrollingEnabled=true ] -->
+<html>
+  <head>
+    <style>
+        .scroller {
+            position: absolute;
+            width: 300px;
+            height: 300px;
+            overflow: scroll;
+            background: white;
+            z-index: 3;
+            margin: 100px;
+            opacity: 0;
+        }
+
+        .contents {
+            height: 120%;
+        }
+
+        #test-clip-text {
+            font-family: system-ui;
+            font-weight: 900;
+            font-size: 60px;
+        }
+        .clipped {
+            position: absolute;
+            width: 200px;
+            height: 100px;
+            z-index: 1;
+            top: 5px;
+            left: 50px;
+            clip-path: url("#test-clip-text");
+            background-color: blue;
+        }
+        .composited {
+            transform: translateZ(0);
+        }
+
+        </style>
+  </head>
+  <body>
+    <svg xmlns="http://www.w3.org/2000/svg" width="0" height="0">
+      <defs>
+        <clipPath id="test-clip-text">
+          <text x="5" y="50">TEST</text>
+        </clipPath>
+      </defs>
+    </svg>
+    <div class="composited clipped"></div>
+
+    <!-- scroller triggers event region generation -->
+    <div class="composited scroller">
+        <div class="contents">
+            scroller
+        </div>
+    </div>
+  </body>
+</html>
diff --git a/Source/WebCore/rendering/svg/RenderSVGResourceClipper.cpp b/Source/WebCore/rendering/svg/RenderSVGResourceClipper.cpp
@@ -141,7 +141,7 @@ bool RenderSVGResourceClipper::pathOnlyClipping(GraphicsContext& context, const
     return true;
 }
 
-ClipperData::Inputs RenderSVGResourceClipper::computeInputs(RenderElement& renderer, const FloatRect& objectBoundingBox, const FloatRect& clippedContentBounds, float effectiveZoom)
+ClipperData::Inputs RenderSVGResourceClipper::computeInputs(const GraphicsContext& context, const RenderElement& renderer, const FloatRect& objectBoundingBox, const FloatRect& clippedContentBounds, float effectiveZoom)
 {
     AffineTransform absoluteTransform = SVGRenderingContext::calculateTransformationToOutermostCoordinateSystem(renderer);
 
@@ -151,7 +151,7 @@ ClipperData::Inputs RenderSVGResourceClipper::computeInputs(RenderElement& rende
     // Determine scale factor for the clipper. The size of intermediate ImageBuffers shouldn't be bigger than kMaxFilterSize.
     ImageBuffer::sizeNeedsClamping(objectBoundingBox.size(), scale);
 
-    return { objectBoundingBox, clippedContentBounds, scale, effectiveZoom };
+    return { objectBoundingBox, clippedContentBounds, scale, effectiveZoom, context.paintingDisabled() };
 }
 
 bool RenderSVGResourceClipper::applyClippingToContext(GraphicsContext& context, RenderElement& renderer, const FloatRect& objectBoundingBox, const FloatRect& clippedContentBounds, float effectiveZoom)
@@ -167,7 +167,7 @@ bool RenderSVGResourceClipper::applyClippingToContext(GraphicsContext& context,
     if (!clipperData.imageBuffer && pathOnlyClipping(context, animatedLocalTransform, objectBoundingBox, effectiveZoom))
         return true;
 
-    if (clipperData.invalidate(computeInputs(renderer, objectBoundingBox, clippedContentBounds, effectiveZoom))) {
+    if (clipperData.invalidate(computeInputs(context, renderer, objectBoundingBox, clippedContentBounds, effectiveZoom))) {
         // FIXME (149469): This image buffer should not be unconditionally unaccelerated. Making it match the context breaks nested clipping, though.
         clipperData.imageBuffer = context.createScaledImageBuffer(clippedContentBounds, clipperData.inputs.scale, DestinationColorSpace::SRGB(), RenderingMode::Unaccelerated); // FIXME
         if (!clipperData.imageBuffer)

diff --git a/Source/WebCore/rendering/svg/RenderSVGResourceClipper.h b/Source/WebCore/rendering/svg/RenderSVGResourceClipper.h
@@ -34,17 +34,14 @@ struct ClipperData {
     WTF_MAKE_STRUCT_FAST_ALLOCATED;
 
     struct Inputs {
-        bool operator==(const Inputs& other) const
-        {
-            return std::tie(objectBoundingBox, clippedContentBounds, scale, effectiveZoom) == std::tie(other.objectBoundingBox, other.clippedContentBounds, other.scale, other.effectiveZoom);
-        }
-
+        bool operator==(const Inputs& other) const = default;
         bool operator!=(const Inputs& other) const { return !(*this == other); }
 
         FloatRect objectBoundingBox;
         FloatRect clippedContentBounds;
         FloatSize scale;
         float effectiveZoom = 1;
+        bool paintingDisabled { false };
     };
 
     bool invalidate(const Inputs& inputs)
@@ -94,7 +91,7 @@ class RenderSVGResourceClipper final : public RenderSVGResourceContainer {
     ASCIILiteral renderName() const override { return "RenderSVGResourceClipper"_s; }
     bool isSVGResourceClipper() const override { return true; }
 
-    ClipperData::Inputs computeInputs(RenderElement&, const FloatRect& objectBoundingBox, const FloatRect& clippedContentBounds, float effectiveZoom);
+    ClipperData::Inputs computeInputs(const GraphicsContext&, const RenderElement&, const FloatRect& objectBoundingBox, const FloatRect& clippedContentBounds, float effectiveZoom);
     bool pathOnlyClipping(GraphicsContext&, const AffineTransform&, const FloatRect&, float effectiveZoom);
     bool drawContentIntoMaskImage(ImageBuffer&, const FloatRect& objectBoundingBox, float effectiveZoom);
     void calculateClipContentRepaintRect();