From d939c4402d072f5f579de45f816a5d6282cfa21c Mon Sep 17 00:00:00 2001 From: Friz64 Date: Sat, 10 Feb 2024 21:17:04 +0100 Subject: [PATCH] Avoid unwraps in winit fullscreen handling code (#11735) # Objective - Get rid of unwraps in winit fullscreen handling code, which are the source of some crashes. - Fix #11275 ## Solution - Replace the unwraps with warnings. Ignore the fullscreen request, do nothing instead. --- crates/bevy_winit/src/system.rs | 290 +++++++++++++------------ crates/bevy_winit/src/winit_windows.rs | 29 ++- 2 files changed, 168 insertions(+), 151 deletions(-) diff --git a/crates/bevy_winit/src/system.rs b/crates/bevy_winit/src/system.rs index 347fbadcac7da..b027461075970 100644 --- a/crates/bevy_winit/src/system.rs +++ b/crates/bevy_winit/src/system.rs @@ -7,7 +7,9 @@ use bevy_ecs::{ system::{NonSendMut, Query, SystemParamItem}, }; use bevy_utils::tracing::{error, info, warn}; -use bevy_window::{RawHandleWrapper, Window, WindowClosed, WindowCreated, WindowResized}; +use bevy_window::{ + RawHandleWrapper, Window, WindowClosed, WindowCreated, WindowMode, WindowResized, +}; use raw_window_handle::{HasDisplayHandle, HasWindowHandle}; use winit::{ @@ -119,181 +121,189 @@ pub(crate) fn changed_windows( mut window_resized: EventWriter, ) { for (entity, mut window, mut cache) in &mut changed_windows { - if let Some(winit_window) = winit_windows.get_window(entity) { - if window.title != cache.window.title { - winit_window.set_title(window.title.as_str()); - } + let Some(winit_window) = winit_windows.get_window(entity) else { + continue; + }; - if window.mode != cache.window.mode { - let new_mode = match window.mode { - bevy_window::WindowMode::BorderlessFullscreen => { - Some(winit::window::Fullscreen::Borderless(None)) - } - bevy_window::WindowMode::Fullscreen => { - Some(winit::window::Fullscreen::Exclusive(get_best_videomode( - &winit_window.current_monitor().unwrap(), - ))) - } - bevy_window::WindowMode::SizedFullscreen => { - Some(winit::window::Fullscreen::Exclusive(get_fitting_videomode( - &winit_window.current_monitor().unwrap(), - window.width() as u32, - window.height() as u32, - ))) + if window.title != cache.window.title { + winit_window.set_title(window.title.as_str()); + } + + if window.mode != cache.window.mode { + let new_mode = match window.mode { + WindowMode::BorderlessFullscreen => { + Some(Some(winit::window::Fullscreen::Borderless(None))) + } + mode @ (WindowMode::Fullscreen | WindowMode::SizedFullscreen) => { + if let Some(current_monitor) = winit_window.current_monitor() { + let videomode = match mode { + WindowMode::Fullscreen => get_best_videomode(¤t_monitor), + WindowMode::SizedFullscreen => get_fitting_videomode( + ¤t_monitor, + window.width() as u32, + window.height() as u32, + ), + _ => unreachable!(), + }; + + Some(Some(winit::window::Fullscreen::Exclusive(videomode))) + } else { + warn!("Could not determine current monitor, ignoring exclusive fullscreen request for window {:?}", window.title); + None } - bevy_window::WindowMode::Windowed => None, - }; + } + WindowMode::Windowed => Some(None), + }; + if let Some(new_mode) = new_mode { if winit_window.fullscreen() != new_mode { winit_window.set_fullscreen(new_mode); } } - if window.resolution != cache.window.resolution { - let physical_size = PhysicalSize::new( - window.resolution.physical_width(), - window.resolution.physical_height(), - ); - if let Some(size_now) = winit_window.request_inner_size(physical_size) { - crate::react_to_resize(&mut window, size_now, &mut window_resized, entity); - } + } + if window.resolution != cache.window.resolution { + let physical_size = PhysicalSize::new( + window.resolution.physical_width(), + window.resolution.physical_height(), + ); + if let Some(size_now) = winit_window.request_inner_size(physical_size) { + crate::react_to_resize(&mut window, size_now, &mut window_resized, entity); } + } - if window.physical_cursor_position() != cache.window.physical_cursor_position() { - if let Some(physical_position) = window.physical_cursor_position() { - let position = PhysicalPosition::new(physical_position.x, physical_position.y); + if window.physical_cursor_position() != cache.window.physical_cursor_position() { + if let Some(physical_position) = window.physical_cursor_position() { + let position = PhysicalPosition::new(physical_position.x, physical_position.y); - if let Err(err) = winit_window.set_cursor_position(position) { - error!("could not set cursor position: {:?}", err); - } + if let Err(err) = winit_window.set_cursor_position(position) { + error!("could not set cursor position: {:?}", err); } } + } - if window.cursor.icon != cache.window.cursor.icon { - winit_window.set_cursor_icon(converters::convert_cursor_icon(window.cursor.icon)); - } + if window.cursor.icon != cache.window.cursor.icon { + winit_window.set_cursor_icon(converters::convert_cursor_icon(window.cursor.icon)); + } - if window.cursor.grab_mode != cache.window.cursor.grab_mode { - crate::winit_windows::attempt_grab(winit_window, window.cursor.grab_mode); - } + if window.cursor.grab_mode != cache.window.cursor.grab_mode { + crate::winit_windows::attempt_grab(winit_window, window.cursor.grab_mode); + } - if window.cursor.visible != cache.window.cursor.visible { - winit_window.set_cursor_visible(window.cursor.visible); - } + if window.cursor.visible != cache.window.cursor.visible { + winit_window.set_cursor_visible(window.cursor.visible); + } - if window.cursor.hit_test != cache.window.cursor.hit_test { - if let Err(err) = winit_window.set_cursor_hittest(window.cursor.hit_test) { - window.cursor.hit_test = cache.window.cursor.hit_test; - warn!( - "Could not set cursor hit test for window {:?}: {:?}", - window.title, err - ); - } + if window.cursor.hit_test != cache.window.cursor.hit_test { + if let Err(err) = winit_window.set_cursor_hittest(window.cursor.hit_test) { + window.cursor.hit_test = cache.window.cursor.hit_test; + warn!( + "Could not set cursor hit test for window {:?}: {:?}", + window.title, err + ); } + } - if window.decorations != cache.window.decorations - && window.decorations != winit_window.is_decorated() - { - winit_window.set_decorations(window.decorations); - } + if window.decorations != cache.window.decorations + && window.decorations != winit_window.is_decorated() + { + winit_window.set_decorations(window.decorations); + } - if window.resizable != cache.window.resizable - && window.resizable != winit_window.is_resizable() - { - winit_window.set_resizable(window.resizable); - } + if window.resizable != cache.window.resizable + && window.resizable != winit_window.is_resizable() + { + winit_window.set_resizable(window.resizable); + } + + if window.enabled_buttons != cache.window.enabled_buttons { + winit_window.set_enabled_buttons(convert_enabled_buttons(window.enabled_buttons)); + } - if window.enabled_buttons != cache.window.enabled_buttons { - winit_window.set_enabled_buttons(convert_enabled_buttons(window.enabled_buttons)); + if window.resize_constraints != cache.window.resize_constraints { + let constraints = window.resize_constraints.check_constraints(); + let min_inner_size = LogicalSize { + width: constraints.min_width, + height: constraints.min_height, + }; + let max_inner_size = LogicalSize { + width: constraints.max_width, + height: constraints.max_height, + }; + + winit_window.set_min_inner_size(Some(min_inner_size)); + if constraints.max_width.is_finite() && constraints.max_height.is_finite() { + winit_window.set_max_inner_size(Some(max_inner_size)); } + } - if window.resize_constraints != cache.window.resize_constraints { - let constraints = window.resize_constraints.check_constraints(); - let min_inner_size = LogicalSize { - width: constraints.min_width, - height: constraints.min_height, + if window.position != cache.window.position { + if let Some(position) = crate::winit_window_position( + &window.position, + &window.resolution, + winit_window.available_monitors(), + winit_window.primary_monitor(), + winit_window.current_monitor(), + ) { + let should_set = match winit_window.outer_position() { + Ok(current_position) => current_position != position, + _ => true, }; - let max_inner_size = LogicalSize { - width: constraints.max_width, - height: constraints.max_height, - }; - - winit_window.set_min_inner_size(Some(min_inner_size)); - if constraints.max_width.is_finite() && constraints.max_height.is_finite() { - winit_window.set_max_inner_size(Some(max_inner_size)); - } - } - if window.position != cache.window.position { - if let Some(position) = crate::winit_window_position( - &window.position, - &window.resolution, - winit_window.available_monitors(), - winit_window.primary_monitor(), - winit_window.current_monitor(), - ) { - let should_set = match winit_window.outer_position() { - Ok(current_position) => current_position != position, - _ => true, - }; - - if should_set { - winit_window.set_outer_position(position); - } + if should_set { + winit_window.set_outer_position(position); } } + } - if let Some(maximized) = window.internal.take_maximize_request() { - winit_window.set_maximized(maximized); - } - - if let Some(minimized) = window.internal.take_minimize_request() { - winit_window.set_minimized(minimized); - } + if let Some(maximized) = window.internal.take_maximize_request() { + winit_window.set_maximized(maximized); + } - if window.focused != cache.window.focused && window.focused { - winit_window.focus_window(); - } + if let Some(minimized) = window.internal.take_minimize_request() { + winit_window.set_minimized(minimized); + } - if window.window_level != cache.window.window_level { - winit_window.set_window_level(convert_window_level(window.window_level)); - } + if window.focused != cache.window.focused && window.focused { + winit_window.focus_window(); + } - // Currently unsupported changes - if window.transparent != cache.window.transparent { - window.transparent = cache.window.transparent; - warn!( - "Winit does not currently support updating transparency after window creation." - ); - } + if window.window_level != cache.window.window_level { + winit_window.set_window_level(convert_window_level(window.window_level)); + } - #[cfg(target_arch = "wasm32")] - if window.canvas != cache.window.canvas { - window.canvas = cache.window.canvas.clone(); - warn!( - "Bevy currently doesn't support modifying the window canvas after initialization." - ); - } + // Currently unsupported changes + if window.transparent != cache.window.transparent { + window.transparent = cache.window.transparent; + warn!("Winit does not currently support updating transparency after window creation."); + } - if window.ime_enabled != cache.window.ime_enabled { - winit_window.set_ime_allowed(window.ime_enabled); - } + #[cfg(target_arch = "wasm32")] + if window.canvas != cache.window.canvas { + window.canvas = cache.window.canvas.clone(); + warn!( + "Bevy currently doesn't support modifying the window canvas after initialization." + ); + } - if window.ime_position != cache.window.ime_position { - winit_window.set_ime_cursor_area( - LogicalPosition::new(window.ime_position.x, window.ime_position.y), - PhysicalSize::new(10, 10), - ); - } + if window.ime_enabled != cache.window.ime_enabled { + winit_window.set_ime_allowed(window.ime_enabled); + } - if window.window_theme != cache.window.window_theme { - winit_window.set_theme(window.window_theme.map(convert_window_theme)); - } + if window.ime_position != cache.window.ime_position { + winit_window.set_ime_cursor_area( + LogicalPosition::new(window.ime_position.x, window.ime_position.y), + PhysicalSize::new(10, 10), + ); + } - if window.visible != cache.window.visible { - winit_window.set_visible(window.visible); - } + if window.window_theme != cache.window.window_theme { + winit_window.set_theme(window.window_theme.map(convert_window_theme)); + } - cache.window = window.clone(); + if window.visible != cache.window.visible { + winit_window.set_visible(window.visible); } + + cache.window = window.clone(); } } diff --git a/crates/bevy_winit/src/winit_windows.rs b/crates/bevy_winit/src/winit_windows.rs index 48757887b883c..085897f21f001 100644 --- a/crates/bevy_winit/src/winit_windows.rs +++ b/crates/bevy_winit/src/winit_windows.rs @@ -55,18 +55,25 @@ impl WinitWindows { WindowMode::BorderlessFullscreen => winit_window_builder.with_fullscreen(Some( winit::window::Fullscreen::Borderless(event_loop.primary_monitor()), )), - WindowMode::Fullscreen => { - winit_window_builder.with_fullscreen(Some(winit::window::Fullscreen::Exclusive( - get_best_videomode(&event_loop.primary_monitor().unwrap()), - ))) + mode @ (WindowMode::Fullscreen | WindowMode::SizedFullscreen) => { + if let Some(primary_monitor) = event_loop.primary_monitor() { + let videomode = match mode { + WindowMode::Fullscreen => get_best_videomode(&primary_monitor), + WindowMode::SizedFullscreen => get_fitting_videomode( + &primary_monitor, + window.width() as u32, + window.height() as u32, + ), + _ => unreachable!(), + }; + + winit_window_builder + .with_fullscreen(Some(winit::window::Fullscreen::Exclusive(videomode))) + } else { + warn!("Could not determine primary monitor, ignoring exclusive fullscreen request for window {:?}", window.title); + winit_window_builder + } } - WindowMode::SizedFullscreen => winit_window_builder.with_fullscreen(Some( - winit::window::Fullscreen::Exclusive(get_fitting_videomode( - &event_loop.primary_monitor().unwrap(), - window.width() as u32, - window.height() as u32, - )), - )), WindowMode::Windowed => { if let Some(position) = winit_window_position( &window.position,