Changeset 180643 in webkit

Timestamp:

Feb 25, 2015 3:05:20 PM (9 years ago)

Author:

Said Abou-Hallawa

Message:

Horizontal and vertical lines are clipped completely if clip-path is included in the tag but the referenced element is defined later.
​https://bugs.webkit.org/show_bug.cgi?id=141776.

Reviewed by Dean Jackson.
Source/WebCore:

Tests: svg/clip-path/clip-path-line-use-before-defined-expected.svg

svg/clip-path/clip-path-line-use-before-defined.svg

• rendering/svg/RenderSVGResourceClipper.cpp:

(WebCore::RenderSVGResourceClipper::applyClippingToContext): Ensure the renderer
is added to m_clipper if it does not exist. The same renderer might have been
added to m_clipper in resourceBoundingBox().

(WebCore::RenderSVGResourceClipper::addRendererToClipper): Add the renderer to
m_clipper if it does not exist. Return the associated ClipperData.

(WebCore::RenderSVGResourceClipper::resourceBoundingBox): If the clipper is
referenced before it is defined, add the renderer to m_clipper. While doing the
layout() for the clipper, we can check if m_clipper has values or not. If it does
have, we are going to mark the clipper for client invalidation which is done by
the SVG root.

• rendering/svg/RenderSVGResourceClipper.h:
• rendering/svg/RenderSVGResourceContainer.h:

(WebCore::RenderSVGResourceContainer::selfNeedsClientInvalidation): Define a
new function selfNeedsClientInvalidation() which controls marking the clipper
for client invalidation. In RenderSVGResourceClipper, override it so it checks
m_clipper to force clients validation even if it the first time we do layout
for this clipper.

• rendering/svg/RenderSVGResourceContainer.cpp:

(WebCore::RenderSVGResourceContainer::layout): Call the virtual function
selfNeedsClientInvalidation() to check whether we need to mark the clipper for
client invalidation.

• svg/SVGElement.cpp: Delete unneeded header file.

LayoutTests:

New test cases for SVG lines which are clipped to a <clipPath>. The <clipPath>
is referenced before it is defined.

• svg/clip-path/clip-path-line-use-before-defined-expected.svg: Added.
• svg/clip-path/clip-path-line-use-before-defined.svg: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/svg/clip-path/clip-path-line-use-before-defined-expected.svg (added)
• LayoutTests/svg/clip-path/clip-path-line-use-before-defined.svg (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/svg/RenderSVGResourceClipper.cpp (modified) (4 diffs)
• Source/WebCore/rendering/svg/RenderSVGResourceClipper.h (modified) (2 diffs)
• Source/WebCore/rendering/svg/RenderSVGResourceContainer.cpp (modified) (1 diff)
• Source/WebCore/rendering/svg/RenderSVGResourceContainer.h (modified) (1 diff)
• Source/WebCore/svg/SVGElement.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2015-02-25  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        Horizontal and vertical lines are clipped completely if clip-path is included in the tag but the referenced element is defined later.
+        https://bugs.webkit.org/show_bug.cgi?id=141776.
+
+        Reviewed by Dean Jackson.
+        
+        New test cases for SVG lines which are clipped to a <clipPath>. The <clipPath>
+        is referenced before it is defined.
+
+        * svg/clip-path/clip-path-line-use-before-defined-expected.svg: Added.
+        * svg/clip-path/clip-path-line-use-before-defined.svg: Added.
+
 2015-02-25  Myles C. Maxfield  <mmaxfield@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2015-02-25  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        Horizontal and vertical lines are clipped completely if clip-path is included in the tag but the referenced element is defined later.
+        https://bugs.webkit.org/show_bug.cgi?id=141776.
+
+        Reviewed by Dean Jackson.
+
+        Tests: svg/clip-path/clip-path-line-use-before-defined-expected.svg
+               svg/clip-path/clip-path-line-use-before-defined.svg
+
+        * rendering/svg/RenderSVGResourceClipper.cpp:
+        (WebCore::RenderSVGResourceClipper::applyClippingToContext): Ensure the renderer
+        is added to m_clipper if it does not exist. The same renderer might have been 
+        added to m_clipper in resourceBoundingBox().
+        
+        (WebCore::RenderSVGResourceClipper::addRendererToClipper): Add the renderer to
+        m_clipper if it does not exist. Return the associated ClipperData.
+        
+        (WebCore::RenderSVGResourceClipper::resourceBoundingBox): If the clipper is
+        referenced before it is defined, add the renderer to m_clipper. While doing the
+        layout() for the clipper, we can check if m_clipper has values or not. If it does
+        have, we are going to mark the clipper for client invalidation which is done by
+        the SVG root.
+        
+        * rendering/svg/RenderSVGResourceClipper.h:
+        * rendering/svg/RenderSVGResourceContainer.h:
+        (WebCore::RenderSVGResourceContainer::selfNeedsClientInvalidation): Define a 
+        new function selfNeedsClientInvalidation() which controls marking the clipper 
+        for client invalidation. In RenderSVGResourceClipper, override it so it checks
+        m_clipper to force clients validation even if it the first time we do layout
+        for this clipper.
+
+        * rendering/svg/RenderSVGResourceContainer.cpp:
+        (WebCore::RenderSVGResourceContainer::layout): Call the virtual function
+        selfNeedsClientInvalidation() to check whether we need to mark the clipper for
+        client invalidation.
+        
+        * svg/SVGElement.cpp: Delete unneeded header file.
+
 2015-02-25  peavo@outlook.com  <peavo@outlook.com>
 

trunk/Source/WebCore/rendering/svg/RenderSVGResourceClipper.cpp

@@ -131 +131 @@
                                                       const FloatRect& repaintRect, GraphicsContext* context)
 {
-    bool missingClipperData = !m_clipper.contains(&renderer);
-    if (missingClipperData)
-        m_clipper.set(&renderer, std::make_unique<ClipperData>());
-
-    bool shouldCreateClipData = false;
+    ClipperData* clipperData = addRendererToClipper(renderer);
+    ASSERT(clipperData);
+    bool shouldCreateClipData = !clipperData->clipMaskImage;
+
     AffineTransform animatedLocalTransform = clipPathElement().animatedLocalTransform();
-    ClipperData* clipperData = m_clipper.get(&renderer);
-    if (!clipperData->clipMaskImage) {
-        if (pathOnlyClipping(context, animatedLocalTransform, objectBoundingBox))
-            return true;
-        shouldCreateClipData = true;
-    }
+
+    if (shouldCreateClipData && pathOnlyClipping(context, animatedLocalTransform, objectBoundingBox))
+        return true;
 
     AffineTransform absoluteTransform;
@@ -178 +174 @@
         return false;
 
-    SVGRenderingContext::clipToImageBuffer(context, absoluteTransform, repaintRect, clipperData->clipMaskImage, missingClipperData);
+    SVGRenderingContext::clipToImageBuffer(context, absoluteTransform, repaintRect, clipperData->clipMaskImage, shouldCreateClipData);
     return true;
 }
