Bug 49311 – Convert SVGMatrix/SVGTransform/SVGTransformList to the new SVGPropertyTearOff concept

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Nikolas Zimmermann]

Unfortunately there was no way to split the patches up, w/o adding new "hybrid wrappers" that support an "old style" parent (SVGTransform) with a "new style" child (SVGMatrix)
SVGTransform contains "readonly attribute SVGMatrix matrix", so I had to switch them over to the new system in one go.
Unfortunately SVGTransformList/SVGAnimatedTransformList had to be converted as well, but those require a new SVGPropertyTearOff classes, that deals with the two extra methods that SVGTransformList provides (consolidate, createSVGTransformFromMatrix)

So the patch is a much larger than I hoped. Anyhow, after this patch there's only SVGPathSegList left to do! (And the cleanup patches removing the old JS/V8SVGPODTypeWrappers, and all the Deprecated* files).

[Nikolas Zimmermann]

Created attachment 73492 [details]
Patch

[Eric Seidel (no email)]

Attachment 73492 [details] did not build on mac:
Build output: http://queues.webkit.org/results/5506085

[Eric Seidel (no email)]

Attachment 73492 [details] did not build on mac:
Build output: http://queues.webkit.org/results/5438080

[Nikolas Zimmermann]

Created attachment 73522 [details]
Patch v2

Not sure why mac build failed, probably an unused variable. If it fails again, I'll try a release build.

This patch is significantly larger, as it contains several new testcases.

[Eric Seidel (no email)]

Attachment 73522 [details] did not build on mac:
Build output: http://queues.webkit.org/results/5613002

[Eric Seidel (no email)]

Attachment 73522 [details] did not build on mac:
Build output: http://queues.webkit.org/results/5501087

[Nikolas Zimmermann]

(In reply to comment #6)
> Attachment 73522 [details] did not build on mac:
> Build output: http://queues.webkit.org/results/5501087

The error is real, I tried a build from scratch, and it turns out I forgot to add SVGAnimatedTransformList.h to Xcode, no works fine. Not sure if it's worth uploading a new version just for that though

[Dirk Schulze]

Comment on attachment 73522 [details]
Patch v2

View in context: https://bugs.webkit.org/attachment.cgi?id=73522&action=review

We already talked about some issues on IRC, just add some notes to bugzilla review as well. I may forgot to note some change wishes here :-) Niko already fixed Mac.

Great Patch Niko! Looking forward to see the same work on SVGPathSegList :-D

r=me

> WebCore/svg/SVGTransformList.cpp:66
> +    return String::format("matrix(%f %f %f %f %f %f)", matrix.a(), matrix.b(), matrix.c(), matrix.d(), matrix.e(), matrix.f());

Don't use String::format here ;-) We talked about this on IRC right now.

> WebCore/svg/SVGTransformable.cpp:192
> +        list.append(t);

Can you rename t to transform please? And please fix the indentation as well on switch .. case later in the code.

[Nikolas Zimmermann]

Fixed all issues, landing.

[Nikolas Zimmermann]

Committed r71802: <http://trac.webkit.org/changeset/71802>