Skip to content

Commit

Permalink
Use safe window handles and wgpu::Instance::create_surface
Browse files Browse the repository at this point in the history
I noticed that bevyengine#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<W>` to an `Arc<dyn Trait>`, so that
makes `WindowWrapper` a little simpler too.
  • Loading branch information
ids1024 committed May 2, 2024
1 parent abddbf2 commit 042d43d
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 32 deletions.
19 changes: 7 additions & 12 deletions crates/bevy_render/src/view/window/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use std::{
sync::PoisonError,
};
use wgpu::{
BufferUsages, SurfaceConfiguration, SurfaceTargetUnsafe, TextureFormat, TextureUsages,
BufferUsages, SurfaceConfiguration, SurfaceTarget, TextureFormat, TextureUsages,
TextureViewDescriptor,
};

Expand Down Expand Up @@ -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")
};
// NOTE: On some OSes this MUST be called from the main thread.
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,
Expand Down
40 changes: 20 additions & 20 deletions crates/bevy_window/src/raw_handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand All @@ -16,16 +15,14 @@ use std::{any::Any, marker::PhantomData, ops::Deref, sync::Arc};
/// which gets picked up by the renderer during extraction.
#[derive(Debug)]
pub struct WindowWrapper<W> {
reference: Arc<dyn Any + Send + Sync>,
ty: PhantomData<W>,
reference: Arc<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,
}
}
}
Expand All @@ -34,22 +31,27 @@ impl<W: 'static> Deref for WindowWrapper<W> {
type Target = W;

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

/// A wrapper over [`RawWindowHandle`] and [`RawDisplayHandle`] that allows us to safely pass it across threads.
trait WindowTrait: HasWindowHandle + HasDisplayHandle {}
impl<T: HasWindowHandle + HasDisplayHandle> 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<dyn Any + Send + Sync>,
/// Raw handle to a window.
pub window_handle: RawWindowHandle,
/// Raw handle to the display server.
pub display_handle: RawDisplayHandle,
window: Arc<dyn WindowTrait>,
}

impl fmt::Debug for RawHandleWrapper {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> {
f.debug_struct("RawHandleWrapper").finish_non_exhaustive()
}
}

impl RawHandleWrapper {
Expand All @@ -58,9 +60,7 @@ impl RawHandleWrapper {
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(),
window: window.reference.clone(),
})
}

Expand Down Expand Up @@ -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()
}
}

Expand All @@ -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()
}
}

0 comments on commit 042d43d

Please sign in to comment.