From 519abbca1141bf904695b1c0cf4184addc6883c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Mockers?= Date: Wed, 5 Jun 2024 20:13:59 +0200 Subject: [PATCH] make sure windows are dropped on main thread (#13686) # Objective - On macOS, closing a window by respawning its entity freezes ## Solution - `WindowWrapper` is keeping an `Arc` of the window, to be able to access it from the rendering thread. Winit windows are closed when they are dropped. This need to happen on the main thread on macOS - Dropping it as soon as the window is closed means the last remaining `Arc` will be in the rendering thread - This PR keeps the `Arc` for one frame in the rendering thread before actually dropping it --- crates/bevy_winit/src/system.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/crates/bevy_winit/src/system.rs b/crates/bevy_winit/src/system.rs index fe77e9bbe0..84975bf1e3 100644 --- a/crates/bevy_winit/src/system.rs +++ b/crates/bevy_winit/src/system.rs @@ -4,12 +4,12 @@ use bevy_ecs::{ prelude::{Changed, Component}, query::QueryFilter, removal_detection::RemovedComponents, - system::{NonSendMut, Query, SystemParamItem}, + system::{Local, NonSendMut, Query, SystemParamItem}, }; use bevy_utils::tracing::{error, info, warn}; use bevy_window::{ ClosingWindow, RawHandleWrapper, Window, WindowClosed, WindowClosing, WindowCreated, - WindowMode, WindowResized, + WindowMode, WindowResized, WindowWrapper, }; use winit::dpi::{LogicalPosition, LogicalSize, PhysicalPosition, PhysicalSize}; @@ -123,7 +123,10 @@ pub(crate) fn despawn_windows( mut closing_events: EventWriter, mut closed_events: EventWriter, mut winit_windows: NonSendMut, + mut windows_to_drop: Local>>, ) { + // Drop all the windows that are waiting to be closed + windows_to_drop.clear(); for window in closing.iter() { closing_events.send(WindowClosing { window }); } @@ -133,7 +136,13 @@ pub(crate) fn despawn_windows( // rather than having the component added // and removed in the same frame. if !window_entities.contains(window) { - winit_windows.remove_window(window); + if let Some(window) = winit_windows.remove_window(window) { + // Keeping WindowWrapper that are dropped for one frame + // Otherwise the last `Arc` of the window could be in the rendering thread, and dropped there + // This would hang on macOS + // Keeping the wrapper and dropping it next frame in this system ensure its dropped in the main thread + windows_to_drop.push(window); + } closed_events.send(WindowClosed { window }); } }