Bug 15389

Summary:

SVG renderers should track transform/path changes, instead of pulling every time from SVG DOM

Product:

WebKit

Reporter:

Eric Seidel (no email) <eric>

Component:

SVG

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

krit, zimmermann

Priority:

Version:

523.x (Safari 3)

Hardware:

Mac

OS:

OS X 10.4

URL:

http://research.sun.com/projects/lively/index.xhtml

Bug Depends on:

15388

Bug Blocks:

Attachments:

Description	Flags
test case using setAttribute	none
another test case using TransformList::initialize	none
potential fix	none
Patch	krit: review+

Description Eric Seidel (no email) 2007-10-05 22:12:09 PDT

setting the transform on an SVG renderer causes a layout even if no change was made

I think the lively kernel has a "bug" where they are setting the same transform on some of their elements repeatedly.  We relayout (and repaint) those elements, even if the transform hasn't changed.  We shouldn't do that. :(

Comment 1 Eric Seidel (no email) 2007-10-05 22:12:26 PDT

Created attachment 16558 [details]
test case using setAttribute

Comment 2 Eric Seidel (no email) 2007-10-05 22:17:58 PDT

Created attachment 16559 [details]
another test case using TransformList::initialize

Comment 3 Eric Seidel (no email) 2007-10-05 22:30:59 PDT

Created attachment 16560 [details]
potential fix

Comment 4 Eric Seidel (no email) 2007-10-05 23:05:18 PDT

So the "potential fix" doesn't actually work for *either* test case due to notifyAttributeChanged and the ultimate-evil which that function is.  nAC is called indiscriminately *any time* any attribute changes, and is the single "notification method" from tear-off javascript bindings back into SVG DOM elements.  We've had at least one bug on the subject before.

Comment 5 Oliver Hunt 2007-10-05 23:14:33 PDT

Comment on attachment 16560 [details]
potential fix

Looks fine -- we might want to go through and do similar on all the other similar attribute setters.

Comment 6 Eric Seidel (no email) 2007-10-05 23:37:27 PDT

Comment on attachment 16560 [details]
potential fix

Hadn't meant for this to be reviewed.  We could apply this, but it doesn't help us much given nAC has already marked us for layout before we even get to this code. :(

Comment 7 Eric Seidel (no email) 2007-10-06 02:01:15 PDT

The potential fix was landed as part of bug 15388: r26077 on feature-branch.

Comment 8 Nikolas Zimmermann 2010-04-11 12:46:56 PDT

The bug is still valid, though the codebase changed a lot since 2007, so I had to change the title to reflect what is really happening these days.
Uploading a patch to fix this problem soon.

Comment 9 Nikolas Zimmermann 2010-04-11 13:30:16 PDT

Created attachment 53110 [details]
Patch

Comment 10 Dirk Schulze 2010-04-12 11:41:44 PDT

Comment on attachment 53110 [details]
Patch

Fantastic patch! r=me

Comment 11 Nikolas Zimmermann 2010-04-13 03:36:12 PDT

Committed r57509: <http://trac.webkit.org/changeset/57509>