[SVGDom] Improved transform parsing

Update 'transform' attribute parsing to a more robust, SkSVGAttributeParser-based implementation.

R=robertphillips@google.com,stephana@google.com
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2220933003

Review-Url: https://codereview.chromium.org/2220933003

experimental/svg/model/SkSVGAttributeParser.cpp

@@ -211,3 +211,144 @@ bool SkSVGAttributeParser::parseViewBox(SkSVGViewBoxType* vb) {
     }
     return parsedValue && this->parseEOSToken();
 }
+
+template <typename Func, typename T>
+bool SkSVGAttributeParser::parseParenthesized(const char* prefix, Func f, T* result) {
+    this->parseWSToken();
+    if (prefix && !this->parseExpectedStringToken(prefix)) {
+        return false;
+    }
+    this->parseWSToken();
+    if (!this->parseExpectedStringToken("(")) {
+        return false;
+    }
+    this->parseWSToken();
+
+    if (!f(result)) {
+        return false;
+    }
+    this->parseWSToken();
+
+    return this->parseExpectedStringToken(")");
+}
+
+bool SkSVGAttributeParser::parseMatrixToken(SkMatrix* matrix) {
+    return this->parseParenthesized("matrix", [this](SkMatrix* m) -> bool {
+        SkScalar scalars[6];
+        for (int i = 0; i < 6; ++i) {
+            if (!(this->parseScalarToken(scalars + i) &&
+                  (i > 4 || this->parseSepToken()))) {
+                return false;
+            }
+        }
+
+        m->setAll(scalars[0], scalars[2], scalars[4], scalars[1], scalars[3], scalars[5], 0, 0, 1);
+        return true;
+    }, matrix);
+}
+
+bool SkSVGAttributeParser::parseTranslateToken(SkMatrix* matrix) {
+    return this->parseParenthesized("translate", [this](SkMatrix* m) -> bool {
+        SkScalar tx, ty;
+        this->parseWSToken();
+        if (!this->parseScalarToken(&tx)) {
+            return false;
+        }
+
+        if (!(this->parseSepToken() && this->parseScalarToken(&ty))) {
+            ty = tx;
+        }
+
+        m->setTranslate(tx, ty);
+        return true;
+    }, matrix);
+}
+
+bool SkSVGAttributeParser::parseScaleToken(SkMatrix* matrix) {
+    return this->parseParenthesized("scale", [this](SkMatrix* m) -> bool {
+        SkScalar sx, sy;
+        if (!this->parseScalarToken(&sx)) {
+            return false;
+        }
+
+        if (!(this->parseSepToken() && this->parseScalarToken(&sy))) {
+            sy = sx;
+        }
+
+        m->setScale(sx, sy);
+        return true;
+    }, matrix);
+}
+
+bool SkSVGAttributeParser::parseRotateToken(SkMatrix* matrix) {
+    return this->parseParenthesized("rotate", [this](SkMatrix* m) -> bool {
+        SkScalar angle;
+        if (!this->parseScalarToken(&angle)) {
+            return false;
+        }
+
+        SkScalar cx = 0;
+        SkScalar cy = 0;
+        // optional [<cx> <cy>]
+        if (this->parseSepToken() && this->parseScalarToken(&cx)) {
+            if (!(this->parseSepToken() && this->parseScalarToken(&cy))) {
+                return false;
+            }
+        }
+
+        m->setRotate(angle, cx, cy);
+        return true;
+    }, matrix);
+}
+
+bool SkSVGAttributeParser::parseSkewXToken(SkMatrix* matrix) {
+    return this->parseParenthesized("skewX", [this](SkMatrix* m) -> bool {
+        SkScalar angle;
+        if (!this->parseScalarToken(&angle)) {
+            return false;
+        }
+        m->setSkewX(angle);
+        return true;
+    }, matrix);
+}
+
+bool SkSVGAttributeParser::parseSkewYToken(SkMatrix* matrix) {
+    return this->parseParenthesized("skewY", [this](SkMatrix* m) -> bool {
+        SkScalar angle;
+        if (!this->parseScalarToken(&angle)) {
+            return false;
+        }
+        m->setSkewY(angle);
+        return true;
+    }, matrix);
+}
+
+// https://www.w3.org/TR/SVG/coords.html#TransformAttribute
+bool SkSVGAttributeParser::parseTransform(SkSVGTransformType* t) {
+    SkMatrix matrix = SkMatrix::I();
+
+    bool parsed = false;
+    while (true) {
+        SkMatrix m;
+
+        if (!( this->parseMatrixToken(&m)
+            || this->parseTranslateToken(&m)
+            || this->parseScaleToken(&m)
+            || this->parseRotateToken(&m)
+            || this->parseSkewXToken(&m)
+            || this->parseSkewYToken(&m))) {
+            break;
+        }
+
+        matrix.preConcat(m);
+        parsed = true;
+    }
+
+    this->parseWSToken();
+    if (!parsed || !this->parseEOSToken()) {
+        return false;
+    }
+
+    *t = SkSVGTransformType(matrix);
+    return true;
+}

