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.

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

Rework WindowMode::Fullscreen API #17525

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

Conversation

jf908
Copy link
Contributor

@jf908 jf908 commented Jan 24, 2025

Objective

Solution

This PR changes the exclusive fullscreen window mode to require the type WindowMode::Fullscreen(MonitorSelection, VideoModeSelection) and removes WindowMode::SizedFullscreen. This API somewhat intentionally more closely resembles Winit 0.31's upcoming fullscreen and video mode API.

The new VideoModeSelection enum is specified as follows:

pub enum VideoModeSelection {
    /// Uses the video mode that the monitor is already in.
    Current,
    /// Uses a given [`crate::monitor::VideoMode`]. A list of video modes supported by the monitor
    /// is supplied by [`crate::monitor::Monitor::video_modes`].
    Specific(VideoMode),
}

Changing default behaviour

This might be contentious because it removes the previous behaviour of WindowMode::Fullscreen which selected the highest resolution possible. While the previous behaviour would be quite easy to re-implement as additional options, or as an impl method on Monitor, I would argue that this isn't an implementation that should be encouraged.

From the perspective of a Windows user, I prefer what the majority of modern games do when entering fullscreen which is to preserve the OS's current resolution settings, which allows exclusive fullscreen to be entered faster, and to only have it change if I manually select it in either the options of the game or the OS. The highest resolution available is not necessarily what the user prefers.

I am open to changing this if I have just missed a good use case for it.

Likewise, the only functionality that WindowMode::SizedFullscreen provided was that it selected the resolution closest to the current size of the window so it was removed since this behaviour can be replicated via the new VideoModeSelection::Specific if necessary.

Testing

  • Tested only on Windows 11 so far.
  • macOS/Linux need to be tested.

Migration Guide

WindowMode::SizedFullscreen(MonitorSelection) and WindowMode::Fullscreen(MonitorSelection) has become WindowMode::Fullscreen(MonitorSelection, VideoModeSelection). Previously, the VideoMode was selected based on the closest resolution to the current window size for SizedFullscreen and the largest resolution for Fullscreen. It is possible to replicate that behaviour by searching Monitor::video_modes and selecting it with VideoModeSelection::Specific(VideoMode) but it is recommended to use VideoModeSelection::Current as the default video mode when entering fullscreen.

@mnmaita mnmaita added A-Windowing Platform-agnostic interface layer to run your app in C-Usability A targeted quality-of-life change that makes Bevy easier to use M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jan 25, 2025
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-Usability A targeted quality-of-life change that makes Bevy easier to use D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Needs-Review Needs reviewer attention (from anyone!) to move forward
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants