Bug 27279 - SVGPathSeg should be a POD Type
: SVGPathSeg should be a POD Type
: WebKit
: 528+ (Nightly build)
: PC Mac OS X 10.5
: P2 Normal
Assigned To:
: 27088
  Show dependency treegraph
Reported: 2009-07-14 16:03 PST by
Modified: 2010-11-17 01:12 PST (History)



You need to log in before you can comment on or make changes to this bug.

Description From 2009-07-14 16:03:55 PST
SVGPathSeg should be a POD Type

I don't think there is any need for SVGPathSeg to be ref-counted.  If we move it to being a POD Type we can use SVGPODTypeWrapper and get the "context/creator" updating for free.  Fixing this brings us one step closer to getting rid of m_context on all SVG binding objects, per bug 27276.
------- Comment #1 From 2009-07-14 16:07:35 PST -------
Hum.  To do this we'd have to get rid of all the subclasses (Which is probably a good thing).  Everything moves into being one big switch statement inside SVGPathSeg.
------- Comment #2 From 2009-07-14 16:19:06 PST -------
That would make SVGPathSeg the maximum size, which would be 4 floats and an a type enum. :(

I think I might leave these a non-pod type for now and fix this another way.
------- Comment #3 From 2009-07-14 16:40:34 PST -------
Currently WebKit supports modification of SVGPathSegs, but I'm not sure the spec supports that.  It would make our jobs easier if they were readonly.  Asking #svg.
------- Comment #4 From 2009-07-14 17:33:52 PST -------
Turns out there is a new version of the 1.1 spec in the works:
------- Comment #5 From 2009-12-21 13:02:22 PST -------
SVGPathSegList and it's objects are not-readonly, see LayoutTests/svg/custom/js-update-path-changes.svg, this is valid SVG 1.1, supported by all implementations.

I've tried making SVGPathSeg a POD type, though it failed, because of the subclass problem. That would need to be resolved first...
I hacked up a version w/o nonfunctional DOM bindings just to see if we see some performance benefits in non-JS SVG code. I couldn't see any real speedup, so I'm not going to work anymore on this at the moment.

The only thing which really makes sense IMHO is to not use SVGPathSegList at all internally, and only use it for the JS bindings (similar to SVGAnimated* code). This way we could use a really speedy approach to store our path data as POD type (w/o tons of subclasses), and use the JSSVGPOD* binding generator and some magic to implement SVGPathSegList & friends on top of that. What do you think?
------- Comment #6 From 2010-11-17 01:12:39 PST -------
Changing SVGPathSeg to be a POD type is not worth it.
We should instead only expose SVGPathSeg when using the SVG DOM bindings (pathElement.pathSegList).
Internally, we can now use SVGPathByteStream, which is much faster.