experimental/svg/model/SkSVGAttributeParser.h

@@ -18,6 +18,7 @@ public:
     bool parseNumber(SkSVGNumberType*);
     bool parseLength(SkSVGLength*);
     bool parseViewBox(SkSVGViewBoxType*);
+    bool parseTransform(SkSVGTransformType*);
 
 private:
     // Stack-only
@@ -37,6 +38,19 @@ private:
     bool parseNamedColorToken(SkColor*);
     bool parseHexColorToken(SkColor*);
 
+    // Transform helpers
+    bool parseMatrixToken(SkMatrix*);
+    bool parseTranslateToken(SkMatrix*);
+    bool parseScaleToken(SkMatrix*);
+    bool parseRotateToken(SkMatrix*);
+    bool parseSkewXToken(SkMatrix*);
+    bool parseSkewYToken(SkMatrix*);
+
+    // Parses a sequence of 'WS* <prefix> WS* (<nested>)', where the nested sequence
+    // is handled by the passed functor.
+    template <typename Func, typename T>
+    bool parseParenthesized(const char* prefix, Func, T* result);
+
     // The current position in the input string.
     const char* fCurPos;
 

experimental/svg/model/SkSVGDOM.cpp

@@ -7,7 +7,6 @@
 
 #include "SkCanvas.h"
 #include "SkDOM.h"
-#include "SkParse.h"
 #include "SkParsePath.h"
 #include "SkString.h"
 #include "SkSVGAttributeParser.h"
