Prefer unprefixed clip-path to (webkit) prefixed

Source/core/rendering/svg/SVGRenderingContext.cpp

Index: Source/core/rendering/svg/SVGRenderingContext.cpp
diff --git a/Source/core/rendering/svg/SVGRenderingContext.cpp b/Source/core/rendering/svg/SVGRenderingContext.cpp
index 0fab1ae1987455247669e0b6d7e3be13170624b0..b9a769120f8cdef65a4f7a0360bc44bb6b4c7477 100644
--- a/Source/core/rendering/svg/SVGRenderingContext.cpp
+++ b/Source/core/rendering/svg/SVGRenderingContext.cpp
@@ -86,7 +86,6 @@ void SVGRenderingContext::prepareToRenderSVGContent(RenderObject* object, PaintI
 
     m_object = object;
     m_paintInfo = &paintInfo;
-    m_filter = 0;
 
     RenderStyle* style = m_object->style();
     ASSERT(style);
@@ -96,10 +95,10 @@ void SVGRenderingContext::prepareToRenderSVGContent(RenderObject* object, PaintI
     // Setup transparency layers before setting up SVG resources!
     bool isRenderingMask = SVGRenderSupport::isRenderingClipPathAsMaskImage(*m_object);
     // RenderLayer takes care of root opacity.
-    float opacity = (object->isSVGRoot() || isRenderingMask) ? 1 : style->opacity();
-    bool hasBlendMode = style->hasBlendMode() && !isRenderingMask;
+    float opacity = object->isSVGRoot() ? 1 : style->opacity();
+    bool hasBlendMode = style->hasBlendMode();
 
-    if (opacity < 1 || hasBlendMode || style->hasIsolation()) {
+    if (!isRenderingMask && (opacity < 1 || hasBlendMode || style->hasIsolation())) {

[f(malita), 2014/10/31 17:24:46]

This shuffling has side effects: hasIsolation() no longer forces a layer when
isRenderingMask == true. Is this intentional?

[fs, 2014/10/31 19:29:22]

I made (maybe too brief?) note about this in the last paragraph of the
description. Since a clipPath is "geometry only", 'isolation' will (should) have
no effect. (Since it helps define grouping for blending.)

[f(malita), 2014/10/31 19:36:35]

Thanks, I missed that. LGTM.

         FloatRect paintInvalidationRect = m_object->paintInvalidationRectInLocalCoordinates();
         m_paintInfo->context->clip(paintInvalidationRect);
 
@@ -119,39 +118,37 @@ void SVGRenderingContext::prepareToRenderSVGContent(RenderObject* object, PaintI
         m_renderingFlags |= EndOpacityLayer;
     }
 
-    ClipPathOperation* clipPathOperation = style->clipPath();
-    if (clipPathOperation && clipPathOperation->type() == ClipPathOperation::SHAPE) {
-        ShapeClipPathOperation* clipPath = toShapeClipPathOperation(clipPathOperation);
-        m_paintInfo->context->clipPath(clipPath->path(object->objectBoundingBox()), clipPath->windRule());
-    }
-
     SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(m_object);
-    if (!resources) {
-        if (svgStyle.hasFilter())
+
+    // Prefer a 'clipper' (non-prefixed 'clip-path') to a 'clip shape'
+    // ('-webkit-clip-path'), until these two properties end up being merged
+    // properly.
+    if (RenderSVGResourceClipper* clipper = resources ? resources->clipper() : nullptr) {
+        if (!clipper->applyStatefulResource(m_object, m_paintInfo->context, m_clipperState))
             return;
+        m_clipper = clipper;
+        m_renderingFlags |= PostApplyResources;
+    } else {
+        ClipPathOperation* clipPathOperation = style->clipPath();
+        if (clipPathOperation && clipPathOperation->type() == ClipPathOperation::SHAPE) {
+            ShapeClipPathOperation* clipPath = toShapeClipPathOperation(clipPathOperation);
+            m_paintInfo->context->clipPath(clipPath->path(object->objectBoundingBox()), clipPath->windRule());
+        }
+    }
 
+    if (isRenderingMask) {
         m_renderingFlags |= RenderingPrepared;
         return;
     }
 
-    if (!isRenderingMask) {
+    if (resources) {
         if (RenderSVGResourceMasker* masker = resources->masker()) {
             if (!masker->prepareEffect(m_object, m_paintInfo->context))
                 return;
             m_masker = masker;
             m_renderingFlags |= PostApplyResources;
         }
-    }
 
-    RenderSVGResourceClipper* clipper = resources->clipper();
-    if (!clipPathOperation && clipper) {
-        if (!clipper->applyStatefulResource(m_object, m_paintInfo->context, m_clipperState))
-            return;
-        m_clipper = clipper;
-        m_renderingFlags |= PostApplyResources;
-    }
-
-    if (!isRenderingMask) {
         m_filter = resources->filter();
         if (m_filter) {
             m_savedContext = m_paintInfo->context;
@@ -168,6 +165,10 @@ void SVGRenderingContext::prepareToRenderSVGContent(RenderObject* object, PaintI
             // be drawn.
             m_paintInfo->rect = IntRect(m_filter->drawingRegion(m_object));
         }
+    } else {
+        // Broken filter disables rendering.
+        if (svgStyle.hasFilter())
+            return;
     }
 
     m_renderingFlags |= RenderingPrepared;