Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Show the menu bar in borderless fullscreen on macOS #2053

Merged
merged 6 commits into from Nov 19, 2021
Merged

Show the menu bar in borderless fullscreen on macOS #2053

merged 6 commits into from Nov 19, 2021

Conversation

irh
Copy link
Contributor

@irh irh commented Nov 4, 2021 •
edited

Fixes #1195.

Following on from @cixel's work in #1326, this PR aims to fix the issue where the menu bar doesn't show up when using the borderless fullscreen mode on macOS.

#2046 was opened recently but following some discussion @francesca64 proposed reactivating this branch in a new PR.

There are a couple of edge cases that need to be manually tested, e.g. this comment

Running cargo run --example multithreaded:

  1. Press esc twice to close the first two windows
  2. Press alt+F. The window goes into exclusive fullscreen
  3. Press F. The window seems to change to borderless fullscreen which is indicated by the fact that I can use cmd+tab to switch between applications. But now the menu bar isn't shown.

Additionally, since I first looked at this issue, I've added a fix for the following path:

Running cargo run --example multithreaded:

  1. Press F. The window goes into borderless fullscreen.
  2. Press alt+F. The window goes into exclusive fullscreen
  3. Press F. The window returns to borderless fullscreen. At this point the menu bar should appear normal and the green 'resize' button should be active.

I assume that there are further edge cases to consider that I haven't found, the logic here is a bit thorny and it can be hard to keep track of all the options and modes, but for the main cases this seems to be working correctly, at least in my testing.

  • Tested on all platforms changed
  • Compilation warnings were addressed
  • cargo fmt has been run on this branch
  • cargo doc builds successfully
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created or updated an example program if it would help users understand this functionality
  • Updated feature matrix, if new features were added or implemented

cixel and others added 5 commits November 4, 2021 15:35
@cixel @irh
In MacOS, only disable menu bar in exclusive fullscreen
3fd62c2
@cixel @irh
Save and restore fullscreen options in set_fullscreen
75e24c6
@irh
Don't always cache presentation options when entering exclusive fulls…
04a71d5 
…creen

This commit caches presentation options when entering exclusive fullscreen
only if we're coming from borderless fullscreen.

Then, when transitioning from exclusive -> borderless, if no cached presentation
options are present, then the default borderless options are applied.

This fixes the menu bar being unavailable when taking the following path:
[not fullscreen] -> [exclusive fullscreen] -> [borderless fullscreen].

Without this commit, the presentation options from [not fullscreen] were being
cached and then applied to [borderless fullscreen].
@irh
Restore the window level when switching to exclusive fullscreen
f297869 
The hack of using `CGShieldingWindowLevel() + 1` in borderless fullscreen needs
to be undone when switching from [borderless] -> [exclusive] fullscreen,
otherwise there are menu bar glitches when following a path through
[borderless] -> [exclusive] -> [borderless] modes.

Now, this might appear to conflict with the 'always on top' feature which uses
the 'floating window' level, but this feature appears to be broken anyway when
entering and exiting fullscreen with an always-on-top window. So, rather than
introducing logic to attempt to restore to the 'floating' level here, I think
it's better to do the simple thing for now and then introduce logic for
always-on-top windows when fixing the overall fullscreen behaviour.
@irh
Update the changelog
5b8b129
@irh irh mentioned this pull request Nov 4, 2021
Closed
8 tasks
@maroider maroider added DS - macos C - waiting on maintainer A maintainer must review this code labels Nov 4, 2021
@francesca64
Copy link
Member

With the first test case, if I click somewhere else on the menu bar in borderless fullscreen, the menu bar disappears again. This only seems to happen after transitioning from exclusive->borderless, and the menu bar doesn't reappear even if I cycle exclusive->borderless again.

With the second test case, I unreliably experience a substantial delay (sometimes even briefly beach-balling) when transitioning from exclusive->borderless. However, this already happen on master, so it's not an issue with this PR.

@irh
Copy link
Contributor Author

irh commented Nov 5, 2021

With the first test case, if I click somewhere else on the menu bar in borderless fullscreen, the menu bar disappears again. This only seems to happen after transitioning from exclusive->borderless, and the menu bar doesn't reappear even if I cycle exclusive->borderless again.

Yes I see that too, well spotted 👍 I'll see if I can find a fix.

@irh
Copy link
Contributor Author

irh commented Nov 5, 2021

OK I can see that I won't be able to find a fix any time soon (I have no clue what the root cause is, other than it likely being related to the hacks involved in exclusive fullscreen), and I'm not going to have more time to dedicate to this until sometime next week. If this needs to be fixed before merging I'd be happy for someone with more knowledge of mac windowing than me to take this over, I'm mostly just pulling on levers I don't really understand here!

@francesca64
Copy link
Member

@irh completely understandable; windowing is a nightmare, and you've already done so much to help!

I also won't be able to investigate this until next week. If anyone wants to swoop in and try to figure this out before then, they'll be handsomely rewarded with at least 1 heart emoji.

@irh
Copy link
Contributor Author

irh commented Nov 13, 2021

An update from me, I've got no spare time for at least another couple of weeks while my daughter's quarantining from a Covid close contact, so I'm not going to be able to help with pushing this forward for quite a while.

I've also noticed that the multithreaded example is badly broken on the M1 macbook that I use at home (the wrong resolution is used in exclusive fullscreen, and interaction seems broken).

My thinking is that the thing to do here would be to merge a fix for the missing menu issue so that at least the idiomatic macOS fullscreen mode is working correctly, and then leave the refinement of mode switching as followup work? I fear that getting the mode switching to 100% will be a bit of a thorny challenge that will continue to drag on, and the fix for the primary issue is pretty straightforward, so it seems reasonable to me to treat the two topics separately.

@madsmtm madsmtm mentioned this pull request Nov 19, 2021
Open
madsmtm
madsmtm approved these changes Nov 19, 2021
View reviewed changes
Copy link
Member

@madsmtm madsmtm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, I even think we should just merge this as-is since it includes helpful considerations and goes part of the way in fixing the mode-switching issue.

I've opened #2068 to track fixing the mode-switching for later.

francesca64
francesca64 approved these changes Nov 19, 2021
View reviewed changes
Copy link
Member

@francesca64 francesca64 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@madsmtm seconded; this PR is strictly an improvement.

@irh thank you for everything you've done, and I wish you and your family the best!

@francesca64 francesca64 merged commit f2de847 into rust-windowing:master Nov 19, 2021
@francesca64 francesca64 mentioned this pull request Nov 19, 2021
Merged
9 tasks
@ayroblu ayroblu mentioned this pull request Nov 21, 2021
Closed
@irh irh deleted the fix-macos-fullscreen branch November 21, 2021 09:05
@lethanix lethanix mentioned this pull request Dec 6, 2021
Closed
@madsmtm madsmtm mentioned this pull request Jan 3, 2022
Closed
@speelbarrow speelbarrow mentioned this pull request Jun 10, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@francesca64 francesca64 francesca64 approved these changes

@madsmtm madsmtm madsmtm approved these changes

Assignees
No one assigned
Labels
C - waiting on maintainer A maintainer must review this code DS - macos
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fullscreen Titlebar Won't Show on macOS
5 participants
@irh @francesca64 @madsmtm @maroider @cixel