1891040 - Regression: Toolbar animation in Fullscreen is not smooth on macOS Catalina

Status: VERIFIED FIXED

(Core :: Widget: Cocoa, defect, P3)

Description

[Mehmet]

Attachment: actual_not_smooth 16.43.02.mov

Nightly 126.0a1 (2024-04-11) (64-Bit)
macOS 10.15.7 Catalina

STR:

1. Open a new window
2. Switch into Fullscreen Mode
3. Move the mouse cursor to the top of the screen so that the Toolbar appears/disappears

Actual: The animation is not smooth.

Expected: The Animation should be smooth.

This is a recent regression.

Comment 1

[Mehmet]

Attachment: Expected_Smooth.mov

• expected demo.

Comment 2

[Mehmet]

I did a bisect: bwerth@: Can you please take a look at this regression? Thanks :)

Bug 1882358 Part 3: Remove nsIWidget::IsResizingNativeWidget, which is no longer used. r=nical

Differential Revision: https://phabricator.services.mozilla.com/D202868

2024-04-11T17:48:02.174000: DEBUG : Did not find a branch, checking all integration branches
2024-04-11T17:48:02.177000: INFO : The bisection is done.
2024-04-11T17:48:02.178000: INFO : Stopped

Comment 3

[Mehmet]

FYI, this is only an issue on older macOS versions like macOS 10.15.7 Catalina. This is not an issue on newer macOS versions like macOS 14.4.1 Sonoma. Thanks for checking in advance.

Comment 4

[Brad Werth [:bradwerth]]

Hmmm... something about changing the sync flushing behavior has caused this problem. Would you please navigate to "about:config" and modify the layers.force-synchronous-resize pref (either true or false) and re-test? Is there a value for that pref where this problem goes away?

Comment 5

[Mehmet]

(In reply to Brad Werth [:bradwerth] from comment #4)

Hmmm... something about changing the sync flushing behavior has caused this problem. Would you please navigate to "about:config" and modify the layers.force-synchronous-resize pref (either true or false) and re-test? Is there a value for that pref where this problem goes away?

Standard config for this flag is "true". Changing to "false" makes no difference unfortunately :(

Comment 6

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

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

Comment 7

[Daniel Holbert [:dholbert]]

Attachment: recording on dholbert's Catalina mac environment, showing both good/bad behavior

For what it's worth, I see both behaviors (smooth and not-smooth) when testing current Nightly, on macOS Catalina.

Here's a screencast. The very first animation here is smooth, and then the ones that follow are mostly not-smooth, except near t=15s where I open a second tab and the animation is smooth again after that.

Not sure if there are particular triggers that make the animation smooth or not-smooth; or, perhaps this is just a perf/synchronization issue where one piece of code is falling behind another, and we miss our chance to play the animation.

Comment 8

[Brad Werth [:bradwerth]]

(In reply to Daniel Holbert [:dholbert] from comment #7)

For what it's worth, I see both behaviors (smooth and not-smooth) when testing current Nightly, on macOS Catalina.

Thanks for this; this is helpful. This is showing that the difference between smooth and not-smooth is whether or not the page content is animated every frame of the resize. That means a sync flush (smooth) vs an async flush (not). The reason why the first animation with a new tab is smooth/sync is because the first flush is always sync. Perfect, makes sense!

So the potential fix is to make this toolbar appearance event be detected as resize-like even though the widget isn't actually resizing. That would allow the layers.force-synchronous-resize pref (on by default) to turn each frame into a sync flush. I'll work on that.

Comment 9

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

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

Comment 10

[Donal Meehan [:dmeehan]]

:bradwerth next week is the final week of beta for Fx126.
This is a P3, wondering if you plan on having a fix in time to uplift before Fx126 hits RC?

Comment 11

[Brad Werth [:bradwerth]]

(In reply to Donal Meehan [:dmeehan] from comment #10)

:bradwerth next week is the final week of beta for Fx126.
This is a P3, wondering if you plan on having a fix in time to uplift before Fx126 hits RC?

I do not expect to have a fix ready. Investigation hasn't really begun yet.

Comment 12

[Brad Werth [:bradwerth]]

Attachment: WIP: Bug 1891040: Make animating content force synchronous scene building.

Comment 13

[Brad Werth [:bradwerth]]

As pointed out to me, the toolbar moving down is not done as a CSS animation, it is done as a repeatedly-set transform property that is driven by a key-value observation of the revealAmount property on the window. So it is an incorrect premise that this issue is due to our failure to sync flush scene build during a CSS animation. However, we clearly did do a sync flush scene build more often prior to the landing of Bug 1882358, and this effect looks smoother on macOS Catalina when that happens.

I'll see if I can find a better balance for our sync/async flush scene builds that will fix this issue.

Comment 14

[Brad Werth [:bradwerth]]

Attachment: Bug 1891040 Part 1: Add a mechanism to make CompositorBridgeChild use sync flush rendering.

Now that we do sync flushes less often, this provides a way to
temporarily force all flushes to be sync in order to achieve smooth
visual effects.

Comment 15

[Brad Werth [:bradwerth]]

Attachment: Bug 1891040 Part 2: Make nsChildView request sync flush rendering during critical animations.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0b67e54c213e
Part 1: Add a mechanism to make CompositorBridgeChild use sync flush rendering. r=gfx-reviewers,nical
https://hg.mozilla.org/integration/autoland/rev/288697e1b5c8
Part 2: Make nsChildView request sync flush rendering during critical animations. r=mac-reviewers,spohl

Comment 17

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/0b67e54c213e
https://hg.mozilla.org/mozilla-central/rev/288697e1b5c8

Comment 18

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

Since nightly and release are affected, beta will likely be affected too.
For more information, please visit BugBot documentation.

Comment 19

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

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

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

For more information, please visit BugBot documentation.

Comment 20

[Brad Werth [:bradwerth]]

This is a fix for a narrow issue; it can ride the trains.

Comment 21

[Ada C.]

I was able to reproduce the issue on macOS 10.15 using Firefox 128.0a1.
Verified as fixed on macOS 10.15 using Firefox 129.0b9 and 130.0a1.