Skip to content

Commit d939c44

Browse files
Friz64Friz64
authored
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.
1 parent b6945e5 commit d939c44

File tree

2 files changed

+168
-151
lines changed

2 files changed

+168
-151
lines changed

‎crates/bevy_winit/src/system.rs

Lines changed: 150 additions & 140 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@ use bevy_ecs::{
77
system::{NonSendMut, Query, SystemParamItem},
88
};
99
use bevy_utils::tracing::{error, info, warn};
10-
use bevy_window::{RawHandleWrapper, Window, WindowClosed, WindowCreated, WindowResized};
10+
use bevy_window::{
11+
RawHandleWrapper, Window, WindowClosed, WindowCreated, WindowMode, WindowResized,
12+
};
1113

1214
use raw_window_handle::{HasDisplayHandle, HasWindowHandle};
1315
use winit::{
@@ -119,181 +121,189 @@ pub(crate) fn changed_windows(
119121
mut window_resized: EventWriter<WindowResized>,
120122
) {
121123
for (entity, mut window, mut cache) in &mut changed_windows {
122-
if let Some(winit_window) = winit_windows.get_window(entity) {
123-
if window.title != cache.window.title {
124-
winit_window.set_title(window.title.as_str());
125-
}
124+
let Some(winit_window) = winit_windows.get_window(entity) else {
125+
continue;
126+
};
126127

127-
if window.mode != cache.window.mode {
128-
let new_mode = match window.mode {
129-
bevy_window::WindowMode::BorderlessFullscreen => {
130-
Some(winit::window::Fullscreen::Borderless(None))
131-
}
132-
bevy_window::WindowMode::Fullscreen => {
133-
Some(winit::window::Fullscreen::Exclusive(get_best_videomode(
134-
&winit_window.current_monitor().unwrap(),
135-
)))
136-
}
137-
bevy_window::WindowMode::SizedFullscreen => {
138-
Some(winit::window::Fullscreen::Exclusive(get_fitting_videomode(
139-
&winit_window.current_monitor().unwrap(),
140-
window.width() as u32,
141-
window.height() as u32,
142-
)))
128+
if window.title != cache.window.title {
129+
winit_window.set_title(window.title.as_str());
130+
}
131+
132+
if window.mode != cache.window.mode {
133+
let new_mode = match window.mode {
134+
WindowMode::BorderlessFullscreen => {
135+
Some(Some(winit::window::Fullscreen::Borderless(None)))
136+
}
137+
mode @ (WindowMode::Fullscreen | WindowMode::SizedFullscreen) => {
138+
if let Some(current_monitor) = winit_window.current_monitor() {
139+
let videomode = match mode {
140+
WindowMode::Fullscreen => get_best_videomode(&current_monitor),
141+
WindowMode::SizedFullscreen => get_fitting_videomode(
142+
&current_monitor,
143+
window.width() as u32,
144+
window.height() as u32,
145+
),
146+
_ => unreachable!(),
147+
};
148+
149+
Some(Some(winit::window::Fullscreen::Exclusive(videomode)))
150+
} else {
151+
warn!("Could not determine current monitor, ignoring exclusive fullscreen request for window {:?}", window.title);
152+
None
143153
}
144-
bevy_window::WindowMode::Windowed => None,
145-
};
154+
}
155+
WindowMode::Windowed => Some(None),
156+
};
146157

158+
if let Some(new_mode) = new_mode {
147159
if winit_window.fullscreen() != new_mode {
148160
winit_window.set_fullscreen(new_mode);
149161
}
150162
}
151-
if window.resolution != cache.window.resolution {
152-
let physical_size = PhysicalSize::new(
153-
window.resolution.physical_width(),
154-
window.resolution.physical_height(),
155-
);
156-
if let Some(size_now) = winit_window.request_inner_size(physical_size) {
157-
crate::react_to_resize(&mut window, size_now, &mut window_resized, entity);
158-
}
163+
}
164+
if window.resolution != cache.window.resolution {
165+
let physical_size = PhysicalSize::new(
166+
window.resolution.physical_width(),
167+
window.resolution.physical_height(),
168+
);
169+
if let Some(size_now) = winit_window.request_inner_size(physical_size) {
170+
crate::react_to_resize(&mut window, size_now, &mut window_resized, entity);
159171
}
172+
}
160173

161-
if window.physical_cursor_position() != cache.window.physical_cursor_position() {
162-
if let Some(physical_position) = window.physical_cursor_position() {
163-
let position = PhysicalPosition::new(physical_position.x, physical_position.y);
174+
if window.physical_cursor_position() != cache.window.physical_cursor_position() {
175+
if let Some(physical_position) = window.physical_cursor_position() {
176+
let position = PhysicalPosition::new(physical_position.x, physical_position.y);
164177

165-
if let Err(err) = winit_window.set_cursor_position(position) {
166-
error!("could not set cursor position: {:?}", err);
167-
}
178+
if let Err(err) = winit_window.set_cursor_position(position) {
179+
error!("could not set cursor position: {:?}", err);
168180
}
169181
}
182+
}
170183

171-
if window.cursor.icon != cache.window.cursor.icon {
172-
winit_window.set_cursor_icon(converters::convert_cursor_icon(window.cursor.icon));
173-
}
184+
if window.cursor.icon != cache.window.cursor.icon {
185+
winit_window.set_cursor_icon(converters::convert_cursor_icon(window.cursor.icon));
186+
}
174187

175-
if window.cursor.grab_mode != cache.window.cursor.grab_mode {
176-
crate::winit_windows::attempt_grab(winit_window, window.cursor.grab_mode);
177-
}
188+
if window.cursor.grab_mode != cache.window.cursor.grab_mode {
189+
crate::winit_windows::attempt_grab(winit_window, window.cursor.grab_mode);
190+
}
178191

179-
if window.cursor.visible != cache.window.cursor.visible {
180-
winit_window.set_cursor_visible(window.cursor.visible);
181-
}
192+
if window.cursor.visible != cache.window.cursor.visible {
193+
winit_window.set_cursor_visible(window.cursor.visible);
194+
}
182195

183-
if window.cursor.hit_test != cache.window.cursor.hit_test {
184-
if let Err(err) = winit_window.set_cursor_hittest(window.cursor.hit_test) {
185-
window.cursor.hit_test = cache.window.cursor.hit_test;
186-
warn!(
187-
"Could not set cursor hit test for window {:?}: {:?}",
188-
window.title, err
189-
);
190-
}
196+
if window.cursor.hit_test != cache.window.cursor.hit_test {
197+
if let Err(err) = winit_window.set_cursor_hittest(window.cursor.hit_test) {
198+
window.cursor.hit_test = cache.window.cursor.hit_test;
199+
warn!(
200+
"Could not set cursor hit test for window {:?}: {:?}",
201+
window.title, err
202+
);
191203
}
204+
}
192205

193-
if window.decorations != cache.window.decorations
194-
&& window.decorations != winit_window.is_decorated()
195-
{
196-
winit_window.set_decorations(window.decorations);
197-
}
206+
if window.decorations != cache.window.decorations
207+
&& window.decorations != winit_window.is_decorated()
208+
{
209+
winit_window.set_decorations(window.decorations);
210+
}
198211

199-
if window.resizable != cache.window.resizable
200-
&& window.resizable != winit_window.is_resizable()
201-
{
202-
winit_window.set_resizable(window.resizable);
203-
}
212+
if window.resizable != cache.window.resizable
213+
&& window.resizable != winit_window.is_resizable()
214+
{
215+
winit_window.set_resizable(window.resizable);
216+
}
217+
218+
if window.enabled_buttons != cache.window.enabled_buttons {
219+
winit_window.set_enabled_buttons(convert_enabled_buttons(window.enabled_buttons));
220+
}
204221

205-
if window.enabled_buttons != cache.window.enabled_buttons {
206-
winit_window.set_enabled_buttons(convert_enabled_buttons(window.enabled_buttons));
222+
if window.resize_constraints != cache.window.resize_constraints {
223+
let constraints = window.resize_constraints.check_constraints();
224+
let min_inner_size = LogicalSize {
225+
width: constraints.min_width,
226+
height: constraints.min_height,
227+
};
228+
let max_inner_size = LogicalSize {
229+
width: constraints.max_width,
230+
height: constraints.max_height,
231+
};
232+
233+
winit_window.set_min_inner_size(Some(min_inner_size));
234+
if constraints.max_width.is_finite() && constraints.max_height.is_finite() {
235+
winit_window.set_max_inner_size(Some(max_inner_size));
207236
}
237+
}
208238

209-
if window.resize_constraints != cache.window.resize_constraints {
210-
let constraints = window.resize_constraints.check_constraints();
211-
let min_inner_size = LogicalSize {
212-
width: constraints.min_width,
213-
height: constraints.min_height,
239+
if window.position != cache.window.position {
240+
if let Some(position) = crate::winit_window_position(
241+
&window.position,
242+
&window.resolution,
243+
winit_window.available_monitors(),
244+
winit_window.primary_monitor(),
245+
winit_window.current_monitor(),
246+
) {
247+
let should_set = match winit_window.outer_position() {
248+
Ok(current_position) => current_position != position,
249+
_ => true,
214250
};
215-
let max_inner_size = LogicalSize {
216-
width: constraints.max_width,
217-
height: constraints.max_height,
218-
};
219-
220-
winit_window.set_min_inner_size(Some(min_inner_size));
221-
if constraints.max_width.is_finite() && constraints.max_height.is_finite() {
222-
winit_window.set_max_inner_size(Some(max_inner_size));
223-
}
224-
}
225251

226-
if window.position != cache.window.position {
227-
if let Some(position) = crate::winit_window_position(
228-
&window.position,
229-
&window.resolution,
230-
winit_window.available_monitors(),
231-
winit_window.primary_monitor(),
232-
winit_window.current_monitor(),
233-
) {
234-
let should_set = match winit_window.outer_position() {
235-
Ok(current_position) => current_position != position,
236-
_ => true,
237-
};
238-
239-
if should_set {
240-
winit_window.set_outer_position(position);
241-
}
252+
if should_set {
253+
winit_window.set_outer_position(position);
242254
}
243255
}
256+
}
244257

245-
if let Some(maximized) = window.internal.take_maximize_request() {
246-
winit_window.set_maximized(maximized);
247-
}
248-
249-
if let Some(minimized) = window.internal.take_minimize_request() {
250-
winit_window.set_minimized(minimized);
251-
}
258+
if let Some(maximized) = window.internal.take_maximize_request() {
259+
winit_window.set_maximized(maximized);
260+
}
252261

253-
if window.focused != cache.window.focused && window.focused {
254-
winit_window.focus_window();
255-
}
262+
if let Some(minimized) = window.internal.take_minimize_request() {
263+
winit_window.set_minimized(minimized);
264+
}
256265

257-
if window.window_level != cache.window.window_level {
258-
winit_window.set_window_level(convert_window_level(window.window_level));
259-
}
266+
if window.focused != cache.window.focused && window.focused {
267+
winit_window.focus_window();
268+
}
260269

261-
// Currently unsupported changes
262-
if window.transparent != cache.window.transparent {
263-
window.transparent = cache.window.transparent;
264-
warn!(
265-
"Winit does not currently support updating transparency after window creation."
266-
);
267-
}
270+
if window.window_level != cache.window.window_level {
271+
winit_window.set_window_level(convert_window_level(window.window_level));
272+
}
268273

269-
#[cfg(target_arch = "wasm32")]
270-
if window.canvas != cache.window.canvas {
271-
window.canvas = cache.window.canvas.clone();
272-
warn!(
273-
"Bevy currently doesn't support modifying the window canvas after initialization."
274-
);
275-
}
274+
// Currently unsupported changes
275+
if window.transparent != cache.window.transparent {
276+
window.transparent = cache.window.transparent;
277+
warn!("Winit does not currently support updating transparency after window creation.");
278+
}
276279

277-
if window.ime_enabled != cache.window.ime_enabled {
278-
winit_window.set_ime_allowed(window.ime_enabled);
279-
}
280+
#[cfg(target_arch = "wasm32")]
281+
if window.canvas != cache.window.canvas {
282+
window.canvas = cache.window.canvas.clone();
283+
warn!(
284+
"Bevy currently doesn't support modifying the window canvas after initialization."
285+
);
286+
}
280287

281-
if window.ime_position != cache.window.ime_position {
282-
winit_window.set_ime_cursor_area(
283-
LogicalPosition::new(window.ime_position.x, window.ime_position.y),
284-
PhysicalSize::new(10, 10),
285-
);
286-
}
288+
if window.ime_enabled != cache.window.ime_enabled {
289+
winit_window.set_ime_allowed(window.ime_enabled);
290+
}
287291

288-
if window.window_theme != cache.window.window_theme {
289-
winit_window.set_theme(window.window_theme.map(convert_window_theme));
290-
}
292+
if window.ime_position != cache.window.ime_position {
293+
winit_window.set_ime_cursor_area(
294+
LogicalPosition::new(window.ime_position.x, window.ime_position.y),
295+
PhysicalSize::new(10, 10),
296+
);
297+
}
291298

292-
if window.visible != cache.window.visible {
293-
winit_window.set_visible(window.visible);
294-
}
299+
if window.window_theme != cache.window.window_theme {
300+
winit_window.set_theme(window.window_theme.map(convert_window_theme));
301+
}
295302

296-
cache.window = window.clone();
303+
if window.visible != cache.window.visible {
304+
winit_window.set_visible(window.visible);
297305
}
306+
307+
cache.window = window.clone();
298308
}
299309
}

‎crates/bevy_winit/src/winit_windows.rs

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -55,18 +55,25 @@ impl WinitWindows {
5555
WindowMode::BorderlessFullscreen => winit_window_builder.with_fullscreen(Some(
5656
winit::window::Fullscreen::Borderless(event_loop.primary_monitor()),
5757
)),
58-
WindowMode::Fullscreen => {
59-
winit_window_builder.with_fullscreen(Some(winit::window::Fullscreen::Exclusive(
60-
get_best_videomode(&event_loop.primary_monitor().unwrap()),
61-
)))
58+
mode @ (WindowMode::Fullscreen | WindowMode::SizedFullscreen) => {
59+
if let Some(primary_monitor) = event_loop.primary_monitor() {
60+
let videomode = match mode {
61+
WindowMode::Fullscreen => get_best_videomode(&primary_monitor),
62+
WindowMode::SizedFullscreen => get_fitting_videomode(
63+
&primary_monitor,
64+
window.width() as u32,
65+
window.height() as u32,
66+
),
67+
_ => unreachable!(),
68+
};
69+
70+
winit_window_builder
71+
.with_fullscreen(Some(winit::window::Fullscreen::Exclusive(videomode)))
72+
} else {
73+
warn!("Could not determine primary monitor, ignoring exclusive fullscreen request for window {:?}", window.title);
74+
winit_window_builder
75+
}
6276
}
63-
WindowMode::SizedFullscreen => winit_window_builder.with_fullscreen(Some(
64-
winit::window::Fullscreen::Exclusive(get_fitting_videomode(
65-
&event_loop.primary_monitor().unwrap(),
66-
window.width() as u32,
67-
window.height() as u32,
68-
)),
69-
)),
7077
WindowMode::Windowed => {
7178
if let Some(position) = winit_window_position(
7279
&window.position,

0 commit comments

Comments
 (0)