Skip to content

Read back full screen state from window on macOS #15678

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.

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

coreh
Copy link
Contributor

@coreh coreh commented Oct 6, 2024

Objective

I'm trying to display on a menu screen of my game the Windowed/Full screen state. However, on macOS it's possible for the user to trigger borderless fullscreen directly from the window controls, which results in a stale/wrong state displayed.

Solution

  • Add logic to read back full screen state and apply it to the window when the user directly makes it full screen via the window controls on macOS

Testing

  • Did you test these changes? If so, how? Yes, mostly on my game
  • Are there any parts that need more testing? I'm not sure if this has performance implications, as the call might be blocking on the UI thread. However, unscientifically I didn't notice any degradation.
  • How can other people (reviewers) test your changes? Is there anything specific they need to know? Adding log statements to the match branches should suffice.
  • If relevant, what platforms did you test these changes on, and are there any important ones you can't test? Just macOS

@tychedelia tychedelia self-requested a review October 6, 2024 18:39
@tychedelia tychedelia added C-Bug An unexpected or incorrect behavior A-Windowing Platform-agnostic interface layer to run your app in S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Oct 6, 2024
@mockersf
Copy link
Member

mockersf commented Oct 6, 2024

does winit sends a resolution changed event (or another one) when the user changes the window controls? I think it would be better to add this when the window reacts to a corresponding event from winit

#[cfg(target_os = "macos")]
match (winit_window.fullscreen(), window.mode) {
(Some(winit::window::Fullscreen::Borderless(_)), WindowMode::Windowed) => {
window.mode = WindowMode::BorderlessFullscreen(MonitorSelection::Current);
Copy link
Member

Choose a reason for hiding this comment

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

We should probably make sure we are copying the correct monitor returned from winit rather than just defaulting to Current.

@alice-i-cecile alice-i-cecile added the O-MacOS Specific to the MacOS (Apple) desktop operating system label Oct 27, 2024
@alice-i-cecile
Copy link
Member

Should we be upstreaming this fix to winit instead?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Windowing Platform-agnostic interface layer to run your app in C-Bug An unexpected or incorrect behavior O-MacOS Specific to the MacOS (Apple) desktop operating system S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants