Unify surface creation by introducing new SurfaceTarget enum (#4984)

Author: Wumpf

CHANGELOG.md

@@ -59,6 +59,14 @@ This feature allowed you to call `global_id` on any wgpu opaque handle to get a
 
 Wgpu now exposes backend feature for the Direct3D 12 (`dx12`) and Metal (`metal`) backend. These are enabled by default, but don't do anything when not targetting the corresponding OS. By @daxpedda in [#4815](https://github.com/gfx-rs/wgpu/pull/4815)
 
+### Unified surface creation
+
+Previously, there were various specialized surface creation functions for various platform specific handles.
+Now, `wgpu::Instance::create_surface` & `wgpu::Instance::create_surface_unsafe` instead each take a value that can be converted to the unified `wgpu::SurfaceTarget`/`wgpu::SurfaceTargetUnsafe` enums.
+Conversion to `wgpu::SurfaceTarget` is automatic for anything implementing `raw-window-handle`'s `HasWindowHandle` & `HasDisplayHandle` traits,
+meaning that you can continue to e.g. pass references to winit windows as before.
+By @wumpf in [#4984](https://github.com/gfx-rs/wgpu/pull/4984)
+
 ### New Features
 
 #### General

player/src/bin/play.rs

@@ -63,7 +63,8 @@ fn main() {
             window.window_handle().unwrap().into(),
             wgc::id::TypedId::zip(0, 1, wgt::Backend::Empty),
         )
-    };
+    }
+    .unwrap();
 
     let device = match actions.pop() {
         Some(trace::Action::Init { desc, backend }) => {

tests/src/init.rs

@@ -51,7 +51,7 @@ pub async fn initialize_adapter(adapter_index: usize) -> (Instance, Adapter, Opt
         let canvas = initialize_html_canvas();
 
         _surface = instance
-            .create_surface_from_canvas(canvas.clone())
+            .create_surface(wgpu::SurfaceTarget::Canvas(canvas.clone()))
             .expect("could not create surface from canvas");
 
         surface_guard = Some(SurfaceGuard { canvas });

tests/tests/create_surface_error.rs

@@ -1,6 +1,6 @@
 //! Test that `create_surface_*()` accurately reports those errors we can provoke.
 
-/// This test applies to those cfgs that have a `create_surface_from_canvas` method, which
+/// This test applies to those cfgs that can create a surface from a canvas, which
 /// include WebGL and WebGPU, but *not* Emscripten GLES.
 #[cfg(all(target_arch = "wasm32", not(target_os = "emscripten")))]
 #[wasm_bindgen_test::wasm_bindgen_test]
@@ -15,7 +15,7 @@ fn canvas_get_context_returned_null() {
 
     #[allow(clippy::redundant_clone)] // false positive — can't and shouldn't move out.
     let error = instance
-        .create_surface_from_canvas(canvas.clone())
+        .create_surface(wgpu::SurfaceTarget::Canvas(canvas.clone()))
         .unwrap_err();
 
     assert!(

wgpu-core/src/instance.rs

@@ -479,68 +479,55 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
         display_handle: raw_window_handle::RawDisplayHandle,
         window_handle: raw_window_handle::RawWindowHandle,
         id_in: Input<G, SurfaceId>,
-    ) -> SurfaceId {
+    ) -> Result<SurfaceId, hal::InstanceError> {
         profiling::scope!("Instance::create_surface");
 
         fn init<A: HalApi>(
-            any_surface: &mut Option<AnySurface>,
             inst: &Option<A::Instance>,
             display_handle: raw_window_handle::RawDisplayHandle,
             window_handle: raw_window_handle::RawWindowHandle,
-        ) {
-            if any_surface.is_none() {
-                if let Some(surface) = inst.as_ref().and_then(|inst| unsafe {
-                    match inst.create_surface(display_handle, window_handle) {
-                        Ok(raw) => Some(HalSurface::<A> { raw: Arc::new(raw) }),
-                        Err(e) => {
-                            log::warn!("Error: {:?}", e);
-                            None
-                        }
-                    }
-                }) {
-                    *any_surface = Some(AnySurface::new(surface));
+        ) -> Option<Result<AnySurface, hal::InstanceError>> {
+            inst.as_ref().map(|inst| unsafe {
+                match inst.create_surface(display_handle, window_handle) {
+                    Ok(raw) => Ok(AnySurface::new(HalSurface::<A> { raw: Arc::new(raw) })),
+                    Err(e) => Err(e),
                 }
-            }
+            })
         }
 
-        let mut hal_surface = None;
+        let mut hal_surface: Option<Result<AnySurface, hal::InstanceError>> = None;
+
         #[cfg(all(feature = "vulkan", not(target_arch = "wasm32")))]
-        init::<hal::api::Vulkan>(
-            &mut hal_surface,
-            &self.instance.vulkan,
-            display_handle,
-            window_handle,
-        );
+        if hal_surface.is_none() {
+            hal_surface =
+                init::<hal::api::Vulkan>(&self.instance.vulkan, display_handle, window_handle);
+        }
         #[cfg(all(feature = "metal", any(target_os = "macos", target_os = "ios")))]
-        init::<hal::api::Metal>(
-            &mut hal_surface,
-            &self.instance.metal,
-            display_handle,
-            window_handle,
-        );
+        if hal_surface.is_none() {
+            hal_surface =
+                init::<hal::api::Metal>(&self.instance.metal, display_handle, window_handle);
+        }
         #[cfg(all(feature = "dx12", windows))]
-        init::<hal::api::Dx12>(
-            &mut hal_surface,
-            &self.instance.dx12,
-            display_handle,
-            window_handle,
-        );
+        if hal_surface.is_none() {
+            hal_surface =
+                init::<hal::api::Dx12>(&self.instance.dx12, display_handle, window_handle);
+        }
         #[cfg(feature = "gles")]
-        init::<hal::api::Gles>(
-            &mut hal_surface,
-            &self.instance.gl,
-            display_handle,
-            window_handle,
-        );
+        if hal_surface.is_none() {
+            hal_surface = init::<hal::api::Gles>(&self.instance.gl, display_handle, window_handle);
+        }
+
+        //  This is only None if there's no instance at all.
+        let hal_surface = hal_surface.unwrap()?;
 
         let surface = Surface {
             presentation: Mutex::new(None),
             info: ResourceInfo::new("<Surface>"),
-            raw: hal_surface.unwrap(),
+            raw: hal_surface,
         };
 
         let (id, _) = self.surfaces.prepare::<G>(id_in).assign(surface);
-        id
+        Ok(id)
     }
 
     /// # Safety
@@ -578,78 +565,6 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
         id
     }
 
-    #[cfg(all(
-        target_arch = "wasm32",
-        not(target_os = "emscripten"),
-        feature = "gles"
-    ))]
-    pub fn create_surface_webgl_canvas(
-        &self,
-        canvas: web_sys::HtmlCanvasElement,
-        id_in: Input<G, SurfaceId>,
-    ) -> Result<SurfaceId, hal::InstanceError> {
-        profiling::scope!("Instance::create_surface_webgl_canvas");
-
-        let surface = Surface {
-            presentation: Mutex::new(None),
-            info: ResourceInfo::new("<Surface>"),
-            raw: {
-                let hal_surface: HalSurface<hal::api::Gles> = self
-                    .instance
-                    .gl
-                    .as_ref()
-                    .map(|inst| {
-                        let raw_surface = inst.create_surface_from_canvas(canvas)?;
-                        Ok(HalSurface {
-                            raw: Arc::new(raw_surface),
-                        })
-                    })
-                    .transpose()?
-                    .unwrap();
-                AnySurface::new(hal_surface)
-            },
-        };
-
-        let (id, _) = self.surfaces.prepare::<G>(id_in).assign(surface);
-        Ok(id)
-    }
-
-    #[cfg(all(
-        target_arch = "wasm32",
-        not(target_os = "emscripten"),
-        feature = "gles"
-    ))]
-    pub fn create_surface_webgl_offscreen_canvas(
-        &self,
-        canvas: web_sys::OffscreenCanvas,
-        id_in: Input<G, SurfaceId>,
-    ) -> Result<SurfaceId, hal::InstanceError> {
-        profiling::scope!("Instance::create_surface_webgl_offscreen_canvas");
-
-        let surface = Surface {
-            presentation: Mutex::new(None),
-            info: ResourceInfo::new("<Surface>"),
-            raw: {
-                let hal_surface: HalSurface<hal::api::Gles> = self
-                    .instance
-                    .gl
-                    .as_ref()
-                    .map(|inst| {
-                        let raw_surface = inst.create_surface_from_offscreen_canvas(canvas)?;
-                        Ok(HalSurface {
-                            raw: Arc::new(raw_surface),
-                        })
-                    })
-                    .transpose()?
-                    .unwrap();
-                AnySurface::new(hal_surface)
-            },
-        };
-
-        let (id, _) = self.surfaces.prepare::<G>(id_in).assign(surface);
-        Ok(id)
-    }
-
     #[cfg(all(feature = "dx12", windows))]
     /// # Safety
     ///

