Closed Bug 1701929 Opened 4 years ago Closed 3 years ago

macOS: New fullscreen menu slide-in animation not smooth when there is a pinned tab

Categories

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

Firefox 89
All
macOS
enhancement
Points:
2

Tracking

()

VERIFIED FIXED
95 Branch
Iteration:
95.1 - Oct 4 - Oct 17
Tracking Status
firefox-esr78 --- unaffected
firefox87 --- unaffected
firefox88 --- unaffected
firefox89 --- wontfix
firefox95 --- verified

People

(Reporter: beingalink, Assigned: bugzilla)

References

(Depends on 1 open bug, Regression)

Details

(Keywords: regression, Whiteboard: [mac:fullscreen:native-affected][mac:fullscreen:nonnative-affected])

Attachments

(2 files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:89.0) Gecko/20100101 Firefox/89.0

Steps to reproduce:

Go into fullscreen mode in macOS 11.2.3. Move mouse to the top of the screen to get the new menu slide-in animation.

System: MacBook Pro 13", 2019 (i5, 2,4 Ghz, 4 cores, Iris Plus Graphics 655 - 1536 MB)

Actual results:

The animation isn't smooth (see attached video).

Expected results:

The animation should be smooth.

Regressed by: 738335
Has Regression Range: --- → yes
Keywords: regression
Status: UNCONFIRMED → NEW
Component: Untriaged → Widget: Cocoa
Ever confirmed: true
OS: Unspecified → macOS
Product: Firefox → Core
Hardware: Unspecified → All
Whiteboard: [mac:fullscreen]

Couldn't reproduce the issue on Firefox 89.0a1 (2021-03-30) either on macOS 10.15.7 or 11.2.3. Tried on two separate systems (one macbook pro 15" 2015 with a I7 2.8 processor and on a mac mini arm with M1 processor). Here's an attachment with what I tried.

That's what I did, too. Does the complexity of this animation correlate with the number of tabs open? I have several hundreds of tabs open in this window?

OK, to answer my own question, this seems to be it. That's a serious weakness of this new animation though. It shouldn't degrade in performance with an increase in the number of tabs open. The old animation didn't have this problem.

The Bugbug bot thinks this bug is a defect, but please change it back in case of error.

Type: enhancement → defect

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

Type: defect → enhancement
Severity: -- → S3
Priority: -- → P3

Does this reproduce for you with full-screen-api.macos-native-full-screen set to true and false?

Flags: needinfo?(beingalink)

full-screen-api.macos-native-full-screenset to true doesn't improve the situation. Having it set to false, which seems to be the default, doesn't change anything either.

Flags: needinfo?(beingalink)

Thank you for confirming.

Whiteboard: [mac:fullscreen] → [mac:fullscreen:native-affected][mac:fullscreen:nonnative-affected]
Depends on: 1720517

beingalink collected this profile: https://share.firefox.dev/2UesNH8

