Bug 199364 – CSS animations in SVG do not repaint when SVG is zoomed

• Component: SVG
• Status: NEW

[jstsch]

CSS animations in SVGs which are in a zoomed element are only partially visible. It looks like a dirty rectangle is defined at load time, not taking the zoom factor into account.

Example case to reproduce: https://jstsch.com/misc/2019/7/safari-svg-zoom.html. As you can see, the animation is only repainted at the size of the original image. If you resize your window you can force a repaint in the entire images.

Interestingly, when you give the first image a width of double the original image width and reload, you can see the other images repaint correctly. It looks like the image objects share the dimensions (but not the locations) of their painting rectangle: https://jstsch.com/misc/2019/7/safari-svg-zoom-2.html

Tested in Safari TP and latest Webkit build. Also reproduced on iPad.

[Radar WebKit Bug Importer]

<rdar://problem/52459928>

[jstsch]

Just would like to confirm that this issue still remains in Safari 14. A real-world example can be seen here. Observe that by dragging any of the words or resizing the window that the lines start to animate.

https://explorer-mag.nationalgeographic.org/voyager_november_december_2020/words_to_explore

[jstsch]

Just encountered this issue again. A (terrible) workaround is to manipulate the opacity of the element each frame slightly to ensure it gets repainted.

I modified the test case so the SVG animated elements get 'jiggled' using requestAnimationFrame. Not so performant, of course...

https://jstsch.com/misc/2019/7/safari-svg-zoom.html

(using a keyframe animation to manipulate the opacity didn't force a repaint, but manipulating the style attribute does)

[jstsch]

Confirmed that Safari TP 138 / WebKit 17613.1.12.1.12 still features this issue

[Ahmad Saleem]

https://github.com/WebKit/WebKit/pull/4727

[Ahmad Saleem]

I tried to fix this bug in following PR:

https://github.com/WebKit/WebKit/pull/4727

by merging following Blink commit:

https://src.chromium.org/viewvc/blink?view=revision&revision=174874

NOTE - Webkit has currently two parallel SVG implementations:

1) Legacy
2) New Layer Based (LBSE)

Blink patch was modifying code present in LBSE layer (might be common code moved from Legacy to be reused in LBSE).

Based on my discussion with @Said on the PR, it seems that Blink patch is not addressing this CSS animation bug which is kind of repairing / style invalidation issue.

Similarly, Blink commit is also not fixing this case and even the test case with it is not showing any clear expected output to reflect what is being fixed with this patch.

This PR compiles and does not have any failed test case.

Additional note - if I try to take newer code of this test case from Blink / Chromium source, it uses 'rAf' but using updated test script, which leverages some tooling not available in Webkit so we cannot take as is Blink / Chromium latest test case and test in Webkit infrastructure.

Just wanted to document the attempt while I try to investigate in background. Thanks!

[Ahmad Saleem]

(In reply to Ahmad Saleem from comment #6)
> I tried to fix this bug in following PR:
> 
> https://github.com/WebKit/WebKit/pull/4727
> 
> by merging following Blink commit:
> 
> https://src.chromium.org/viewvc/blink?view=revision&revision=174874
> 
> NOTE - Webkit has currently two parallel SVG implementations:
> 
> 1) Legacy
> 2) New Layer Based (LBSE)
> 
> Blink patch was modifying code present in LBSE layer (might be common code
> moved from Legacy to be reused in LBSE).
> 
> Based on my discussion with @Said on the PR, it seems that Blink patch is
> not addressing this CSS animation bug which is kind of repairing / style
> invalidation issue.
> 
> Similarly, Blink commit is also not fixing this case and even the test case
> with it is not showing any clear expected output to reflect what is being
> fixed with this patch.
> 
> This PR compiles and does not have any failed test case.
> 
> Additional note - if I try to take newer code of this test case from Blink /
> Chromium source, it uses 'rAf' but using updated test script, which
> leverages some tooling not available in Webkit so we cannot take as is Blink
> / Chromium latest test case and test in Webkit infrastructure.
> 
> Just wanted to document the attempt while I try to investigate in
> background. Thanks!

I built local WebKit Release build based of (259195@main) with and without this Blink merge and this blink merge indeed fix the test case mentioned in the Comment 0.

I know Blink patch might be flaky but if everyone is happy for me to fix this without test case, I am happy to do PR.

[Ahmad Saleem]

(In reply to Ahmad Saleem from comment #7)
> (In reply to Ahmad Saleem from comment #6)
> > I tried to fix this bug in following PR:
> > 
> > https://github.com/WebKit/WebKit/pull/4727
> > 
> > by merging following Blink commit:
> > 
> > https://src.chromium.org/viewvc/blink?view=revision&revision=174874
> > 
> > NOTE - Webkit has currently two parallel SVG implementations:
> > 
> > 1) Legacy
> > 2) New Layer Based (LBSE)
> > 
> > Blink patch was modifying code present in LBSE layer (might be common code
> > moved from Legacy to be reused in LBSE).
> > 
> > Based on my discussion with @Said on the PR, it seems that Blink patch is
> > not addressing this CSS animation bug which is kind of repairing / style
> > invalidation issue.
> > 
> > Similarly, Blink commit is also not fixing this case and even the test case
> > with it is not showing any clear expected output to reflect what is being
> > fixed with this patch.
> > 
> > This PR compiles and does not have any failed test case.
> > 
> > Additional note - if I try to take newer code of this test case from Blink /
> > Chromium source, it uses 'rAf' but using updated test script, which
> > leverages some tooling not available in Webkit so we cannot take as is Blink
> > / Chromium latest test case and test in Webkit infrastructure.
> > 
> > Just wanted to document the attempt while I try to investigate in
> > background. Thanks!
> 
> I built local WebKit Release build based of (259195@main) with and without
> this Blink merge and this blink merge indeed fix the test case mentioned in
> the Comment 0.
> 
> I know Blink patch might be flaky but if everyone is happy for me to fix
> this without test case, I am happy to do PR.

It might be because we also removed "useZoomRules" quirk with this commit on border-width that this patch now works:

https://github.com/WebKit/WebKit/commit/e7f78cb4cd8136f0b3c6e0c02b36009022514452

[Ahmad Saleem]

Created attachment 464674 [details]
Local Build showing fixed test-case

[Ahmad Saleem]

SVN is gone - so Chromium GitHub repo URL - https://github.com/chromium/chromium/commit/8f79cf8a43b2a5db9b2cc301733277d1f27b90b5

[Ahmad Saleem]

Pull request: https://github.com/WebKit/WebKit/pull/44003