Skip to content

Commit

Permalink
Adds token system to back-channel whether a Window's surface is closed.
Browse files Browse the repository at this point in the history
To prevent a Winit Window from being closed, and potentially invalidating
the underlying RawWindowHandle, this creates a SurfaceToken  which
should be held by the `wgpu` backend as long as a Window's render
surface exists.
  • Loading branch information
robojeb committed Oct 28, 2023
1 parent a830530 commit 9f806b7
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 24 deletions.
55 changes: 34 additions & 21 deletions crates/bevy_render/src/view/window/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ use bevy_app::{App, Plugin};
use bevy_ecs::prelude::*;
use bevy_utils::{default, tracing::debug, HashMap, HashSet};
use bevy_window::{
CompositeAlphaMode, PresentMode, PrimaryWindow, RawHandleWrapper, Window, WindowClosed,
CompositeAlphaMode, PresentMode, PrimaryWindow, RawHandleWrapper, SurfaceToken, Window,
WindowCloseRequested,
};
use std::{
ops::{Deref, DerefMut},
Expand Down Expand Up @@ -71,6 +72,7 @@ pub struct ExtractedWindow {
pub present_mode_changed: bool,
pub alpha_mode: CompositeAlphaMode,
pub screenshot_func: Option<screenshot::ScreenshotFn>,
pub surface_token: SurfaceToken,
}

impl ExtractedWindow {
Expand Down Expand Up @@ -109,12 +111,20 @@ impl DerefMut for ExtractedWindows {
fn extract_windows(
mut extracted_windows: ResMut<ExtractedWindows>,
screenshot_manager: Extract<Res<ScreenshotManager>>,
mut closed: Extract<EventReader<WindowClosed>>,
windows: Extract<Query<(Entity, &Window, &RawHandleWrapper, Option<&PrimaryWindow>)>>,
mut closed: Extract<EventReader<WindowCloseRequested>>,
windows: Extract<
Query<(
Entity,
&Window,
&SurfaceToken,
&RawHandleWrapper,
Option<&PrimaryWindow>,
)>,
>,
mut removed: Extract<RemovedComponents<RawHandleWrapper>>,
mut window_surfaces: ResMut<WindowSurfaces>,
) {
for (entity, window, handle, primary) in windows.iter() {
for (entity, window, surface_token, handle, primary) in windows.iter() {
if primary.is_some() {
extracted_windows.primary = Some(entity);
}
Expand All @@ -124,21 +134,24 @@ fn extract_windows(
window.resolution.physical_height().max(1),
);

let extracted_window = extracted_windows.entry(entity).or_insert(ExtractedWindow {
entity,
handle: handle.clone(),
physical_width: new_width,
physical_height: new_height,
present_mode: window.present_mode,
swap_chain_texture: None,
swap_chain_texture_view: None,
size_changed: false,
swap_chain_texture_format: None,
present_mode_changed: false,
alpha_mode: window.composite_alpha_mode,
screenshot_func: None,
screenshot_memory: None,
});
let extracted_window = extracted_windows
.entry(entity)
.or_insert_with(|| ExtractedWindow {
entity,
handle: handle.clone(),
physical_width: new_width,
physical_height: new_height,
present_mode: window.present_mode,
swap_chain_texture: None,
swap_chain_texture_view: None,
size_changed: false,
swap_chain_texture_format: None,
present_mode_changed: false,
alpha_mode: window.composite_alpha_mode,
screenshot_func: None,
screenshot_memory: None,
surface_token: surface_token.clone(),
});

// NOTE: Drop the swap chain frame here
extracted_window.swap_chain_texture_view = None;
Expand Down Expand Up @@ -169,12 +182,12 @@ fn extract_windows(
}

for closed_window in closed.read() {
extracted_windows.remove(&closed_window.window);
window_surfaces.remove(&closed_window.window);
extracted_windows.remove(&closed_window.window);
}
for removed_window in removed.read() {
extracted_windows.remove(&removed_window);
window_surfaces.remove(&removed_window);
extracted_windows.remove(&removed_window);
}
// This lock will never block because `callbacks` is `pub(crate)` and this is the singular callsite where it's locked.
// Even if a user had multiple copies of this system, since the system has a mutable resource access the two systems would never run
Expand Down
3 changes: 3 additions & 0 deletions crates/bevy_window/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,15 @@ impl Plugin for WindowPlugin {
.world
.spawn(primary_window.clone())
.insert(PrimaryWindow)
.insert(SurfaceToken::default())
.id();
if let Some(mut focus) = app.world.get_resource_mut::<Focus>() {
**focus = Some(initial_focus);
}
}

app.add_systems(Last, fixup_window_surface);

match self.exit_condition {
ExitCondition::OnPrimaryClosed => {
app.add_systems(PostUpdate, exit_on_primary_closed);
Expand Down
42 changes: 39 additions & 3 deletions crates/bevy_window/src/system.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use crate::{PrimaryWindow, Window, WindowCloseRequested};
use crate::{PrimaryWindow, SurfaceToken, Window, WindowCloseRequested};

use bevy_app::AppExit;
use bevy_ecs::prelude::*;
use bevy_input::{keyboard::KeyCode, Input};
use bevy_utils::HashSet;

/// Exit the application when there are no open windows.
///
Expand Down Expand Up @@ -40,10 +41,34 @@ pub fn exit_on_primary_closed(
/// Ensure that you read the caveats documented on that field if doing so.
///
/// [`WindowPlugin`]: crate::WindowPlugin
pub fn close_when_requested(mut commands: Commands, mut closed: EventReader<WindowCloseRequested>) {
pub fn close_when_requested(
mut commands: Commands,
tokens: Query<&SurfaceToken>,
mut closed: EventReader<WindowCloseRequested>,
mut waiting_to_close: Local<HashSet<Entity>>,
) {
for event in closed.read() {
commands.entity(event.window).despawn();
if let Ok(token) = tokens.get(event.window) {
// Check if that is okay
if token.is_safe_to_close_window() {
commands.entity(event.window).despawn();
} else {
// Stash for later when the renderer cleans up the surface
waiting_to_close.insert(event.window);
}
}
}

waiting_to_close.retain(|window_entity| {
if let Ok(token) = tokens.get(*window_entity) {
if token.is_safe_to_close_window() {
commands.entity(*window_entity).despawn();
return false;
}
}

true
})
}

/// Close the focused window whenever the escape key (<kbd>Esc</kbd>) is pressed
Expand All @@ -64,3 +89,14 @@ pub fn close_on_esc(
}
}
}

/// Windows need to hold on to a unique [SurfaceToken] to know if they are able
/// to be despawned
pub fn fixup_window_surface(
mut commands: Commands,
missing_surface_token: Query<Entity, (With<Window>, Without<SurfaceToken>)>,
) {
missing_surface_token.for_each(|entity| {
commands.entity(entity).insert(SurfaceToken::default());
});
}
24 changes: 24 additions & 0 deletions crates/bevy_window/src/window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use bevy_ecs::{
};
use bevy_math::{DVec2, IVec2, Vec2};
use bevy_reflect::{std_traits::ReflectDefault, Reflect};
use std::sync::Arc;

#[cfg(feature = "serialize")]
use bevy_reflect::{ReflectDeserialize, ReflectSerialize};
Expand Down Expand Up @@ -1072,6 +1073,29 @@ impl Default for EnabledButtons {
}
}

/// A Token to track the presence of an active rendering surface for this Window.
///
/// The rendering back-end will hold a copy of this Token while the surface is
/// active. The Window should be kept alive until [is_safe_to_close_window()]
/// returns `true`
#[derive(Debug, Component, Clone, Default)]
#[cfg_attr(
feature = "serialize",
derive(serde::Serialize, serde::Deserialize),
reflect(Serialize, Deserialize)
)]
pub struct SurfaceToken {
state: Arc<()>,
}

impl SurfaceToken {
/// Check if this [SurfaceToken] is unique and thus the Window can be
/// closed.
pub fn is_safe_to_close_window(&self) -> bool {
dbg!(Arc::strong_count(&self.state)) == 1
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down

0 comments on commit 9f806b7

Please sign in to comment.