Skip to content

Commit

Permalink
Avoid unwraps in winit fullscreen handling code (#11735)
Browse files Browse the repository at this point in the history 
# 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.
  • Loading branch information
@Friz64
Friz64 authored Feb 10, 2024
1 parent b6945e5 commit d939c44
Show file tree
Hide file tree
Showing 2 changed files with 168 additions and 151 deletions.
290 changes: 150 additions & 140 deletions crates/bevy_winit/src/system.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -119,181 +121,189 @@ pub(crate) fn changed_windows(
mut window_resized: EventWriter<WindowResized>,
) {
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(&current_monitor),
WindowMode::SizedFullscreen => get_fitting_videomode(
&current_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();
}
}
29 changes: 18 additions & 11 deletions crates/bevy_winit/src/winit_windows.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit d939c44

Please sign in to comment.