Skip to content

Commit bef74b7

Browse files
smfrsmfr
committed
Make parsing SVG transform lists more efficient
https://bugs.webkit.org/show_bug.cgi?id=290192 rdar://147591008 Reviewed by Said Abou-Hallawa. Content that animates SVG transforms will often keep the same list of transform operations, but just change the values. Currently, `SVGTransformList::parse()` implements this by clearing the list and re-build it, but this involves a lot of object churn: each SVGTransform is heap-allocated, and itself has a heap-allocated SVGMatrix. In addition, `detach()` and `attach()` have overhead. Make this more efficient by re-using the existing SVGTransforms when possible. Do this by moving the list management into `SVGTransformList::parseGeneric()`. Callers of `parse()` expect the list to be preserved between calls, so the `ListReplacement` argument controls that behavior. Pass the existing object to `SVGTransformable::parseTransform` if the type matches; we compare that with the return value to know if the item was re-used. If not, we go back to appending. Well tested by existing SVG tests. * Source/WebCore/svg/SVGTransformList.cpp: (WebCore::SVGTransformList::parseGeneric): (WebCore::SVGTransformList::parse): * Source/WebCore/svg/SVGTransformList.h: * Source/WebCore/svg/SVGTransformable.cpp: (WebCore::parseTransformGeneric): (WebCore::SVGTransformable::parseTransform): (WebCore::parseTransformValueGeneric): Deleted. (WebCore::SVGTransformable::parseTransformValue): Deleted. * Source/WebCore/svg/SVGTransformable.h: Canonical link: https://commits.webkit.org/292545@main
1 parent 2394366 commit bef74b7

4 files changed

Lines changed: 114 additions & 37 deletions

File tree

‎Source/WebCore/svg/SVGTransformList.cpp‎

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -62,54 +62,73 @@ AffineTransform SVGTransformList::concatenate() const
6262
return result;
6363
}
6464

65-
template<typename CharacterType> bool SVGTransformList::parseGeneric(StringParsingBuffer<CharacterType>& buffer)
65+
template<typename CharacterType> bool SVGTransformList::parseGeneric(StringParsingBuffer<CharacterType>& buffer, ListReplacement listReplacement)
6666
{
6767
bool delimParsed = false;
6868
skipOptionalSVGSpaces(buffer);
6969

70+
size_t itemIndex = 0;
71+
auto currentListReplacement = listReplacement;
72+
7073
while (buffer.hasCharactersRemaining()) {
7174
delimParsed = false;
72-
75+
7376
auto parsedTransformType = SVGTransformable::parseTransformType(buffer);
7477
if (!parsedTransformType)
7578
return false;
7679

77-
auto parsedTransformValue = SVGTransformable::parseTransformValue(*parsedTransformType, buffer);
78-
if (!parsedTransformValue)
80+
RefPtr<SVGTransform> existingTransform;
81+
if (currentListReplacement == ListReplacement::Replace && itemIndex < m_items.size() && parsedTransformType == m_items[itemIndex]->type())
82+
existingTransform = m_items[itemIndex].ptr();
83+
84+
RefPtr parsedTransform = SVGTransformable::parseTransform(*parsedTransformType, buffer, existingTransform);
85+
if (!parsedTransform)
7986
return false;
8087

81-
append(SVGTransform::create(WTFMove(*parsedTransformValue)));
88+
if (currentListReplacement == ListReplacement::Replace && parsedTransform.get() != existingTransform.get()) {
89+
currentListReplacement = ListReplacement::Append;
90+
resize(itemIndex);
91+
}
92+
93+
if (currentListReplacement == ListReplacement::Append)
94+
append(parsedTransform.releaseNonNull());
8295

8396
skipOptionalSVGSpaces(buffer);
8497

8598
if (skipExactly(buffer, ','))
8699
delimParsed = true;
87100

88101
skipOptionalSVGSpaces(buffer);
102+
103+
++itemIndex;
89104
}
105+
106+
if (itemIndex < m_items.size()) {
107+
ASSERT(currentListReplacement == ListReplacement::Replace);
108+
resize(itemIndex);
109+
}
110+
90111
return !delimParsed;
91112
}
92113

93114
void SVGTransformList::parse(StringView value)
94115
{
95-
clearItems();
96-
97116
bool parsingSucceeded = readCharactersForParsing(value, [&](auto buffer) {
98-
return parseGeneric(buffer);
117+
return parseGeneric(buffer, ListReplacement::Replace);
99118
});
100-
119+
101120
if (!parsingSucceeded)
102121
clearItems();
103122
}
104123

105124
bool SVGTransformList::parse(StringParsingBuffer<LChar>& buffer)
106125
{
107-
return parseGeneric(buffer);
126+
return parseGeneric(buffer, ListReplacement::Append);
108127
}
109128

