Unreviewed, reverting 270992@main.

https://bugs.webkit.org/show_bug.cgi?id=265179 ASSERTION FAILED: document().settings().layerBasedSVGEngineEnabled() under RenderLayerModelObject::svgClipperResourceFromStyle() Reverted changeset: "[LBSE] Rework SVG resource invalidation" https://bugs.webkit.org/show_bug.cgi?id=264230 https://commits.webkit.org/270992@main Canonical link: https://commits.webkit.org/271001@main
WebKit · Nov 21, 2023 · ac48763 · ac48763
1 parent 7e36e3e
commit ac48763
Show file tree

Hide file tree

Showing 47 changed files with 246 additions and 425 deletions.
diff --git a/...sts/platform/mac-sonoma-wk2-lbse-text/svg/custom/svg-fonts-in-html-expected.png b/...sts/platform/mac-sonoma-wk2-lbse-text/svg/custom/svg-fonts-in-html-expected.png
diff --git a/LayoutTests/platform/mac-sonoma-wk2-lbse-text/svg/custom/svg-fonts-in-html-expected.txt b/LayoutTests/platform/mac-sonoma-wk2-lbse-text/svg/custom/svg-fonts-in-html-expected.txt
@@ -22,8 +22,8 @@ layer at (173,32) size 454x454
 layer at (173,47) size 454x190
   RenderBlock (positioned) {H1} at (0,15) size 454x191 [color=#DD9955]
     RenderInline {SPAN} at (0,0) size 343x192
-      RenderText {#text} at (63,-1) size 343x192
-        text run at (63,-1) width 164: "CSS"
+      RenderText {#text} at (62,-1) size 343x192
+        text run at (62,-1) width 165: "CSS"
         text run at (198,-1) width 193: " ZEN"
         text run at (55,94) width 343: "GARDEN"
 layer at (173,119) size 454x21

diff --git a/...-sonoma-wk2-lbse-text/svg/repaint/repaint-non-scaling-stroke-text-decoration-expected.txt b/...-sonoma-wk2-lbse-text/svg/repaint/repaint-non-scaling-stroke-text-decoration-expected.txt
@@ -1,5 +1,6 @@
 Hello
  (repaint rects
+  (rect 13 16 40 22)
   (rect 9 12 48 30)
 )
 
diff --git a/...latform/mac-sonoma-wk2-lbse-text/svg/repaint/repaint-non-scaling-stroke-text-expected.txt b/...latform/mac-sonoma-wk2-lbse-text/svg/repaint/repaint-non-scaling-stroke-text-expected.txt
@@ -1,5 +1,6 @@
 Hello
  (repaint rects
+  (rect 13 16 40 22)
   (rect 9 12 48 30)
 )
 
diff --git a/LayoutTests/svg/repaint/clip-path-object-bounding-box-expected.txt b/LayoutTests/svg/repaint/clip-path-object-bounding-box-expected.txt
@@ -1,4 +1,4 @@
 (repaint rects
-  (rect 25 25 50 50)
+  (rect 0 0 100 100)
 )
 
diff --git a/LayoutTests/svg/repaint/clip-path-object-bounding-box-shrink-expected.txt b/LayoutTests/svg/repaint/clip-path-object-bounding-box-shrink-expected.txt
@@ -1,5 +1,4 @@
 (repaint rects
-  (rect 50 50 50 50)
-  (rect 0 0 50 50)
+  (rect 0 0 100 100)
 )
 
diff --git a/LayoutTests/svg/repaint/clip-path-object-bounding-box-shrink.html b/LayoutTests/svg/repaint/clip-path-object-bounding-box-shrink.html
@@ -7,15 +7,17 @@
 <svg style="position: absolute; top: 0px; left: 0px; width: 500px; height: 200px">
     <defs>
       <clipPath id="clip" clipPathUnits="objectBoundingBox">
-        <rect id="clipContent" x="0.5" y="0.5" width="0.5" height="0.5"></rect>
+        <rect id="clipContent" x="0" y="0" width="1" height="1"></rect>
       </clipPath>
     </defs>
     <rect id="target" x="0" y="0" width="100" height="100" clip-path="url(#clip)" fill="red"/>
 </svg>
 <script>
     function repaintTest() {
-        document.getElementById("clipContent").setAttribute('x', '0');
-        document.getElementById("clipContent").setAttribute('y', '0');
+        document.getElementById("clipContent").setAttribute('x', '.25');
+        document.getElementById("clipContent").setAttribute('y', '.25');
+        document.getElementById("clipContent").setAttribute('width', '.5');
+        document.getElementById("clipContent").setAttribute('height', '.5');
     }
 </script>
 </body>

diff --git a/LayoutTests/svg/repaint/clip-path-object-bounding-box-transformed-expected.txt b/LayoutTests/svg/repaint/clip-path-object-bounding-box-transformed-expected.txt
@@ -1,4 +1 @@
-(repaint rects
-  (rect 25 25 50 50)
-)
 
diff --git a/LayoutTests/svg/repaint/clip-path-user-space-on-use-expected.txt b/LayoutTests/svg/repaint/clip-path-user-space-on-use-expected.txt
@@ -1,4 +1,4 @@
 (repaint rects
-  (rect 25 25 50 50)
+  (rect 0 0 100 100)
 )
 
diff --git a/LayoutTests/svg/repaint/clip-path-user-space-on-use-shrink-expected.txt b/LayoutTests/svg/repaint/clip-path-user-space-on-use-shrink-expected.txt
@@ -1,5 +1,4 @@
 (repaint rects
-  (rect 50 50 50 50)
-  (rect 0 0 50 50)
+  (rect 0 0 100 100)
 )
 
diff --git a/LayoutTests/svg/repaint/clip-path-user-space-on-use-shrink.html b/LayoutTests/svg/repaint/clip-path-user-space-on-use-shrink.html
@@ -7,15 +7,17 @@
 <svg style="position: absolute; top: 0px; left: 0px; width: 500px; height: 200px">
     <defs>
       <clipPath id="clip" clipPathUnits="userSpaceOnUse">
-        <rect id="clipContent" x="50" y="50" width="50" height="50"></rect>
+        <rect id="clipContent" x="0" y="0" width="100" height="100"></rect>
       </clipPath>
     </defs>
     <rect id="target" x="0" y="0" width="100" height="100" clip-path="url(#clip)" fill="red"/>
 </svg>
 <script>
     function repaintTest() {
-        document.getElementById("clipContent").setAttribute('x', '0');
-        document.getElementById("clipContent").setAttribute('y', '0');
+        document.getElementById("clipContent").setAttribute('x', '25');
+        document.getElementById("clipContent").setAttribute('y', '25');
+        document.getElementById("clipContent").setAttribute('width', '50');
+        document.getElementById("clipContent").setAttribute('height', '50');
     }
 </script>
 </body>

diff --git a/LayoutTests/svg/repaint/clip-path-user-space-on-use-transformed-expected.txt b/LayoutTests/svg/repaint/clip-path-user-space-on-use-transformed-expected.txt
@@ -1,4 +1 @@
-(repaint rects
-  (rect 25 25 50 50)
-)
 
diff --git a/LayoutTests/svg/resource-invalidation/clip-path-resource-invalidation-expected.html b/LayoutTests/svg/resource-invalidation/clip-path-resource-invalidation-expected.html
@@ -22,6 +22,9 @@
         <clipPath id="c6" clipPathUnits="objectBoundingBox">
             <rect width="0.5" height="0.5"/>
         </clipPath>
+        <clipPath id="c6" clipPathUnits="objectBoundingBox">
+            <rect width="175" height="175"/>
+        </clipPath>
         <clipPath id="c7" clipPathUnits="objectBoundingBox">
             <rect width="0.5" height="0.5"/>
         </clipPath>
@@ -35,7 +38,7 @@
     <rect id="r3" x="250" y="50" width="50" height="50"/>
     <rect id="r4" x="350" y="50" width="50" height="50" clip-path="url(#c4)" fill="blue"/>
     <rect id="r5" x="50" y="150" width="75" height="50" clip-path="url(#c5)"/>
-    <rect id="r6" x="150" y="150" width="100" height="100" clip-path="url(#c6)"/>
+    <rect id="r6" x="150" y="150" width="50" height="50" clip-path="url(#c6)"/>
     <rect id="r7" x="250" y="150" width="50" height="50" clip-path="url(#c7)"/>
     <rect id="r8" x="350" y="150" width="50" height="50" clip-path="url(#c8)"/>
 </svg>
diff --git a/LayoutTests/svg/resource-invalidation/clip-path-resource-invalidation.html b/LayoutTests/svg/resource-invalidation/clip-path-resource-invalidation.html
@@ -1,4 +1,4 @@
-<!DOCTYPE html> <!-- webkit-test-runner [ LayerBasedSVGEngineEnabled=true ] -->
+<!DOCTYPE html>
 <body onload="run();">
 <svg width="450" height="250">
     <defs>
@@ -42,19 +42,19 @@
 </svg>
 <script>
 function run() {
-    if (window.testRunner)
+    if (window.testRunner) {
         testRunner.waitUntilDone();
-    requestAnimationFrame(function() {
-        r1.setAttribute("clip-path", "url(#c1)"); // change from no clipPath to some clipPath
-        r2.setAttribute("clip-path", "url(#c2b)"); // change from one clipPath to another
-        r3.removeAttribute("clip-path"); // change from a clipPath to no clipPath
-        r4.setAttribute("fill", "blue"); // change style of clipPath resource client
-        r5.setAttribute("width", "75"); // change layout of clipPath resource client
-        c6.setAttribute("clipPathUnits", "objectBoundingBox"); // change clipPath attribute
-        c7.firstElementChild.setAttribute("visibility", "visible"); // change style of clipPath contents
-        c8.firstElementChild.setAttribute("width", "1"); // change other attribute of clipPath contents
-        if (window.testRunner)
+        requestAnimationFrame(function() {
+            r1.setAttribute("clip-path", "url(#c1)"); // change from no clipPath to some clipPath
+            r2.setAttribute("clip-path", "url(#c2b)"); // change from one clipPath to another
+            r3.removeAttribute("clip-path"); // change from a clipPath to no clipPath
+            r4.setAttribute("fill", "blue"); // change style of clipPath resource client
+            r5.setAttribute("width", "75"); // change layout of clipPath resource client
+            c6.setAttribute("clipPathUnits", "objectBoundingBox"); // change clipPath attribute
+            c7.firstElementChild.setAttribute("visibility", "visible"); // change style of clipPath contents
+            c8.firstElementChild.setAttribute("width", "1"); // change other attribute of clipPath contents
             testRunner.notifyDone();
-    });
+        });
+    }
 }
 </script>
diff --git a/Source/WebCore/dom/TreeScope.cpp b/Source/WebCore/dom/TreeScope.cpp
@@ -640,6 +640,9 @@ struct SVGResourcesMap {
 
     MemoryCompactRobinHoodHashMap<AtomString, WeakHashSet<SVGElement, WeakPtrImplWithEventTargetData>> pendingResources;
     MemoryCompactRobinHoodHashMap<AtomString, WeakHashSet<SVGElement, WeakPtrImplWithEventTargetData>> pendingResourcesForRemoval;
+#if ENABLE(LAYER_BASED_SVG_ENGINE)
+    MemoryCompactRobinHoodHashMap<AtomString, RenderSVGResourceContainer*> resources;
+#endif
     MemoryCompactRobinHoodHashMap<AtomString, LegacyRenderSVGResourceContainer*> legacyResources;
 };
 
@@ -650,6 +653,17 @@ SVGResourcesMap& TreeScope::svgResourcesMap() const
     return *m_svgResourcesMap;
 }
 
+#if ENABLE(LAYER_BASED_SVG_ENGINE)
+void TreeScope::addSVGResource(const AtomString& id, RenderSVGResourceContainer& resource)
+{
+    if (id.isEmpty())
+        return;
+
+    // Replaces resource if already present, to handle potential id changes
+    svgResourcesMap().resources.set(id, &resource);
+}
+#endif
+
 void TreeScope::addSVGResource(const AtomString& id, LegacyRenderSVGResourceContainer& resource)
 {
     if (id.isEmpty())
@@ -664,9 +678,22 @@ void TreeScope::removeSVGResource(const AtomString& id)
     if (id.isEmpty())
         return;
 
+#if ENABLE(LAYER_BASED_SVG_ENGINE)
+    svgResourcesMap().resources.remove(id);
+#endif
     svgResourcesMap().legacyResources.remove(id);
 }
 
+#if ENABLE(LAYER_BASED_SVG_ENGINE)
+RenderSVGResourceContainer* TreeScope::lookupSVGResourceById(const AtomString& id) const
+{
+    if (id.isEmpty())
+        return nullptr;
+
+    return svgResourcesMap().resources.get(id);
+}
+#endif
+
 LegacyRenderSVGResourceContainer* TreeScope::lookupLegacySVGResoureById(const AtomString& id) const
 {
     if (id.isEmpty())

diff --git a/Source/WebCore/dom/TreeScope.h b/Source/WebCore/dom/TreeScope.h
@@ -54,6 +54,7 @@ class LegacyRenderSVGResourceContainer;
 class IdTargetObserverRegistry;
 class Node;
 class RadioButtonGroups;
+class RenderSVGResourceContainer;
 class SVGElement;
 class ShadowRoot;
 class TreeScopeOrderedMap;
@@ -142,8 +143,14 @@ class TreeScope {
     std::span<const RefPtr<CSSStyleSheet>> adoptedStyleSheets() const;
     ExceptionOr<void> setAdoptedStyleSheets(Vector<RefPtr<CSSStyleSheet>>&&);
 
+#if ENABLE(LAYER_BASED_SVG_ENGINE)
+    void addSVGResource(const AtomString& id, RenderSVGResourceContainer&);
+#endif
     void addSVGResource(const AtomString& id, LegacyRenderSVGResourceContainer&);
     void removeSVGResource(const AtomString& id);
+#if ENABLE(LAYER_BASED_SVG_ENGINE)
+    RenderSVGResourceContainer* lookupSVGResourceById(const AtomString& id) const;
+#endif
     LegacyRenderSVGResourceContainer* lookupLegacySVGResoureById(const AtomString& id) const;
 
     void addPendingSVGResource(const AtomString& id, SVGElement&);

diff --git a/Source/WebCore/rendering/ReferencedSVGResources.cpp b/Source/WebCore/rendering/ReferencedSVGResources.cpp
@@ -33,8 +33,6 @@
 #include "SVGClipPathElement.h"
 #include "SVGElementTypeHelpers.h"
 #include "SVGFilterElement.h"
-#include "SVGMarkerElement.h"
-#include "SVGMaskElement.h"
 #include "SVGResourceElementClient.h"
 
 #include <wtf/IsoMallocInlines.h>
@@ -148,7 +146,6 @@ RefPtr<SVGElement> ReferencedSVGResources::elementForResourceID(TreeScope& treeS
     return downcast<SVGElement>(WTFMove(element));
 }
 
-#if ENABLE(LAYER_BASED_SVG_ENGINE)
 RefPtr<SVGClipPathElement> ReferencedSVGResources::referencedClipPathElement(TreeScope& treeScope, const ReferencePathOperation& clipPath)
 {
     if (clipPath.fragment().isEmpty())
@@ -157,31 +154,6 @@ RefPtr<SVGClipPathElement> ReferencedSVGResources::referencedClipPathElement(Tre
     return element ? downcast<SVGClipPathElement>(WTFMove(element)) : nullptr;
 }
 
-RefPtr<SVGMarkerElement> ReferencedSVGResources::referencedMarkerElement(TreeScope& treeScope, const String& markerResource)
-{
-    auto resourceID = SVGURIReference::fragmentIdentifierFromIRIString(markerResource, treeScope.documentScope());
-    if (resourceID.isEmpty())
-        return nullptr;
-
-    RefPtr element = elementForResourceID(treeScope, resourceID, SVGNames::maskTag);
-    return element ? downcast<SVGMarkerElement>(WTFMove(element)) : nullptr;
-}
-
-RefPtr<SVGMaskElement> ReferencedSVGResources::referencedMaskElement(TreeScope& treeScope, const StyleImage& maskImage)
-{
-    auto reresolvedURL = maskImage.reresolvedURL(treeScope.documentScope());
-    if (reresolvedURL.isEmpty())
-        return nullptr;
-
-    auto resourceID = SVGURIReference::fragmentIdentifierFromIRIString(reresolvedURL.string(), treeScope.documentScope());
-    if (resourceID.isEmpty())
-        return nullptr;
-
-    RefPtr element = elementForResourceID(treeScope, resourceID, SVGNames::maskTag);
-    return element ? downcast<SVGMaskElement>(WTFMove(element)) : nullptr;
-}
-#endif
-
 RefPtr<SVGFilterElement> ReferencedSVGResources::referencedFilterElement(TreeScope& treeScope, const ReferenceFilterOperation& referenceFilter)
 {
     if (referenceFilter.fragment().isEmpty())

diff --git a/Source/WebCore/rendering/ReferencedSVGResources.h b/Source/WebCore/rendering/ReferencedSVGResources.h
@@ -37,18 +37,15 @@ class CSSSVGResourceElementClient;
 class Document;
 class LegacyRenderSVGResourceClipper;
 class LegacyRenderSVGResourceContainer;
-class QualifiedName;
-class ReferenceFilterOperation;
 class ReferencePathOperation;
+class ReferenceFilterOperation;
 class RenderElement;
 class RenderSVGResourceFilter;
 class RenderStyle;
+class QualifiedName;
 class SVGClipPathElement;
 class SVGElement;
 class SVGFilterElement;
-class SVGMarkerElement;
-class SVGMaskElement;
-class StyleImage;
 class TreeScope;
 
 class ReferencedSVGResources {
@@ -60,19 +57,13 @@ class ReferencedSVGResources {
     static Vector<std::pair<AtomString, QualifiedName>> referencedSVGResourceIDs(const RenderStyle&);
     void updateReferencedResources(TreeScope&, const Vector<std::pair<AtomString, QualifiedName>>&);
 
-    // Legacy: Clipping needs a renderer, filters use an element.
+    // Clipping needs a renderer, filters use an element.
     static LegacyRenderSVGResourceClipper* referencedClipperRenderer(TreeScope&, const ReferencePathOperation&);
+    static RefPtr<SVGClipPathElement> referencedClipPathElement(TreeScope&, const ReferencePathOperation&);
     static RefPtr<SVGFilterElement> referencedFilterElement(TreeScope&, const ReferenceFilterOperation&);
 
     static LegacyRenderSVGResourceContainer* referencedRenderResource(TreeScope&, const AtomString& fragment);
 
-#if ENABLE(LAYER_BASED_SVG_ENGINE)
-    // LBSE: All element based.
-    static RefPtr<SVGClipPathElement> referencedClipPathElement(TreeScope&, const ReferencePathOperation&);
-    static RefPtr<SVGMarkerElement> referencedMarkerElement(TreeScope&, const String&);
-    static RefPtr<SVGMaskElement> referencedMaskElement(TreeScope&, const StyleImage&);
-#endif
-
 private:
     static RefPtr<SVGElement> elementForResourceID(TreeScope&, const AtomString& resourceID, const QualifiedName& tagName);
 

diff --git a/Source/WebCore/rendering/RenderElement.cpp b/Source/WebCore/rendering/RenderElement.cpp
@@ -71,7 +71,6 @@
 #include "RenderLineBreak.h"
 #include "RenderListItem.h"
 #include "RenderMultiColumnSpannerPlaceholder.h"
-#include "RenderSVGResourceContainer.h"
 #include "RenderSVGViewportContainer.h"
 #include "RenderStyleSetters.h"
 #include "RenderTableCaption.h"
@@ -971,13 +970,8 @@ void RenderElement::styleDidChange(StyleDifference diff, const RenderStyle* oldS
 
     SVGRenderSupport::styleChanged(*this, oldStyle);
 
-    if (diff >= StyleDifference::Repaint) {
+    if (diff >= StyleDifference::Repaint)
         updateReferencedSVGResources();
-#if ENABLE(LAYER_BASED_SVG_ENGINE)
-        if (oldStyle && diff <= StyleDifference::RepaintLayer)
-            repaintClientsOfReferencedSVGResources();
-#endif
-    }
 
     if (!m_parent)
         return;
@@ -2104,29 +2098,6 @@ void RenderElement::updateReferencedSVGResources()
         clearReferencedSVGResources();
 }
 
-#if ENABLE(LAYER_BASED_SVG_ENGINE)
-void RenderElement::repaintRendererOrClientsOfReferencedSVGResources() const
-{
-    auto* enclosingResourceContainer = lineageOfType<RenderSVGResourceContainer>(*this).first();
-    if (!enclosingResourceContainer) {
-        repaint();
-        return;
-    }
-
-    // This implicitly checks if LBSE is activated. If not, no 'RenderSVGResourceContainer' objects are present in the render tree.
-    enclosingResourceContainer->repaintAllClients();
-}
-
-void RenderElement::repaintClientsOfReferencedSVGResources() const
-{
-    if (!document().settings().layerBasedSVGEngineEnabled())
-        return;
-
-    if (auto* enclosingResourceContainer = lineageOfType<RenderSVGResourceContainer>(*this).first())
-        enclosingResourceContainer->repaintAllClients();
-}
-#endif
-
 #if ENABLE(TEXT_AUTOSIZING)
 static RenderObject::BlockContentHeightType includeNonFixedHeight(const RenderObject& renderer)
 {

diff --git a/Source/WebCore/rendering/RenderElement.h b/Source/WebCore/rendering/RenderElement.h
@@ -173,11 +173,6 @@ class RenderElement : public RenderObject {
     enum class RequiresFullRepaint : bool { No, Yes };
     bool repaintAfterLayoutIfNeeded(const RenderLayerModelObject* repaintContainer, RequiresFullRepaint, const LayoutRect& oldBounds, const LayoutRect& oldOutlineBox, const LayoutRect* newBoundsPtr = nullptr, const LayoutRect* newOutlineBoxPtr = nullptr);
 
-#if ENABLE(LAYER_BASED_SVG_ENGINE)
-    void repaintClientsOfReferencedSVGResources() const;
-    void repaintRendererOrClientsOfReferencedSVGResources() const;
-#endif
-
     bool borderImageIsLoadedAndCanBeRendered() const;
     bool isVisibleIgnoringGeometry() const;
     bool mayCauseRepaintInsideViewport(const IntRect* visibleRect = nullptr) const;