diff --git a/CHANGELOG.md b/CHANGELOG.md index 00e8230df..84773843a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,14 @@ Bottom level categories: - Log vulkan validation layer messages during instance creation and destruction: By @exrook in [#4586](https://github.com/gfx-rs/wgpu/pull/4586) - `TextureFormat::block_size` is deprecated, use `TextureFormat::block_copy_size` instead: By @wumpf in [#4647](https://github.com/gfx-rs/wgpu/pull/4647) +#### Safe `Surface` creation + +It is now possible to safely create a `wgpu::Surface` with `Surface::create_surface()` by letting `Surface` hold a lifetime to `window`. + +Passing an owned value `window` to `Surface` will return a `Surface<'static>`. Shared ownership over `window` can still be achieved with e.g. an `Arc`. Alternatively a reference could be passed, which will return a `Surface<'window>`. + +`Surface::create_surface_from_raw()` can be used to continue producing a `Surface<'static>` without any lifetime requirements over `window`, which also remains `unsafe`. + #### Naga - Introduce a new `Scalar` struct type for use in Naga's IR, and update all frontend, middle, and backend code appropriately. By @jimblandy in [#4673](https://github.com/gfx-rs/wgpu/pull/4673). diff --git a/examples/common/src/framework.rs b/examples/common/src/framework.rs index 90c1a853a..a62423f61 100644 --- a/examples/common/src/framework.rs +++ b/examples/common/src/framework.rs @@ -1,3 +1,5 @@ +use std::sync::Arc; + use wgpu::{Instance, Surface, WasmNotSend, WasmNotSync}; use wgpu_test::GpuTestConfiguration; use winit::{ @@ -90,7 +92,7 @@ fn init_logger() { struct EventLoopWrapper { event_loop: EventLoop<()>, - window: Window, + window: Arc, } impl EventLoopWrapper { @@ -98,7 +100,7 @@ impl EventLoopWrapper { let event_loop = EventLoop::new().unwrap(); let mut builder = winit::window::WindowBuilder::new(); builder = builder.with_title(title); - let window = builder.build(&event_loop).unwrap(); + let window = Arc::new(builder.build(&event_loop).unwrap()); #[cfg(target_arch = "wasm32")] { @@ -121,7 +123,7 @@ impl EventLoopWrapper { /// /// As surface usage varies per platform, wrapping this up cleans up the event loop code. struct SurfaceWrapper { - surface: Option, + surface: Option>, config: Option, } @@ -141,9 +143,9 @@ impl SurfaceWrapper { /// /// We cannot unconditionally create a surface here, as Android requires /// us to wait until we recieve the `Resumed` event to do so. - fn pre_adapter(&mut self, instance: &Instance, window: &Window) { + fn pre_adapter(&mut self, instance: &Instance, window: Arc) { if cfg!(target_arch = "wasm32") { - self.surface = Some(unsafe { instance.create_surface(&window).unwrap() }); + self.surface = Some(instance.create_surface(window).unwrap()); } } @@ -163,7 +165,7 @@ impl SurfaceWrapper { /// On all native platforms, this is where we create the surface. /// /// Additionally, we configure the surface based on the (now valid) window size. - fn resume(&mut self, context: &ExampleContext, window: &Window, srgb: bool) { + fn resume(&mut self, context: &ExampleContext, window: Arc, srgb: bool) { // Window size is only actually valid after we enter the event loop. let window_size = window.inner_size(); let width = window_size.width.max(1); @@ -173,7 +175,7 @@ impl SurfaceWrapper { // We didn't create the surface in pre_adapter, so we need to do so now. if !cfg!(target_arch = "wasm32") { - self.surface = Some(unsafe { context.instance.create_surface(&window).unwrap() }); + self.surface = Some(context.instance.create_surface(window).unwrap()); } // From here on, self.surface should be Some. @@ -252,7 +254,7 @@ struct ExampleContext { } impl ExampleContext { /// Initializes the example context. - async fn init_async(surface: &mut SurfaceWrapper, window: &Window) -> Self { + async fn init_async(surface: &mut SurfaceWrapper, window: Arc) -> Self { log::info!("Initializing wgpu..."); let backends = wgpu::util::backend_bits_from_env().unwrap_or_default(); @@ -357,8 +359,7 @@ async fn start(title: &str) { init_logger(); let window_loop = EventLoopWrapper::new(title); let mut surface = SurfaceWrapper::new(); - let context = ExampleContext::init_async::(&mut surface, &window_loop.window).await; - + let context = ExampleContext::init_async::(&mut surface, window_loop.window.clone()).await; let mut frame_counter = FrameCounter::new(); // We wait to create the example until we have a valid surface. @@ -384,7 +385,7 @@ async fn start(title: &str) { match event { ref e if SurfaceWrapper::start_condition(e) => { - surface.resume(&context, &window_loop.window, E::SRGB); + surface.resume(&context, window_loop.window.clone(), E::SRGB); // If we haven't created the example yet, do so now. if example.is_none() { diff --git a/examples/hello-triangle/src/main.rs b/examples/hello-triangle/src/main.rs index 6a9d1414d..55db4eb92 100644 --- a/examples/hello-triangle/src/main.rs +++ b/examples/hello-triangle/src/main.rs @@ -12,7 +12,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(), @@ -84,6 +84,7 @@ async fn run(event_loop: EventLoop<()>, window: Window) { surface.configure(&device, &config); + let window = &window; event_loop .run(move |event, target| { // Have the closure take ownership of the resources. diff --git a/examples/hello-windows/src/main.rs b/examples/hello-windows/src/main.rs index 43bd98696..22065e362 100644 --- a/examples/hello-windows/src/main.rs +++ b/examples/hello-windows/src/main.rs @@ -1,6 +1,6 @@ #![cfg_attr(target_arch = "wasm32", allow(dead_code))] -use std::collections::HashMap; +use std::{collections::HashMap, sync::Arc}; use winit::{ event::{Event, WindowEvent}, event_loop::EventLoop, @@ -8,9 +8,9 @@ use winit::{ }; struct ViewportDesc { - window: Window, + window: Arc, background: wgpu::Color, - surface: wgpu::Surface, + surface: wgpu::Surface<'static>, } struct Viewport { @@ -19,8 +19,8 @@ struct Viewport { } impl ViewportDesc { - fn new(window: Window, background: wgpu::Color, instance: &wgpu::Instance) -> Self { - let surface = unsafe { instance.create_surface(&window) }.unwrap(); + fn new(window: Arc, background: wgpu::Color, instance: &wgpu::Instance) -> Self { + let surface = instance.create_surface(window.clone()).unwrap(); Self { window, background, @@ -62,7 +62,7 @@ impl Viewport { } } -async fn run(event_loop: EventLoop<()>, viewports: Vec<(Window, wgpu::Color)>) { +async fn run(event_loop: EventLoop<()>, viewports: Vec<(Arc, wgpu::Color)>) { let instance = wgpu::Instance::default(); let viewports: Vec<_> = viewports .into_iter() @@ -180,6 +180,7 @@ fn main() { .with_inner_size(winit::dpi::PhysicalSize::new(WINDOW_SIZE, WINDOW_SIZE)) .build(&event_loop) .unwrap(); + let window = Arc::new(window); window.set_outer_position(winit::dpi::PhysicalPosition::new( WINDOW_PADDING + column * WINDOW_OFFSET, WINDOW_PADDING + row * (WINDOW_OFFSET + WINDOW_TITLEBAR), diff --git a/examples/uniform-values/src/main.rs b/examples/uniform-values/src/main.rs index 48faf857c..c8895bef6 100644 --- a/examples/uniform-values/src/main.rs +++ b/examples/uniform-values/src/main.rs @@ -16,6 +16,7 @@ //! The usage of the uniform buffer within the shader itself is pretty self-explanatory given //! some understanding of WGSL. +use std::sync::Arc; // We won't bring StorageBuffer into scope as that might be too easy to confuse // with actual GPU-allocated WGPU storage buffers. use encase::ShaderType; @@ -84,8 +85,8 @@ impl Default for AppState { } struct WgpuContext { - pub window: Window, - pub surface: wgpu::Surface, + pub window: Arc, + pub surface: wgpu::Surface<'static>, pub surface_config: wgpu::SurfaceConfiguration, pub device: wgpu::Device, pub queue: wgpu::Queue, @@ -95,11 +96,11 @@ struct WgpuContext { } impl WgpuContext { - async fn new(window: Window) -> WgpuContext { + async fn new(window: Arc) -> WgpuContext { let size = window.inner_size(); let instance = wgpu::Instance::default(); - let surface = unsafe { instance.create_surface(&window) }.unwrap(); + let surface = instance.create_surface(window.clone()).unwrap(); let adapter = instance .request_adapter(&wgpu::RequestAdapterOptions { power_preference: wgpu::PowerPreference::HighPerformance, @@ -223,7 +224,7 @@ impl WgpuContext { } } -async fn run(event_loop: EventLoop<()>, window: Window) { +async fn run(event_loop: EventLoop<()>, window: Arc) { let mut wgpu_context = Some(WgpuContext::new(window).await); // (6) let mut state = Some(AppState::default()); @@ -351,6 +352,7 @@ fn main() { .with_inner_size(winit::dpi::LogicalSize::new(900, 900)) .build(&event_loop) .unwrap(); + let window = Arc::new(window); #[cfg(not(target_arch = "wasm32"))] { env_logger::builder().format_timestamp_nanos().init(); diff --git a/wgpu/src/backend/web.rs b/wgpu/src/backend/web.rs index 30d67d864..6883bfb63 100644 --- a/wgpu/src/backend/web.rs +++ b/wgpu/src/backend/web.rs @@ -1109,7 +1109,7 @@ impl crate::context::Context for Context { fn instance_request_adapter( &self, - options: &crate::RequestAdapterOptions<'_>, + options: &crate::RequestAdapterOptions<'_, '_>, ) -> Self::RequestAdapterFuture { //TODO: support this check, return `None` if the flag is not set. // It's not trivial, since we need the Future logic to have this check, diff --git a/wgpu/src/context.rs b/wgpu/src/context.rs index 0d1e95f3b..2ada20f4a 100644 --- a/wgpu/src/context.rs +++ b/wgpu/src/context.rs @@ -103,7 +103,7 @@ pub trait Context: Debug + WasmNotSend + WasmNotSync + Sized { ) -> Result<(Self::SurfaceId, Self::SurfaceData), crate::CreateSurfaceError>; fn instance_request_adapter( &self, - options: &RequestAdapterOptions<'_>, + options: &RequestAdapterOptions<'_, '_>, ) -> Self::RequestAdapterFuture; fn adapter_request_device( &self, @@ -1239,7 +1239,7 @@ pub(crate) trait DynContext: Debug + WasmNotSend + WasmNotSync { #[allow(clippy::type_complexity)] fn instance_request_adapter( &self, - options: &RequestAdapterOptions<'_>, + options: &RequestAdapterOptions<'_, '_>, ) -> Pin; fn adapter_request_device( &self, @@ -2103,7 +2103,7 @@ where fn instance_request_adapter( &self, - options: &RequestAdapterOptions<'_>, + options: &RequestAdapterOptions<'_, '_>, ) -> Pin { let future: T::RequestAdapterFuture = Context::instance_request_adapter(self, options); Box::pin(async move { diff --git a/wgpu/src/lib.rs b/wgpu/src/lib.rs index 914e234f1..b8e603c60 100644 --- a/wgpu/src/lib.rs +++ b/wgpu/src/lib.rs @@ -27,6 +27,7 @@ use std::{ use context::{Context, DeviceRequest, DynContext, ObjectId}; use parking_lot::Mutex; +use raw_window_handle::{HasDisplayHandle, HasWindowHandle}; pub use wgt::{ AdapterInfo, AddressMode, AstcBlock, AstcChannel, Backend, Backends, BindGroupLayoutEntry, BindingType, BlendComponent, BlendFactor, BlendOperation, BlendState, BufferAddress, @@ -379,6 +380,10 @@ impl Drop for Sampler { pub type SurfaceConfiguration = wgt::SurfaceConfiguration>; static_assertions::assert_impl_all!(SurfaceConfiguration: Send, Sync); +trait WgpuSurfaceRequirement: WasmNotSend + WasmNotSync {} + +impl WgpuSurfaceRequirement for T {} + /// Handle to a presentable surface. /// /// A `Surface` represents a platform-specific surface (e.g. a window) onto which rendered images may @@ -387,9 +392,9 @@ static_assertions::assert_impl_all!(SurfaceConfiguration: Send, Sync); /// This type is unique to the Rust API of `wgpu`. In the WebGPU specification, /// [`GPUCanvasContext`](https://gpuweb.github.io/gpuweb/#canvas-context) /// serves a similar role. -#[derive(Debug)] -pub struct Surface { +pub struct Surface<'window> { context: Arc, + _surface: Option>, id: ObjectId, data: Box, // Stores the latest `SurfaceConfiguration` that was set using `Surface::configure`. @@ -400,6 +405,28 @@ pub struct Surface { // been created is is additionally wrapped in an option. config: Mutex>, } + +// This custom implementation is required because [`Surface::_surface`] doesn't +// require [`Debug`](fmt::Debug), which we should not require from the user. +impl<'window> fmt::Debug for Surface<'window> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("Surface") + .field("context", &self.context) + .field( + "_surface", + &if self._surface.is_some() { + "Some" + } else { + "None" + }, + ) + .field("id", &self.id) + .field("data", &self.data) + .field("config", &self.config) + .finish() + } +} + #[cfg(any( not(target_arch = "wasm32"), all( @@ -409,7 +436,7 @@ pub struct Surface { ))] static_assertions::assert_impl_all!(Surface: Send, Sync); -impl Drop for Surface { +impl Drop for Surface<'_> { fn drop(&mut self) { if !thread::panicking() { self.context.surface_drop(&self.id, self.data.as_ref()) @@ -1171,7 +1198,7 @@ pub use wgt::RequestAdapterOptions as RequestAdapterOptionsBase; /// /// Corresponds to [WebGPU `GPURequestAdapterOptions`]( /// https://gpuweb.github.io/gpuweb/#dictdef-gpurequestadapteroptions). -pub type RequestAdapterOptions<'a> = RequestAdapterOptionsBase<&'a Surface>; +pub type RequestAdapterOptions<'a, 'b> = RequestAdapterOptionsBase<&'a Surface<'b>>; #[cfg(any( not(target_arch = "wasm32"), all( @@ -1920,11 +1947,9 @@ 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. + /// If a reference is passed in `window`, the returned [`Surface`] will + /// hold a lifetime to it. Owned values will return a [`Surface<'static>`] + /// instead. /// /// # Errors /// @@ -1936,12 +1961,37 @@ 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::HasWindowHandle + raw_window_handle::HasDisplayHandle, - >( + pub fn create_surface<'window, W>( + &self, + window: W, + ) -> Result, CreateSurfaceError> + where + W: HasWindowHandle + HasDisplayHandle + WasmNotSend + WasmNotSync + 'window, + { + let mut surface = unsafe { self.create_surface_from_raw(&window) }?; + surface._surface = Some(Box::new(window)); + Ok(surface) + } + + /// An alternative version to [`create_surface()`](Self::create_surface) + /// which has no lifetime requirements to `window` and doesn't require + /// [`Send`] or [`Sync`] (on non-Wasm targets). This makes it `unsafe` + /// instead and always returns a [`Surface<'static>`]. + /// + /// See [`create_surface()`](Self::create_surface) for more details. + /// + /// # 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. + pub unsafe fn create_surface_from_raw( &self, window: &W, - ) -> Result { + ) -> Result, CreateSurfaceError> + where + W: HasWindowHandle + HasDisplayHandle, + { let raw_display_handle = window .display_handle() .map_err(|e| CreateSurfaceError { @@ -1963,6 +2013,7 @@ impl Instance { }?; Ok(Surface { context: Arc::clone(&self.context), + _surface: None, id, data, config: Mutex::new(None), @@ -1978,7 +2029,7 @@ impl Instance { pub unsafe fn create_surface_from_core_animation_layer( &self, layer: *mut std::ffi::c_void, - ) -> Surface { + ) -> Surface<'static> { let surface = unsafe { self.context .as_any() @@ -1988,6 +2039,7 @@ impl Instance { }; Surface { context: Arc::clone(&self.context), + _surface: None, id: ObjectId::from(surface.id()), data: Box::new(surface), config: Mutex::new(None), @@ -2000,7 +2052,10 @@ impl Instance { /// /// - visual must be a valid IDCompositionVisual to create a surface upon. #[cfg(target_os = "windows")] - pub unsafe fn create_surface_from_visual(&self, visual: *mut std::ffi::c_void) -> Surface { + pub unsafe fn create_surface_from_visual( + &self, + visual: *mut std::ffi::c_void, + ) -> Surface<'static> { let surface = unsafe { self.context .as_any() @@ -2010,6 +2065,7 @@ impl Instance { }; Surface { context: Arc::clone(&self.context), + _surface: None, id: ObjectId::from(surface.id()), data: Box::new(surface), config: Mutex::new(None), @@ -2025,7 +2081,7 @@ impl Instance { pub unsafe fn create_surface_from_surface_handle( &self, surface_handle: *mut std::ffi::c_void, - ) -> Surface { + ) -> Surface<'static> { let surface = unsafe { self.context .as_any() @@ -2035,6 +2091,7 @@ impl Instance { }; Surface { context: Arc::clone(&self.context), + _surface: None, id: ObjectId::from(surface.id()), data: Box::new(surface), config: Mutex::new(None), @@ -2050,7 +2107,7 @@ impl Instance { pub unsafe fn create_surface_from_swap_chain_panel( &self, swap_chain_panel: *mut std::ffi::c_void, - ) -> Surface { + ) -> Surface<'static> { let surface = unsafe { self.context .as_any() @@ -2060,6 +2117,7 @@ impl Instance { }; Surface { context: Arc::clone(&self.context), + _surface: None, id: ObjectId::from(surface.id()), data: Box::new(surface), config: Mutex::new(None), @@ -2079,7 +2137,7 @@ impl Instance { pub fn create_surface_from_canvas( &self, canvas: web_sys::HtmlCanvasElement, - ) -> Result { + ) -> Result, CreateSurfaceError> { let surface = self .context .as_any() @@ -2090,6 +2148,7 @@ impl Instance { // TODO: This is ugly, a way to create things from a native context needs to be made nicer. Ok(Surface { context: Arc::clone(&self.context), + _surface: None, #[cfg(any(not(target_arch = "wasm32"), feature = "webgl"))] id: ObjectId::from(surface.id()), #[cfg(any(not(target_arch = "wasm32"), feature = "webgl"))] @@ -2115,7 +2174,7 @@ impl Instance { pub fn create_surface_from_offscreen_canvas( &self, canvas: web_sys::OffscreenCanvas, - ) -> Result { + ) -> Result, CreateSurfaceError> { let surface = self .context .as_any() @@ -2126,6 +2185,7 @@ impl Instance { // TODO: This is ugly, a way to create things from a native context needs to be made nicer. Ok(Surface { context: Arc::clone(&self.context), + _surface: None, #[cfg(any(not(target_arch = "wasm32"), feature = "webgl"))] id: ObjectId::from(surface.id()), #[cfg(any(not(target_arch = "wasm32"), feature = "webgl"))] @@ -4913,7 +4973,7 @@ impl Drop for SurfaceTexture { } } -impl Surface { +impl Surface<'_> { /// Returns the capabilities of the surface when used with the given adapter. /// /// Returns specified values (see [`SurfaceCapabilities`]) if surface is incompatible with the adapter. @@ -5303,7 +5363,7 @@ impl RenderBundle { } #[cfg(feature = "expose-ids")] -impl Surface { +impl Surface<'_> { /// Returns a globally-unique identifier for this `Surface`. /// /// Calling this method multiple times on the same object will always return the same value. diff --git a/wgpu/src/util/init.rs b/wgpu/src/util/init.rs index 987e8400f..3b28753e4 100644 --- a/wgpu/src/util/init.rs +++ b/wgpu/src/util/init.rs @@ -80,7 +80,7 @@ pub fn initialize_adapter_from_env( /// Initialize the adapter obeying the WGPU_ADAPTER_NAME environment variable and if it doesn't exist fall back on a default adapter. pub async fn initialize_adapter_from_env_or_default( instance: &Instance, - compatible_surface: Option<&Surface>, + compatible_surface: Option<&Surface<'_>>, ) -> Option { match initialize_adapter_from_env(instance, compatible_surface) { Some(a) => Some(a),