@@ -262 +258 @@
 }
 
+ClipperData* RenderSVGResourceClipper::addRendererToClipper(const RenderObject& object)
+{
+    if (!m_clipper.contains(&object))
+        m_clipper.set(&object, std::make_unique<ClipperData>());
+    return m_clipper.get(&object);
+}
+
 bool RenderSVGResourceClipper::hitTestClipContent(const FloatRect& objectBoundingBox, const FloatPoint& nodeAtPoint)
 {
@@ -295 +298 @@
 {
     // Resource was not layouted yet. Give back the boundingBox of the object.
-    if (selfNeedsLayout())
+    if (selfNeedsLayout()) {
+        addRendererToClipper(object);
         return object.objectBoundingBox();
+    }
     
     if (m_clipBoundaries.isEmpty())

trunk/Source/WebCore/rendering/svg/RenderSVGResourceClipper.h

@@ -61 +61 @@
     SVGUnitTypes::SVGUnitType clipPathUnits() const { return clipPathElement().clipPathUnits(); }
 
+protected:
+    virtual bool selfNeedsClientInvalidation() const override { return (everHadLayout() || m_clipper.size()) && selfNeedsLayout(); }
+
 private:
     void element() const = delete;
@@ -69 +72 @@
     bool drawContentIntoMaskImage(ClipperData*, const FloatRect& objectBoundingBox);
     void calculateClipContentRepaintRect();
+    ClipperData* addRendererToClipper(const RenderObject&);
 
     FloatRect m_clipBoundaries;
-    HashMap<RenderObject*, std::unique_ptr<ClipperData>> m_clipper;
+    HashMap<const RenderObject*, std::unique_ptr<ClipperData>> m_clipper;
 };
 

trunk/Source/WebCore/rendering/svg/RenderSVGResourceContainer.cpp

@@ -53 +53 @@
     StackStats::LayoutCheckPoint layoutCheckPoint;
     // Invalidate all resources if our layout changed.
-    if (everHadLayout() && selfNeedsLayout())
+    if (selfNeedsClientInvalidation())
         RenderSVGRoot::addResourceForClientInvalidation(this);
 

trunk/Source/WebCore/rendering/svg/RenderSVGResourceContainer.h

@@ -56 +56 @@
 
     // Used from the invalidateClient/invalidateClients methods from classes, inheriting from us.
+    virtual bool selfNeedsClientInvalidation() const { return everHadLayout() && selfNeedsLayout(); }
+
     void markAllClientsForInvalidation(InvalidationMode);
     void markAllClientLayersForInvalidation();

trunk/Source/WebCore/svg/SVGElement.cpp

@@ -40 +40 @@
 #include "RenderObject.h"
 #include "RenderSVGResource.h"
-#include "RenderSVGResourceClipper.h"
 #include "RenderSVGResourceFilter.h"
 #include "RenderSVGResourceMasker.h"