@@ -24,53 +23,6 @@
 
 namespace {
 
-const char* ParseScalarPair(const char* str, SkScalar v[2]) {
-    str = SkParse::FindScalar(str, v);
-    if (str) {
-        const char* second = SkParse::FindScalar(str, v + 1);
-        if (!second) {
-            v[1] = v[0];
-        } else {
-            str = second;
-        }
-    }
-
-    return str;
-}
-
-SkMatrix ParseTransform(const char* str) {
-    SkMatrix m = SkMatrix::I();
-
-    // FIXME: real parser
-    if (!strncmp(str, "matrix(", 7)) {
-        SkScalar values[6];
-        str = SkParse::FindScalars(str + 7, values, 6);
-        if (str) {
-            m.setAffine(values);
-        }
-    } else if (!strncmp(str, "scale(", 6)) {
-        SkScalar values[2];
-        str = ParseScalarPair(str + 6, values);
-        if (str) {
-            m.setScale(values[0], values[1]);
-        }
-    } else if (!strncmp(str, "translate(", 10)) {
-        SkScalar values[2];
-        str = ParseScalarPair(str + 10, values);
-        if (str) {
-            m.setTranslate(values[0], values[1]);
-        }
-    } else if (!strncmp(str, "rotate(", 7)) {
-        SkScalar value;
-        str = SkParse::FindScalar(str + 7, &value);
-        if (str) {
-            m.setRotate(value);
-        }
-    }
-
-    return m;
-}
-
 bool SetPaintAttribute(const sk_sp<SkSVGNode>& node, SkSVGAttribute attr,
                        const char* stringValue) {
     SkSVGColorType color;
@@ -96,7 +48,13 @@ bool SetPathDataAttribute(const sk_sp<SkSVGNode>& node, SkSVGAttribute attr,
 
 bool SetTransformAttribute(const sk_sp<SkSVGNode>& node, SkSVGAttribute attr,
                            const char* stringValue) {
-    node->setAttribute(attr, SkSVGTransformValue(ParseTransform(stringValue)));
+    SkSVGTransformType transform;
+    SkSVGAttributeParser parser(stringValue);
+    if (!parser.parseTransform(&transform)) {
+        return false;
+    }
+
+    node->setAttribute(attr, SkSVGTransformValue(transform));
     return true;
 }
 

experimental/svg/model/SkSVGTransformableNode.cpp

@@ -12,13 +12,13 @@
 
 SkSVGTransformableNode::SkSVGTransformableNode(SkSVGTag tag)
     : INHERITED(tag)
-    , fMatrix(SkMatrix::I()) { }
+    , fTransform(SkMatrix::I()) { }
 
 
 bool SkSVGTransformableNode::onPrepareToRender(SkSVGRenderContext* ctx) const {
-    if (!fMatrix.isIdentity()) {
+    if (!fTransform.value().isIdentity()) {
         ctx->canvas()->save();
-        ctx->canvas()->concat(fMatrix);
+        ctx->canvas()->concat(fTransform);
     }
 
     return this->INHERITED::onPrepareToRender(ctx);

experimental/svg/model/SkSVGTransformableNode.h

@@ -15,7 +15,7 @@ class SkSVGTransformableNode : public SkSVGNode {
 public:
     virtual ~SkSVGTransformableNode() = default;
 
-    void setTransform(const SkMatrix& m) { fMatrix = m; }
+    void setTransform(const SkSVGTransformType& t) { fTransform = t; }
 
 protected:
     SkSVGTransformableNode(SkSVGTag);
@@ -26,7 +26,7 @@ protected:
 
 private:
     // FIXME: should be sparse
-    SkMatrix fMatrix;
+    SkSVGTransformType fTransform;
 
     typedef SkSVGNode INHERITED;
 };

experimental/svg/model/SkSVGTypes.h

@@ -9,6 +9,7 @@
 #define SkSVGTypes_DEFINED
 
 #include "SkColor.h"
+#include "SkMatrix.h"
 #include "SkRect.h"
 #include "SkScalar.h"
 #include "SkTypes.h"
@@ -29,9 +30,10 @@ private:
     T fValue;
 };
 
-using SkSVGColorType   = SkSVGPrimitiveTypeWrapper<SkColor >;
-using SkSVGNumberType  = SkSVGPrimitiveTypeWrapper<SkScalar>;
-using SkSVGViewBoxType = SkSVGPrimitiveTypeWrapper<SkRect  >;
+using SkSVGColorType      = SkSVGPrimitiveTypeWrapper<SkColor >;
+using SkSVGNumberType     = SkSVGPrimitiveTypeWrapper<SkScalar>;
+using SkSVGViewBoxType    = SkSVGPrimitiveTypeWrapper<SkRect  >;
+using SkSVGTransformType  = SkSVGPrimitiveTypeWrapper<SkMatrix>;
 
 class SkSVGLength {
 public:

experimental/svg/model/SkSVGValue.h

@@ -61,10 +61,10 @@ private:
     typedef SkSVGValue INHERITED;
 };
 
-using SkSVGColorValue     = SkSVGWrapperValue<SkSVGColorType  , SkSVGValue::Type::kColor    >;
-using SkSVGLengthValue    = SkSVGWrapperValue<SkSVGLength     , SkSVGValue::Type::kLength   >;
-using SkSVGPathValue      = SkSVGWrapperValue<SkPath          , SkSVGValue::Type::kPath     >;
-using SkSVGTransformValue = SkSVGWrapperValue<SkMatrix        , SkSVGValue::Type::kTransform>;
-using SkSVGViewBoxValue   = SkSVGWrapperValue<SkSVGViewBoxType, SkSVGValue::Type::kViewBox  >;
+using SkSVGColorValue     = SkSVGWrapperValue<SkSVGColorType    , SkSVGValue::Type::kColor    >;
+using SkSVGLengthValue    = SkSVGWrapperValue<SkSVGLength       , SkSVGValue::Type::kLength   >;
+using SkSVGPathValue      = SkSVGWrapperValue<SkPath            , SkSVGValue::Type::kPath     >;
+using SkSVGTransformValue = SkSVGWrapperValue<SkSVGTransformType, SkSVGValue::Type::kTransform>;
+using SkSVGViewBoxValue   = SkSVGWrapperValue<SkSVGViewBoxType  , SkSVGValue::Type::kViewBox  >;
 
 #endif // SkSVGValue_DEFINED