[svg] Use new code path for rest of presentation attrs

This is another necessary step in order to remove kInherit from all of
the base SVG type enums.

Change-Id: I2185e744f7b27369f7bad36591f896d3a9982b42
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335817
Commit-Queue: Tyler Denniston <tdenniston@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>

modules/svg/include/SkSVGAttributeParser.h

@@ -16,8 +16,6 @@ class SkSVGAttributeParser : public SkNoncopyable {
 public:
     SkSVGAttributeParser(const char[]);
 
-    bool parseColor(SkSVGColorType*);
-    bool parseNumber(SkSVGNumberType*);
     bool parseInteger(SkSVGIntegerType*);
     bool parseViewBox(SkSVGViewBoxType*);
     bool parsePoints(SkSVGPointsType*);
@@ -26,7 +24,6 @@ class SkSVGAttributeParser : public SkNoncopyable {
 
     // TODO: Migrate all parse*() functions to this style (and delete the old version)
     //      so they can be used by parse<T>():
-    bool parse(SkSVGNumberType* v) { return parseNumber(v); }
     bool parse(SkSVGIntegerType* v) { return parseInteger(v); }
 
     template <typename T> using ParseResult = SkTLazy<T>;

modules/svg/include/SkSVGNode.h

@@ -94,32 +94,31 @@ class SkSVGNode : public SkRefCnt {
     // TODO: consolidate with existing setAttribute
     virtual bool parseAndSetAttribute(const char* name, const char* value);
 
-    void setColor(const SkSVGColorType&);
-    void setFillOpacity(const SkSVGNumberType&);
-    void setOpacity(const SkSVGNumberType&);
-    void setStrokeDashOffset(const SkSVGLength&);
-    void setStrokeOpacity(const SkSVGNumberType&);
-    void setStrokeMiterLimit(const SkSVGNumberType&);
-    void setStrokeWidth(const SkSVGLength&);
-
     // inherited
-    SVG_PRES_ATTR(ClipRule       , SkSVGFillRule  , true)
-    SVG_PRES_ATTR(FillRule       , SkSVGFillRule  , true)
-    SVG_PRES_ATTR(Fill           , SkSVGPaint     , true)
-    SVG_PRES_ATTR(FontFamily     , SkSVGFontFamily, true)
-    SVG_PRES_ATTR(FontSize       , SkSVGFontSize  , true)
-    SVG_PRES_ATTR(FontStyle      , SkSVGFontStyle , true)
-    SVG_PRES_ATTR(FontWeight     , SkSVGFontWeight, true)
-    SVG_PRES_ATTR(Stroke         , SkSVGPaint     , true)
-    SVG_PRES_ATTR(StrokeDashArray, SkSVGDashArray , true)
-    SVG_PRES_ATTR(StrokeLineCap  , SkSVGLineCap   , true)
-    SVG_PRES_ATTR(StrokeLineJoin , SkSVGLineJoin  , true)
-    SVG_PRES_ATTR(TextAnchor     , SkSVGTextAnchor, true)
-    SVG_PRES_ATTR(Visibility     , SkSVGVisibility, true)
+    SVG_PRES_ATTR(ClipRule        , SkSVGFillRule  , true)
+    SVG_PRES_ATTR(Color           , SkSVGColorType , true)
+    SVG_PRES_ATTR(FillRule        , SkSVGFillRule  , true)
+    SVG_PRES_ATTR(Fill            , SkSVGPaint     , true)
+    SVG_PRES_ATTR(FillOpacity     , SkSVGNumberType, true)
+    SVG_PRES_ATTR(FontFamily      , SkSVGFontFamily, true)
+    SVG_PRES_ATTR(FontSize        , SkSVGFontSize  , true)
+    SVG_PRES_ATTR(FontStyle       , SkSVGFontStyle , true)
+    SVG_PRES_ATTR(FontWeight      , SkSVGFontWeight, true)
+    SVG_PRES_ATTR(Stroke          , SkSVGPaint     , true)
+    SVG_PRES_ATTR(StrokeDashArray , SkSVGDashArray , true)
+    SVG_PRES_ATTR(StrokeDashOffset, SkSVGLength    , true)
+    SVG_PRES_ATTR(StrokeLineCap   , SkSVGLineCap   , true)
+    SVG_PRES_ATTR(StrokeLineJoin  , SkSVGLineJoin  , true)
+    SVG_PRES_ATTR(StrokeMiterLimit, SkSVGNumberType, true)
+    SVG_PRES_ATTR(StrokeOpacity   , SkSVGNumberType, true)
+    SVG_PRES_ATTR(StrokeWidth     , SkSVGLength    , true)
+    SVG_PRES_ATTR(TextAnchor      , SkSVGTextAnchor, true)
+    SVG_PRES_ATTR(Visibility      , SkSVGVisibility, true)
 
     // not inherited
-    SVG_PRES_ATTR(ClipPath       , SkSVGClip      , false)
-    SVG_PRES_ATTR(Filter         , SkSVGFilterType, false)
+    SVG_PRES_ATTR(ClipPath        , SkSVGClip      , false)
+    SVG_PRES_ATTR(Filter          , SkSVGFilterType, false)
+    SVG_PRES_ATTR(Opacity         , SkSVGNumberType, false)
 
 protected:
     SkSVGNode(SkSVGTag);
@@ -138,7 +137,7 @@ class SkSVGNode : public SkRefCnt {
 
     virtual SkPath onAsPath(const SkSVGRenderContext&) const = 0;
 
-    virtual void onSetAttribute(SkSVGAttribute, const SkSVGValue&);
+    virtual void onSetAttribute(SkSVGAttribute, const SkSVGValue&) {}
 
     virtual bool hasChildren() const { return false; }
 

modules/svg/src/SkSVGAttributeParser.cpp

@@ -224,7 +224,8 @@ bool SkSVGAttributeParser::parseRGBColorToken(SkColor* c) {
 // https://www.w3.org/TR/SVG11/types.html#DataTypeColor
 // And https://www.w3.org/TR/CSS2/syndata.html#color-units for the alternative
 // forms supported by SVG (e.g. RGB percentages).
-bool SkSVGAttributeParser::parseColor(SkSVGColorType* color) {
+template <>
+bool SkSVGAttributeParser::parse(SkSVGColorType* color) {
     SkColor c;
 
     // consume preceding whitespace
@@ -286,7 +287,8 @@ bool SkSVGAttributeParser::parse(SkSVGStringType* result) {
 }
 
 // https://www.w3.org/TR/SVG11/types.html#DataTypeNumber
-bool SkSVGAttributeParser::parseNumber(SkSVGNumberType* number) {
+template <>
+bool SkSVGAttributeParser::parse(SkSVGNumberType* number) {
     // consume WS
     this->parseWSToken();
 
@@ -506,7 +508,7 @@ bool SkSVGAttributeParser::parse(SkSVGPaint* paint) {
     SkSVGColorType c;
     SkSVGStringType iri;
     bool parsedValue = false;
-    if (this->parseColor(&c)) {
+    if (this->parse(&c)) {
         *paint = SkSVGPaint(c);
         parsedValue = true;
     } else if (this->parseExpectedStringToken("none")) {
@@ -599,7 +601,7 @@ bool SkSVGAttributeParser::parse(SkSVGLineJoin* join) {
 bool SkSVGAttributeParser::parseStopColor(SkSVGStopColor* stopColor) {
     SkSVGColorType c;
     bool parsedValue = false;
-    if (this->parseColor(&c)) {
+    if (this->parse(&c)) {
         *stopColor = SkSVGStopColor(c);
         parsedValue = true;
     } else if (this->parseExpectedStringToken("currentColor")) {

modules/svg/src/SkSVGDOM.cpp

@@ -41,18 +41,6 @@
 
 namespace {
 
-bool SetColorAttribute(const sk_sp<SkSVGNode>& node, SkSVGAttribute attr,
-                       const char* stringValue) {
-    SkSVGColorType color;
-    SkSVGAttributeParser parser(stringValue);
-    if (!parser.parseColor(&color)) {
-        return false;
-    }
-
-    node->setAttribute(attr, SkSVGColorValue(color));
-    return true;
-}
-
 bool SetIRIAttribute(const sk_sp<SkSVGNode>& node, SkSVGAttribute attr,
                       const char* stringValue) {
     auto parseResult = SkSVGAttributeParser::parse<SkSVGIRI>(stringValue);
@@ -107,13 +95,12 @@ bool SetLengthAttribute(const sk_sp<SkSVGNode>& node, SkSVGAttribute attr,
 
 bool SetNumberAttribute(const sk_sp<SkSVGNode>& node, SkSVGAttribute attr,
                         const char* stringValue) {
-    SkSVGNumberType number;
-    SkSVGAttributeParser parser(stringValue);
-    if (!parser.parseNumber(&number)) {
+    auto parseResult = SkSVGAttributeParser::parse<SkSVGNumberType>(stringValue);
+    if (!parseResult.isValid()) {
         return false;
     }
 
-    node->setAttribute(attr, SkSVGNumberValue(number));
+    node->setAttribute(attr, SkSVGNumberValue(*parseResult));
     return true;
 }
 
@@ -255,19 +242,16 @@ struct AttrParseInfo {
 };
 
 SortedDictionaryEntry<AttrParseInfo> gAttributeParseInfo[] = {
-    { "color"              , { SkSVGAttribute::kColor            , SetColorAttribute        }},
     { "cx"                 , { SkSVGAttribute::kCx               , SetLengthAttribute       }},
     { "cy"                 , { SkSVGAttribute::kCy               , SetLengthAttribute       }},
     { "d"                  , { SkSVGAttribute::kD                , SetPathDataAttribute     }},
-    { "fill-opacity"       , { SkSVGAttribute::kFillOpacity      , SetNumberAttribute       }},
     { "filterUnits"        , { SkSVGAttribute::kFilterUnits      ,
                                SetObjectBoundingBoxUnitsAttribute }},
     // focal point x & y
     { "fx"                 , { SkSVGAttribute::kFx               , SetLengthAttribute       }},
     { "fy"                 , { SkSVGAttribute::kFy               , SetLengthAttribute       }},
     { "height"             , { SkSVGAttribute::kHeight           , SetLengthAttribute       }},
     { "offset"             , { SkSVGAttribute::kOffset           , SetLengthAttribute       }},
-    { "opacity"            , { SkSVGAttribute::kOpacity          , SetNumberAttribute       }},
     { "patternTransform"   , { SkSVGAttribute::kPatternTransform , SetTransformAttribute    }},
     { "points"             , { SkSVGAttribute::kPoints           , SetPointsAttribute       }},
     { "preserveAspectRatio", { SkSVGAttribute::kPreserveAspectRatio,
@@ -277,10 +261,6 @@ SortedDictionaryEntry<AttrParseInfo> gAttributeParseInfo[] = {
     { "ry"                 , { SkSVGAttribute::kRy               , SetLengthAttribute       }},
     { "stop-color"         , { SkSVGAttribute::kStopColor        , SetStopColorAttribute    }},
     { "stop-opacity"       , { SkSVGAttribute::kStopOpacity      , SetNumberAttribute       }},
-    { "stroke-dashoffset"  , { SkSVGAttribute::kStrokeDashOffset , SetLengthAttribute       }},
-    { "stroke-miterlimit"  , { SkSVGAttribute::kStrokeMiterLimit , SetNumberAttribute       }},
-    { "stroke-opacity"     , { SkSVGAttribute::kStrokeOpacity    , SetNumberAttribute       }},
-    { "stroke-width"       , { SkSVGAttribute::kStrokeWidth      , SetLengthAttribute       }},
     { "style"              , { SkSVGAttribute::kUnknown          , SetStyleAttributes       }},
     { "text"               , { SkSVGAttribute::kText             , SetStringAttribute       }},
     { "transform"          , { SkSVGAttribute::kTransform        , SetTransformAttribute    }},

modules/svg/src/SkSVGFeColorMatrix.cpp

@@ -98,13 +98,13 @@ sk_sp<SkImageFilter> SkSVGFeColorMatrix::onMakeImageFilter(const SkSVGRenderCont
 
 template <> bool SkSVGAttributeParser::parse(SkSVGFeColorMatrixValues* values) {
     SkSVGNumberType value;
-    if (!this->parseNumber(&value)) {
+    if (!this->parse(&value)) {
         return false;
     }
 
     values->push_back(value);
     while (true) {
-        if (!this->parseNumber(&value) || values->count() >= 20) {
+        if (!this->parse(&value) || values->count() >= 20) {
             break;
         }
         values->push_back(value);

modules/svg/src/SkSVGFeTurbulence.cpp

@@ -28,13 +28,13 @@ template <>
 bool SkSVGAttributeParser::parse<SkSVGFeTurbulenceBaseFrequency>(
         SkSVGFeTurbulenceBaseFrequency* freq) {
     SkSVGNumberType freqX;
-    if (!this->parseNumber(&freqX)) {
+    if (!this->parse(&freqX)) {
         return false;
     }
 
     SkSVGNumberType freqY;
     this->parseCommaWspToken();
-    if (this->parseNumber(&freqY)) {
+    if (this->parse(&freqY)) {
         *freq = SkSVGFeTurbulenceBaseFrequency(freqX, freqY);
     } else {
         *freq = SkSVGFeTurbulenceBaseFrequency(freqX, freqX);

modules/svg/src/SkSVGNode.cpp

@@ -76,101 +76,34 @@ void SetInheritedByDefault(SkTLazy<T>& presentation_attribute, const T& value) {
     }
 }
 
-void SkSVGNode::setColor(const SkSVGColorType& color) {
-    // TODO: Color should be inherited by default
-    fPresentationAttributes.fColor.set(color);
-}
-
-void SkSVGNode::setFillOpacity(const SkSVGNumberType& opacity) {
-    fPresentationAttributes.fFillOpacity.set(SkSVGNumberType(SkTPin<SkScalar>(opacity, 0, 1)));
-}
-
-void SkSVGNode::setOpacity(const SkSVGNumberType& opacity) {
-    fPresentationAttributes.fOpacity.set(SkSVGNumberType(SkTPin<SkScalar>(opacity, 0, 1)));
-}
-
-void SkSVGNode::setStrokeDashOffset(const SkSVGLength& dashOffset) {
-    fPresentationAttributes.fStrokeDashOffset.set(dashOffset);
-}
-
-void SkSVGNode::setStrokeOpacity(const SkSVGNumberType& opacity) {
-    fPresentationAttributes.fStrokeOpacity.set(SkSVGNumberType(SkTPin<SkScalar>(opacity, 0, 1)));
-}
-
-void SkSVGNode::setStrokeMiterLimit(const SkSVGNumberType& ml) {
-    fPresentationAttributes.fStrokeMiterLimit.set(ml);
-}
-
-void SkSVGNode::setStrokeWidth(const SkSVGLength& strokeWidth) {
-    fPresentationAttributes.fStrokeWidth.set(strokeWidth);
-}
-
-void SkSVGNode::onSetAttribute(SkSVGAttribute attr, const SkSVGValue& v) {
-    switch (attr) {
-    case SkSVGAttribute::kColor:
-        if (const SkSVGColorValue* color = v.as<SkSVGColorValue>()) {
-            this->setColor(*color);
-        }
-        break;
-    case SkSVGAttribute::kFillOpacity:
-        if (const SkSVGNumberValue* opacity = v.as<SkSVGNumberValue>()) {
-            this->setFillOpacity(*opacity);
-        }
-        break;
-    case SkSVGAttribute::kOpacity:
-        if (const SkSVGNumberValue* opacity = v.as<SkSVGNumberValue>()) {
-            this->setOpacity(*opacity);
-        }
-        break;
-    case SkSVGAttribute::kStrokeDashOffset:
-        if (const SkSVGLengthValue* dashOffset= v.as<SkSVGLengthValue>()) {
-            this->setStrokeDashOffset(*dashOffset);
-        }
-        break;
-    case SkSVGAttribute::kStrokeOpacity:
-        if (const SkSVGNumberValue* opacity = v.as<SkSVGNumberValue>()) {
-            this->setStrokeOpacity(*opacity);
-        }
-        break;
-    case SkSVGAttribute::kStrokeMiterLimit:
-        if (const SkSVGNumberValue* miterLimit = v.as<SkSVGNumberValue>()) {
-            this->setStrokeMiterLimit(*miterLimit);
-        }
-        break;
-    case SkSVGAttribute::kStrokeWidth:
-        if (const SkSVGLengthValue* strokeWidth = v.as<SkSVGLengthValue>()) {
-            this->setStrokeWidth(*strokeWidth);
-        }
-        break;
-    default:
-#if defined(SK_VERBOSE_SVG_PARSING)
-        SkDebugf("attribute ID <%d> ignored for node <%d>\n", attr, fTag);
-#endif
-        break;
-    }
-}
-
 bool SkSVGNode::parseAndSetAttribute(const char* n, const char* v) {
 #define PARSE_AND_SET(svgName, attrName)                                                        \
     this->set##attrName(                                                                        \
             SkSVGAttributeParser::parseProperty<decltype(fPresentationAttributes.f##attrName)>( \
                     svgName, n, v))
 
-    return PARSE_AND_SET(   "clip-path"       , ClipPath)
-           || PARSE_AND_SET("clip-rule"       , ClipRule)
-           || PARSE_AND_SET("fill"            , Fill)
-           || PARSE_AND_SET("fill-rule"       , FillRule)
-           || PARSE_AND_SET("filter"          , Filter)
-           || PARSE_AND_SET("font-family"     , FontFamily)
-           || PARSE_AND_SET("font-size"       , FontSize)
-           || PARSE_AND_SET("font-style"      , FontStyle)
-           || PARSE_AND_SET("font-weight"     , FontWeight)
-           || PARSE_AND_SET("stroke"          , Stroke)
-           || PARSE_AND_SET("stroke-dasharray", StrokeDashArray)
-           || PARSE_AND_SET("stroke-linecap"  , StrokeLineCap)
-           || PARSE_AND_SET("stroke-linejoin" , StrokeLineJoin)
-           || PARSE_AND_SET("text-anchor"     , TextAnchor)
-           || PARSE_AND_SET("visibility"      , Visibility);
+    return PARSE_AND_SET(   "clip-path"        , ClipPath)
+           || PARSE_AND_SET("clip-rule"        , ClipRule)
+           || PARSE_AND_SET("color"            , Color)
+           || PARSE_AND_SET("fill"             , Fill)
+           || PARSE_AND_SET("fill-opacity"     , FillOpacity)
+           || PARSE_AND_SET("fill-rule"        , FillRule)
+           || PARSE_AND_SET("filter"           , Filter)
+           || PARSE_AND_SET("font-family"      , FontFamily)
+           || PARSE_AND_SET("font-size"        , FontSize)
+           || PARSE_AND_SET("font-style"       , FontStyle)
+           || PARSE_AND_SET("font-weight"      , FontWeight)
+           || PARSE_AND_SET("opacity"          , Opacity)
+           || PARSE_AND_SET("stroke"           , Stroke)
+           || PARSE_AND_SET("stroke-dasharray" , StrokeDashArray)
+           || PARSE_AND_SET("stroke-dashoffset", StrokeDashOffset)
+           || PARSE_AND_SET("stroke-linecap"   , StrokeLineCap)
+           || PARSE_AND_SET("stroke-linejoin"  , StrokeLineJoin)
+           || PARSE_AND_SET("stroke-miterlimit", StrokeMiterLimit)
+           || PARSE_AND_SET("stroke-opacity"   , StrokeOpacity)
+           || PARSE_AND_SET("stroke-width"     , StrokeWidth)
+           || PARSE_AND_SET("text-anchor"      , TextAnchor)
+           || PARSE_AND_SET("visibility"       , Visibility);
 
 #undef PARSE_AND_SET
 }

modules/svg/src/SkSVGRenderContext.cpp

@@ -128,7 +128,7 @@ void applySvgPaint(const SkSVGRenderContext& ctx, const SkSVGPaint& svgPaint, Sk
 }
 
 inline uint8_t opacity_to_alpha(SkScalar o) {
-    return SkTo<uint8_t>(SkScalarRoundToInt(o * 255));
+    return SkTo<uint8_t>(SkScalarRoundToInt(SkTPin<SkScalar>(o, 0, 1) * 255));
 }
 
 // Commit the selected attribute to the paint cache.