Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce a WindowWrapper to extend the lifetime of the window when using pipelined rendering #12978

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions crates/bevy_window/src/raw_handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,38 @@ use raw_window_handle::{
DisplayHandle, HandleError, HasDisplayHandle, HasWindowHandle, RawDisplayHandle,
RawWindowHandle, WindowHandle,
};
use std::{any::Any, marker::PhantomData, ops::Deref, sync::Arc};

/// A wrapper over a window.
///
/// This allows us to extend the lifetime of the window, so it doesn't get eagerly dropped while a
/// pipelined renderer still has frames in flight that need to draw to it.
///
/// This is achieved by storing a shared reference to the window in the [`RawHandleWrapper`],
/// which gets picked up by the renderer during extraction.
#[derive(Debug)]
pub struct WindowWrapper<W> {
reference: Arc<dyn Any + Send + Sync>,
Copy link
Contributor

@hymm hymm Apr 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this need to be type erased instead of storing a Arc<W>?

Copy link
Contributor Author

@Friz64 Friz64 Apr 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is so that we can later store this Arc in the RawHandleWrapper without static typing:

pub struct RawHandleWrapper {
_window: Arc<dyn Any + Send + Sync>,

ty: PhantomData<W>,
}

impl<W: Send + Sync + 'static> WindowWrapper<W> {
/// Creates a `WindowWrapper` from a window.
pub fn new(window: W) -> WindowWrapper<W> {
WindowWrapper {
reference: Arc::new(window),
ty: PhantomData,
}
}
}

impl<W: 'static> Deref for WindowWrapper<W> {
type Target = W;

fn deref(&self) -> &Self::Target {
self.reference.downcast_ref::<W>().unwrap()
}
}

/// A wrapper over [`RawWindowHandle`] and [`RawDisplayHandle`] that allows us to safely pass it across threads.
///
Expand All @@ -13,13 +45,25 @@ use raw_window_handle::{
/// thread-safe.
#[derive(Debug, Clone, Component)]
pub struct RawHandleWrapper {
_window: Arc<dyn Any + Send + Sync>,
/// Raw handle to a window.
pub window_handle: RawWindowHandle,
/// Raw handle to the display server.
pub display_handle: RawDisplayHandle,
}

impl RawHandleWrapper {
/// Creates a `RawHandleWrapper` from a `WindowWrapper`.
pub fn new<W: HasWindowHandle + HasDisplayHandle + 'static>(
window: &WindowWrapper<W>,
) -> Result<RawHandleWrapper, HandleError> {
Ok(RawHandleWrapper {
_window: window.reference.clone(),
window_handle: window.window_handle()?.as_raw(),
display_handle: window.display_handle()?.as_raw(),
})
}

/// Returns a [`HasWindowHandle`] + [`HasDisplayHandle`] impl, which exposes [`WindowHandle`] and [`DisplayHandle`].
///
/// # Safety
Expand Down
6 changes: 1 addition & 5 deletions crates/bevy_winit/src/system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ use bevy_window::{
RawHandleWrapper, Window, WindowClosed, WindowCreated, WindowMode, WindowResized,
};

use raw_window_handle::{HasDisplayHandle, HasWindowHandle};
use winit::{
dpi::{LogicalPosition, LogicalSize, PhysicalPosition, PhysicalSize},
event_loop::EventLoopWindowTarget,
Expand Down Expand Up @@ -75,10 +74,7 @@ pub fn create_windows<F: QueryFilter + 'static>(
.set_scale_factor(winit_window.scale_factor() as f32);
commands
.entity(entity)
.insert(RawHandleWrapper {
window_handle: winit_window.window_handle().unwrap().as_raw(),
display_handle: winit_window.display_handle().unwrap().as_raw(),
})
.insert(RawHandleWrapper::new(winit_window).unwrap())
.insert(CachedWindow {
window: window.clone(),
});
Expand Down
17 changes: 11 additions & 6 deletions crates/bevy_winit/src/winit_windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ use bevy_ecs::entity::Entity;

use bevy_ecs::entity::EntityHashMap;
use bevy_utils::{tracing::warn, HashMap};
use bevy_window::{CursorGrabMode, Window, WindowMode, WindowPosition, WindowResolution};
use bevy_window::{
CursorGrabMode, Window, WindowMode, WindowPosition, WindowResolution, WindowWrapper,
};

use winit::{
dpi::{LogicalSize, PhysicalPosition},
Expand All @@ -20,7 +22,7 @@ use crate::{
#[derive(Debug, Default)]
pub struct WinitWindows {
/// Stores [`winit`] windows by window identifier.
pub windows: HashMap<winit::window::WindowId, winit::window::Window>,
pub windows: HashMap<winit::window::WindowId, WindowWrapper<winit::window::Window>>,
/// Maps entities to `winit` window identifiers.
pub entity_to_winit: EntityHashMap<winit::window::WindowId>,
/// Maps `winit` window identifiers to entities.
Expand All @@ -41,7 +43,7 @@ impl WinitWindows {
adapters: &mut AccessKitAdapters,
handlers: &mut WinitActionHandlers,
accessibility_requested: &AccessibilityRequested,
) -> &winit::window::Window {
) -> &WindowWrapper<winit::window::Window> {
let mut winit_window_builder = winit::window::WindowBuilder::new();

// Due to a UIA limitation, winit windows need to be invisible for the
Expand Down Expand Up @@ -240,12 +242,12 @@ impl WinitWindows {

self.windows
.entry(winit_window.id())
.insert(winit_window)
.insert(WindowWrapper::new(winit_window))
.into_mut()
}

/// Get the winit window that is associated with our entity.
pub fn get_window(&self, entity: Entity) -> Option<&winit::window::Window> {
pub fn get_window(&self, entity: Entity) -> Option<&WindowWrapper<winit::window::Window>> {
self.entity_to_winit
.get(&entity)
.and_then(|winit_id| self.windows.get(winit_id))
Expand All @@ -261,7 +263,10 @@ impl WinitWindows {
/// Remove a window from winit.
///
/// This should mostly just be called when the window is closing.
pub fn remove_window(&mut self, entity: Entity) -> Option<winit::window::Window> {
pub fn remove_window(
&mut self,
entity: Entity,
) -> Option<WindowWrapper<winit::window::Window>> {
let winit_id = self.entity_to_winit.remove(&entity)?;
self.winit_to_entity.remove(&winit_id);
self.windows.remove(&winit_id)
Expand Down