[SVGDom] Add clip-rule support

Currently we use 'fill-rule' when emitting clip paths.  This is wrong:
per spec [1], clip paths observe 'clip-rule', not 'fill-rule'.

[1] https://www.w3.org/TR/SVG/masking.html#ClipRuleProperty

Change-Id: Idf81de05e9601663c8dbc9856900ffa679daf4a5
Reviewed-on: https://skia-review.googlesource.com/57661
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Stephan Altmueller <stephana@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>

11 files changed, 35 insertions, 22 deletions

diff --git a/experimental/svg/model/SkSVGAttribute.cpp b/experimental/svg/model/SkSVGAttribute.cpp
index 6d318ca2f2..d16b479e1a 100644
--- a/experimental/svg/model/SkSVGAttribute.cpp
+++ b/experimental/svg/model/SkSVGAttribute.cpp
@@ -13,6 +13,7 @@ SkSVGPresentationAttributes SkSVGPresentationAttributes::MakeInitial() {
     result.fFill.set(SkSVGPaint(SkSVGColorType(SK_ColorBLACK)));
     result.fFillOpacity.set(SkSVGNumberType(1));
     result.fFillRule.set(SkSVGFillRule(SkSVGFillRule::Type::kNonZero));
+    result.fClipRule.set(SkSVGFillRule(SkSVGFillRule::Type::kNonZero));
 
     result.fStroke.set(SkSVGPaint(SkSVGPaint::Type::kNone));
     result.fStrokeLineCap.set(SkSVGLineCap(SkSVGLineCap::Type::kButt));
diff --git a/experimental/svg/model/SkSVGAttribute.h b/experimental/svg/model/SkSVGAttribute.h
index 6c88326420..9ba38f944c 100644
--- a/experimental/svg/model/SkSVGAttribute.h
+++ b/experimental/svg/model/SkSVGAttribute.h
@@ -15,6 +15,7 @@ class SkSVGRenderContext;
 
 enum class SkSVGAttribute {
     kClipPath,
+    kClipRule,
     kCx, // <circle>, <ellipse>, <radialGradient>: center x position
     kCy, // <circle>, <ellipse>, <radialGradient>: center y position
     kD,
@@ -62,6 +63,7 @@ struct SkSVGPresentationAttributes {
     SkTLazy<SkSVGPaint>      fFill;
     SkTLazy<SkSVGNumberType> fFillOpacity;
     SkTLazy<SkSVGFillRule>   fFillRule;
+    SkTLazy<SkSVGFillRule>   fClipRule;
 
     SkTLazy<SkSVGPaint>      fStroke;
     SkTLazy<SkSVGLineCap>    fStrokeLineCap;
diff --git a/experimental/svg/model/SkSVGDOM.cpp b/experimental/svg/model/SkSVGDOM.cpp
index 70db9330a4..6625e21705 100644
--- a/experimental/svg/model/SkSVGDOM.cpp
+++ b/experimental/svg/model/SkSVGDOM.cpp
@@ -279,6 +279,7 @@ struct AttrParseInfo {
 
 SortedDictionaryEntry<AttrParseInfo> gAttributeParseInfo[] = {
     { "clip-path"        , { SkSVGAttribute::kClipPath         , SetClipPathAttribute     }},
+    { "clip-rule"        , { SkSVGAttribute::kClipRule         , SetFillRuleAttribute     }},
     { "cx"               , { SkSVGAttribute::kCx               , SetLengthAttribute       }},
     { "cy"               , { SkSVGAttribute::kCy               , SetLengthAttribute       }},
     { "d"                , { SkSVGAttribute::kD                , SetPathDataAttribute     }},
diff --git a/experimental/svg/model/SkSVGNode.cpp b/experimental/svg/model/SkSVGNode.cpp
index 46d6423af2..dfd25bafbb 100644
--- a/experimental/svg/model/SkSVGNode.cpp
+++ b/experimental/svg/model/SkSVGNode.cpp
@@ -61,6 +61,10 @@ void SkSVGNode::setClipPath(const SkSVGClip& clip) {
     fPresentationAttributes.fClipPath.set(clip);
 }
 
+void SkSVGNode::setClipRule(const SkSVGFillRule& clipRule) {
+    fPresentationAttributes.fClipRule.set(clipRule);
+}
+
 void SkSVGNode::setFill(const SkSVGPaint& svgPaint) {
     fPresentationAttributes.fFill.set(svgPaint);
 }
@@ -99,6 +103,11 @@ void SkSVGNode::onSetAttribute(SkSVGAttribute attr, const SkSVGValue& v) {
             this->setClipPath(*clip);
         }
         break;
+    case SkSVGAttribute::kClipRule:
+        if (const SkSVGFillRuleValue* clipRule = v.as<SkSVGFillRuleValue>()) {
+            this->setClipRule(*clipRule);
+        }
+        break;
     case SkSVGAttribute::kFill:
         if (const SkSVGPaintValue* paint = v.as<SkSVGPaintValue>()) {
             this->setFill(*paint);
diff --git a/experimental/svg/model/SkSVGNode.h b/experimental/svg/model/SkSVGNode.h
index 86c98d3064..e6567134c8 100644
--- a/experimental/svg/model/SkSVGNode.h
+++ b/experimental/svg/model/SkSVGNode.h
@@ -50,6 +50,7 @@ public:
     void setAttribute(SkSVGAttribute, const SkSVGValue&);
 
     void setClipPath(const SkSVGClip&);
+    void setClipRule(const SkSVGFillRule&);
     void setFill(const SkSVGPaint&);
     void setFillOpacity(const SkSVGNumberType&);
     void setFillRule(const SkSVGFillRule&);
diff --git a/experimental/svg/model/SkSVGPath.cpp b/experimental/svg/model/SkSVGPath.cpp
index 7568744035..3c09ab9885 100644
--- a/experimental/svg/model/SkSVGPath.cpp
+++ b/experimental/svg/model/SkSVGPath.cpp
@@ -33,10 +33,9 @@ void SkSVGPath::onDraw(SkCanvas* canvas, const SkSVGLengthContext&, const SkPain
 }
 
 SkPath SkSVGPath::onAsPath(const SkSVGRenderContext& ctx) const {
-    // the computed fillType follows inheritance rules and needs to be applied at draw time.
-    fPath.setFillType(FillRuleToFillType(*ctx.presentationContext().fInherited.fFillRule.get()));
-
     SkPath path = fPath;
+    // clip-rule can be inherited and needs to be applied at clip time.
+    path.setFillType(ctx.presentationContext().fInherited.fClipRule.get()->asFillType());
     this->mapToParent(&path);
     return path;
 }
diff --git a/experimental/svg/model/SkSVGPoly.cpp b/experimental/svg/model/SkSVGPoly.cpp
index 479638e93f..e2bc5bc606 100644
--- a/experimental/svg/model/SkSVGPoly.cpp
+++ b/experimental/svg/model/SkSVGPoly.cpp
@@ -40,10 +40,11 @@ void SkSVGPoly::onDraw(SkCanvas* canvas, const SkSVGLengthContext&, const SkPain
 }
 
 SkPath SkSVGPoly::onAsPath(const SkSVGRenderContext& ctx) const {
-    // the computed fillType follows inheritance rules and needs to be applied at draw time.
-    fPath.setFillType(FillRuleToFillType(*ctx.presentationContext().fInherited.fFillRule.get()));
-
     SkPath path = fPath;
+
+    // clip-rule can be inherited and needs to be applied at clip time.
+    path.setFillType(ctx.presentationContext().fInherited.fClipRule.get()->asFillType());
+
     this->mapToParent(&path);
     return path;
 }
diff --git a/experimental/svg/model/SkSVGRenderContext.cpp b/experimental/svg/model/SkSVGRenderContext.cpp
index f071d9696f..d8c269a56f 100644
--- a/experimental/svg/model/SkSVGRenderContext.cpp
+++ b/experimental/svg/model/SkSVGRenderContext.cpp
@@ -203,6 +203,13 @@ void commitToPaint<SkSVGAttribute::kFillRule>(const SkSVGPresentationAttributes&
     // Not part of the SkPaint state; applied to the path at render time.
 }
 
+template <>
+void commitToPaint<SkSVGAttribute::kClipRule>(const SkSVGPresentationAttributes&,
+                                              const SkSVGRenderContext&,
+                                              SkSVGPresentationContext*) {
+    // Not part of the SkPaint state; applied to the path at clip time.
+}
+
 } // anonymous ns
 
 SkSVGPresentationContext::SkSVGPresentationContext()
@@ -275,6 +282,7 @@ void SkSVGRenderContext::applyPresentationAttributes(const SkSVGPresentationAttr
     ApplyLazyInheritedAttribute(Fill);
     ApplyLazyInheritedAttribute(FillOpacity);
     ApplyLazyInheritedAttribute(FillRule);
+    ApplyLazyInheritedAttribute(ClipRule);
     ApplyLazyInheritedAttribute(Stroke);
     ApplyLazyInheritedAttribute(StrokeLineCap);
     ApplyLazyInheritedAttribute(StrokeLineJoin);
diff --git a/experimental/svg/model/SkSVGShape.cpp b/experimental/svg/model/SkSVGShape.cpp
index 9351a2f096..7394bc67b4 100644
--- a/experimental/svg/model/SkSVGShape.cpp
+++ b/experimental/svg/model/SkSVGShape.cpp
@@ -11,8 +11,7 @@
 SkSVGShape::SkSVGShape(SkSVGTag t) : INHERITED(t) {}
 
 void SkSVGShape::onRender(const SkSVGRenderContext& ctx) const {
-    const SkPath::FillType fillType =
-        FillRuleToFillType(*ctx.presentationContext().fInherited.fFillRule.get());
+    const auto fillType = ctx.presentationContext().fInherited.fFillRule.get()->asFillType();
 
     // TODO: this approach forces duplicate geometry resolution in onDraw(); refactor to avoid.
     if (const SkPaint* fillPaint = ctx.fillPaint()) {
@@ -27,15 +26,3 @@ void SkSVGShape::onRender(const SkSVGRenderContext& ctx) const {
 void SkSVGShape::appendChild(sk_sp<SkSVGNode>) {
     SkDebugf("cannot append child nodes to an SVG shape.\n");
 }
-
-SkPath::FillType SkSVGShape::FillRuleToFillType(const SkSVGFillRule& fillRule) {
-    switch (fillRule.type()) {
-    case SkSVGFillRule::Type::kNonZero:
-        return SkPath::kWinding_FillType;
-    case SkSVGFillRule::Type::kEvenOdd:
-        return SkPath::kEvenOdd_FillType;
-    default:
-        SkASSERT(false);
-        return SkPath::kWinding_FillType;
-    }
-}
diff --git a/experimental/svg/model/SkSVGShape.h b/experimental/svg/model/SkSVGShape.h
index f40609e8f8..98d9b4bcbb 100644
--- a/experimental/svg/model/SkSVGShape.h
+++ b/experimental/svg/model/SkSVGShape.h
@@ -28,8 +28,6 @@ protected:
     virtual void onDraw(SkCanvas*, const SkSVGLengthContext&, const SkPaint&,
                         SkPath::FillType) const = 0;
 
-    static SkPath::FillType FillRuleToFillType(const SkSVGFillRule&);
-
 private:
     typedef SkSVGTransformableNode INHERITED;
 };
diff --git a/experimental/svg/model/SkSVGTypes.h b/experimental/svg/model/SkSVGTypes.h
index 42a09b93b5..c83fd7d10b 100644
--- a/experimental/svg/model/SkSVGTypes.h
+++ b/experimental/svg/model/SkSVGTypes.h
@@ -10,6 +10,7 @@
 
 #include "SkColor.h"
 #include "SkMatrix.h"
+#include "SkPath.h"
 #include "SkPoint.h"
 #include "SkRect.h"
 #include "SkScalar.h"
@@ -238,6 +239,11 @@ public:
 
     Type type() const { return fType; }
 
+    SkPath::FillType asFillType() const {
+        SkASSERT(fType != Type::kInherit); // should never be called for unresolved values.
+        return fType == Type::kEvenOdd ? SkPath::kEvenOdd_FillType : SkPath::kWinding_FillType;
+    }
+
 private:
     Type fType;
 };