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

Use safe window handles and wgpu::Instance::create_surface #13172

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
21 changes: 8 additions & 13 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")
};
// 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,
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()
}
}
Comment on lines +51 to 55
Copy link
Member

Choose a reason for hiding this comment

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

Can this be automatically derived by #[derive(Debug)], or does it complain that dyn WindowTrait does not implement Debug?

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, it's needed since dyn WindowTrait doesn't implement Debug. (The standard library has an implementation for dyn Any.) It seems simplest to just implement Debug for RawHandleWrapper.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, sounds good!


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()
}
}