Bug 18450 – SVG animation flickers when restarting

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Cameron McCormack (:heycam)]

When an animation restarts, it seems to briefly apply the end value before interpolating from the beginning again.

Load the URL above, click the rect, wait the 2 seconds for the animation to finish, then click the rect again.  Upon the second click, you can see that the rect briefly has width 100 before interpolating from 50 to 100.

[Felician Marton]

I can confirm this, it also accours on qt-linux.

[Felician Marton]

Created attachment 97110 [details]
Patch for SVG animation flickers when restarting

[Andreas Kling]

Comment on attachment 97110 [details]
Patch for SVG animation flickers when restarting

View in context: https://bugs.webkit.org/attachment.cgi?id=97110&action=review

> Source/WebCore/ChangeLog:8
> +        This change eliminates the flickering when SVG animation restarting.

This is not a sufficient ChangeLog. Please explain what your patch is doing and why. :-)

[Felician Marton]

Created attachment 97126 [details]
Patch for SVG animation flickers when restarting

[Nikolas Zimmermann]

Dirk, do you think this patch is reasonable as-is?

[Dirk Schulze]

Comment on attachment 97126 [details]
Patch for SVG animation flickers when restarting

Patch looks great! We need regression tests for changes like that. Sadly I don't know a way how to test. Maybe with SVGSVGElements getCurrentTime()? Can you check this? Anyway, even if there is no way to test it, this needs to be noted in the changelog. But please try to find  a way first.

[Felician Marton]

Created attachment 98691 [details]
Patch for SVG animation flickers when restarting

I think there is no way to tests SVG flickering. I noted it in the ChangeLog.

[Nikolas Zimmermann]

Comment on attachment 98691 [details]
Patch for SVG animation flickers when restarting

View in context: https://bugs.webkit.org/attachment.cgi?id=98691&action=review

Looks good to me, you forgot to ask for review, though - can you upload a new one that I can r+ and cq+ ?

> Source/WebCore/ChangeLog:18
> +        The following methods just modified for standardization:

s/standardization/consistency/.

> Source/WebCore/ChangeLog:22
> +         No new tests, because currently there is no way to test SVG flickering at zero time.

One space too much.

[Felician Marton]

The latest patch is incorrect. Thats why I marked it to obsolate.

[Felician Marton]

(In reply to comment #9)
> The latest patch is incorrect. Thats why I marked it to obsolate.

The patch is ok. There was some problem with my repo.

[Felician Marton]

Created attachment 98869 [details]
Patch for SVG animation flickers when restarting

Only spelling and spacing fixed according to the prev patch.

[Nikolas Zimmermann]

Comment on attachment 98869 [details]
Patch for SVG animation flickers when restarting

r=me!

[WebKit Review Bot]

Comment on attachment 98869 [details]
Patch for SVG animation flickers when restarting

Clearing flags on attachment: 98869

Committed r89907: <http://trac.webkit.org/changeset/89907>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.