|Summary:||feImage doesn't invalidate when its target SVG element is animated|
|Product:||WebKit||Reporter:||Tim Horton <thorton>|
|Component:||SVG||Assignee:||Nikolas Zimmermann <zimmermann>|
|Severity:||Normal||CC:||dglazkov, simon.fraser, webkit-bug-importer, webkit.review.bot, zimmermann|
|Version:||528+ (Nightly build)|
|Bug Depends on:|
Description Tim Horton 2011-12-05 13:32:32 PST
Created attachment 117934 [details] repro This is blocking SVG Wow!'s "Twirl" demo, and potentially others. If you have an <feImage> that references a SVG element (in my case, a <rect>), and the target element is animated (I have an <animateTransform>), the feImage doesn't invalidate along with the animation. See the attached test-case in WebKit vs. Opera. (in Opera, the rectangle rotates, as expected; in WebKit, it stays still).
Comment 2 Tim Horton 2011-12-05 14:48:30 PST
feImage is rather unfortunate; it's supposed to act like <use> in many cases, but we have special-cases dotted throughout the code for <use> that can't be used for feImage...
Comment 3 Nikolas Zimmermann 2012-01-27 02:13:06 PST
I have a fix for this. Will be uploaded soon on bug 10403.
Comment 4 Nikolas Zimmermann 2012-01-27 06:33:41 PST
Reversing the logic, let 10403 depend on this bug report, as its better suited for the upcoming fix.
Comment 6 Nikolas Zimmermann 2012-01-27 12:35:19 PST
Note, that this only fully fixes bug 10403 modulo the unload issue. SVG Twirl now starts reacting, but there's still another bug besides the repro that Tim found out. The ChangeLog is a bit incorrect, please ignore the sentence that it also fixes SVG Twirl demo fully.
Comment 7 Tim Horton 2012-01-27 12:38:45 PST
(In reply to comment #6) > Note, that this only fully fixes bug 10403 modulo the unload issue. SVG Twirl now starts reacting, but there's still another bug besides the repro that Tim found out. The ChangeLog is a bit incorrect, please ignore the sentence that it also fixes SVG Twirl demo fully. IIRC the next issue with Twirl is that feDisplacement is broken somehow; I have a bunch of bugs written down that I need to file. This looks like a reasonable approach to fixing this; I haven't tried it out, but it looks good to me!
Comment 8 WebKit Review Bot 2012-01-27 13:47:14 PST
Comment on attachment 124351 [details] Patch Attachment 124351 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11191856 New failing tests: svg/filters/feImage-target-attribute-change.svg svg/filters/feImage-late-indirect-update.svg svg/filters/feImage-target-inline-style-change.svg svg/filters/feImage-target-attribute-change-with-use-indirection.svg svg/filters/feImage-target-style-change.svg svg/filters/feImage-target-attribute-change-with-use-indirection-2.svg svg/filters/feImage-mutliple-targets-id-change.svg svg/filters/feImage-animated-transform-on-target-rect.svg svg/filters/feImage-target-property-change.svg
Comment 9 Dirk Schulze 2012-02-01 02:39:15 PST
Comment on attachment 124351 [details] Patch LGTM. r=me! We talked about the 2 setTimeouts per test. Niko wants to avoid that before landing. Please check chromium, I forgot to do that.
Comment 11 Dirk Schulze 2012-02-08 04:00:51 PST
Comment on attachment 126045 [details] Patch v2 View in context: https://bugs.webkit.org/attachment.cgi?id=126045&action=review Still looks good to me ;) r=me. Great when it is fixed. > LayoutTests/svg/filters/feImage-animated-transform-on-target-rect.svg:32 > + if (window.layoutTestController) > + layoutTestController.notifyDone(); Niko already noted, that he will remove them on landing, since they are unnecessary.
Comment 12 Nikolas Zimmermann 2012-02-08 04:08:06 PST
Comment on attachment 126045 [details] Patch v2 View in context: https://bugs.webkit.org/attachment.cgi?id=126045&action=review >> LayoutTests/svg/filters/feImage-animated-transform-on-target-rect.svg:32 >> + layoutTestController.notifyDone(); > > Niko already noted, that he will remove them on landing, since they are unnecessary. For clarification, that notifyDone() has to stay, we're using setTimeouts() to poll the animation status... > LayoutTests/svg/filters/feImage-change-target-id.svg:20 > + if (window.layoutTestController) > + layoutTestController.notifyDone(); ... this one is obsolete, and will be removed.