Cleanup RenderSVGResourceClipper class.
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 26 Feb 2015 18:37:14 +0000 (18:37 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 26 Feb 2015 18:37:14 +0000 (18:37 +0000)
https://bugs.webkit.org/show_bug.cgi?id=142032.

Patch by Said Abou-Hallawa <sabouhallawa@apple.com> on 2015-02-26
Reviewed by Darin Adler.
Source/WebCore:

This is a follow up for r180643: <http://trac.webkit.org/changeset/180643>.
It includes cleanup for RenderSVGResourceClipper class.

* rendering/svg/RenderSVGResourceClipper.cpp:
(WebCore::RenderSVGResourceClipper::applyClippingToContext):
(WebCore::RenderSVGResourceClipper::drawContentIntoMaskImage):
* rendering/svg/RenderSVGResourceClipper.h: Change ClipperData to be a
typedef instead of a class and rename it to ClipperMaskImage. The purpose
of having it a class even though it includes only one member was because
we wanted it to be WTF_MAKE_FAST_ALLOCATED. We do not need to allocate it
as a separate object on the heap anymore.

(WebCore::RenderSVGResourceClipper::addRendererToClipper): Instead of doing
double hash table lookups by calling HashMap::contains() and then HashMap::get(),
we can use HashMap::add() instead.

LayoutTests:

* svg/clip-path/clip-path-line-use-before-defined-expected.svg:
* svg/clip-path/clip-path-line-use-before-defined.svg: Simplify the test
and make separate drawings for different cases.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@180685 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/svg/clip-path/clip-path-line-use-before-defined-expected.svg
LayoutTests/svg/clip-path/clip-path-line-use-before-defined.svg
Source/WebCore/ChangeLog
Source/WebCore/rendering/svg/RenderSVGResourceClipper.cpp
Source/WebCore/rendering/svg/RenderSVGResourceClipper.h

index bd4d255..0811734 100644 (file)
@@ -1,5 +1,16 @@
 2015-02-26  Said Abou-Hallawa  <sabouhallawa@apple.com>
 
+        Cleanup RenderSVGResourceClipper class.
+        https://bugs.webkit.org/show_bug.cgi?id=142032.
+
+        Reviewed by Darin Adler.
+
+        * svg/clip-path/clip-path-line-use-before-defined-expected.svg:
+        * svg/clip-path/clip-path-line-use-before-defined.svg: Simplify the test
+        and make separate drawings for different cases.
+
+2015-02-26  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
         Setting any of the <object> element plugin controlling attributes does not have any affect.
         https://bugs.webkit.org/show_bug.cgi?id=141936.
 
index 8bb3c63..84f6a5e 100644 (file)
@@ -1,33 +1,44 @@
-<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="200" height="200">
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
   <defs>
     <style>
       line, path {
         stroke: black;
+        stroke-width: 3;
       }
     </style>
     <clipPath id="clip-circle">
-        <circle cx="50%" cy="50%" r="25%"/>
+      <circle cx="75" cy="75" r="50"/>
     </clipPath>
+    <g id="circle-in-rect">
+      <rect x="0" y="0" width="150" height="150" fill="lime"/>
+      <rect x="0" y="0" width="150" height="150" fill="white" clip-path="url(#clip-circle)"/>
+    </g>
   </defs>
-  
-  <rect x="0" y="0" width="100%" height="100%" fill="lime"/>
-  <rect x="0" y="0" width="100%" height="100%" fill="white" clip-path="url(#clip-circle)"/>
 
-  <!-- diagonal -->
-  <line x1="170" y1="30" x2="30" y2="170" clip-path="url(#clip-circle)"/>
+  <use xlink:href="#circle-in-rect" transform="translate(10, 10)">
+    <g clip-path="url(#clip-circle)">
+      <!-- center lines -->
+      <path d="M0 75 L150 75"/>
+      <line x1="75" y1="0" x2="75" y2="150"/>
+    </g>
+  </use>
   
-  <!-- top and left lines -->
-  <path d="M30 75 L170 75" clip-path="url(#clip-circle)"/>
-  <line x1="75" y1="30" x2="75" y2="170" clip-path="url(#clip-circle)"/>
+  <use xlink:href="#circle-in-rect" transform="translate(170, 10)">
+    <g clip-path="url(#clip-circle)">
+      <!-- diagonal lines -->
+      <path d="M0 0 L150 150"/>
+      <line x1="0" y1="150" x2="150" y2="0"/>
+    </g>
+  </use>
 
-  <!-- right and bottom lines -->
-  <g clip-path="url(#clip-circle)">
-    <path d="M30 125 L170 125"/>
-    <line x1="125" y1="30" x2="125" y2="170"/>
-  </g>
-  
-  <!-- center lines -->
-  <path d="M30 100 L170 100" clip-path="url(#clip-circle)"/>
-  <line x1="100" y1="30" x2="100" y2="170" clip-path="url(#clip-circle)"/>
-  
+  <use xlink:href="#circle-in-rect" transform="translate(330, 10)">
+    <g clip-path="url(#clip-circle)">
+      <!-- top and left lines -->
+      <path d="M0 50 L150 50"/>
+      <line x1="50" y1="0" x2="50" y2="150"/>
+      <!-- bottom and right lines -->
+      <path d="M0 100 L150 100"/>
+      <line x1="100" y1="0" x2="100" y2="150"/>
+    </g>
+  </use>
 </svg>
index 6f2ee07..dd203c4 100644 (file)
@@ -1,38 +1,46 @@
-<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="200" height="200">
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
   <defs>
     <style>
       line, path {
         stroke: black;
+        stroke-width: 3;
       }
     </style>
+    <g id="circle-in-rect">
+      <rect x="0" y="0" width="150" height="150" fill="lime"/>
+      <rect x="0" y="0" width="150" height="150" fill="white" clip-path="url(#clip-circle)"/>
+    </g>
   </defs>
-  
-  <rect x="0" y="0" width="100%" height="100%" fill="lime"/>
-  <rect x="0" y="0" width="100%" height="100%" fill="white" clip-path="url(#clip-circle)"/>
 
-  <!-- diagonal -->
-  <line x1="170" y1="30" x2="30" y2="170" clip-path="url(#clip-circle)"/>
+  <use xlink:href="#circle-in-rect" transform="translate(10, 10)">
+    <!-- center lines -->
+    <path d="M0 75 L150 75" clip-path="url(#clip-circle)"/>
+    <line x1="75" y1="0" x2="75" y2="150" clip-path="url(#clip-circle)"/>
+  </use>
   
-  <!-- top and left lines -->
-  <path d="M30 75 L170 75" clip-path="url(#clip-circle)"/>
-  <line x1="75" y1="30" x2="75" y2="170" clip-path="url(#clip-circle)"/>
+  <use xlink:href="#circle-in-rect" transform="translate(170, 10)">
+    <!-- diagonal lines -->
+    <path d="M0 0 L150 150" clip-path="url(#clip-circle)"/>
+    <line x1="0" y1="150" x2="150" y2="0" clip-path="url(#clip-circle)"/>
+  </use>
 
-  <!-- bottom and right lines -->
-  <g clip-path="url(#clip-circle)">
-    <path d="M30 125 L170 125"/>
-    <line x1="125" y1="30" x2="125" y2="170"/>
-  </g>
+  <use xlink:href="#circle-in-rect" transform="translate(330, 10)">
+    <!-- top and left lines -->
+    <path d="M0 50 L150 50" clip-path="url(#clip-circle)"/>
+    <line x1="50" y1="0" x2="50" y2="150" clip-path="url(#clip-circle)"/>
+    <g clip-path="url(#clip-circle)">
+      <!-- bottom and right lines -->
+      <path d="M0 100 L150 100"/>
+      <line x1="100" y1="0" x2="100" y2="150"/>
+    </g>
+  </use>
   
   <defs>
     <clipPath id="clip-circle">
-      <circle cx="50%" cy="50%" r="50%" clip-path="url(#clip-small-circle)"/>
+      <circle cx="75" cy="75" r="75" clip-path="url(#clip-small-circle)"/>
     </clipPath>
     <clipPath id="clip-small-circle">
-      <circle cx="50%" cy="50%" r="25%"/>
+      <circle cx="75" cy="75" r="50"/>
     </clipPath>
   </defs>
-
-  <!-- center lines -->
-  <path d="M30 100 L170 100" clip-path="url(#clip-circle)"/>
-  <line x1="100" y1="30" x2="100" y2="170" clip-path="url(#clip-circle)"/>
 </svg>
index acac680..fd94076 100644 (file)
@@ -1,5 +1,28 @@
 2015-02-26  Said Abou-Hallawa  <sabouhallawa@apple.com>
 
+        Cleanup RenderSVGResourceClipper class.
+        https://bugs.webkit.org/show_bug.cgi?id=142032.
+
+        Reviewed by Darin Adler.
+        
+        This is a follow up for r180643: <http://trac.webkit.org/changeset/180643>.
+        It includes cleanup for RenderSVGResourceClipper class.
+
+        * rendering/svg/RenderSVGResourceClipper.cpp:
+        (WebCore::RenderSVGResourceClipper::applyClippingToContext):
+        (WebCore::RenderSVGResourceClipper::drawContentIntoMaskImage):
+        * rendering/svg/RenderSVGResourceClipper.h: Change ClipperData to be a
+        typedef instead of a class and rename it to ClipperMaskImage. The purpose
+        of having it a class even though it includes only one member was because
+        we wanted it to be WTF_MAKE_FAST_ALLOCATED. We do not need to allocate it
+        as a separate object on the heap anymore.
+        
+        (WebCore::RenderSVGResourceClipper::addRendererToClipper): Instead of doing
+        double hash table lookups by calling HashMap::contains() and then HashMap::get(),
+        we can use HashMap::add() instead.
+
+2015-02-26  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
         Setting any of the <object> element plugin controlling attributes does not have any affect.
         https://bugs.webkit.org/show_bug.cgi?id=141936.
 
index d8e48a0..d170f28 100644 (file)
@@ -130,23 +130,22 @@ bool RenderSVGResourceClipper::pathOnlyClipping(GraphicsContext* context, const
 bool RenderSVGResourceClipper::applyClippingToContext(RenderElement& renderer, const FloatRect& objectBoundingBox,
                                                       const FloatRect& repaintRect, GraphicsContext* context)
 {
-    ClipperData* clipperData = addRendererToClipper(renderer);
-    ASSERT(clipperData);
-    bool shouldCreateClipData = !clipperData->clipMaskImage;
+    ClipperMaskImage& clipperMaskImage = addRendererToClipper(renderer);
+    bool shouldCreateClipperMaskImage = !clipperMaskImage;
 
     AffineTransform animatedLocalTransform = clipPathElement().animatedLocalTransform();
 
-    if (shouldCreateClipData && pathOnlyClipping(context, animatedLocalTransform, objectBoundingBox))
+    if (shouldCreateClipperMaskImage && pathOnlyClipping(context, animatedLocalTransform, objectBoundingBox))
         return true;
 
     AffineTransform absoluteTransform;
     SVGRenderingContext::calculateTransformationToOutermostCoordinateSystem(renderer, absoluteTransform);
 
-    if (shouldCreateClipData && !repaintRect.isEmpty()) {
-        if (!SVGRenderingContext::createImageBuffer(repaintRect, absoluteTransform, clipperData->clipMaskImage, ColorSpaceDeviceRGB, Unaccelerated))
+    if (shouldCreateClipperMaskImage && !repaintRect.isEmpty()) {
+        if (!SVGRenderingContext::createImageBuffer(repaintRect, absoluteTransform, clipperMaskImage, ColorSpaceDeviceRGB, Unaccelerated))
             return false;
 
-        GraphicsContext* maskContext = clipperData->clipMaskImage->context();
+        GraphicsContext* maskContext = clipperMaskImage->context();
         ASSERT(maskContext);
 
         maskContext->concatCTM(animatedLocalTransform);
@@ -161,28 +160,27 @@ bool RenderSVGResourceClipper::applyClippingToContext(RenderElement& renderer, c
             if (!clipper->applyClippingToContext(*this, objectBoundingBox, repaintRect, maskContext))
                 return false;
 
-            succeeded = drawContentIntoMaskImage(clipperData, objectBoundingBox);
+            succeeded = drawContentIntoMaskImage(clipperMaskImage, objectBoundingBox);
             // The context restore applies the clipping on non-CG platforms.
         } else
-            succeeded = drawContentIntoMaskImage(clipperData, objectBoundingBox);
+            succeeded = drawContentIntoMaskImage(clipperMaskImage, objectBoundingBox);
 
         if (!succeeded)
-            clipperData->clipMaskImage.reset();
+            clipperMaskImage.reset();
     }
 
-    if (!clipperData->clipMaskImage)
+    if (!clipperMaskImage)
         return false;
 
-    SVGRenderingContext::clipToImageBuffer(context, absoluteTransform, repaintRect, clipperData->clipMaskImage, shouldCreateClipData);
+    SVGRenderingContext::clipToImageBuffer(context, absoluteTransform, repaintRect, clipperMaskImage, shouldCreateClipperMaskImage);
     return true;
 }
 
-bool RenderSVGResourceClipper::drawContentIntoMaskImage(ClipperData* clipperData, const FloatRect& objectBoundingBox)
+bool RenderSVGResourceClipper::drawContentIntoMaskImage(const ClipperMaskImage& clipperMaskImage, const FloatRect& objectBoundingBox)
 {
-    ASSERT(clipperData);
-    ASSERT(clipperData->clipMaskImage);
+    ASSERT(clipperMaskImage);
 
-    GraphicsContext* maskContext = clipperData->clipMaskImage->context();
+    GraphicsContext* maskContext = clipperMaskImage->context();
     ASSERT(maskContext);
 
     AffineTransform maskContentTransformation;
@@ -233,7 +231,7 @@ bool RenderSVGResourceClipper::drawContentIntoMaskImage(ClipperData* clipperData
         // In the case of a <use> element, we obtained its renderere above, to retrieve its clipRule.
         // We have to pass the <use> renderer itself to renderSubtreeToImageBuffer() to apply it's x/y/transform/etc. values when rendering.
         // So if isUseElement is true, refetch the childNode->renderer(), as renderer got overriden above.
-        SVGRenderingContext::renderSubtreeToImageBuffer(clipperData->clipMaskImage.get(), isUseElement ? *child.renderer() : *renderer, maskContentTransformation);
+        SVGRenderingContext::renderSubtreeToImageBuffer(clipperMaskImage.get(), isUseElement ? *child.renderer() : *renderer, maskContentTransformation);
     }
 
     view().frameView().setPaintBehavior(oldBehavior);
@@ -257,11 +255,9 @@ void RenderSVGResourceClipper::calculateClipContentRepaintRect()
     m_clipBoundaries = clipPathElement().animatedLocalTransform().mapRect(m_clipBoundaries);
 }
 
-ClipperData* RenderSVGResourceClipper::addRendererToClipper(const RenderObject& object)
+ClipperMaskImage& RenderSVGResourceClipper::addRendererToClipper(const RenderObject& object)
 {
-    if (!m_clipper.contains(&object))
-        m_clipper.set(&object, std::make_unique<ClipperData>());
-    return m_clipper.get(&object);
+    return m_clipper.add(&object, ClipperMaskImage()).iterator->value;
 }
 
 bool RenderSVGResourceClipper::hitTestClipContent(const FloatRect& objectBoundingBox, const FloatPoint& nodeAtPoint)
index 3d309ff..4415b94 100644 (file)
 
 namespace WebCore {
 
-struct ClipperData {
-    WTF_MAKE_FAST_ALLOCATED;
-public:
-    std::unique_ptr<ImageBuffer> clipMaskImage;
-};
+typedef std::unique_ptr<ImageBuffer> ClipperMaskImage;
 
 class RenderSVGResourceClipper final : public RenderSVGResourceContainer {
 public:
@@ -69,12 +65,12 @@ private:
     virtual const char* renderName() const override { return "RenderSVGResourceClipper"; }
 
     bool pathOnlyClipping(GraphicsContext*, const AffineTransform&, const FloatRect&);
-    bool drawContentIntoMaskImage(ClipperData*, const FloatRect& objectBoundingBox);
+    bool drawContentIntoMaskImage(const ClipperMaskImage&, const FloatRect& objectBoundingBox);
     void calculateClipContentRepaintRect();
-    ClipperData* addRendererToClipper(const RenderObject&);
+    ClipperMaskImage& addRendererToClipper(const RenderObject&);
 
     FloatRect m_clipBoundaries;
-    HashMap<const RenderObject*, std::unique_ptr<ClipperData>> m_clipper;
+    HashMap<const RenderObject*, ClipperMaskImage> m_clipper;
 };
 
 }