1888748 - rAF movement applied to spaced elements with a transition, coming into the viewport from off screen, has a delay

Status: VERIFIED FIXED

(Core :: CSS Transitions and Animations, defect, P2)

Description

[Tudor]

Attachment: firefox.html

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:124.0) Gecko/20100101 Firefox/124.0

Steps to reproduce:

The test case is something similar to a slider in which several elements are translated, each using its own requestAnimationFrame function, while also applying a transition on the elements as they are getting translated.
So basically a group of elements in a stage with hidden overflow, with a bit of spacing between them.

Actual results:

The elements that are currently off screen but coming into the viewport are translating with a delay compared to the element that was initially in the viewport.

Expected results:

All the elements should translate in lockstep, displaying the same pace of movement, same easing from the transitions, as the same numerical transformations and transitions are applied in parallel.

Just to make sure we're on the same page, here are some videos demonstrating the rendering bug:

Latest Firefox (124.0)
https://i.imgur.com/TzfeESK.mp4

Previous major version (123.0)
https://i.imgur.com/VRY9FET.mp4

Chrome latest (123.0) for comparison
https://i.imgur.com/4t3w8eb.mp4

I should mention that if I disable the transition applied to the elements, the delay disappears. Same if I remove any spacing bewteen them and they're all flush within their container.

It's the second rendering bug I'm reporting as I work on a library and every several Firefox versions there's a rendering bug that ruins a significant part of the library. Which doesn't make me very optimistic about providing a consistent experience for users when even core rendering issues like this appear frequently in Firefox.

Comment 1

[Tudor]

What changes did you make to the renderer from 123 to 124 that created this problem?
It affects a very basic feature: mixing requestAnimationFrame with a CSS transition leads to delays in the rendering of elements that have some space between them and come from outside the viewport.
I would say it's a pretty serious bug and likely to break any sites that mix rAF with a transition applied to elements that aren't perfectly adjacent.

Comment 2

[Tudor]

If you don't take this bug seriously I will have to disable the library feature in Firefox and users will be recommended to use other browsers. Or just get a subpar experience with Firefox.

Comment 3

[Tudor]

You can click on the demo attached to this report (firefox.html) to see how the rendering bug manifests itself (in Firefox 124) and check out the code to get an idea what kind of combination of elements leads to such rendering malfunction.
I spent some hours building the demo and making the videos to make sure your job is much easier and you don't have to build your own test case to see it happening.
And I've also explained disabling/enabling which features would make the issue disappear, which again should make your job of identifying the cause of this bug in the rendering engine.
Not sure what more I can do to make things easier to get processed.

Comment 4

[Tudor]

Do I have to get your CEO on the phone for someone to take a look at this bug?
I will do it

Comment 5

[Daniel Bodea [:danibodea]]

I can confirm this report in both Windows 10 and MacOS 11 with Nightly v126.0a1 and Release v124.0.2, but not in ESR v115.9.1esr.
The regressor appears to be bug 1864425.

I am sorry for the late reply. Thank you for your report!

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

:emilio, since you are the author of the regressor, bug 1864425, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1888748 - Ensure animations started on the same tick share ready time. r=birtles,#firefox-animation-reviewers

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Thanks for the report and the (super useful) test-case! This actually got to my radar just in time since a couple other related bugs had made it but those didn't have reduced (ish) test-cases :)

