From bfe932d1f32452f671cc1e1ac0646579f205a7a9 Mon Sep 17 00:00:00 2001 From: jf908 Date: Tue, 11 Mar 2025 01:20:53 +0000 Subject: [PATCH] Rework WindowMode::Fullscreen API (#17525) # Objective - #16883 - Improve the default behaviour of the exclusive fullscreen API. ## 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: ```rust 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. ## Out of scope WindowResolution and scale factor act strangely in exclusive fullscreen, this PR doesn't address it or regress it. ## Testing - Tested on Windows 11 and macOS 12.7 - Linux untested ## 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. --- crates/bevy_render/src/camera/camera.rs | 2 +- crates/bevy_window/src/lib.rs | 5 +- crates/bevy_window/src/monitor.rs | 2 +- crates/bevy_window/src/window.rs | 44 ++++++++------ crates/bevy_winit/src/system.rs | 51 +++++++--------- crates/bevy_winit/src/winit_windows.rs | 77 +++++++++++++------------ examples/window/monitor_info.rs | 5 +- tests/window/change_window_mode.rs | 11 ++-- 8 files changed, 103 insertions(+), 94 deletions(-) diff --git a/crates/bevy_render/src/camera/camera.rs b/crates/bevy_render/src/camera/camera.rs index a1f4918f74..c2687f4dea 100644 --- a/crates/bevy_render/src/camera/camera.rs +++ b/crates/bevy_render/src/camera/camera.rs @@ -980,7 +980,7 @@ pub fn camera_system( } } } - // This check is needed because when changing WindowMode to SizedFullscreen, the viewport may have invalid + // This check is needed because when changing WindowMode to Fullscreen, the viewport may have invalid // arguments due to a sudden change on the window size to a lower value. // If the size of the window is lower, the viewport will match that lower value. if let Some(viewport) = &mut camera.viewport { diff --git a/crates/bevy_window/src/lib.rs b/crates/bevy_window/src/lib.rs index add17c7efe..851c8a681f 100644 --- a/crates/bevy_window/src/lib.rs +++ b/crates/bevy_window/src/lib.rs @@ -45,8 +45,9 @@ pub use window::*; pub mod prelude { #[doc(hidden)] pub use crate::{ - CursorEntered, CursorLeft, CursorMoved, FileDragAndDrop, Ime, MonitorSelection, Window, - WindowMoved, WindowPlugin, WindowPosition, WindowResizeConstraints, + CursorEntered, CursorLeft, CursorMoved, FileDragAndDrop, Ime, MonitorSelection, + VideoModeSelection, Window, WindowMoved, WindowPlugin, WindowPosition, + WindowResizeConstraints, }; } diff --git a/crates/bevy_window/src/monitor.rs b/crates/bevy_window/src/monitor.rs index 0f7d5134a7..b5ebf73975 100644 --- a/crates/bevy_window/src/monitor.rs +++ b/crates/bevy_window/src/monitor.rs @@ -55,7 +55,7 @@ impl Monitor { } /// Represents a video mode that a monitor supports -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] #[cfg_attr(feature = "bevy_reflect", derive(Reflect), reflect(Debug))] #[cfg_attr(feature = "serialize", derive(serde::Serialize, serde::Deserialize))] #[cfg_attr( diff --git a/crates/bevy_window/src/window.rs b/crates/bevy_window/src/window.rs index bc4ca5f23d..9ef026fe74 100644 --- a/crates/bevy_window/src/window.rs +++ b/crates/bevy_window/src/window.rs @@ -17,6 +17,8 @@ use { #[cfg(all(feature = "serialize", feature = "bevy_reflect"))] use bevy_reflect::{ReflectDeserialize, ReflectSerialize}; +use crate::VideoMode; + /// Marker [`Component`] for the window considered the primary window. /// /// Currently this is assumed to only exist on 1 entity at a time. @@ -127,11 +129,12 @@ impl EntityBorrow for NormalizedWindowRef { /// ``` /// # use bevy_ecs::query::With; /// # use bevy_ecs::system::Query; -/// # use bevy_window::{WindowMode, PrimaryWindow, Window, MonitorSelection}; +/// # use bevy_window::{WindowMode, PrimaryWindow, Window, MonitorSelection, VideoModeSelection}; /// fn change_window_mode(mut windows: Query<&mut Window, With>) { /// // Query returns one window typically. /// for mut window in windows.iter_mut() { -/// window.mode = WindowMode::Fullscreen(MonitorSelection::Current); +/// window.mode = +/// WindowMode::Fullscreen(MonitorSelection::Current, VideoModeSelection::Current); /// } /// } /// ``` @@ -1123,6 +1126,24 @@ pub enum MonitorSelection { Entity(Entity), } +/// References an exclusive fullscreen video mode. +/// +/// Used when setting [`WindowMode::Fullscreen`] on a window. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Reflect)] +#[cfg_attr( + feature = "serialize", + derive(serde::Serialize, serde::Deserialize), + reflect(Serialize, Deserialize) +)] +#[reflect(Debug, PartialEq)] +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), +} + /// Presentation mode for a [`Window`]. /// /// The presentation mode specifies when a frame is presented to the window. The [`Fifo`] @@ -1286,28 +1307,15 @@ pub enum WindowMode { /// If you want to avoid that behavior, you can use the [`WindowResolution::set_scale_factor_override`] function /// or the [`WindowResolution::with_scale_factor_override`] builder method to set the scale factor to 1.0. BorderlessFullscreen(MonitorSelection), - /// The window should be in "true"/"legacy" Fullscreen mode on the given [`MonitorSelection`]. + /// The window should be in "true"/"legacy"/"exclusive" Fullscreen mode on the given [`MonitorSelection`]. /// - /// When setting this, the operating system will be requested to use the - /// **closest** resolution available for the current monitor to match as - /// closely as possible the window's physical size. - /// After that, the window's physical size will be modified to match - /// that monitor resolution, and the logical size will follow based on the - /// scale factor, see [`WindowResolution`]. - SizedFullscreen(MonitorSelection), - /// The window should be in "true"/"legacy" Fullscreen mode on the given [`MonitorSelection`]. - /// - /// When setting this, the operating system will be requested to use the - /// **biggest** resolution available for the current monitor. - /// After that, the window's physical size will be modified to match - /// that monitor resolution, and the logical size will follow based on the - /// scale factor, see [`WindowResolution`]. + /// The resolution, refresh rate, and bit depth are selected based on the given [`VideoModeSelection`]. /// /// Note: As this mode respects the scale factor provided by the operating system, /// the window's logical size may be different from its physical size. /// If you want to avoid that behavior, you can use the [`WindowResolution::set_scale_factor_override`] function /// or the [`WindowResolution::with_scale_factor_override`] builder method to set the scale factor to 1.0. - Fullscreen(MonitorSelection), + Fullscreen(MonitorSelection, VideoModeSelection), } /// Specifies where a [`Window`] should appear relative to other overlapping windows (on top or under) . diff --git a/crates/bevy_winit/src/system.rs b/crates/bevy_winit/src/system.rs index 5bcd871025..f4ed1a59a3 100644 --- a/crates/bevy_winit/src/system.rs +++ b/crates/bevy_winit/src/system.rs @@ -34,7 +34,7 @@ use crate::{ convert_enabled_buttons, convert_resize_direction, convert_window_level, convert_window_theme, convert_winit_theme, }, - get_best_videomode, get_fitting_videomode, select_monitor, + get_selected_videomode, select_monitor, state::react_to_resize, winit_monitors::WinitMonitors, CreateMonitorParams, CreateWindowParams, WinitWindows, @@ -314,36 +314,27 @@ pub(crate) fn changed_windows( &monitor_selection, )))) } - mode @ (WindowMode::Fullscreen(_) | WindowMode::SizedFullscreen(_)) => { - let videomode = match mode { - WindowMode::Fullscreen(monitor_selection) => get_best_videomode( - &select_monitor( - &monitors, - winit_window.primary_monitor(), - winit_window.current_monitor(), - &monitor_selection, - ) - .unwrap_or_else(|| { - panic!("Could not find monitor for {:?}", monitor_selection) - }), - ), - WindowMode::SizedFullscreen(monitor_selection) => get_fitting_videomode( - &select_monitor( - &monitors, - winit_window.primary_monitor(), - winit_window.current_monitor(), - &monitor_selection, - ) - .unwrap_or_else(|| { - panic!("Could not find monitor for {:?}", monitor_selection) - }), - window.width() as u32, - window.height() as u32, - ), - _ => unreachable!(), - }; + WindowMode::Fullscreen(monitor_selection, video_mode_selection) => { + let monitor = &select_monitor( + &monitors, + winit_window.primary_monitor(), + winit_window.current_monitor(), + &monitor_selection, + ) + .unwrap_or_else(|| { + panic!("Could not find monitor for {:?}", monitor_selection) + }); - Some(Some(winit::window::Fullscreen::Exclusive(videomode))) + if let Some(video_mode) = get_selected_videomode(monitor, &video_mode_selection) + { + Some(Some(winit::window::Fullscreen::Exclusive(video_mode))) + } else { + warn!( + "Could not find valid fullscreen video mode for {:?} {:?}", + monitor_selection, video_mode_selection + ); + None + } } WindowMode::Windowed => Some(None), }; diff --git a/crates/bevy_winit/src/winit_windows.rs b/crates/bevy_winit/src/winit_windows.rs index cae20f15af..49698f9f97 100644 --- a/crates/bevy_winit/src/winit_windows.rs +++ b/crates/bevy_winit/src/winit_windows.rs @@ -4,8 +4,8 @@ use bevy_ecs::entity::Entity; use bevy_ecs::entity::hash_map::EntityHashMap; use bevy_platform_support::collections::HashMap; use bevy_window::{ - CursorGrabMode, MonitorSelection, Window, WindowMode, WindowPosition, WindowResolution, - WindowWrapper, + CursorGrabMode, MonitorSelection, VideoModeSelection, Window, WindowMode, WindowPosition, + WindowResolution, WindowWrapper, }; use tracing::warn; @@ -61,8 +61,7 @@ impl WinitWindows { let maybe_selected_monitor = &match window.mode { WindowMode::BorderlessFullscreen(monitor_selection) - | WindowMode::Fullscreen(monitor_selection) - | WindowMode::SizedFullscreen(monitor_selection) => select_monitor( + | WindowMode::Fullscreen(monitor_selection, _) => select_monitor( monitors, event_loop.primary_monitor(), None, @@ -74,23 +73,22 @@ impl WinitWindows { winit_window_attributes = match window.mode { WindowMode::BorderlessFullscreen(_) => winit_window_attributes .with_fullscreen(Some(Fullscreen::Borderless(maybe_selected_monitor.clone()))), - WindowMode::Fullscreen(_) => { + WindowMode::Fullscreen(monitor_selection, video_mode_selection) => { let select_monitor = &maybe_selected_monitor .clone() .expect("Unable to get monitor."); - let videomode = get_best_videomode(select_monitor); - winit_window_attributes.with_fullscreen(Some(Fullscreen::Exclusive(videomode))) - } - WindowMode::SizedFullscreen(_) => { - let select_monitor = &maybe_selected_monitor - .clone() - .expect("Unable to get monitor."); - let videomode = get_fitting_videomode( - select_monitor, - window.width() as u32, - window.height() as u32, - ); - winit_window_attributes.with_fullscreen(Some(Fullscreen::Exclusive(videomode))) + + if let Some(video_mode) = + get_selected_videomode(select_monitor, &video_mode_selection) + { + winit_window_attributes.with_fullscreen(Some(Fullscreen::Exclusive(video_mode))) + } else { + warn!( + "Could not find valid fullscreen video mode for {:?} {:?}", + monitor_selection, video_mode_selection + ); + winit_window_attributes + } } WindowMode::Windowed => { if let Some(position) = winit_window_position( @@ -337,30 +335,35 @@ impl WinitWindows { } } -/// Gets the "best" video mode which fits the given dimensions. -/// -/// The heuristic for "best" prioritizes width, height, and refresh rate in that order. -pub fn get_fitting_videomode(monitor: &MonitorHandle, width: u32, height: u32) -> VideoModeHandle { - monitor - .video_modes() - .max_by_key(|x| { - ( - x.size().width.abs_diff(width), - x.size().height.abs_diff(height), - x.refresh_rate_millihertz(), - ) - }) - .unwrap() +/// Returns some [`winit::monitor::VideoModeHandle`] given a [`MonitorHandle`] and a +/// [`VideoModeSelection`] or None if no valid matching video mode was found. +pub fn get_selected_videomode( + monitor: &MonitorHandle, + selection: &VideoModeSelection, +) -> Option { + match selection { + VideoModeSelection::Current => get_current_videomode(monitor), + VideoModeSelection::Specific(specified) => monitor.video_modes().find(|mode| { + mode.size().width == specified.physical_size.x + && mode.size().height == specified.physical_size.y + && mode.refresh_rate_millihertz() == specified.refresh_rate_millihertz + && mode.bit_depth() == specified.bit_depth + }), + } } -/// Gets the "best" video-mode handle from a monitor. +/// Gets a monitor's current video-mode. /// -/// The heuristic for "best" prioritizes width, height, and refresh rate in that order. -pub fn get_best_videomode(monitor: &MonitorHandle) -> VideoModeHandle { +/// TODO: When Winit 0.31 releases this function can be removed and replaced with +/// `MonitorHandle::current_video_mode()` +fn get_current_videomode(monitor: &MonitorHandle) -> Option { monitor .video_modes() - .max_by_key(|x| (x.size(), x.refresh_rate_millihertz())) - .unwrap() + .filter(|mode| { + mode.size() == monitor.size() + && Some(mode.refresh_rate_millihertz()) == monitor.refresh_rate_millihertz() + }) + .max_by_key(VideoModeHandle::bit_depth) } pub(crate) fn attempt_grab( diff --git a/examples/window/monitor_info.rs b/examples/window/monitor_info.rs index c5d6a3386e..c4a26982f8 100644 --- a/examples/window/monitor_info.rs +++ b/examples/window/monitor_info.rs @@ -44,7 +44,10 @@ fn update( .spawn(( Window { title: name.clone(), - mode: WindowMode::Fullscreen(MonitorSelection::Entity(entity)), + mode: WindowMode::Fullscreen( + MonitorSelection::Entity(entity), + VideoModeSelection::Current, + ), position: WindowPosition::Centered(MonitorSelection::Entity(entity)), ..default() }, diff --git a/tests/window/change_window_mode.rs b/tests/window/change_window_mode.rs index 682dd9168b..dce4c9cbee 100644 --- a/tests/window/change_window_mode.rs +++ b/tests/window/change_window_mode.rs @@ -1,8 +1,8 @@ -//! a test that confirms that 'bevy' does not panic while changing from Windowed to SizedFullscreen when viewport is set +//! a test that confirms that 'bevy' does not panic while changing from Windowed to Fullscreen when viewport is set use bevy::{prelude::*, render::camera::Viewport, window::WindowMode}; -//Having a viewport set to the same size as a window used to cause panic on some occasions when switching to SizedFullscreen +//Having a viewport set to the same size as a window used to cause panic on some occasions when switching to Fullscreen const WINDOW_WIDTH: f32 = 1366.0; const WINDOW_HEIGHT: f32 = 768.0; @@ -47,9 +47,12 @@ fn toggle_window_mode(mut qry_window: Query<&mut Window>) { window.mode = match window.mode { WindowMode::Windowed => { - //it takes a while for the window to change from windowed to sizedfullscreen and back + //it takes a while for the window to change from windowed to fullscreen and back std::thread::sleep(std::time::Duration::from_secs(4)); - WindowMode::SizedFullscreen + WindowMode::Fullscreen( + MonitorSelection::Entity(entity), + VideoModeSelection::Current, + ) } _ => { std::thread::sleep(std::time::Duration::from_secs(4));