wgpu/src/backend/direct.rs

@@ -7,7 +7,8 @@ use crate::{
     DownlevelCapabilities, Features, Label, Limits, LoadOp, MapMode, Operations,
     PipelineLayoutDescriptor, RenderBundleEncoderDescriptor, RenderPipelineDescriptor,
     SamplerDescriptor, ShaderModuleDescriptor, ShaderModuleDescriptorSpirV, ShaderSource, StoreOp,
-    SurfaceStatus, TextureDescriptor, TextureViewDescriptor, UncapturedErrorHandler,
+    SurfaceStatus, SurfaceTargetUnsafe, TextureDescriptor, TextureViewDescriptor,
+    UncapturedErrorHandler,
 };
 
 use arrayvec::ArrayVec;
@@ -231,81 +232,6 @@ impl Context {
         self.0.generate_report()
     }
 
-    #[cfg(metal)]
-    pub unsafe fn create_surface_from_core_animation_layer(
-        &self,
-        layer: *mut std::ffi::c_void,
-    ) -> Surface {
-        let id = unsafe { self.0.instance_create_surface_metal(layer, ()) };
-        Surface {
-            id,
-            configured_device: Mutex::default(),
-        }
-    }
-
-    #[cfg(any(webgpu, webgl))]
-    pub fn instance_create_surface_from_canvas(
-        &self,
-        canvas: web_sys::HtmlCanvasElement,
-    ) -> Result<Surface, crate::CreateSurfaceError> {
-        let id = self.0.create_surface_webgl_canvas(canvas, ())?;
-        Ok(Surface {
-            id,
-            configured_device: Mutex::default(),
-        })
-    }
-
-    #[cfg(any(webgpu, webgl))]
-    pub fn instance_create_surface_from_offscreen_canvas(
-        &self,
-        canvas: web_sys::OffscreenCanvas,
-    ) -> Result<Surface, crate::CreateSurfaceError> {
-        let id = self.0.create_surface_webgl_offscreen_canvas(canvas, ())?;
-        Ok(Surface {
-            id,
-            configured_device: Mutex::default(),
-        })
-    }
-
-    #[cfg(dx12)]
-    pub unsafe fn create_surface_from_visual(&self, visual: *mut std::ffi::c_void) -> Surface {
-        let id = unsafe { self.0.instance_create_surface_from_visual(visual, ()) };
-        Surface {
-            id,
-            configured_device: Mutex::default(),
-        }
-    }
-
-    #[cfg(dx12)]
-    pub unsafe fn create_surface_from_surface_handle(
-        &self,
-        surface_handle: *mut std::ffi::c_void,
-    ) -> Surface {
-        let id = unsafe {
-            self.0
-                .instance_create_surface_from_surface_handle(surface_handle, ())
-        };
-        Surface {
-            id,
-            configured_device: Mutex::default(),
-        }
-    }
-
-    #[cfg(dx12)]
-    pub unsafe fn create_surface_from_swap_chain_panel(
-        &self,
-        swap_chain_panel: *mut std::ffi::c_void,
-    ) -> Surface {
-        let id = unsafe {
-            self.0
-                .instance_create_surface_from_swap_chain_panel(swap_chain_panel, ())
-        };
-        Surface {
-            id,
-            configured_device: Mutex::default(),
-        }
-    }
-
     fn handle_error(
         &self,
         sink_mutex: &Mutex<ErrorSinkRaw>,
@@ -594,19 +520,45 @@ impl crate::Context for Context {
 
     unsafe fn instance_create_surface(
         &self,
-        display_handle: raw_window_handle::RawDisplayHandle,
-        window_handle: raw_window_handle::RawWindowHandle,
+        target: SurfaceTargetUnsafe,
     ) -> Result<(Self::SurfaceId, Self::SurfaceData), crate::CreateSurfaceError> {
-        let id = unsafe {
-            self.0
-                .instance_create_surface(display_handle, window_handle, ())
+        let id = match target {
+            SurfaceTargetUnsafe::RawHandle {
+                raw_display_handle,
+                raw_window_handle,
+            } => unsafe {
+                self.0
+                    .instance_create_surface(raw_display_handle, raw_window_handle, ())?
+            },
+
+            #[cfg(metal)]
+            SurfaceTargetUnsafe::CoreAnimationLayer(layer) => unsafe {
+                self.0.instance_create_surface_metal(layer, ())
+            },
+
+            #[cfg(dx12)]
+            SurfaceTargetUnsafe::CompositionVisual(visual) => unsafe {
+                self.0.instance_create_surface_from_visual(visual, ())
+            },
+
+            #[cfg(dx12)]
+            SurfaceTargetUnsafe::SurfaceHandle(surface_handle) => unsafe {
+                self.0
+                    .instance_create_surface_from_surface_handle(surface_handle, ())
+            },
+
+            #[cfg(dx12)]
+            SurfaceTargetUnsafe::SwapChainPanel(swap_chain_panel) => unsafe {
+                self.0
+                    .instance_create_surface_from_swap_chain_panel(swap_chain_panel, ())
+            },
         };
 
         Ok((
             id,
             Surface {
                 id,
-                configured_device: Mutex::new(None),
+                configured_device: Mutex::default(),
             },
         ))
     }