Ensure clean exit (#13236)

# Objective

Fixes two issues related to #13208.

First, we ensure render resources for a window are always dropped first
to ensure that the `winit::Window` always drops on the main thread when
it is removed from `WinitWindows`. Previously, changes in #12978 caused
the window to drop in the render world, causing issues.

We accomplish this by delaying despawning the window by a frame by
inserting a marker component `ClosingWindow` that indicates the window
has been requested to close and is in the process of closing. The render
world now responds to the equivalent `WindowClosing` event rather than
`WindowCloseed` which now fires after the render resources are
guarunteed to be cleaned up.

Secondly, fixing the above caused (revealed?) that additional events
were being delivered to the the event loop handler after exit had
already been requested: in my testing `RedrawRequested` and
`LoopExiting`. This caused errors to be reported try to send an exit
event on the close channel. There are two options here:
- Guard the handler so no additional events are delivered once the app
is exiting. I ~considered this but worried it might be confusing or bug
prone if in the future someone wants to handle `LoopExiting` or some
other event to clean-up while exiting.~ We are now taking this approach.
- Only send an exit signal if we are not already exiting. ~It doesn't
appear to cause any problems to handle the extra events so this seems
safer.~
 
Fixing this also appears to have fixed #13231.

Fixes #10260.

## Testing

Tested on mac only.

---

## Changelog

### Added
- A `WindowClosing` event has been added that indicates the window will
be despawned on the next frame.

### Changed
- Windows now close a frame after their exit has been requested.

## Migration Guide
- Ensure custom exit logic does not rely on the app exiting the same
frame as a window is closed.
This commit is contained in:
charlotte 2024-05-12 08:56:01 -07:00 committed by GitHub
parent 278380394f
commit dc0fdd6ad9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 55 additions and 12 deletions

View File

@ -12,7 +12,7 @@ use bevy_ecs::{entity::EntityHashMap, prelude::*};
use bevy_utils::warn_once; use bevy_utils::warn_once;
use bevy_utils::{default, tracing::debug, HashSet}; use bevy_utils::{default, tracing::debug, HashSet};
use bevy_window::{ use bevy_window::{
CompositeAlphaMode, PresentMode, PrimaryWindow, RawHandleWrapper, Window, WindowClosed, CompositeAlphaMode, PresentMode, PrimaryWindow, RawHandleWrapper, Window, WindowClosing,
}; };
use std::{ use std::{
num::NonZeroU32, num::NonZeroU32,
@ -117,7 +117,7 @@ impl DerefMut for ExtractedWindows {
fn extract_windows( fn extract_windows(
mut extracted_windows: ResMut<ExtractedWindows>, mut extracted_windows: ResMut<ExtractedWindows>,
screenshot_manager: Extract<Res<ScreenshotManager>>, screenshot_manager: Extract<Res<ScreenshotManager>>,
mut closed: Extract<EventReader<WindowClosed>>, mut closing: Extract<EventReader<WindowClosing>>,
windows: Extract<Query<(Entity, &Window, &RawHandleWrapper, Option<&PrimaryWindow>)>>, windows: Extract<Query<(Entity, &Window, &RawHandleWrapper, Option<&PrimaryWindow>)>>,
mut removed: Extract<RemovedComponents<RawHandleWrapper>>, mut removed: Extract<RemovedComponents<RawHandleWrapper>>,
mut window_surfaces: ResMut<WindowSurfaces>, mut window_surfaces: ResMut<WindowSurfaces>,
@ -177,9 +177,9 @@ fn extract_windows(
} }
} }
for closed_window in closed.read() { for closing_window in closing.read() {
extracted_windows.remove(&closed_window.window); extracted_windows.remove(&closing_window.window);
window_surfaces.remove(&closed_window.window); window_surfaces.remove(&closing_window.window);
} }
for removed_window in removed.read() { for removed_window in removed.read() {
extracted_windows.remove(&removed_window); extracted_windows.remove(&removed_window);

View File

@ -94,6 +94,20 @@ pub struct WindowClosed {
pub window: Entity, pub window: Entity,
} }
/// An event that is sent whenever a window is closing. This will be sent when
/// after a [`WindowCloseRequested`] event is received and the window is in the process of closing.
#[derive(Event, Debug, Clone, PartialEq, Eq, Reflect)]
#[reflect(Debug, PartialEq)]
#[cfg_attr(
feature = "serialize",
derive(serde::Serialize, serde::Deserialize),
reflect(Serialize, Deserialize)
)]
pub struct WindowClosing {
/// Window that has been requested to close and is the process of closing.
pub window: Entity,
}
/// An event that is sent whenever a window is destroyed by the underlying window system. /// An event that is sent whenever a window is destroyed by the underlying window system.
/// ///
/// Note that if your application only has a single window, this event may be your last chance to /// Note that if your application only has a single window, this event may be your last chance to

View File

@ -89,6 +89,7 @@ impl Plugin for WindowPlugin {
#[allow(deprecated)] #[allow(deprecated)]
app.add_event::<WindowResized>() app.add_event::<WindowResized>()
.add_event::<WindowCreated>() .add_event::<WindowCreated>()
.add_event::<WindowClosing>()
.add_event::<WindowClosed>() .add_event::<WindowClosed>()
.add_event::<WindowCloseRequested>() .add_event::<WindowCloseRequested>()
.add_event::<WindowDestroyed>() .add_event::<WindowDestroyed>()
@ -139,6 +140,7 @@ impl Plugin for WindowPlugin {
.register_type::<RequestRedraw>() .register_type::<RequestRedraw>()
.register_type::<WindowCreated>() .register_type::<WindowCreated>()
.register_type::<WindowCloseRequested>() .register_type::<WindowCloseRequested>()
.register_type::<WindowClosing>()
.register_type::<WindowClosed>() .register_type::<WindowClosed>()
.register_type::<CursorMoved>() .register_type::<CursorMoved>()
.register_type::<CursorEntered>() .register_type::<CursorEntered>()

View File

@ -1,4 +1,4 @@
use crate::{PrimaryWindow, Window, WindowCloseRequested}; use crate::{ClosingWindow, PrimaryWindow, Window, WindowCloseRequested};
use bevy_app::AppExit; use bevy_app::AppExit;
use bevy_ecs::prelude::*; use bevy_ecs::prelude::*;
@ -39,8 +39,17 @@ pub fn exit_on_primary_closed(
/// Ensure that you read the caveats documented on that field if doing so. /// Ensure that you read the caveats documented on that field if doing so.
/// ///
/// [`WindowPlugin`]: crate::WindowPlugin /// [`WindowPlugin`]: crate::WindowPlugin
pub fn close_when_requested(mut commands: Commands, mut closed: EventReader<WindowCloseRequested>) { pub fn close_when_requested(
mut commands: Commands,
mut closed: EventReader<WindowCloseRequested>,
closing: Query<Entity, With<ClosingWindow>>,
) {
// This was inserted by us on the last frame so now we can despawn the window
for window in closing.iter() {
commands.entity(window).despawn();
}
// Mark the window as closing so we can despawn it on the next frame
for event in closed.read() { for event in closed.read() {
commands.entity(event.window).despawn(); commands.entity(event.window).insert(ClosingWindow);
} }
} }

View File

@ -1171,6 +1171,11 @@ impl Default for EnabledButtons {
} }
} }
/// Marker component for a [`Window`] that has been requested to close and
/// is in the process of closing (on the next frame).
#[derive(Component)]
pub struct ClosingWindow;
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;

View File

@ -302,6 +302,11 @@ pub fn winit_runner(mut app: App) -> AppExit {
let mut winit_events = Vec::default(); let mut winit_events = Vec::default();
// set up the event loop // set up the event loop
let event_handler = move |event, event_loop: &EventLoopWindowTarget<UserEvent>| { let event_handler = move |event, event_loop: &EventLoopWindowTarget<UserEvent>| {
// The event loop is in the process of exiting, so don't deliver any new events
if event_loop.exiting() {
return;
}
handle_winit_event( handle_winit_event(
&mut app, &mut app,
&mut runner_state, &mut runner_state,

View File

@ -8,7 +8,8 @@ use bevy_ecs::{
}; };
use bevy_utils::tracing::{error, info, warn}; use bevy_utils::tracing::{error, info, warn};
use bevy_window::{ use bevy_window::{
RawHandleWrapper, Window, WindowClosed, WindowCreated, WindowMode, WindowResized, ClosingWindow, RawHandleWrapper, Window, WindowClosed, WindowClosing, WindowCreated,
WindowMode, WindowResized,
}; };
use winit::{ use winit::{
@ -16,6 +17,7 @@ use winit::{
event_loop::EventLoopWindowTarget, event_loop::EventLoopWindowTarget,
}; };
use bevy_ecs::query::With;
#[cfg(target_arch = "wasm32")] #[cfg(target_arch = "wasm32")]
use winit::platform::web::WindowExtWebSys; use winit::platform::web::WindowExtWebSys;
@ -97,18 +99,24 @@ pub fn create_windows<F: QueryFilter + 'static>(
} }
pub(crate) fn despawn_windows( pub(crate) fn despawn_windows(
closing: Query<Entity, With<ClosingWindow>>,
mut closed: RemovedComponents<Window>, mut closed: RemovedComponents<Window>,
window_entities: Query<&Window>, window_entities: Query<&Window>,
mut close_events: EventWriter<WindowClosed>, mut closing_events: EventWriter<WindowClosing>,
mut closed_events: EventWriter<WindowClosed>,
mut winit_windows: NonSendMut<WinitWindows>, mut winit_windows: NonSendMut<WinitWindows>,
) { ) {
for window in closing.iter() {
closing_events.send(WindowClosing { window });
}
for window in closed.read() { for window in closed.read() {
info!("Closing window {:?}", window); info!("Closing window {:?}", window);
// Guard to verify that the window is in fact actually gone, // Guard to verify that the window is in fact actually gone,
// rather than having the component added and removed in the same frame. // rather than having the component added
// and removed in the same frame.
if !window_entities.contains(window) { if !window_entities.contains(window) {
winit_windows.remove_window(window); winit_windows.remove_window(window);
close_events.send(WindowClosed { window }); closed_events.send(WindowClosed { window });
} }
} }
} }