In this profile, there seem to be two problems, and slow scene building (which I filed bug 1720517 for) does not seem to be one of them. The problems are:

  • At the end of the animation, there's a long jank with frame reconstruction and reflow. This seems to be triggered when the style attribute is added or removed. Not sure exactly what's going on.
  • During the animation (https://share.firefox.dev/36CisHO), the slowest part of each frame update are:
    • DisplayPortUtils::MaybeCreateDisplayPortInFirstScrollFrameEncountered (40%), and
    • SyncViewsAndInvalidateDescendants (21%).

We add/remove position:relative during the animation. That's probably what triggers the reflow.
https://searchfox.org/mozilla-central/rev/04b41051dd2b08df2a06f86f8bf9230c0848fc51/browser/base/content/browser-fullScreenAndPointerLock.js#368-390

Harry, could we find a solution which doesn't do this? Also, could you make a test build which doesn't dynamically add/remove position:relative and give it to beingalink for testing?

Flags: needinfo?(htwyford)

beingalink said on Element that this window has 1000+ tabs, and the bookmarks toolbar is visible and contains a few hundred bookmarks.

Here's another profile, with screenshots: https://share.firefox.dev/36Dltrb
It shows that the slow frames are the first frame of the slide-in animation and the last frame of the slide-out animation.

Isn't it a case that partial pre-render should handle? It's quite hard to tell by watching the recording. I am not familiar with the browser chrome UIs to tell whether the browser tabbar(?) is overflowed or not.

This animation is not a CSS animation; it's driven by JS, which is driven by events from the OS, so that the slide-down offset can be in sync with the offset of the native menubar. So pre-render should not apply here.

I didn't see any obvious ill effects from giving #browser and #navigation-toolbox relative positioning in CSS, so I made a build containing that change. beingalink, could you please try this build to see if it smooths out the animation? It would also be useful to know if you see anything that's broken.

DMG: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/Ob6JnatJS_iWV0nMhHYIqw/runs/0/artifacts/public/build/target.dmg

Flags: needinfo?(htwyford)
Assignee: nobody → htwyford
Status: NEW → ASSIGNED
Iteration: --- → 92.2 - Jul 26 - Aug 8
Points: --- → 2
Flags: needinfo?(beingalink)

Just tried the build but it doesn't change anything, the animation is still stuttering for me.

Flags: needinfo?(beingalink)

Can you capture a new profile? There were two problems in the original profiles: 1. A pause at the start and end of the animation, and 2. Stuttering. This build is supposed to fix the former issue but not the latter.

Flags: needinfo?(beingalink)

See #macdev on matrix.

Flags: needinfo?(beingalink)

I looked at the profile, and indeed, the pause is still there. Looks like the reframe wasn't due position:relative, but due to adding/removing the transform property. Harry, could you make another build with transform defaulting to translateY(0px)?

Flags: needinfo?(htwyford)

Both Harry and I are having trouble reproducing the frame reconstruction that the profile from comment 12 shows.
Emilio, do you have ideas why beingalink might be running into frame construction here?

Flags: needinfo?(emilio)

fwiw, here's a profile with 500+ tabs open and 200 items in the bookmarks toolbar: https://share.firefox.dev/2WV7jAh. I didn't encounter any jank. I'm on macOS 11.4 on an M1 MacBook Pro.

Flags: needinfo?(htwyford)

Adding / removing transform might cause reframes if there's abspos / fixedpos descendants parented outside of that... Perhaps there's a fixed-pos element that gets added dynamically by the front-end or something?

Flags: needinfo?(emilio)

Thanks Emilio. Markus pointed out it could be a pinned tab when tabs are overflowing. Sure enough, I was able to reproduce the jank with a pinned tab: https://share.firefox.dev/3rVBFht

Summary: macOS: New fullscreen menu slide-in animation not smooth → macOS: New fullscreen menu slide-in animation not smooth when there is a pinned tab

Is this reproducible on Linux somehow? Can that be switched to absolute positioning? The tab bar is always on top.

Blocks: 1725151

(In reply to Emilio Cobos Álvarez (:emilio) from comment #25)

Is this reproducible on Linux somehow?

You could hack up the CSS to have toolbox:hover { transform: translateY(70px); } or something like that, I suppose. The Mac code is called in response to MacFullscreenMenubarRevealUpdate events, it would probably be annoying to emulate those.

Can that be switched to absolute positioning? The tab bar is always on top.

You mean, use absolute positioning for pinned tabs rather than fixed positioning? I'm not aware of a reason why this wouldn't be possible, unless there's a relative positioned ancestor in the tabbar's overflow scrollbox which would get in the way.

The frame reconstruction has another unfortunate side-effect: Any open popups in the toolbox close, see bug 1725151.

I'm going to unassign myself from this since I don't expect I'll have time to work on it for at least two weeks.

Assignee: htwyford → nobody
Status: ASSIGNED → NEW
Iteration: 92.2 - Jul 26 - Aug 8 → ---
Assignee: nobody → htwyford
Attachment #9234036 - Attachment description: WIP: Bug 1701929 - Move relative positioning of main chrome elements from JS to CSS. → Bug 1701929 - Define positioning required for Mac fullscreen slide in CSS to avoid frame reconstruction. r?mstange!
Status: NEW → ASSIGNED
Iteration: --- → 95.1 - Oct 4 - Oct 17
Pushed by htwyford@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/bde13f051e73 Define positioning required for Mac fullscreen slide in CSS to avoid frame reconstruction. r=mstange,Gijs
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 95 Branch

Hello! Are there any steps that we could use to reliably verify this? I tried to reproduce this with multiple tabs opened and some pinned tabs but I don't think I can reproduce the issue. The lag is happening only when multiple websites are loading at the same time while in Fullscreen and opening the Apple Menu bar but then it's working as expected after they are loaded for me on macOS 11.6 ARM device. Thank you!

Flags: needinfo?(htwyford)

I can verify that the fix works. It used to be very laggy and not smooth at all, which has been drastically improved. Thanks a lot!

(In reply to beingalink from comment #32)

I can verify that the fix works. It used to be very laggy and not smooth at all, which has been drastically improved. Thanks a lot!

Thank you beingalink! Changing the flags based on comment 32. Cancelling the ni? as well.

Status: RESOLVED → VERIFIED
Flags: needinfo?(htwyford)

beingalink, sorry for the spam, can you please verify this with full-screen-api.macos-native-full-screen set to true as well? To see if there are any differences. Thank's again!

Flags: needinfo?(beingalink)

Toggling full-screen-api.macos-native-full-screen (+ restart) doesn't seem to make a difference here. I'm not totally sure if the animation is as smooth as with apple's apps but the animation is too short and I guess it works well enough for me now.

Out of curiosity, what difference is full-screen-api.macos-native-full-screen supposed to make as I assume the window is a native widget in both cases?

Flags: needinfo?(beingalink)

(In reply to beingalink from comment #35)

Toggling full-screen-api.macos-native-full-screen (+ restart) doesn't seem to make a difference here. I'm not totally sure if the animation is as smooth as with apple's apps but the animation is too short and I guess it works well enough for me now.

Out of curiosity, what difference is full-screen-api.macos-native-full-screen supposed to make as I assume the window is a native widget in both cases?

:haik can answer this in more detail, but that's the pref toggling a new feature where fullscreen web content is treated as its own Space in macOS. For example, with that pref enabled, fullscreen videos will get their own desktop in Mission Control. Previously, (I could be wrong here...) we just expanded fullscreen content to fill the whole desktop, meaning it didn't get any special treatment at the OS level.

(In reply to Harry Twyford [:harry] from comment #36)

:haik can answer this in more detail, but that's the pref toggling a new feature where fullscreen web content is treated as its own Space in macOS. For example, with that pref enabled, fullscreen videos will get their own desktop in Mission Control. Previously, (I could be wrong here...) we just expanded fullscreen content to fill the whole desktop, meaning it didn't get any special treatment at the OS level.

Full screen video in its own desktop doesn't seem to be implemented yet but if that's the goal, I'm really happy with it because current behavior is a bit annoying. Thanks for the info!

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: