[LBSE] Fix clipping + SVGUseElement support

https://bugs.webkit.org/show_bug.cgi?id=265244 Reviewed by Rob Buis. Teach pathFromGraphicsElement() about SVGUseElement, and use that to support <use> as <clipPath> children in LBSE. The legacy SVG engine uses the deprecated virtual SVGUseElement::toClipPath() function -- LBSE no longer makes use of these code paths. Covered by existing tests - 5+ fixed tests in LBSE. * LayoutTests/platform/mac-sonoma-wk2-lbse-text/TestExpectations: * Source/WebCore/rendering/svg/SVGPathData.cpp: (WebCore::pathFromUseElement): (WebCore::pathFromGraphicsElement): * Source/WebCore/svg/SVGClipPathElement.cpp: (WebCore::SVGClipPathElement::shouldApplyPathClipping const): * Source/WebCore/svg/SVGGraphicsElement.cpp: (WebCore::SVGGraphicsElement::toClipPath): * Source/WebCore/svg/SVGUseElement.cpp: (WebCore::SVGUseElement::toClipPath): (WebCore::SVGUseElement::clipChild const): * Source/WebCore/svg/SVGUseElement.h: Canonical link: https://commits.webkit.org/271101@main
WebKit · Nov 24, 2023 · e94cf19 · e94cf19
1 parent a1a92e7
commit e94cf19
Show file tree

Hide file tree

Showing 6 changed files with 65 additions and 22 deletions.
diff --git a/LayoutTests/platform/mac-sonoma-wk2-lbse-text/TestExpectations b/LayoutTests/platform/mac-sonoma-wk2-lbse-text/TestExpectations
@@ -341,11 +341,6 @@ svg/W3C-SVG-1.1/masking-path-01-b.svg                     [ ImageOnlyFailure ]
 svg/W3C-SVG-1.1/masking-path-04-b.svg                     [ ImageOnlyFailure ]
 svg/batik/text/textEffect2.svg                            [ ImageOnlyFailure ]
 svg/batik/text/textProperties.svg                         [ ImageOnlyFailure ]
-svg/clip-path/clip-path-content-use-001.svg               [ ImageOnlyFailure ]
-svg/clip-path/clip-path-content-use-003.svg               [ ImageOnlyFailure ]
-svg/clip-path/clip-path-content-use-004.svg               [ ImageOnlyFailure ]
-svg/clip-path/clip-path-content-use-005.svg               [ ImageOnlyFailure ]
-svg/clip-path/clip-path-content-use-006.svg               [ ImageOnlyFailure ]
 svg/clip-path/clip-path-css-transform-001.svg             [ ImageOnlyFailure ]
 svg/clip-path/clip-path-css-transform-002.svg             [ ImageOnlyFailure ]
 svg/clip-path/clip-path-objectboundingbox-004.svg         [ ImageOnlyFailure ]
@@ -360,9 +355,7 @@ svg/clip-path/clip-path-on-svg-004.svg                    [ ImageOnlyFailure ]
 svg/clip-path/clip-path-on-svg-005.svg                    [ ImageOnlyFailure ]
 svg/clip-path/clip-path-precision-001.svg                 [ ImageOnlyFailure ]
 svg/clip-path/clip-path-recursion-001.svg                 [ ImageOnlyFailure ]
-svg/clip-path/clip-path-recursion-002.svg                 [ Crash ImageOnlyFailure ]
 svg/clip-path/clip-path-use-referencing-clipped-text.html [ ImageOnlyFailure ]
-svg/clip-path/clip-path-use-referencing-text.html         [ ImageOnlyFailure ]
 svg/clip-path/mask-nested-clip-path-001.svg               [ ImageOnlyFailure ]
 svg/clip-path/mask-nested-clip-path-002.svg               [ ImageOnlyFailure ]
 svg/clip-path/mask-nested-clip-path-003.svg               [ ImageOnlyFailure ]
