1700178 - Attempt to reduce stutter during full-screen menubar reveal animation

Status: VERIFIED FIXED

(Core :: Widget: Cocoa, task)

Description

[Markus Stange [:mstange]]

With the patch from bug 1699506, when the full screen revealAmount changes, we set a different CSS transform on the toolbox. This visual update is delayed by up to two vsync intervals: First, the main thread paint is delayed until the refresh driver ticks. Then the composite is delayed until the next vsync event.
If a CoreAnimation composition happens during this window on the main thread, our drawing will probably not be in sync with it. This can create a stutter effect.

Comment 1

[Markus Stange [:mstange]]

Attachment: Bug 1700178 - Trigger a main thread repaint immediately when the revealAmount changed, to reduce stuttering during the animation. r=#mac-reviewers

Comment 2

[Markus Stange [:mstange]]

Profile before: https://share.firefox.dev/3lMpb8L
Profile after: https://share.firefox.dev/3cTDc0w

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/089f6c9bbcc9
Trigger a main thread repaint immediately when the revealAmount changed, to reduce stuttering during the animation. r=mac-reviewers,bradwerth

Comment 4

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/089f6c9bbcc9

Comment 5

[Alexandru Trif, Desktop QA [:atrif]]

Hello Markus! Are there any steps that manual QA can do to verify this issue? Thank you!

Comment 6

[Markus Stange [:mstange]]

The fix was only intended to result in a minor, barely noticeable animation quality improvement. So the only QA that's needed here is to check that it didn't introduce new bugs.

However, Martin says that it may also have fixed the sliding animation 10.15. Can you confirm that?

Comment 7

[Alexandru Trif, Desktop QA [:atrif]]

Martin is correct! The sliding animation is smooth now on the latest Nightly 89.0a1 (20210324084848). We tested this on macOS 10.15, 10.14, 10.13, 10.12, and 11.2.3 M1 to be sure with native fullscreen enabled and disabled as well.
Can we close this as verified given the above? Thank you!

Comment 8

[Markus Stange [:mstange]]

Sure!

Comment 9

[Alexandru Trif, Desktop QA [:atrif]]

Thank you! Closing this as verified per comment 7.