From a6c6f03556d2df727fa2b596d9d553bf27c401f8 Mon Sep 17 00:00:00 2001 From: Ian Douglas Scott Date: Wed, 1 May 2024 19:22:46 -0700 Subject: [PATCH] Use safe window handles and `wgpu::Instance::create_surface` I noticed that https://github.com/bevyengine/bevy/pull/12978 introduces a reference counted wrapper around windows, but still uses `RawWindowHandle` and `RawDisplayHandle`, with `wgpu::Instance::create_surface_unsafe`. This can be changed easily enough to use `wgpu::Instance::create_surface`, and not use the raw handles anywhere. Apparently we can just cast a `Arc` to an `Arc`, so that makes `WindowWrapper` a little simpler too. --- crates/bevy_render/src/view/window/mod.rs | 21 +++++------- crates/bevy_window/src/raw_handle.rs | 40 +++++++++++------------ 2 files changed, 28 insertions(+), 33 deletions(-) diff --git a/crates/bevy_render/src/view/window/mod.rs b/crates/bevy_render/src/view/window/mod.rs index 762771a41edd5..783eff8223925 100644 --- a/crates/bevy_render/src/view/window/mod.rs +++ b/crates/bevy_render/src/view/window/mod.rs @@ -20,7 +20,7 @@ use std::{ sync::PoisonError, }; use wgpu::{ - BufferUsages, SurfaceConfiguration, SurfaceTargetUnsafe, TextureFormat, TextureUsages, + BufferUsages, SurfaceConfiguration, SurfaceTarget, TextureFormat, TextureUsages, TextureViewDescriptor, }; @@ -456,18 +456,13 @@ pub fn create_surfaces( .surfaces .entry(window.entity) .or_insert_with(|| { - let surface_target = SurfaceTargetUnsafe::RawHandle { - raw_display_handle: window.handle.display_handle, - raw_window_handle: window.handle.window_handle, - }; - // SAFETY: The window handles in ExtractedWindows will always be valid objects to create surfaces on - let surface = unsafe { - // NOTE: On some OSes this MUST be called from the main thread. - // As of wgpu 0.15, only fallible if the given window is a HTML canvas and obtaining a WebGPU or WebGL2 context fails. - render_instance - .create_surface_unsafe(surface_target) - .expect("Failed to create wgpu surface") - }; + // SAFETY: On some OSes this MUST be called from the main thread. Otherwise this is + // safe. + let surface_target = SurfaceTarget::from(unsafe { window.handle.get_handle() }); + // As of wgpu 0.15, only fallible if the given window is a HTML canvas and obtaining a WebGPU or WebGL2 context fails. + let surface = render_instance + .create_surface(surface_target) + .expect("Failed to create wgpu surface"); let caps = surface.get_capabilities(&render_adapter); let formats = caps.formats; // For future HDR output support, we'll need to request a format that supports HDR, diff --git a/crates/bevy_window/src/raw_handle.rs b/crates/bevy_window/src/raw_handle.rs index 0b799a1142936..145f8889d1cf0 100644 --- a/crates/bevy_window/src/raw_handle.rs +++ b/crates/bevy_window/src/raw_handle.rs @@ -2,10 +2,9 @@ use bevy_ecs::prelude::Component; use raw_window_handle::{ - DisplayHandle, HandleError, HasDisplayHandle, HasWindowHandle, RawDisplayHandle, - RawWindowHandle, WindowHandle, + DisplayHandle, HandleError, HasDisplayHandle, HasWindowHandle, WindowHandle, }; -use std::{any::Any, marker::PhantomData, ops::Deref, sync::Arc}; +use std::{fmt, ops::Deref, sync::Arc}; /// A wrapper over a window. /// @@ -16,8 +15,7 @@ use std::{any::Any, marker::PhantomData, ops::Deref, sync::Arc}; /// which gets picked up by the renderer during extraction. #[derive(Debug)] pub struct WindowWrapper { - reference: Arc, - ty: PhantomData, + reference: Arc, } impl WindowWrapper { @@ -25,7 +23,6 @@ impl WindowWrapper { pub fn new(window: W) -> WindowWrapper { WindowWrapper { reference: Arc::new(window), - ty: PhantomData, } } } @@ -34,22 +31,27 @@ impl Deref for WindowWrapper { type Target = W; fn deref(&self) -> &Self::Target { - self.reference.downcast_ref::().unwrap() + &self.reference } } -/// A wrapper over [`RawWindowHandle`] and [`RawDisplayHandle`] that allows us to safely pass it across threads. +trait WindowTrait: HasWindowHandle + HasDisplayHandle {} +impl WindowTrait for T {} + +/// A wrapper over [`HasWindowHandle`] and [`HasDisplayHandle`] that allows us to safely pass it across threads. /// /// Depending on the platform, the underlying pointer-containing handle cannot be used on all threads, -/// and so we cannot simply make it (or any type that has a safe operation to get a [`RawWindowHandle`] or [`RawDisplayHandle`]) +/// and so we cannot simply make it (or any type that has a safe operation to get a [`WindowHandle`] or [`DisplayHandle`]) /// thread-safe. -#[derive(Debug, Clone, Component)] +#[derive(Clone, Component)] pub struct RawHandleWrapper { - _window: Arc, - /// Raw handle to a window. - pub window_handle: RawWindowHandle, - /// Raw handle to the display server. - pub display_handle: RawDisplayHandle, + window: Arc, +} + +impl fmt::Debug for RawHandleWrapper { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> { + f.debug_struct("RawHandleWrapper").finish_non_exhaustive() + } } impl RawHandleWrapper { @@ -58,9 +60,7 @@ impl RawHandleWrapper { window: &WindowWrapper, ) -> Result { Ok(RawHandleWrapper { - _window: window.reference.clone(), - window_handle: window.window_handle()?.as_raw(), - display_handle: window.display_handle()?.as_raw(), + window: window.reference.clone(), }) } @@ -101,7 +101,7 @@ impl HasWindowHandle for ThreadLockedRawWindowHandleWrapper { // as the `raw_window_handle` method is safe. We cannot guarantee that all calls // of this method are correct (as it may be off the main thread on an incompatible platform), // and so exposing a safe method to get a [`RawWindowHandle`] directly would be UB. - Ok(unsafe { WindowHandle::borrow_raw(self.0.window_handle) }) + self.0.window.window_handle() } } @@ -113,6 +113,6 @@ impl HasDisplayHandle for ThreadLockedRawWindowHandleWrapper { // as the `raw_display_handle` method is safe. We cannot guarantee that all calls // of this method are correct (as it may be off the main thread on an incompatible platform), // and so exposing a safe method to get a [`RawDisplayHandle`] directly would be UB. - Ok(unsafe { DisplayHandle::borrow_raw(self.0.display_handle) }) + self.0.window.display_handle() } }