Bug 27279 - SVGPathSeg should be a POD Type
Summary: SVGPathSeg should be a POD Type
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
Depends on:
Blocks: 27088
  Show dependency treegraph
Reported: 2009-07-14 16:03 PDT by Eric Seidel (no email)
Modified: 2010-11-17 01:12 PST (History)
1 user (show)

See Also:


Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2009-07-14 16:03:55 PDT
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 Eric Seidel (no email) 2009-07-14 16:07:35 PDT
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 Eric Seidel (no email) 2009-07-14 16:19:06 PDT
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 Eric Seidel (no email) 2009-07-14 16:40:34 PDT
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 Eric Seidel (no email) 2009-07-14 17:33:52 PDT
Turns out there is a new version of the 1.1 spec in the works:
Comment 5 Nikolas Zimmermann 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 Nikolas Zimmermann 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.