Unify surface creation by introducing new SurfaceTarget enum

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` instead always take a value that can be converted to the unified `wgpu::SurfaceTarget` enum.
+Conversion 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

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/src/backend/direct.rs

@@ -4,10 +4,11 @@ use crate::{
     context::{ObjectId, Unused},
     AdapterInfo, BindGroupDescriptor, BindGroupLayoutDescriptor, BindingResource, BufferBinding,
     BufferDescriptor, CommandEncoderDescriptor, ComputePassDescriptor, ComputePipelineDescriptor,
-    DownlevelCapabilities, Features, Label, Limits, LoadOp, MapMode, Operations,
-    PipelineLayoutDescriptor, RenderBundleEncoderDescriptor, RenderPipelineDescriptor,
-    SamplerDescriptor, ShaderModuleDescriptor, ShaderModuleDescriptorSpirV, ShaderSource, StoreOp,
-    SurfaceStatus, TextureDescriptor, TextureViewDescriptor, UncapturedErrorHandler,
+    CreateSurfaceError, CreateSurfaceErrorKind, DownlevelCapabilities, Features, Label, Limits,
+    LoadOp, MapMode, Operations, PipelineLayoutDescriptor, RenderBundleEncoderDescriptor,
+    RenderPipelineDescriptor, SamplerDescriptor, ShaderModuleDescriptor,
+    ShaderModuleDescriptorSpirV, ShaderSource, StoreOp, SurfaceStatus, SurfaceTarget,
+    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,67 @@ impl crate::Context for Context {
 
     unsafe fn instance_create_surface(
         &self,
-        display_handle: raw_window_handle::RawDisplayHandle,
-        window_handle: raw_window_handle::RawWindowHandle,
+        target: &SurfaceTarget<'_>,
     ) -> Result<(Self::SurfaceId, Self::SurfaceData), crate::CreateSurfaceError> {
-        let id = unsafe {
-            self.0
-                .instance_create_surface(display_handle, window_handle, ())
+        let id = match target {
+            SurfaceTarget::WindowHandle(window) => {
+                let raw_display_handle = window
+                    .display_handle()
+                    .map_err(|e| CreateSurfaceError {
+                        inner: CreateSurfaceErrorKind::RawHandle(e),
+                    })?
+                    .as_raw();
+                let raw_window_handle = window
+                    .window_handle()
+                    .map_err(|e| CreateSurfaceError {
+                        inner: CreateSurfaceErrorKind::RawHandle(e),
+                    })?
+                    .as_raw();
+
+                unsafe {
+                    self.0
+                        .instance_create_surface(raw_display_handle, raw_window_handle, ())
+                }
+            }
+
+            #[cfg(metal)]
+            SurfaceTarget::CoreAnimationLayer(layer) => unsafe {
+                self.0.instance_create_surface_metal(*layer, ())
+            },
+
+            #[cfg(dx12)]
+            SurfaceTarget::CompositionVisual(visual) => unsafe {
+                self.0.instance_create_surface_from_visual(*visual, ())
+            },
+
+            #[cfg(dx12)]
+            SurfaceTarget::SurfaceHandle(surface_handle) => unsafe {
+                self.0
+                    .instance_create_surface_from_surface_handle(*surface_handle, ())
+            },
+
+            #[cfg(dx12)]
+            SurfaceTarget::SwapChainPanel(swap_chain_panel) => unsafe {
+                self.0
+                    .instance_create_surface_from_swap_chain_panel(*swap_chain_panel, ())
+            },
+
+            #[cfg(any(webgpu, webgl))]
+            SurfaceTarget::Canvas(canvas) => {
+                self.0.create_surface_webgl_canvas(canvas.clone(), ())?
+            }
+
+            #[cfg(any(webgpu, webgl))]
+            SurfaceTarget::OffscreenCanvas(canvas) => self
+                .0
+                .create_surface_webgl_offscreen_canvas(canvas.clone(), ())?,
         };
 
         Ok((
             id,
             Surface {
                 id,
-                configured_device: Mutex::new(None),
+                configured_device: Mutex::default(),
             },
         ))
     }

wgpu/src/backend/web.rs

@@ -18,7 +18,7 @@ use wasm_bindgen::{prelude::*, JsCast};
 
 use crate::{
     context::{downcast_ref, ObjectId, QueueWriteBuffer, Unused},
-    UncapturedErrorHandler,
+    CreateSurfaceError, CreateSurfaceErrorKind, SurfaceTarget, UncapturedErrorHandler,
 };
 
 fn create_identified<T>(value: T) -> (Identified<T>, Sendable<T>) {
@@ -878,35 +878,7 @@ where
 }
 
 impl Context {
-    pub fn instance_create_surface_from_canvas(
-        &self,
-        canvas: web_sys::HtmlCanvasElement,
-    ) -> Result<
-        (
-            <Self as crate::Context>::SurfaceId,
-            <Self as crate::Context>::SurfaceData,
-        ),
-        crate::CreateSurfaceError,
-    > {
-        let result = canvas.get_context("webgpu");
-        self.create_surface_from_context(Canvas::Canvas(canvas), result)
-    }
-
-    pub fn instance_create_surface_from_offscreen_canvas(
-        &self,
-        canvas: web_sys::OffscreenCanvas,
-    ) -> Result<
-        (
-            <Self as crate::Context>::SurfaceId,
-            <Self as crate::Context>::SurfaceData,
-        ),
-        crate::CreateSurfaceError,
-    > {
-        let result = canvas.get_context("webgpu");
-        self.create_surface_from_context(Canvas::Offscreen(canvas), result)
-    }
-
-    /// Common portion of public `instance_create_surface_from_*` functions.
+    /// Common portion of the internal branches of the public `instance_create_surface` function.
     ///
     /// Note: Analogous code also exists in the WebGL2 backend at
     /// `wgpu_hal::gles::web::Instance`.
@@ -1068,36 +1040,64 @@ impl crate::context::Context for Context {
 
     unsafe fn instance_create_surface(
         &self,
-        _display_handle: raw_window_handle::RawDisplayHandle,
-        window_handle: raw_window_handle::RawWindowHandle,
+        target: &SurfaceTarget<'_>,
     ) -> Result<(Self::SurfaceId, Self::SurfaceData), crate::CreateSurfaceError> {
-        let canvas_element: web_sys::HtmlCanvasElement = match window_handle {
-            raw_window_handle::RawWindowHandle::Web(handle) => {
-                let canvas_node: wasm_bindgen::JsValue = web_sys::window()
-                    .and_then(|win| win.document())
-                    .and_then(|doc| {
-                        doc.query_selector_all(&format!("[data-raw-handle=\"{}\"]", handle.id))
-                            .ok()
-                    })
-                    .and_then(|nodes| nodes.get(0))
-                    .expect("expected to find single canvas")
-                    .into();
-                canvas_node.into()
-            }
-            raw_window_handle::RawWindowHandle::WebCanvas(handle) => {
-                let value: &JsValue = unsafe { handle.obj.cast().as_ref() };
-                value.clone().unchecked_into()
+        match target {
+            SurfaceTarget::WindowHandle(window) => {
+                let raw_window_handle = window
+                    .window_handle()
+                    .map_err(|e| CreateSurfaceError {
+                        inner: CreateSurfaceErrorKind::RawHandle(e),
+                    })?
+                    .as_raw();
+
+                let canvas_element: web_sys::HtmlCanvasElement = match raw_window_handle {
+                    raw_window_handle::RawWindowHandle::Web(handle) => {
+                        let canvas_node: wasm_bindgen::JsValue = web_sys::window()
+                            .and_then(|win| win.document())
+                            .and_then(|doc| {
+                                doc.query_selector_all(&format!(
+                                    "[data-raw-handle=\"{}\"]",
+                                    handle.id
+                                ))
+                                .ok()
+                            })
+                            .and_then(|nodes| nodes.get(0))
+                            .expect("expected to find single canvas")
+                            .into();
+                        canvas_node.into()
+                    }
+                    raw_window_handle::RawWindowHandle::WebCanvas(handle) => {
+                        let value: &JsValue = unsafe { handle.obj.cast().as_ref() };
+                        value.clone().unchecked_into()
+                    }
+                    raw_window_handle::RawWindowHandle::WebOffscreenCanvas(handle) => {
+                        let value: &JsValue = unsafe { handle.obj.cast().as_ref() };
+                        let canvas: web_sys::OffscreenCanvas = value.clone().unchecked_into();
+                        let context_result = canvas.get_context("webgpu");
+
+                        return self.create_surface_from_context(
+                            Canvas::Offscreen(canvas),
+                            context_result,
+                        );
+                    }
+                    _ => panic!("expected valid handle for canvas"),
+                };
+
+                let context_result = canvas_element.get_context("webgpu");
+                self.create_surface_from_context(Canvas::Canvas(canvas_element), context_result)
             }
-            raw_window_handle::RawWindowHandle::WebOffscreenCanvas(handle) => {
-                let value: &JsValue = unsafe { handle.obj.cast().as_ref() };
-                let canvas: web_sys::OffscreenCanvas = value.clone().unchecked_into();
 
-                return self.instance_create_surface_from_offscreen_canvas(canvas);
+            SurfaceTarget::Canvas(canvas) => {
+                let context_result = canvas.get_context("webgpu");
+                self.create_surface_from_context(Canvas::Canvas(canvas.clone()), context_result)
             }
-            _ => panic!("expected valid handle for canvas"),
-        };
 
-        self.instance_create_surface_from_canvas(canvas_element)
+            SurfaceTarget::OffscreenCanvas(canvas) => {
+                let context_result = canvas.get_context("webgpu");
+                self.create_surface_from_context(Canvas::Offscreen(canvas.clone()), context_result)
+            }
+        }
     }
 
     fn instance_request_adapter(
@@ -2146,7 +2146,7 @@ impl crate::context::Context for Context {
         _pipeline_layout: &Self::PipelineLayoutId,
         _pipeline_layout_data: &Self::PipelineLayoutData,
     ) {
-        // Dropped automatically
+        // Dropped automaticaly
     }
 
     fn shader_module_drop(