@@ -375,7 +368,6 @@ svg/clip-path/mask-nested-clip-path-009.svg               [ ImageOnlyFailure ]
 svg/clip-path/mask-nested-clip-path-010.svg               [ ImageOnlyFailure ]
 svg/clip-path/mask-nested-clip-path-panning-001.svg       [ ImageOnlyFailure ]
 svg/clip-path/mask-nested-clip-path-panning-002.svg       [ ImageOnlyFailure ]
-svg/custom/intersection-list-clipping.svg                 [ Failure ]
 svg/custom/js-late-clipPath-and-object-creation.svg       [ Failure ]
 svg/custom/js-late-clipPath-creation.svg                  [ Failure ]
 svg/custom/use-clipped-transform.svg                      [ Failure ]

diff --git a/Source/WebCore/rendering/svg/SVGPathData.cpp b/Source/WebCore/rendering/svg/SVGPathData.cpp
@@ -38,6 +38,7 @@
 #include "SVGPolylineElement.h"
 #include "SVGRectElement.h"
 #include "SVGRenderStyle.h"
+#include "SVGUseElement.h"
 #include <wtf/HashMap.h>
 
 namespace WebCore {
@@ -183,6 +184,25 @@ static Path pathFromRectElement(const SVGElement& element)
     return path;
 }
 
