Bug 199364 - CSS animations in SVG do not repaint when SVG is zoomed
Summary: CSS animations in SVG do not repaint when SVG is zoomed
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: WebKit Nightly Build
Hardware: All macOS 12
: P2 Normal
Assignee: Nobody
URL:
Keywords: BrowserCompat, InRadar
Depends on:
Blocks:
 
Reported: 2019-07-01 07:42 PDT by jstsch
Modified: 2023-01-26 14:02 PST (History)
6 users (show)

See Also:


Attachments
Local Build showing fixed test-case (41.64 MB, video/quicktime)
2023-01-26 14:02 PST, Ahmad Saleem
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description jstsch 2019-07-01 07:42:28 PDT
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.
Comment 1 Radar WebKit Bug Importer 2019-07-01 08:07:57 PDT
<rdar://problem/52459928>
Comment 2 jstsch 2020-10-13 04:10:13 PDT
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
Comment 3 jstsch 2021-06-22 10:05:06 PDT
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)
Comment 4 jstsch 2022-01-21 05:55:17 PST
Confirmed that Safari TP 138 / WebKit 17613.1.12.1.12 still features this issue
Comment 5 Ahmad Saleem 2022-09-26 16:11:56 PDT
https://github.com/WebKit/WebKit/pull/4727
Comment 6 Ahmad Saleem 2022-09-28 10:53:49 PDT
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!
Comment 7 Ahmad Saleem 2023-01-22 14:44:18 PST
(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.
Comment 8 Ahmad Saleem 2023-01-23 03:29:09 PST
(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
Comment 9 Ahmad Saleem 2023-01-26 14:02:09 PST
Created attachment 464674 [details]
Local Build showing fixed test-case