Skip to content

Commit 5715499

Browse files
tdennistonSkia Commit-Bot
tdenniston
authored and
Skia Commit-Bot
committed
[svg] Add plumbing for bottom-up attribute parsing
One significant source of boilerplate in the SVG frontend is the plumbing to ensure type safety all the way down from the XML layer to the SkSVGNode layer. This is mostly an artifact of the top-down parsing approach currently used by the SkDOM -> SkSVGDom building process. One way to help remove some boilerplate is to perform attribute parsing bottom-up, where each SVG node knows how to parse and populate its own attribute values from a string-valued KV pair. Additionally, bottom-up parsing allows us to support the case of the same SVG attribute name having different meanings on different nodes (e.g. the "type" attribute has different meaning on <feTurbulence> versus <feColorMatrix>). This CL adds some initial work to start us down that road, and ports the attributes previously added for <feTurbulence> to use the new code path. Change-Id: I2973cfab96891475d05ebf1228117626ca48ef4d Reviewed-on: https://skia-review.googlesource.com/c/skia/+/331477 Commit-Queue: Tyler Denniston <tdenniston@google.com> Reviewed-by: Florin Malita <fmalita@chromium.org>
1 parent 450eb04 commit 5715499

File tree

9 files changed

+121
-132
lines changed

9 files changed

+121
-132
lines changed

