Summary: | SVG needs to be wired into the shared attribute system | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Eric Seidel (no email) <eric> | ||||||
Component: | SVG | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | sam | ||||||
Priority: | P2 | ||||||||
Version: | 420+ | ||||||||
Hardware: | Mac | ||||||||
OS: | OS X 10.4 | ||||||||
Attachments: |
|
Description
Eric Seidel (no email)
2007-01-01 14:42: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.
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.
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. 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. I filed: http://bugs.webkit.org/show_bug.cgi?id=12281 asking for the style sharing system to support inline styles. 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
|