Skip to content

Commit

Permalink
[WIP] Make surface creation safe
Browse files Browse the repository at this point in the history
- Partially implements gfx-rs#1463
  • Loading branch information
parasyte committed Jan 27, 2023
1 parent fe2b230 commit 73fbb52
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 29 deletions.
13 changes: 6 additions & 7 deletions wgpu/examples/framework.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ pub trait Example: 'static + Sized {
}

struct Setup {
window: winit::window::Window,
event_loop: EventLoop<()>,
instance: wgpu::Instance,
size: winit::dpi::PhysicalSize<u32>,
Expand Down Expand Up @@ -164,11 +163,11 @@ async fn setup<E: Example>(title: &str) -> Setup {
backends,
dx12_shader_compiler,
});
let (size, surface) = unsafe {
let (size, surface) = {
let size = window.inner_size();

#[cfg(any(not(target_arch = "wasm32"), target_os = "emscripten"))]
let surface = instance.create_surface(&window).unwrap();
let surface = instance.create_surface(window).unwrap();
#[cfg(all(target_arch = "wasm32", not(target_os = "emscripten")))]
let surface = {
if let Some(offscreen_canvas_setup) = &offscreen_canvas_setup {
Expand Down Expand Up @@ -235,7 +234,6 @@ async fn setup<E: Example>(title: &str) -> Setup {
.expect("Unable to find a suitable GPU adapter!");

Setup {
window,
event_loop,
instance,
size,
Expand All @@ -250,7 +248,6 @@ async fn setup<E: Example>(title: &str) -> Setup {

fn start<E: Example>(
#[cfg(not(target_arch = "wasm32"))] Setup {
window,
event_loop,
instance,
size,
Expand All @@ -260,7 +257,6 @@ fn start<E: Example>(
queue,
}: Setup,
#[cfg(target_arch = "wasm32")] Setup {
window,
event_loop,
instance,
size,
Expand Down Expand Up @@ -298,7 +294,10 @@ fn start<E: Example>(
#[cfg(not(target_arch = "wasm32"))]
spawner.run_until_stalled();

window.request_redraw();
surface
.window::<winit::window::Window>()
.unwrap()
.request_redraw();
}
event::Event::WindowEvent {
event:
Expand Down
4 changes: 2 additions & 2 deletions wgpu/examples/hello-triangle/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ async fn run(event_loop: EventLoop<()>, window: Window) {

let instance = wgpu::Instance::default();

let surface = unsafe { instance.create_surface(&window) }.unwrap();
let surface = instance.create_surface(window).unwrap();
let adapter = instance
.request_adapter(&wgpu::RequestAdapterOptions {
power_preference: wgpu::PowerPreference::default(),
Expand Down Expand Up @@ -99,7 +99,7 @@ async fn run(event_loop: EventLoop<()>, window: Window) {
config.height = size.height;
surface.configure(&device, &config);
// On macos the window needs to be redrawn manually after resizing
window.request_redraw();
surface.window::<Window>().unwrap().request_redraw();
}
Event::RedrawRequested(_) => {
let frame = surface
Expand Down
20 changes: 14 additions & 6 deletions wgpu/examples/hello-windows/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ use winit::{
};

struct ViewportDesc {
window: Window,
background: wgpu::Color,
surface: wgpu::Surface,
}
Expand All @@ -20,16 +19,15 @@ struct Viewport {

impl ViewportDesc {
fn new(window: Window, background: wgpu::Color, instance: &wgpu::Instance) -> Self {
let surface = unsafe { instance.create_surface(&window) }.unwrap();
let surface = instance.create_surface(window).unwrap();
Self {
window,
background,
surface,
}
}

fn build(self, adapter: &wgpu::Adapter, device: &wgpu::Device) -> Viewport {
let size = self.window.inner_size();
let size = self.surface.window::<Window>().unwrap().inner_size();

let caps = self.surface.get_capabilities(adapter);
let config = wgpu::SurfaceConfiguration {
Expand Down Expand Up @@ -92,7 +90,12 @@ async fn run(event_loop: EventLoop<()>, viewports: Vec<(Window, wgpu::Color)>) {

let mut viewports: HashMap<WindowId, Viewport> = viewports
.into_iter()
.map(|desc| (desc.window.id(), desc.build(&adapter, &device)))
.map(|desc| {
(
desc.surface.window::<Window>().unwrap().id(),
desc.build(&adapter, &device),
)
})
.collect();

event_loop.run(move |event, _, control_flow| {
Expand All @@ -112,7 +115,12 @@ async fn run(event_loop: EventLoop<()>, viewports: Vec<(Window, wgpu::Color)>) {
if let Some(viewport) = viewports.get_mut(&window_id) {
viewport.resize(&device, size);
// On macos the window needs to be redrawn manually after resizing
viewport.desc.window.request_redraw();
viewport
.desc
.surface
.window::<Window>()
.unwrap()
.request_redraw();
}
}
Event::RedrawRequested(window_id) => {
Expand Down
59 changes: 45 additions & 14 deletions wgpu/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))]
#![doc(html_logo_url = "https://raw.githubusercontent.com/gfx-rs/wgpu/master/logo.png")]
#![warn(missing_docs, unsafe_op_in_unsafe_fn)]
#![feature(trait_upcasting)]

mod backend;
mod context;
Expand Down Expand Up @@ -307,6 +308,7 @@ pub struct Surface {
// be wrapped in a mutex and since the configuration is only supplied after the surface has
// been created is is additionally wrapped in an option.
config: Mutex<Option<SurfaceConfiguration>>,
window: Option<OwnedSurfaceWindowHandle>,
}
static_assertions::assert_impl_all!(Surface: Send, Sync);

Expand Down Expand Up @@ -1460,12 +1462,6 @@ impl Instance {
/// If the specified display and window handle are not supported by any of the backends, then the surface
/// will not be supported by any adapters.
///
/// # Safety
///
/// - `raw_window_handle` must be a valid object to create a surface upon.
/// - `raw_window_handle` must remain valid until after the returned [`Surface`] is
/// dropped.
///
/// # Errors
///
/// - On WebGL2: Will return an error if the browser does not support WebGL2,
Expand All @@ -1476,22 +1472,21 @@ impl Instance {
/// - On macOS/Metal: will panic if not called on the main thread.
/// - On web: will panic if the `raw_window_handle` does not properly refer to a
/// canvas element.
pub unsafe fn create_surface<
W: raw_window_handle::HasRawWindowHandle + raw_window_handle::HasRawDisplayHandle,
>(
&self,
window: &W,
) -> Result<Surface, CreateSurfaceError> {
pub fn create_surface<W>(&self, window: W) -> Result<Surface, CreateSurfaceError>
where
W: SurfaceWindowHandle + 'static,
{
let (id, data) = DynContext::instance_create_surface(
&*self.context,
raw_window_handle::HasRawDisplayHandle::raw_display_handle(window),
raw_window_handle::HasRawWindowHandle::raw_window_handle(window),
raw_window_handle::HasRawDisplayHandle::raw_display_handle(&window),
raw_window_handle::HasRawWindowHandle::raw_window_handle(&window),
)?;
Ok(Surface {
context: Arc::clone(&self.context),
id,
data,
config: Mutex::new(None),
window: Some(OwnedSurfaceWindowHandle(Box::new(window))),
})
}

Expand All @@ -1517,6 +1512,7 @@ impl Instance {
id: ObjectId::from(surface.id()),
data: Box::new(surface),
config: Mutex::new(None),
window: None,
}
}

Expand All @@ -1539,6 +1535,7 @@ impl Instance {
id: ObjectId::from(surface.id()),
data: Box::new(surface),
config: Mutex::new(None),
window: None,
}
}

Expand All @@ -1564,6 +1561,7 @@ impl Instance {
id: ObjectId::from(surface.id()),
data: Box::new(surface),
config: Mutex::new(None),
window: None,
}
}

Expand Down Expand Up @@ -1600,6 +1598,7 @@ impl Instance {
#[cfg(all(target_arch = "wasm32", not(feature = "webgl")))]
data: Box::new(()),
config: Mutex::new(None),
window: None,
})
}

Expand Down Expand Up @@ -1636,6 +1635,7 @@ impl Instance {
#[cfg(all(target_arch = "wasm32", not(feature = "webgl")))]
data: Box::new(()),
config: Mutex::new(None),
window: None,
})
}

Expand Down Expand Up @@ -4180,6 +4180,13 @@ impl Surface {
.ok_or(SurfaceError::Lost)
}

/// Returns a reference to the window owned by this surface.
pub fn window<W: 'static>(&self) -> Option<&W> {
self.window
.as_ref()
.and_then(|window| (&*window.0 as &dyn std::any::Any).downcast_ref::<W>())
}

/// Returns the inner hal Surface using a callback. The hal surface will be `None` if the
/// backend type argument does not match with this wgpu Surface
///
Expand Down Expand Up @@ -4403,6 +4410,30 @@ impl Surface {
}
}

#[derive(Debug)]
struct OwnedSurfaceWindowHandle(Box<dyn SurfaceWindowHandle + 'static>);

/// Type for the window owned by [`Surface`]
pub trait SurfaceWindowHandle:
std::any::Any
+ raw_window_handle::HasRawWindowHandle
+ raw_window_handle::HasRawDisplayHandle
+ Debug
+ Send
+ Sync
{
}

impl<T> SurfaceWindowHandle for T where
T: std::any::Any
+ raw_window_handle::HasRawWindowHandle
+ raw_window_handle::HasRawDisplayHandle
+ Debug
+ Send
+ Sync
{
}

/// Type for the callback of uncaptured error handler
pub trait UncapturedErrorHandler: Fn(Error) + Send + 'static {}
impl<T> UncapturedErrorHandler for T where T: Fn(Error) + Send + 'static {}
Expand Down

0 comments on commit 73fbb52

Please sign in to comment.