‎modules/svg/include/SkSVGAttribute.h‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,6 @@ enum class SkSVGAttribute {
2323
kFill,
2424
kFillOpacity,
2525
kFillRule,
26-
kFeTurbulenceBaseFrequency,
27-
kFeTurbulenceNumOctaves,
28-
kFeTurbulenceSeed,
29-
kFeTurbulenceType,
3026
kFilter,
3127
kFilterUnits,
3228
kFontFamily,

‎modules/svg/include/SkSVGAttributeParser.h‎

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
#include "include/private/SkNoncopyable.h"
1212
#include "modules/svg/include/SkSVGTypes.h"
13+
#include "src/core/SkTLazy.h"
1314

1415
class SkSVGAttributeParser : public SkNoncopyable {
1516
public:
@@ -36,15 +37,49 @@ class SkSVGAttributeParser : public SkNoncopyable {
3637
bool parseDashArray(SkSVGDashArray*);
3738
bool parsePreserveAspectRatio(SkSVGPreserveAspectRatio*);
3839

39-
bool parseFeTurbulenceBaseFrequency(SkSVGFeTurbulenceBaseFrequency*);
40-
bool parseFeTurbulenceType(SkSVGFeTurbulenceType*);
41-
4240
bool parseFontFamily(SkSVGFontFamily*);
4341
bool parseFontSize(SkSVGFontSize*);
4442
bool parseFontStyle(SkSVGFontStyle*);
4543
bool parseFontWeight(SkSVGFontWeight*);
4644
bool parseTextAnchor(SkSVGTextAnchor*);
4745

46+
bool parseEOSToken();
47+
bool parseCommaWspToken();
48+
bool parseExpectedStringToken(const char*);
49+
50+
// TODO: Migrate all parse*() functions to this style (and delete the old version)
51+
// so they can be used by parse<T>():
52+
bool parse(SkSVGNumberType* v) { return parseNumber(v); }
53+
bool parse(SkSVGIntegerType* v) { return parseInteger(v); }
54+
55+
template <typename T> using ParseResult = SkTLazy<T>;
56+
57+
template <typename T>
58+
static ParseResult<T> parse(const char* expectedName,
59+
const char* name,
60+
const char* value,
61+
bool (*parseFnc)(const char*, T*)) {
62+
if (strcmp(name, expectedName) != 0) {
63+
return ParseResult<T>();
64+
}
65+
66+
T parsedValue;
67+
if (parseFnc(value, &parsedValue)) {
68+
return ParseResult<T>(&parsedValue);
69+
}
70+
71+
return ParseResult<T>();
72+
}
73+
74+
template <typename T>
75+
static ParseResult<T> parse(const char* expectedName, const char* name, const char* value) {
76+
const auto parseFnc = +[](const char* str, T* v) {
77+
SkSVGAttributeParser parser(str);
78+
return parser.parse(v);
79+
};
80+
return parse(expectedName, name, value, parseFnc);
81+
}
82+
4883
private:
4984
// Stack-only
5085
void* operator new(size_t) = delete;
@@ -54,10 +89,7 @@ class SkSVGAttributeParser : public SkNoncopyable {
5489
bool advanceWhile(F func);
5590

5691
bool parseWSToken();
57-
bool parseEOSToken();
5892
bool parseSepToken();
59-
bool parseCommaWspToken();
60-
bool parseExpectedStringToken(const char*);
6193
bool parseScalarToken(SkScalar*);
6294
bool parseInt32Token(int32_t*);
6395
bool parseHexToken(uint32_t*);

‎modules/svg/include/SkSVGFeTurbulence.h‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,17 @@ class SkSVGFeTurbulence : public SkSVGFe {
2626
SkSVGFeTurbulenceType(SkSVGFeTurbulenceType::Type::kTurbulence))
2727

2828
protected:
29-
void onSetAttribute(SkSVGAttribute, const SkSVGValue&) override;
30-
3129
sk_sp<SkImageFilter> onMakeImageFilter(const SkSVGRenderContext&,
3230
const SkSVGFilterContext&) const override;
3331

32+
bool parseAndSetAttribute(const char*, const char*) override;
33+
3434
private:
3535
SkSVGFeTurbulence() : INHERITED(SkSVGTag::kFeTurbulence) {}
3636

37+
static bool parse(const char*, SkSVGFeTurbulenceBaseFrequency*);
38+
static bool parse(const char*, SkSVGFeTurbulenceType*);
39+
3740
using INHERITED = SkSVGFe;
3841
};
3942

‎modules/svg/include/SkSVGNode.h‎

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
#include "include/core/SkRefCnt.h"
1212
#include "modules/svg/include/SkSVGAttribute.h"
13+
#include "modules/svg/include/SkSVGAttributeParser.h"
1314

1415
class SkCanvas;
1516
class SkMatrix;
@@ -80,6 +81,9 @@ class SkSVGNode : public SkRefCnt {
8081
void setAttribute(SkSVGAttribute, const SkSVGValue&);
8182
bool setAttribute(const char* attributeName, const char* attributeValue);
8283

84+
// TODO: consolidate with existing setAttribute
85+
virtual bool parseAndSetAttribute(const char* name, const char* value);
86+
8387
void setClipPath(const SkSVGClip&);
8488
void setClipRule(const SkSVGFillRule&);
8589
void setColor(const SkSVGColorType&);
@@ -140,12 +144,22 @@ class SkSVGNode : public SkRefCnt {
140144

141145
#undef SVG_PRES_ATTR // presentation attributes are only defined for the base class
142146

143-
#define SVG_ATTR(attr_name, attr_type, attr_default) \
144-
private: \
145-
attr_type f##attr_name = attr_default; \
146-
public: \
147-
const attr_type& get##attr_name() const { return f##attr_name; } \
148-
void set##attr_name(const attr_type& a) { f##attr_name = a; } \
147+
#define SVG_ATTR(attr_name, attr_type, attr_default) \
148+
private: \
149+
attr_type f##attr_name = attr_default; \
150+
bool set##attr_name( \
151+
const SkSVGAttributeParser::ParseResult<attr_type>& pr) { \
152+
if (pr.isValid()) { this->set##attr_name(*pr); } \
153+
return pr.isValid(); \
154+
} \
155+
bool set##attr_name( \
156+
SkSVGAttributeParser::ParseResult<attr_type>&& pr) { \
157+
if (pr.isValid()) { this->set##attr_name(std::move(*pr)); } \
158+
return pr.isValid(); \
159+
} \
160+
public: \
161+
const attr_type& get##attr_name() const { return f##attr_name; } \
162+
void set##attr_name(const attr_type& a) { f##attr_name = a; } \
149163
void set##attr_name(attr_type&& a) { f##attr_name = std::move(a); }
150164

151165
#endif // SkSVGNode_DEFINED

‎modules/svg/include/SkSVGValue.h‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,12 @@ class SkSVGValue : public SkNoncopyable {
2121
kClip,
2222
kColor,
2323
kDashArray,
24-
kFeTurbulenceBaseFrequency,
25-
kFeTurbulenceType,
2624
kFillRule,
2725
kFilter,
2826
kFontFamily,
2927
kFontSize,
3028
kFontStyle,
3129
kFontWeight,
32-
kInteger,
3330
kLength,
3431
kLineCap,
3532
kLineJoin,
@@ -97,7 +94,6 @@ using SkSVGViewBoxValue = SkSVGWrapperValue<SkSVGViewBoxType , SkSVGValue:
9794
using SkSVGPaintValue = SkSVGWrapperValue<SkSVGPaint , SkSVGValue::Type::kPaint >;
9895
using SkSVGLineCapValue = SkSVGWrapperValue<SkSVGLineCap , SkSVGValue::Type::kLineCap >;
9996
using SkSVGLineJoinValue = SkSVGWrapperValue<SkSVGLineJoin , SkSVGValue::Type::kLineJoin >;
100-
using SkSVGIntegerValue = SkSVGWrapperValue<SkSVGIntegerType , SkSVGValue::Type::kInteger >;
10197
using SkSVGNumberValue = SkSVGWrapperValue<SkSVGNumberType , SkSVGValue::Type::kNumber >;
10298
using SkSVGPointsValue = SkSVGWrapperValue<SkSVGPointsType , SkSVGValue::Type::kPoints >;
10399
using SkSVGStringValue = SkSVGWrapperValue<SkSVGStringType , SkSVGValue::Type::kString >;
@@ -119,10 +115,4 @@ using SkSVGPreserveAspectRatioValue = SkSVGWrapperValue<SkSVGPreserveAspectRa
119115
using SkSVGObjectBoundingBoxUnitsValue = SkSVGWrapperValue<SkSVGObjectBoundingBoxUnits,
120116
SkSVGValue::Type::kObjectBoundingBoxUnits>;
121117

122-
using SkSVGFeTurbulenceBaseFrequencyValue =
123-
SkSVGWrapperValue<SkSVGFeTurbulenceBaseFrequency,
124-
SkSVGValue::Type::kFeTurbulenceBaseFrequency>;
125-
using SkSVGFeTurbulenceTypeValue =
126-
SkSVGWrapperValue<SkSVGFeTurbulenceType, SkSVGValue::Type::kFeTurbulenceType>;
127-
128118
#endif // SkSVGValue_DEFINED

‎modules/svg/src/SkSVGAttributeParser.cpp‎

Lines changed: 5 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -56,25 +56,25 @@ inline bool SkSVGAttributeParser::advanceWhile(F f) {
5656
return fCurPos != initial;
5757
}
5858

59-
inline bool SkSVGAttributeParser::parseEOSToken() {
59+
bool SkSVGAttributeParser::parseEOSToken() {
6060
return is_eos(*fCurPos);
6161
}
6262

63-
inline bool SkSVGAttributeParser::parseSepToken() {
63+
bool SkSVGAttributeParser::parseSepToken() {
6464
return this->advanceWhile(is_sep);
6565
}
6666

67-
inline bool SkSVGAttributeParser::parseWSToken() {
67+
bool SkSVGAttributeParser::parseWSToken() {
6868
return this->advanceWhile(is_ws);
6969
}
7070

71-
inline bool SkSVGAttributeParser::parseCommaWspToken() {
71+
bool SkSVGAttributeParser::parseCommaWspToken() {
7272
// comma-wsp:
7373
// (wsp+ comma? wsp*) | (comma wsp*)
7474
return this->parseWSToken() || this->parseExpectedStringToken(",");
7575
}
7676

77-
inline bool SkSVGAttributeParser::parseExpectedStringToken(const char* expected) {
77+
bool SkSVGAttributeParser::parseExpectedStringToken(const char* expected) {
7878
const char* c = fCurPos;
7979

8080
while (*c && *expected && *c == *expected) {
@@ -927,34 +927,3 @@ bool SkSVGAttributeParser::parsePreserveAspectRatio(SkSVGPreserveAspectRatio* pa
927927

928928
return parsedValue && this->parseEOSToken();
929929
}
930-
931-
bool SkSVGAttributeParser::parseFeTurbulenceBaseFrequency(SkSVGFeTurbulenceBaseFrequency* freq) {
932-
SkSVGNumberType freqX;
933-
if (!this->parseNumber(&freqX)) {
934-
return false;
935-
}
936-
937-
SkSVGNumberType freqY;
938-
this->parseCommaWspToken();
939-
if (this->parseNumber(&freqY)) {
940-
*freq = SkSVGFeTurbulenceBaseFrequency(freqX, freqY);
941-
} else {
942-
*freq = SkSVGFeTurbulenceBaseFrequency(freqX, freqX);
943-
}
944-
945-
return this->parseEOSToken();
946-
}
947-
948-
bool SkSVGAttributeParser::parseFeTurbulenceType(SkSVGFeTurbulenceType* type) {
949-
bool parsedValue = false;
950-
951-
if (this->parseExpectedStringToken("fractalNoise")) {
952-
*type = SkSVGFeTurbulenceType(SkSVGFeTurbulenceType::kFractalNoise);
953-
parsedValue = true;
954-
} else if (this->parseExpectedStringToken("turbulence")) {
955-
*type = SkSVGFeTurbulenceType(SkSVGFeTurbulenceType::kTurbulence);
956-
parsedValue = true;
957-
}
958-
959-
return parsedValue && this->parseEOSToken();
960-
}

‎modules/svg/src/SkSVGDOM.cpp‎

Lines changed: 5 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -142,18 +142,6 @@ bool SetNumberAttribute(const sk_sp<SkSVGNode>& node, SkSVGAttribute attr,
142142
return true;
143143
}
144144

145-
bool SetIntegerAttribute(const sk_sp<SkSVGNode>& node, SkSVGAttribute attr,
146-
const char* stringValue) {
147-
SkSVGIntegerType number;
148-
SkSVGAttributeParser parser(stringValue);
149-
if (!parser.parseInteger(&number)) {
150-
return false;
151-
}
152-
153-
node->setAttribute(attr, SkSVGIntegerValue(number));
154-
return true;
155-
}
156-
157145
bool SetViewBoxAttribute(const sk_sp<SkSVGNode>& node, SkSVGAttribute attr,
158146
const char* stringValue) {
159147
SkSVGViewBoxType viewBox;
@@ -360,31 +348,6 @@ bool SetPreserveAspectRatioAttribute(const sk_sp<SkSVGNode>& node, SkSVGAttribut
360348
return true;
361349
}
362350

363-
bool SetFeTurbulenceBaseFrequencyAttribute(const sk_sp<SkSVGNode>& node,
364-
SkSVGAttribute attr,
365-
const char* stringValue) {
366-
SkSVGFeTurbulenceBaseFrequency baseFreq;
367-
SkSVGAttributeParser parser(stringValue);
368-
if (!parser.parseFeTurbulenceBaseFrequency(&baseFreq)) {
369-
return false;
370-
}
371-
372-
node->setAttribute(attr, SkSVGFeTurbulenceBaseFrequencyValue(baseFreq));
373-
return true;
374-
}
375-
376-
bool SetFeTurbulenceTypeAttribute(const sk_sp<SkSVGNode>& node, SkSVGAttribute attr,
377-
const char* stringValue) {
378-
SkSVGFeTurbulenceType type;
379-
SkSVGAttributeParser parser(stringValue);
380-
if (!parser.parseFeTurbulenceType(&type)) {
381-
return false;
382-
}
383-
384-
node->setAttribute(attr, SkSVGFeTurbulenceTypeValue(type));
385-
return true;
386-
}
387-
388351
SkString TrimmedString(const char* first, const char* last) {
389352
SkASSERT(first);
390353
SkASSERT(last);
@@ -463,8 +426,6 @@ struct AttrParseInfo {
463426
};
464427

465428
SortedDictionaryEntry<AttrParseInfo> gAttributeParseInfo[] = {
466-
{ "baseFrequency" , { SkSVGAttribute::kFeTurbulenceBaseFrequency,
467-
SetFeTurbulenceBaseFrequencyAttribute }},
468429
{ "clip-path" , { SkSVGAttribute::kClipPath , SetClipPathAttribute }},
469430
{ "clip-rule" , { SkSVGAttribute::kClipRule , SetFillRuleAttribute }},
470431
{ "color" , { SkSVGAttribute::kColor , SetColorAttribute }},
@@ -488,8 +449,6 @@ SortedDictionaryEntry<AttrParseInfo> gAttributeParseInfo[] = {
488449
{ "gradientUnits" , { SkSVGAttribute::kGradientUnits ,
489450
SetObjectBoundingBoxUnitsAttribute }},
490451
{ "height" , { SkSVGAttribute::kHeight , SetLengthAttribute }},
491-
{ "numOctaves" , { SkSVGAttribute::kFeTurbulenceNumOctaves,
492-
SetIntegerAttribute }},
493452
{ "offset" , { SkSVGAttribute::kOffset , SetLengthAttribute }},
494453
{ "opacity" , { SkSVGAttribute::kOpacity , SetNumberAttribute }},
495454
{ "patternTransform" , { SkSVGAttribute::kPatternTransform , SetTransformAttribute }},
@@ -499,7 +458,6 @@ SortedDictionaryEntry<AttrParseInfo> gAttributeParseInfo[] = {
499458
{ "r" , { SkSVGAttribute::kR , SetLengthAttribute }},
500459
{ "rx" , { SkSVGAttribute::kRx , SetLengthAttribute }},
501460
{ "ry" , { SkSVGAttribute::kRy , SetLengthAttribute }},
502-
{ "seed" , { SkSVGAttribute::kFeTurbulenceSeed , SetNumberAttribute }},
503461
{ "spreadMethod" , { SkSVGAttribute::kSpreadMethod , SetSpreadMethodAttribute }},
504462
{ "stop-color" , { SkSVGAttribute::kStopColor , SetStopColorAttribute }},
505463
{ "stop-opacity" , { SkSVGAttribute::kStopOpacity , SetNumberAttribute }},
@@ -515,8 +473,6 @@ SortedDictionaryEntry<AttrParseInfo> gAttributeParseInfo[] = {
515473
{ "text" , { SkSVGAttribute::kText , SetStringAttribute }},
516474
{ "text-anchor" , { SkSVGAttribute::kTextAnchor , SetTextAnchorAttribute }},
517475
{ "transform" , { SkSVGAttribute::kTransform , SetTransformAttribute }},
518-
{ "type" , { SkSVGAttribute::kFeTurbulenceType ,
519-
SetFeTurbulenceTypeAttribute }},
520476
{ "viewBox" , { SkSVGAttribute::kViewBox , SetViewBoxAttribute }},
521477
{ "visibility" , { SkSVGAttribute::kVisibility , SetVisibilityAttribute }},
522478
{ "width" , { SkSVGAttribute::kWidth , SetLengthAttribute }},
@@ -562,6 +518,11 @@ struct ConstructionContext {
562518
};
563519

564520
bool set_string_attribute(const sk_sp<SkSVGNode>& node, const char* name, const char* value) {
521+
if (node->parseAndSetAttribute(name, value)) {
522+
// Handled by new code path
523+
return true;
524+
}
525+
565526
const int attrIndex = SkStrSearch(&gAttributeParseInfo[0].fKey,
566527
SkTo<int>(SK_ARRAY_COUNT(gAttributeParseInfo)),
567528
name, sizeof(gAttributeParseInfo[0]));

0 commit comments

Comments
 (0)