ImageBuffer-less SVG masking and clipping.

Source/core/rendering/svg/RenderSVGResourceClipper.cpp

Index: Source/core/rendering/svg/RenderSVGResourceClipper.cpp
diff --git a/Source/core/rendering/svg/RenderSVGResourceClipper.cpp b/Source/core/rendering/svg/RenderSVGResourceClipper.cpp
index 9da09f9d9f0c299df32978dd0d74385de5bcab12..e5a9b827b279aa630b5a71feb3a1b14cebd38ee4 100644
--- a/Source/core/rendering/svg/RenderSVGResourceClipper.cpp
+++ b/Source/core/rendering/svg/RenderSVGResourceClipper.cpp
@@ -28,11 +28,13 @@
 #include "SVGNames.h"
 #include "core/page/FrameView.h"
 #include "core/platform/graphics/GraphicsContextStateSaver.h"
+#include "core/platform/graphics/skia/SkiaUtils.h"

[Stephen White, 2013/08/28 14:11:10]

We're calling into skia-specific code from outside platform/ here. :( Would it
help if we changed GraphicsContext::saveLayer() to take Blink types?

[f(malita), 2013/08/28 19:54:38]

Fixed, see other comment.

 #include "core/rendering/HitTestResult.h"
 #include "core/rendering/svg/SVGRenderingContext.h"
 #include "core/rendering/svg/SVGResources.h"
 #include "core/rendering/svg/SVGResourcesCache.h"
 #include "core/svg/SVGUseElement.h"