(In reply to Tudor from comment #4)

Do I have to get your CEO on the phone for someone to take a look at this bug?
I will do it

I understand the frustration, but a bug filed in the general component and without a regression range has to be triaged appropriately in order to find the right person to look at it (me in this case). For the next time, if the bug is a regression, it's more productive to use mozregression to figure out what broke it, rather than making this kind of comment.

[Tracking Requested - why for this release]: Regression from 124 which affects perceived performance. Fix should be simple enough.

Comment 9

[Tudor]

RE: Comment 8

I understand the frustration, but a bug filed in the general component and without a regression range has to be triaged appropriately in order to find the right person to look at it (me in this case). For the next time, if the bug is a regression, it's more productive to use mozregression to figure out what broke it, rather than making this kind of comment.

I have been given this suggestion before but I didn't have time to invest in learning how to use the regression tool. I did my best just downloading a few previous Firefox versions, disabling automatic updates, and checking which major release was still working right and which deviated. So I tracked it down to version 124 being the likely culprit. Then built a simplified test case with some comments to make it more explicit what was being done on each step.

Maybe my tone was harsh, but I've been working for years on a library that relies on synchronised translation of elements and this update just broke a significant feature and I was thinking my work just went out the window and now I had to disable features to keep it minimally functional.
And I saw that newer bugs that were not so critical were being triaged while this one was ignored and I didn't understand why.

Thanks for picking it up eventually. 👌

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Too late to land a fix for 125.0 at this point (it ships Tuesday and the RC build is already created), but we can keep it on the radar for the scheduled dot release going out on April 30 if all goes well.

Comment 12

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1864425

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d06a0d7b3413
Ensure animations started on the same tick share ready time. r=birtles

Comment 14

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/45758 for changes under testing/web-platform/tests

Comment 15

[Aron Cseh]

https://hg.mozilla.org/mozilla-central/rev/d06a0d7b3413

Comment 16

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 17

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 18

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:emilio, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox126 to wontfix.

For more information, please visit BugBot documentation.

Comment 19

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9396438 [details]
Bug 1888748 - Ensure animations started on the same tick share ready time. r=birtles,#firefox-animation-reviewers

Beta/Release Uplift Approval Request

• User impact if declined: Comment 0 and dependencies
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: comment 0
• List of other uplifts needed: none
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Relatively small fix.
• String changes made/needed: none
• Is Android affected?: Yes

Comment 20

[Donal Meehan [:dmeehan]]

Comment on attachment 9396438 [details]
Bug 1888748 - Ensure animations started on the same tick share ready time. r=birtles,#firefox-animation-reviewers

Approved for 126.0b3

Comment 21

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/52f995110e98

Comment 22

[Daniel Bodea [:danibodea]]

I can confirm the fix in Nightly v127.0a1 In Windows 10, MacOS 11 and Ubuntu 22. The branch 126 will be verified when a fixed build is available.

Comment 23

[Daniel Bodea [:danibodea]]

I have also verified this fix in Beta v126.0b3 on Windows 10 and MacOS 11.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9396438 [details]
Bug 1888748 - Ensure animations started on the same tick share ready time. r=birtles,#firefox-animation-reviewers

Approved for 125.0.3

Comment 25

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-release/rev/79f9e900c545

Comment 26

[Andrej (:aglavic)]

(In reply to Pulsebot from comment #13)

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d06a0d7b3413
Ensure animations started on the same tick share ready time. r=birtles

Perfherder has detected a talos performance change from push d06a0d7b341361ec7d0e996636f94a7ebd68b92e.

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
34%	tart	linux1804-64-shippable-qr	e10s fission stylo webrender-sw	3.73 -> 2.47
32%	tart	windows10-64-shippable-qr	e10s fission stylo webrender-sw	3.18 -> 2.16
32%	tart	linux1804-64-shippable-qr	e10s fission stylo webrender	3.98 -> 2.71
32%	tart	windows10-64-shippable-qr	e10s fission stylo webrender	3.48 -> 2.38
31%	tart	windows10-64-shippable-qr	e10s fission stylo webrender-sw	3.18 -> 2.19
...	...	...	...	...
30%	tart	macosx1015-64-shippable-qr	e10s fission stylo webrender-sw	2.60 -> 1.83

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.

If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a sheriff to do that for you.

You can run these tests on try with ./mach try perf --alert 16

For more information on performance sheriffing please see our FAQ.

Comment 27

[Daniel Bodea [:danibodea]]

I can also confirm this fix in Firefox Release v125.0.3 in Windows 10 and MacOS 11.