Bug 12060 - SVG needs to be wired into the shared attribute system
Summary: SVG needs to be wired into the shared attribute system
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 420+
Hardware: Macintosh OS X 10.4
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-01-01 14:42 PST by Eric Seidel (no email)
Modified: 2007-01-15 20:21 PST (History)
1 user (show)

See Also:


Attachments
a first step in that direction (breaks layout tests) (3.79 KB, patch)
2007-01-15 03:52 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Fixed patch (9.21 KB, patch)
2007-01-15 05:53 PST, Eric Seidel (no email)
hyatt: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2007-01-01 14:42:34 PST
SVG needs to be wired into the shared attribute system

HTML has a mechanism for sharing attribute nodes.  SVG has not yet implemented the proper DOM methods in order to wire into this system.

I just haven't bothered to learn much about the attribute sharing system.  But anyone who has, should find this to be an easy bug to fix.  I don't expect one needs to know much about the actual SVG attributes to be able to fix this.
Comment 1 Eric Seidel (no email) 2007-01-15 03:52:34 PST
Created attachment 12452 [details]
a first step in that direction (breaks layout tests)

Unfortunately this breaks all the SVG layout tests.  I expect that this patch is causing us to share RenderStyles and as a result exposing other bugs in SVG code.  I'm not certain however.
Comment 2 Eric Seidel (no email) 2007-01-15 05:53:53 PST
Created attachment 12453 [details]
Fixed patch

This patch has a beautiful attribute lookup system.  Eventually perhaps that system should be a .in file and auto-generated instead of being inline, but it's OK for now.

One could make a test to check that <g width="10px"> is not actually mapped to g.style.width, but I didn't bother.
Comment 3 Eric Seidel (no email) 2007-01-15 06:41:21 PST
My patch seems to have no affect on SVG style sharing however.  I turned on STYLE_SHARING_STATS and saw that simple SVGs are already sharing styles (and share no more after this change), and complicated SVGs (like plan.svg) are sharing no styles at all, even with this change.

It looks (from my minimal testing) that style sharing work when only attributes are used (those are mapped correctly -- at least after my change).  inline style declarations seem to kill style sharing, plan.svg is full of inline style declarations.
Comment 4 Eric Seidel (no email) 2007-01-15 06:43:35 PST
It looks like
bool CSSStyleSelector::canShareStyleWithElement(Node* n)
has !s->inlineStyleDecl() as part of its check.  That would explain why style sharing doesn't work at all for plan.svg.
Comment 5 Eric Seidel (no email) 2007-01-15 06:52:28 PST
I filed:
http://bugs.webkit.org/show_bug.cgi?id=12281
asking for the style sharing system to support inline styles.
Comment 6 Dave Hyatt 2007-01-15 20:10:52 PST
Comment on attachment 12453 [details]
Fixed patch

static HashMap<AtomicStringImpl*, int>* propertyNameToIdMap = 0;

Remove the "= 0".  Statics in a function are already initialized to 0.

r=me
Comment 7 Eric Seidel (no email) 2007-01-15 20:21:48 PST
Well, lots more could be wired up.  But this is a big step in the right direction.

r18876,