110129
bool SVGTransformList::parse(StringParsingBuffer<UChar>& buffer)
111130
{
112-
return parseGeneric(buffer);
131+
return parseGeneric(buffer, ListReplacement::Append);
113132
}
114133

115134
String SVGTransformList::valueAsString() const

‎Source/WebCore/svg/SVGTransformList.h‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,12 @@ class SVGTransformList final : public SVGValuePropertyList<SVGTransform> {
6363
String valueAsString() const override;
6464

6565
private:
66-
template<typename CharacterType> bool parseGeneric(StringParsingBuffer<CharacterType>&);
66+
67+
enum class ListReplacement : bool {
68+
Append,
69+
Replace
70+
};
71+
template<typename CharacterType> bool parseGeneric(StringParsingBuffer<CharacterType>&, ListReplacement = ListReplacement::Append);
6772
bool parse(StringParsingBuffer<LChar>&);
6873
bool parse(StringParsingBuffer<UChar>&);
6974
};

‎Source/WebCore/svg/SVGTransformable.cpp‎

Lines changed: 75 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -99,67 +99,119 @@ template<typename CharacterType> static int parseTransformParamList(StringParsin
9999
static constexpr std::array requiredValuesForType { 0, 6, 1, 1, 1, 1, 1 };
100100
static constexpr std::array optionalValuesForType { 0, 0, 1, 1, 2, 0, 0 };
101101

102-
template<typename CharacterType> static std::optional<SVGTransformValue> parseTransformValueGeneric(SVGTransformValue::SVGTransformType type, StringParsingBuffer<CharacterType>& buffer)
102+
template<typename CharacterType> static RefPtr<SVGTransform> parseTransformGeneric(SVGTransformValue::SVGTransformType type, StringParsingBuffer<CharacterType>& buffer, RefPtr<SVGTransform> reusableTransform)
103103
{
104+
ASSERT_IMPLIES(reusableTransform, type == reusableTransform->value().type());
105+
104106
if (type == SVGTransformValue::SVG_TRANSFORM_UNKNOWN)
105-
return std::nullopt;
107+
return nullptr;
106108

107109
int valueCount = 0;
108110
std::array<float, 6> values { 0, 0, 0, 0, 0, 0 };
109111
if ((valueCount = parseTransformParamList(buffer, values, requiredValuesForType[type], optionalValuesForType[type])) < 0)
110-
return std::nullopt;
112+
return nullptr;
111113

112114
switch (type) {
113115
case SVGTransformValue::SVG_TRANSFORM_UNKNOWN:
114116
ASSERT_NOT_REACHED();
115-
return std::nullopt;
117+
return nullptr;
116118

117119
case SVGTransformValue::SVG_TRANSFORM_SKEWX: {
120+
if (reusableTransform) {
121+
ASSERT(reusableTransform->value().type() == SVGTransformValue::SVG_TRANSFORM_SKEWX);
122+
reusableTransform->value().setSkewX(values[0]);
123+
return reusableTransform;
124+
}
125+
118126
SVGTransformValue transform;
119127
transform.setSkewX(values[0]);
120-
return transform;
128+
return SVGTransform::create(WTFMove(transform));
121129
}
122130
case SVGTransformValue::SVG_TRANSFORM_SKEWY: {
131+
if (reusableTransform) {
132+
reusableTransform->value().setSkewY(values[0]);
133+
return reusableTransform;
134+
}
135+
123136
SVGTransformValue transform;
124137
transform.setSkewY(values[0]);
125-
return transform;
138+
return SVGTransform::create(WTFMove(transform));
126139
}
127-
case SVGTransformValue::SVG_TRANSFORM_SCALE:
128-
if (valueCount == 1) // Spec: if only one param given, assume uniform scaling
129-
return SVGTransformValue::scaleTransformValue({ values[0], values[0] });
140+
case SVGTransformValue::SVG_TRANSFORM_SCALE: {
141+
if (reusableTransform) {
142+
if (valueCount == 1)
143+
reusableTransform->value().setScale(values[0], values[0]);
144+
else
145+
reusableTransform->value().setScale(values[0], values[1]);
146+
147+
return reusableTransform;
148+
}
149+
150+
auto resultValue = [&]() {
151+
if (valueCount == 1) // Spec: if only one param given, assume uniform scaling
152+
return SVGTransformValue::scaleTransformValue({ values[0], values[0] });
130153

131-
return SVGTransformValue::scaleTransformValue({ values[0], values[1] });
154+
return SVGTransformValue::scaleTransformValue({ values[0], values[1] });
155+
};
156+
157+
return SVGTransform::create(resultValue());
158+
}
159+
case SVGTransformValue::SVG_TRANSFORM_TRANSLATE: {
160+
if (reusableTransform) {
161+
if (valueCount == 1)
162+
reusableTransform->value().setTranslate(values[0], values[0]);
163+
else
164+
reusableTransform->value().setTranslate(values[0], values[1]);
165+
166+
return reusableTransform;
167+
}
132168

133-
case SVGTransformValue::SVG_TRANSFORM_TRANSLATE:
134169
if (valueCount == 1) // Spec: if only one param given, assume 2nd param to be 0
135-
return SVGTransformValue::translateTransformValue({ values[0], 0 });
170+
return SVGTransform::create(SVGTransformValue::translateTransformValue({ values[0], 0 }));
136171

137-
return SVGTransformValue::translateTransformValue({ values[0], values[1] });
172+
return SVGTransform::create(SVGTransformValue::translateTransformValue({ values[0], values[1] }));
173+
}
174+
case SVGTransformValue::SVG_TRANSFORM_ROTATE: {
175+
if (reusableTransform) {
176+
if (valueCount == 1)
177+
reusableTransform->value().setRotate(values[0], 0, 0);
178+
else
179+
reusableTransform->value().setRotate(values[0], values[1], values[2]);
138180

139-
case SVGTransformValue::SVG_TRANSFORM_ROTATE:
140-
if (valueCount == 1)
141-
return SVGTransformValue::rotateTransformValue(values[0], { });
181+
return reusableTransform;
182+
}
142183

143-
return SVGTransformValue::rotateTransformValue(values[0], { values[1], values[2] });
184+
auto resultValue = [&]() {
185+
if (valueCount == 1)
186+
return SVGTransformValue::rotateTransformValue(values[0], { });
144187

188+
return SVGTransformValue::rotateTransformValue(values[0], { values[1], values[2] });
189+
};
190+
return SVGTransform::create(resultValue());
191+
}
145192
case SVGTransformValue::SVG_TRANSFORM_MATRIX: {
193+
if (reusableTransform) {
194+
reusableTransform->value().setMatrix(AffineTransform(values[0], values[1], values[2], values[3], values[4], values[5]));
195+
return reusableTransform;
196+
}
197+
146198
SVGTransformValue transform;
147199
transform.setMatrix(AffineTransform(values[0], values[1], values[2], values[3], values[4], values[5]));
148-
return transform;
200+
return SVGTransform::create(transform);
149201
}
150202
}
151203

152-
return std::nullopt;
204+
return nullptr;
153205
}
154206

155-
std::optional<SVGTransformValue> SVGTransformable::parseTransformValue(SVGTransformValue::SVGTransformType type, StringParsingBuffer<LChar>& buffer)
207+
RefPtr<SVGTransform> SVGTransformable::parseTransform(SVGTransformValue::SVGTransformType type, StringParsingBuffer<LChar>& buffer, RefPtr<SVGTransform> reusableTransform)
156208
{
157-
return parseTransformValueGeneric(type, buffer);
209+
return parseTransformGeneric(type, buffer, reusableTransform);
158210
}
159211

160-
std::optional<SVGTransformValue> SVGTransformable::parseTransformValue(SVGTransformValue::SVGTransformType type, StringParsingBuffer<UChar>& buffer)
212+
RefPtr<SVGTransform> SVGTransformable::parseTransform(SVGTransformValue::SVGTransformType type, StringParsingBuffer<UChar>& buffer, RefPtr<SVGTransform> reusableTransform)
161213
{
162-
return parseTransformValueGeneric(type, buffer);
214+
return parseTransformGeneric(type, buffer, reusableTransform);
163215
}
164216

165217
template<typename CharacterType> static constexpr std::array<CharacterType, 5> skewXDesc { 's', 'k', 'e', 'w', 'X' };

‎Source/WebCore/svg/SVGTransformable.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,14 @@
2626
namespace WebCore {
2727

2828
class AffineTransform;
29+
class SVGTransform;
2930

3031
class SVGTransformable : public SVGLocatable {
3132
public:
3233
virtual ~SVGTransformable();
3334

34-
static std::optional<SVGTransformValue> parseTransformValue(SVGTransformValue::SVGTransformType, StringParsingBuffer<LChar>&);
35-
static std::optional<SVGTransformValue> parseTransformValue(SVGTransformValue::SVGTransformType, StringParsingBuffer<UChar>&);
35+
static RefPtr<SVGTransform> parseTransform(SVGTransformValue::SVGTransformType, StringParsingBuffer<LChar>&, RefPtr<SVGTransform> reusableValue);
36+
static RefPtr<SVGTransform> parseTransform(SVGTransformValue::SVGTransformType, StringParsingBuffer<UChar>&, RefPtr<SVGTransform> reusableValue);
3637

3738
static std::optional<SVGTransformValue::SVGTransformType> parseTransformType(StringView);
3839
static std::optional<SVGTransformValue::SVGTransformType> parseTransformType(StringParsingBuffer<LChar>&);

0 commit comments

Comments
 (0)