+#include "wtf/TemporaryChange.h"
 
 namespace WebCore {
 
@@ -40,35 +42,25 @@ RenderSVGResourceType RenderSVGResourceClipper::s_resourceType = ClipperResource
 
 RenderSVGResourceClipper::RenderSVGResourceClipper(SVGClipPathElement* node)
     : RenderSVGResourceContainer(node)
+    , m_inClipExpansion(false)
 {
 }
 
 RenderSVGResourceClipper::~RenderSVGResourceClipper()
 {
-    if (m_clipper.isEmpty())
-        return;
-
-    deleteAllValues(m_clipper);
-    m_clipper.clear();
 }
 
 void RenderSVGResourceClipper::removeAllClientsFromCache(bool markForInvalidation)
 {
     m_clipBoundaries = FloatRect();
-    if (!m_clipper.isEmpty()) {
-        deleteAllValues(m_clipper);
-        m_clipper.clear();
-    }
-
+    m_clipperMap.clear();
     markAllClientsForInvalidation(markForInvalidation ? LayoutAndBoundariesInvalidation : ParentOnlyInvalidation);
 }
 
 void RenderSVGResourceClipper::removeClientFromCache(RenderObject* client, bool markForInvalidation)
 {
     ASSERT(client);
-    if (m_clipper.contains(client))
-        delete m_clipper.take(client);
-
+    m_clipperMap.remove(client);
     markClientForInvalidation(client, markForInvalidation ? BoundariesInvalidation : ParentOnlyInvalidation);
 }
 
@@ -144,74 +136,96 @@ bool RenderSVGResourceClipper::pathOnlyClipping(GraphicsContext* context, const
     return true;
 }
 
-bool RenderSVGResourceClipper::applyClippingToContext(RenderObject* object, const FloatRect& objectBoundingBox,
+bool RenderSVGResourceClipper::applyClippingToContext(RenderObject* target, const FloatRect& targetBoundingBox,
                                                       const FloatRect& repaintRect, GraphicsContext* context)
 {
-    bool missingClipperData = !m_clipper.contains(object);
-    if (missingClipperData)
-        m_clipper.set(object, new ClipperData);
+    ASSERT(target);
+    ASSERT(context);
+    ASSERT_WITH_SECURITY_IMPLICATION(!needsLayout());
 
-    bool shouldCreateClipData = false;
-    AffineTransform animatedLocalTransform = toSVGClipPathElement(node())->animatedLocalTransform();
-    ClipperData* clipperData = m_clipper.get(object);
-    if (!clipperData->clipMaskImage) {
-        if (pathOnlyClipping(context, animatedLocalTransform, objectBoundingBox))
-            return true;
-        shouldCreateClipData = true;
-    }
+    if (repaintRect.isEmpty() || m_inClipExpansion)
+        return false;
+    TemporaryChange<bool> inClipExpansionChange(m_inClipExpansion, true);
 
-    AffineTransform absoluteTransform;
-    SVGRenderingContext::calculateTransformationToOutermostCoordinateSystem(object, absoluteTransform);
+    // add(obj, 0) idiom -> lookup & add if not found.

[pdr., 2013/08/28 03:32:36]

(not for this patch)
We should add HashMap::lookupAndAddIfNotFound so we don't all have to
independently discover this trick.

+    OwnPtr<ClipperData>& clipperData = m_clipperMap.add(target, nullptr).iterator->value;
+    if (!clipperData)
+        clipperData = adoptPtr(new ClipperData);
 
-    if (shouldCreateClipData && !repaintRect.isEmpty()) {
-        if (!SVGRenderingContext::createImageBuffer(repaintRect, absoluteTransform, clipperData->clipMaskImage, Unaccelerated))
-            return false;
+    // First, try to apply the clip as a clipPath.
+    AffineTransform animatedLocalTransform = toSVGClipPathElement(node())->animatedLocalTransform();
+    if (pathOnlyClipping(context, animatedLocalTransform, targetBoundingBox)) {

[pdr., 2013/08/28 03:32:36]

Nit: could you rename this tryPathOnlyClipping?

[f(malita), 2013/08/28 19:54:38]

Done.

+        clipperData->pathOnly = true;
+        return true;
+    }
 
-        GraphicsContext* maskContext = clipperData->clipMaskImage->context();
-        ASSERT(maskContext);
+    // Fall back to masking.
+    clipperData->pathOnly = false;
+    SkRect layerRect = WebCoreFloatRectToSKRect(repaintRect);
 
-        maskContext->concatCTM(animatedLocalTransform);
+    // Mask layer start
+    context->saveLayer(&layerRect, 0);
+    {
+        GraphicsContextStateSaver maskContentSaver(*context);
+        context->concatCTM(animatedLocalTransform);
 
         // clipPath can also be clipped by another clipPath.
         SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(this);
-        RenderSVGResourceClipper* clipper;
-        bool succeeded;
-        if (resources && (clipper = resources->clipper())) {
-            GraphicsContextStateSaver stateSaver(*maskContext);
-
-            if (!clipper->applyClippingToContext(this, objectBoundingBox, repaintRect, maskContext))
+        RenderSVGResourceClipper* selfClipper = 0;

[pdr., 2013/08/28 03:32:36]

Nit: clipPathClipper? or clipOnClip?
I was confused that this isn't the case where we clip ourselves.

[f(malita), 2013/08/28 19:54:38]

Done.

+        if (resources && (selfClipper = resources->clipper())) {
+            if (!selfClipper->applyClippingToContext(this, targetBoundingBox, repaintRect, context)) {
+                // FIXME: Awkward state micro-management. Ideally, GraphicsContextStateSaver should
+                //   a) pop saveLayers also
+                //   b) pop multiple states if needed (similarly to SkCanvas::restoreToCount())
+                // Then we should be able to replace this mess with a single, top-level GCSS.
+                maskContentSaver.restore();
+                context->restoreLayer();
                 return false;
+            }
+        }
 
-            succeeded = drawContentIntoMaskImage(clipperData, objectBoundingBox);
-            // The context restore applies the clipping on non-CG platforms.
-        } else
-            succeeded = drawContentIntoMaskImage(clipperData, objectBoundingBox);
+        drawMaskContent(context, targetBoundingBox);
 
-        if (!succeeded)
-            clipperData->clipMaskImage.clear();
+        if (selfClipper)
+            selfClipper->postApplyResource(this, context, ApplyToDefaultMode, 0, 0);
     }
 
-    if (!clipperData->clipMaskImage)
-        return false;
+    // Start the content layer in SrcIn mode. This ensures that it gets masked against the current
+    // (mask content) layer on restore.
+    SkPaint contentPaint;
+    contentPaint.setXfermodeMode(SkXfermode::kSrcIn_Mode);
+    context->saveLayer(&layerRect, &contentPaint);
 
-    SVGRenderingContext::clipToImageBuffer(context, absoluteTransform, repaintRect, clipperData->clipMaskImage, missingClipperData);
     return true;
 }
 
-bool RenderSVGResourceClipper::drawContentIntoMaskImage(ClipperData* clipperData, const FloatRect& objectBoundingBox)
-{
-    ASSERT(frame());
+void RenderSVGResourceClipper::postApplyResource(RenderObject* object, GraphicsContext*& context,
+    unsigned short resourceMode, const Path*, const RenderSVGShape*) {
+    ASSERT_UNUSED(resourceMode, resourceMode == ApplyToDefaultMode);
+
+    ClipperData* clipperData = m_clipperMap.get(object);
     ASSERT(clipperData);
-    ASSERT(clipperData->clipMaskImage);
 
-    GraphicsContext* maskContext = clipperData->clipMaskImage->context();
-    ASSERT(maskContext);
+    // Path-only clipping, no layers to restore.
+    if (clipperData->pathOnly)
+        return;
+
+    // Transfer content layer -> mask layer (SrcIn)
+    context->restoreLayer();
+    // Transfer mask layer -> bg layer (SrcOver)
+    context->restoreLayer();
+}
+
+void RenderSVGResourceClipper::drawMaskContent(GraphicsContext* context, const FloatRect& targetBoundingBox)
+{
+    ASSERT(frame());
+    ASSERT(context);
 
+    // Adjust the mask image context according to the target objectBoundingBox.
     AffineTransform maskContentTransformation;
     if (toSVGClipPathElement(node())->clipPathUnitsCurrentValue() == SVGUnitTypes::SVG_UNIT_TYPE_OBJECTBOUNDINGBOX) {
-        maskContentTransformation.translate(objectBoundingBox.x(), objectBoundingBox.y());
-        maskContentTransformation.scaleNonUniform(objectBoundingBox.width(), objectBoundingBox.height());
-        maskContext->concatCTM(maskContentTransformation);
+        maskContentTransformation = makeMapBetweenRects(FloatRect(0, 0, 1, 1), targetBoundingBox);

[pdr., 2013/08/28 03:32:36]

Can you switch this back to the previous method? makeMapBetweenRects is harder
to understand and does unnecessary math.

[f(malita), 2013/08/28 19:54:38]

Done.

+        context->concatCTM(maskContentTransformation);
     }
 
     // Switch to a paint behavior where all children of this <clipPath> will be rendered using special constraints:
@@ -222,15 +236,11 @@ bool RenderSVGResourceClipper::drawContentIntoMaskImage(ClipperData* clipperData
     PaintBehavior oldBehavior = frame()->view()->paintBehavior();
     frame()->view()->setPaintBehavior(oldBehavior | PaintBehaviorRenderingSVGMask);
 
-    // Draw all clipPath children into a global mask.
     for (Node* childNode = node()->firstChild(); childNode; childNode = childNode->nextSibling()) {
         RenderObject* renderer = childNode->renderer();
         if (!childNode->isSVGElement() || !renderer)
             continue;
-        if (renderer->needsLayout()) {
-            frame()->view()->setPaintBehavior(oldBehavior);
-            return false;
-        }
+
         RenderStyle* style = renderer->style();
         if (!style || style->display() == NONE || style->visibility() != VISIBLE)
             continue;
@@ -250,16 +260,15 @@ bool RenderSVGResourceClipper::drawContentIntoMaskImage(ClipperData* clipperData
         if (!renderer->isSVGShape() && !renderer->isSVGText())
             continue;
 
-        maskContext->setFillRule(newClipRule);
+        context->setFillRule(newClipRule);
 
-        // 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 ? childNode->renderer() : renderer, maskContentTransformation);
+        if (isUseElement)
+            renderer = childNode->renderer();
+
+        SVGRenderingContext::renderSubtree(context, renderer, maskContentTransformation);
     }
 
     frame()->view()->setPaintBehavior(oldBehavior);
-    return true;
 }
 
 void RenderSVGResourceClipper::calculateClipContentRepaintRect()