+static Path pathFromUseElement(const SVGElement& element)
+{
+    const auto& useElement = downcast<SVGUseElement>(element);
+
+    RefPtr clipChildElement = useElement.clipChild();
+    if (!clipChildElement)
+        return { };
+
+    SVGLengthContext lengthContext(&element);
+    auto x = useElement.x().value(lengthContext);
+    auto y = useElement.y().value(lengthContext);
+
+    auto path = pathFromGraphicsElement(*clipChildElement.get());
+    if (x || y)
+        path.translate(FloatSize(x, y));
+
+    return path;
+}
+
 Path pathFromGraphicsElement(const SVGElement& element)
 {
     switch (element.tagQName().nodeName()) {
@@ -200,6 +220,8 @@ Path pathFromGraphicsElement(const SVGElement& element)
         return pathFromPolylineElement(element);
     case ElementNames::SVG::rect:
         return pathFromRectElement(element);
+    case ElementNames::SVG::use:
+        return pathFromUseElement(element);
     default:
         break;
     }

diff --git a/Source/WebCore/svg/SVGClipPathElement.cpp b/Source/WebCore/svg/SVGClipPathElement.cpp
@@ -31,6 +31,7 @@
 #include "RenderStyleInlines.h"
 #include "SVGElementInlines.h"
 #include "SVGNames.h"
+#include "SVGUseElement.h"
 #include "StyleResolver.h"
 #include <wtf/IsoMallocInlines.h>
 #include <wtf/NeverDestroyed.h>
@@ -102,31 +103,44 @@ SVGGraphicsElement* SVGClipPathElement::shouldApplyPathClipping() const
     if (renderer() && renderer()->style().clipPath())
         return nullptr;
 
+    auto rendererRequiresMaskClipping = [](const RenderObject& renderer) -> bool {
+        // Only shapes or paths are supported for direct clipping. We need to fall back to masking for texts.
+        if (is<RenderSVGText>(renderer))
+            return true;
+        auto& style = renderer.style();
+        if (style.display() == DisplayType::None || style.visibility() != Visibility::Visible)
+            return false;
+        // Current shape in clip-path gets clipped too. Fall back to masking.
+        return style.clipPath();
+    };
+
     SVGGraphicsElement* useGraphicsElement = nullptr;
 
     // If clip-path only contains one visible shape or path, we can use path-based clipping. Invisible
     // shapes don't affect the clipping and can be ignored. If clip-path contains more than one
     // visible shape, the additive clipping may not work, caused by the clipRule. EvenOdd
     // as well as NonZero can cause self-clipping of the elements.
     // See also http://www.w3.org/TR/SVG/painting.html#FillRuleProperty
-    for (Node* childNode = firstChild(); childNode; childNode = childNode->nextSibling()) {
-        auto* renderer = childNode->renderer();
-        if (!renderer)
-            continue;
-        // Only shapes or paths are supported for direct clipping. We need to fall back to masking for texts.
-        if (is<RenderSVGText>(renderer))
-            return nullptr;
-        if (!is<SVGGraphicsElement>(*childNode))
+    for (auto* childNode = firstChild(); childNode; childNode = childNode->nextSibling()) {
+        auto* graphicsElement = dynamicDowncast<SVGGraphicsElement>(*childNode);
+        if (!graphicsElement)
             continue;
-        auto& style = renderer->style();
-        if (style.display() == DisplayType::None || style.visibility() != Visibility::Visible)
+        auto* renderer = graphicsElement->renderer();
+        if (!renderer)
             continue;
-        // Current shape in clip-path gets clipped too. Fall back to masking.
-        if (style.clipPath())
+        if (rendererRequiresMaskClipping(*renderer))
             return nullptr;
         // Fallback to masking, if there is more than one clipping path.
         if (useGraphicsElement)
             return nullptr;
+
+        // For <use> elements, delegate the decision whether to use mask clipping or not to the referenced element.
+        if (auto* useElement = dynamicDowncast<SVGUseElement>(graphicsElement)) {
+            auto* clipChildRenderer = useElement->rendererClipChild();
+            if (clipChildRenderer && rendererRequiresMaskClipping(*clipChildRenderer))
+                return nullptr;
+        }
+
         useGraphicsElement = downcast<SVGGraphicsElement>(childNode);
     }
 

diff --git a/Source/WebCore/svg/SVGGraphicsElement.cpp b/Source/WebCore/svg/SVGGraphicsElement.cpp
@@ -207,7 +207,10 @@ void SVGGraphicsElement::didAttachRenderers()
 
 Path SVGGraphicsElement::toClipPath()
 {
-    // FIXME: [LBSE] Stop mutating the path here and stop calling animatedLocalTransform().
+#if ENABLE(LAYER_BASED_SVG_ENGINE)
+    RELEASE_ASSERT(!document().settings().layerBasedSVGEngineEnabled());
+#endif
+
     Path path = pathFromGraphicsElement(*this);
     // FIXME: How do we know the element has done a layout?
     path.transform(animatedLocalTransform());

diff --git a/Source/WebCore/svg/SVGUseElement.cpp b/Source/WebCore/svg/SVGUseElement.cpp
@@ -319,6 +319,10 @@ static bool isDirectReference(const SVGElement& element)
 
 Path SVGUseElement::toClipPath()
 {
+#if ENABLE(LAYER_BASED_SVG_ENGINE)
+    RELEASE_ASSERT(!document().settings().layerBasedSVGEngineEnabled());
+#endif
+
     auto targetClone = this->targetClone();
     if (!is<SVGGraphicsElement>(targetClone))
         return { };
@@ -329,7 +333,6 @@ Path SVGUseElement::toClipPath()
         return { };
     }
 
-    // FIXME: [LBSE] Stop mutating the path here and stop calling animatedLocalTransform().
     Path path = downcast<SVGGraphicsElement>(*targetClone).toClipPath();
     SVGLengthContext lengthContext(this);
     // FIXME: Find a way to do this without manual resolution of x/y here. It's potentially incorrect.
@@ -338,6 +341,14 @@ Path SVGUseElement::toClipPath()
     return path;
 }
 
+RefPtr<SVGElement> SVGUseElement::clipChild() const
+{
+    auto targetClone = this->targetClone();
+    if (!targetClone || !isDirectReference(*targetClone))
+        return nullptr;
+    return targetClone;
+}
+
 RenderElement* SVGUseElement::rendererClipChild() const
 {
     auto targetClone = this->targetClone();

diff --git a/Source/WebCore/svg/SVGUseElement.h b/Source/WebCore/svg/SVGUseElement.h
@@ -39,6 +39,7 @@ class SVGUseElement final : public SVGGraphicsElement, public SVGURIReference, p
     void invalidateShadowTree();
     void updateUserAgentShadowTree() final;
 
+    RefPtr<SVGElement> clipChild() const;
     RenderElement* rendererClipChild() const;
 
     const SVGLengthValue& x() const { return m_x->currentValue(); }