Ensure display items are unique when multiple nested clip paths are used

third_party/WebKit/Source/core/paint/SVGClipPainter.cpp

Index: third_party/WebKit/Source/core/paint/SVGClipPainter.cpp
diff --git a/third_party/WebKit/Source/core/paint/SVGClipPainter.cpp b/third_party/WebKit/Source/core/paint/SVGClipPainter.cpp
index 244719831325e44ef7ce14476052c622c4b38446..806cad631f9bcec3e41a5356a486cf659604acad 100644
--- a/third_party/WebKit/Source/core/paint/SVGClipPainter.cpp
+++ b/third_party/WebKit/Source/core/paint/SVGClipPainter.cpp
@@ -6,6 +6,7 @@
 
 #include "core/dom/ElementTraversal.h"
 #include "core/layout/svg/LayoutSVGResourceClipper.h"
+#include "core/layout/svg/SVGLayoutSupport.h"
 #include "core/layout/svg/SVGResources.h"
 #include "core/layout/svg/SVGResourcesCache.h"
 #include "core/paint/LayoutObjectDrawingRecorder.h"
@@ -15,6 +16,7 @@
 #include "platform/graphics/paint/CompositingRecorder.h"
 #include "platform/graphics/paint/DrawingDisplayItem.h"
 #include "platform/graphics/paint/PaintController.h"
+#include "platform/graphics/paint/SkPictureBuilder.h"
 
 namespace blink {
 
@@ -66,22 +68,11 @@ bool SVGClipPainter::prepareEffect(const LayoutObject& target, const FloatRect&
     // Begin compositing the clip mask.
     CompositingRecorder::beginCompositing(context, target, SkXfermode::kSrcOver_Mode, 1, &paintInvalidationRect);
     {
-        TransformRecorder recorder(context, target, animatedLocalTransform);
-
-        // clipPath can also be clipped by another clipPath.
-        SVGResources* resources = SVGResourcesCache::cachedResourcesForLayoutObject(&m_clip);
-        LayoutSVGResourceClipper* clipPathClipper = resources ? resources->clipper() : 0;
-        ClipperState clipPathClipperState = ClipperNotApplied;
-        if (clipPathClipper && !SVGClipPainter(*clipPathClipper).prepareEffect(m_clip, targetBoundingBox, paintInvalidationRect, context, clipPathClipperState)) {
+        if (!drawClipAsMask(context, target, targetBoundingBox, paintInvalidationRect, animatedLocalTransform)) {
             // End the clip mask's compositor.
             CompositingRecorder::endCompositing(context, target);
             return false;
         }
-
-        drawClipMaskContent(context, target, targetBoundingBox, paintInvalidationRect);
-
-        if (clipPathClipper)
-            SVGClipPainter(*clipPathClipper).finishEffect(m_clip, context, clipPathClipperState);
     }
 
     // Masked content layer start.
@@ -109,19 +100,44 @@ void SVGClipPainter::finishEffect(const LayoutObject& target, GraphicsContext& c
     }
 }
 
-void SVGClipPainter::drawClipMaskContent(GraphicsContext& context, const LayoutObject& layoutObject, const FloatRect& targetBoundingBox, const FloatRect& targetPaintInvalidationRect)
+bool SVGClipPainter::drawClipAsMask(GraphicsContext& context, const LayoutObject& layoutObject, const FloatRect& targetBoundingBox, const FloatRect& targetPaintInvalidationRect, const AffineTransform& localTransform)
 {
-    AffineTransform contentTransformation;
-    RefPtr<const SkPicture> clipContentPicture = m_clip.createContentPicture(contentTransformation, targetBoundingBox, context);
-
     if (LayoutObjectDrawingRecorder::useCachedDrawingIfPossible(context, layoutObject, DisplayItem::SVGClip))
-        return;
+        return true;
+
+    SkPictureBuilder maskPictureBuilder(targetPaintInvalidationRect, nullptr, &context);
+    GraphicsContext& maskContext = maskPictureBuilder.context();
+    {
+        TransformRecorder recorder(maskContext, layoutObject, localTransform);
+
+        // Create a clipPathClipper if this clipPath is clipped by another clipPath.
+        SVGResources* resources = SVGResourcesCache::cachedResourcesForLayoutObject(&m_clip);
+        LayoutSVGResourceClipper* clipPathClipper = resources ? resources->clipper() : nullptr;
+        ClipperState clipPathClipperState = ClipperNotApplied;
+        if (clipPathClipper && !SVGClipPainter(*clipPathClipper).prepareEffect(m_clip, targetBoundingBox, targetPaintInvalidationRect, maskContext, clipPathClipperState))
+            return false;
+
+        {
+            AffineTransform contentTransform;
+            if (m_clip.clipPathUnits() == SVGUnitTypes::SVG_UNIT_TYPE_OBJECTBOUNDINGBOX) {
+                contentTransform.translate(targetBoundingBox.x(), targetBoundingBox.y());
+                contentTransform.scaleNonUniform(targetBoundingBox.width(), targetBoundingBox.height());
+            }
+            SubtreeContentTransformScope contentTransformScope(contentTransform);

[chrishtr, 2016/03/29 18:45:40]

This used to be allocated only if m_clipContentPicture was null. Is it still
correct?

[chrishtr, 2016/03/29 19:05:31]

Per offline conversation: SubtreeContentTransformScope has no effect here; it
just sets a hidden global
variable.  (gross).

Add a TODO in a followup to fix this?

+
+            TransformRecorder contentTransformRecorder(maskContext, layoutObject, contentTransform);
+            RefPtr<const SkPicture> clipContentPicture = m_clip.createContentPicture();
+            maskContext.getPaintController().createAndAppend<DrawingDisplayItem>(layoutObject, DisplayItem::SVGClip, clipContentPicture.get());
+        }
+
+        if (clipPathClipper)
+            SVGClipPainter(*clipPathClipper).finishEffect(m_clip, maskContext, clipPathClipperState);
+    }
 
     LayoutObjectDrawingRecorder drawingRecorder(context, layoutObject, DisplayItem::SVGClip, targetPaintInvalidationRect);
-    context.save();
-    context.concatCTM(contentTransformation);
-    context.drawPicture(clipContentPicture.get());
-    context.restore();
+    RefPtr<SkPicture> maskPicture = maskPictureBuilder.endRecording();
+    context.drawPicture(maskPicture.get());
+    return true;
 }
 
 } // namespace blink