[wgpu-hal] require a Surface to be passed to `Instance.enumerate_ad…

…apters()` on WebGL2 Also makes wgpu's `enumerate_adapters()` native only.
gfx-rs · Jul 4, 2024 · 7910fd8 · 7910fd8
1 parent 37f2836
commit 7910fd8
Show file tree

Hide file tree

Showing 15 changed files with 80 additions and 53 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -129,6 +129,16 @@ Platform support:
 
 By @atlv24 in [#5383](https://github.com/gfx-rs/wgpu/pull/5383)
 
+#### A compatible surface is now required for `request_adapter()` on WebGL2 + `enumerate_adapters()` is now native only.
+
+When targeting WebGL2, it has always been the case that a surface had to be created before calling `request_adapter()`.
+We now make this requirement explicit.
+
+Calling `enumerate_adapters()` when targeting WebGPU used to return an empty `Vec` and since we now require users
+to pass a compatible surface when targeting WebGL2, having `enumerate_adapters()` doesn't make sense.
+
+By @teoxoy in [#5901](https://github.com/gfx-rs/wgpu/pull/5901)
+
 ### New features
 #### Vulkan
 

diff --git a/tests/src/init.rs b/tests/src/init.rs
@@ -41,15 +41,17 @@ pub fn initialize_instance() -> Instance {
 pub async fn initialize_adapter(adapter_index: usize) -> (Instance, Adapter, Option<SurfaceGuard>) {
     let instance = initialize_instance();
     #[allow(unused_variables)]
-    let _surface: wgpu::Surface;
+    let surface: Option<wgpu::Surface>;
     let surface_guard: Option<SurfaceGuard>;
 
-    // Create a canvas iff we need a WebGL2RenderingContext to have a working device.
+    #[allow(unused_assignments)]
+    // Create a canvas if we need a WebGL2RenderingContext to have a working device.
     #[cfg(not(all(
         target_arch = "wasm32",
         any(target_os = "emscripten", feature = "webgl")
     )))]
     {
+        surface = None;
         surface_guard = None;
     }
     #[cfg(all(
@@ -60,23 +62,28 @@ pub async fn initialize_adapter(adapter_index: usize) -> (Instance, Adapter, Opt
         // On wasm, append a canvas to the document body for initializing the adapter
         let canvas = initialize_html_canvas();
 
-        _surface = instance
-            .create_surface(wgpu::SurfaceTarget::Canvas(canvas.clone()))
-            .expect("could not create surface from canvas");
+        surface = Some(
+            instance
+                .create_surface(wgpu::SurfaceTarget::Canvas(canvas.clone()))
+                .expect("could not create surface from canvas"),
+        );
 
         surface_guard = Some(SurfaceGuard { canvas });
     }
 
     cfg_if::cfg_if! {
-        if #[cfg(any(not(target_arch = "wasm32"), feature = "webgl"))] {
+        if #[cfg(not(target_arch = "wasm32"))] {
             let adapter_iter = instance.enumerate_adapters(wgpu::Backends::all());
             let adapter_count = adapter_iter.len();
             let adapter = adapter_iter.into_iter()
                 .nth(adapter_index)
                 .unwrap_or_else(|| panic!("Tried to get index {adapter_index} adapter, but adapter list was only {adapter_count} long. Is .gpuconfig out of date?"));
         } else {
             assert_eq!(adapter_index, 0);
-            let adapter = instance.request_adapter(&wgpu::RequestAdapterOptions::default()).await.unwrap();
+            let adapter = instance.request_adapter(&wgpu::RequestAdapterOptions {
+                compatible_surface: surface.as_ref(),
+                ..Default::default()
+            }).await.unwrap();
         }
     }
 

diff --git a/wgpu-core/src/instance.rs b/wgpu-core/src/instance.rs
@@ -717,7 +717,7 @@ impl Global {
         profiling::scope!("enumerating", &*format!("{:?}", A::VARIANT));
         let hub = HalApi::hub(self);
 
-        let hal_adapters = unsafe { inst.enumerate_adapters() };
+        let hal_adapters = unsafe { inst.enumerate_adapters(None) };
         for raw in hal_adapters {
             let adapter = Adapter::new(raw);
             log::info!("Adapter {:?} {:?}", A::VARIANT, adapter.raw.info);
@@ -796,7 +796,9 @@ impl Global {
             let id = inputs.find(A::VARIANT);
             match (id, instance) {
                 (Some(id), Some(inst)) => {
-                    let mut adapters = unsafe { inst.enumerate_adapters() };
+                    let compatible_hal_surface =
+                        compatible_surface.and_then(|surface| A::surface_as_hal(surface));
+                    let mut adapters = unsafe { inst.enumerate_adapters(compatible_hal_surface) };
                     if force_software {
                         adapters.retain(|exposed| exposed.info.device_type == wgt::DeviceType::Cpu);
                     }

diff --git a/wgpu-hal/examples/halmark/main.rs b/wgpu-hal/examples/halmark/main.rs
@@ -111,7 +111,7 @@ impl<A: hal::Api> Example<A> {
         };
 
         let (adapter, capabilities) = unsafe {
-            let mut adapters = instance.enumerate_adapters();
+            let mut adapters = instance.enumerate_adapters(Some(&surface));
             if adapters.is_empty() {
                 return Err("no adapters found".into());
             }

diff --git a/wgpu-hal/examples/ray-traced-triangle/main.rs b/wgpu-hal/examples/ray-traced-triangle/main.rs
@@ -237,7 +237,7 @@ impl<A: hal::Api> Example<A> {
         };
 
         let (adapter, features) = unsafe {
-            let mut adapters = instance.enumerate_adapters();
+            let mut adapters = instance.enumerate_adapters(Some(&surface));
             if adapters.is_empty() {
                 panic!("No adapters found");
             }

diff --git a/wgpu-hal/src/dx12/instance.rs b/wgpu-hal/src/dx12/instance.rs
@@ -147,7 +147,10 @@ impl crate::Instance for super::Instance {
         // just drop
     }
 
-    unsafe fn enumerate_adapters(&self) -> Vec<crate::ExposedAdapter<super::Api>> {
+    unsafe fn enumerate_adapters(
+        &self,
+        _surface_hint: Option<&super::Surface>,
+    ) -> Vec<crate::ExposedAdapter<super::Api>> {
         let adapters = auxil::dxgi::factory::enumerate_adapters(self.factory.clone());
 
         adapters

diff --git a/wgpu-hal/src/empty.rs b/wgpu-hal/src/empty.rs
@@ -54,7 +54,10 @@ impl crate::Instance for Context {
         Ok(Context)
     }
     unsafe fn destroy_surface(&self, surface: Context) {}
-    unsafe fn enumerate_adapters(&self) -> Vec<crate::ExposedAdapter<Api>> {
+    unsafe fn enumerate_adapters(
+        &self,
+        _surface_hint: Option<&Context>,
+    ) -> Vec<crate::ExposedAdapter<Api>> {
         Vec::new()
     }
 }

diff --git a/wgpu-hal/src/gles/egl.rs b/wgpu-hal/src/gles/egl.rs
@@ -1001,7 +1001,10 @@ impl crate::Instance for Instance {
 
     unsafe fn destroy_surface(&self, _surface: Surface) {}
 
-    unsafe fn enumerate_adapters(&self) -> Vec<crate::ExposedAdapter<super::Api>> {
+    unsafe fn enumerate_adapters(
+        &self,
+        _surface_hint: Option<&Surface>,
+    ) -> Vec<crate::ExposedAdapter<super::Api>> {
         let inner = self.inner.lock();
         inner.egl.make_current();
 

diff --git a/wgpu-hal/src/gles/web.rs b/wgpu-hal/src/gles/web.rs
@@ -24,10 +24,7 @@ impl AdapterContext {
 }
 
 #[derive(Debug)]
-pub struct Instance {
-    /// Set when a canvas is provided, and used to implement [`Instance::enumerate_adapters()`].
-    webgl2_context: Mutex<Option<web_sys::WebGl2RenderingContext>>,
-}
+pub struct Instance;
 
 impl Instance {
     pub fn create_surface_from_canvas(
@@ -85,10 +82,6 @@ impl Instance {
             .dyn_into()
             .expect("canvas context is not a WebGl2RenderingContext");
 
-        // It is not inconsistent to overwrite an existing context, because the only thing that
-        // `self.webgl2_context` is used for is producing the response to `enumerate_adapters()`.
-        *self.webgl2_context.lock() = Some(webgl2_context.clone());
-
         Ok(Surface {
             canvas,
             webgl2_context,
@@ -121,21 +114,22 @@ impl crate::Instance for Instance {
 
     unsafe fn init(_desc: &crate::InstanceDescriptor) -> Result<Self, crate::InstanceError> {
         profiling::scope!("Init OpenGL (WebGL) Backend");
-        Ok(Instance {
-            webgl2_context: Mutex::new(None),
-        })
+        Ok(Instance)
     }
 
-    unsafe fn enumerate_adapters(&self) -> Vec<crate::ExposedAdapter<super::Api>> {
-        let context_guard = self.webgl2_context.lock();
-        let gl = match *context_guard {
-            Some(ref webgl2_context) => glow::Context::from_webgl2_context(webgl2_context.clone()),
-            None => return Vec::new(),
-        };
-
-        unsafe { super::Adapter::expose(AdapterContext { glow_context: gl }) }
-            .into_iter()
-            .collect()
+    unsafe fn enumerate_adapters(
+        &self,
+        surface_hint: Option<&Surface>,
+    ) -> Vec<crate::ExposedAdapter<super::Api>> {
+        if let Some(surface_hint) = surface_hint {
+            let gl = glow::Context::from_webgl2_context(surface_hint.webgl2_context.clone());
+
+            unsafe { super::Adapter::expose(AdapterContext { glow_context: gl }) }
+                .into_iter()
+                .collect()
+        } else {
+            Vec::new()
+        }
     }
 
     unsafe fn create_surface(
@@ -172,15 +166,7 @@ impl crate::Instance for Instance {
         self.create_surface_from_canvas(canvas)
     }
 
-    unsafe fn destroy_surface(&self, surface: Surface) {
-        let mut context_option_ref = self.webgl2_context.lock();
-
-        if let Some(context) = context_option_ref.as_ref() {
-            if context == &surface.webgl2_context {
-                *context_option_ref = None;
-            }
-        }
-    }
+    unsafe fn destroy_surface(&self, _surface: Surface) {}
 }
 
 #[derive(Debug)]

diff --git a/wgpu-hal/src/gles/wgl.rs b/wgpu-hal/src/gles/wgl.rs
@@ -558,7 +558,10 @@ impl crate::Instance for Instance {
     }
     unsafe fn destroy_surface(&self, _surface: Surface) {}
 
-    unsafe fn enumerate_adapters(&self) -> Vec<crate::ExposedAdapter<super::Api>> {
+    unsafe fn enumerate_adapters(
+        &self,
+        _surface_hint: Option<&Surface>,
+    ) -> Vec<crate::ExposedAdapter<super::Api>> {
         unsafe {
             super::Adapter::expose(AdapterContext {
                 inner: self.inner.clone(),

diff --git a/wgpu-hal/src/lib.rs b/wgpu-hal/src/lib.rs
@@ -447,7 +447,11 @@ pub trait Instance: Sized + WasmNotSendSync {
         window_handle: raw_window_handle::RawWindowHandle,
     ) -> Result<<Self::A as Api>::Surface, InstanceError>;
     unsafe fn destroy_surface(&self, surface: <Self::A as Api>::Surface);
-    unsafe fn enumerate_adapters(&self) -> Vec<ExposedAdapter<Self::A>>;
+    /// `surface_hint` is only used by the GLES backend targeting WebGL2
+    unsafe fn enumerate_adapters(
+        &self,
+        surface_hint: Option<&<Self::A as Api>::Surface>,
+    ) -> Vec<ExposedAdapter<Self::A>>;
 }
 
 pub trait Surface: WasmNotSendSync {

diff --git a/wgpu-hal/src/metal/mod.rs b/wgpu-hal/src/metal/mod.rs
@@ -121,7 +121,10 @@ impl crate::Instance for Instance {
         unsafe { surface.dispose() };
     }
 
-    unsafe fn enumerate_adapters(&self) -> Vec<crate::ExposedAdapter<Api>> {
+    unsafe fn enumerate_adapters(
+        &self,
+        _surface_hint: Option<&Surface>,
+    ) -> Vec<crate::ExposedAdapter<Api>> {
         let devices = metal::Device::all();
         let mut adapters: Vec<crate::ExposedAdapter<Api>> = devices
             .into_iter()

diff --git a/wgpu-hal/src/vulkan/instance.rs b/wgpu-hal/src/vulkan/instance.rs
@@ -884,7 +884,10 @@ impl crate::Instance for super::Instance {
         unsafe { surface.functor.destroy_surface(surface.raw, None) };
     }
 
-    unsafe fn enumerate_adapters(&self) -> Vec<crate::ExposedAdapter<super::Api>> {
+    unsafe fn enumerate_adapters(
+        &self,
+        _surface_hint: Option<&super::Surface>,
+    ) -> Vec<crate::ExposedAdapter<super::Api>> {
         use crate::auxil::db;
 
         let raw_devices = match unsafe { self.shared.raw.enumerate_physical_devices() } {

diff --git a/wgpu/src/backend/wgpu_core.rs b/wgpu/src/backend/wgpu_core.rs
@@ -61,6 +61,7 @@ impl ContextWgpuCore {
         Self(unsafe { wgc::global::Global::from_instance(core_instance) })
     }
 
+    #[cfg(native)]
     pub fn enumerate_adapters(&self, backends: wgt::Backends) -> Vec<wgc::id::AdapterId> {
         self.0
             .enumerate_adapters(wgc::instance::AdapterInputs::Mask(backends, |_| None))

diff --git a/wgpu/src/lib.rs b/wgpu/src/lib.rs
@@ -2369,13 +2369,10 @@ impl Instance {
 
     /// Retrieves all available [`Adapter`]s that match the given [`Backends`].
     ///
-    /// Always returns an empty vector if the instance decided upon creation to
-    /// target WebGPU since adapter creation is always async on WebGPU.
-    ///
     /// # Arguments
     ///
     /// - `backends` - Backends from which to enumerate adapters.
-    #[cfg(wgpu_core)]
+    #[cfg(native)]
     pub fn enumerate_adapters(&self, backends: Backends) -> Vec<Adapter> {
         let context = Arc::clone(&self.context);
         self.context
@@ -2391,14 +2388,16 @@ impl Instance {
                     })
                     .collect()
             })
-            .unwrap_or_default()
+            .unwrap()
     }
 
     /// Retrieves an [`Adapter`] which matches the given [`RequestAdapterOptions`].
     ///
     /// Some options are "soft", so treated as non-mandatory. Others are "hard".
     ///
     /// If no adapters are found that suffice all the "hard" options, `None` is returned.
+    ///
+    /// A `compatible_surface` is required when targeting WebGL2.
     pub fn request_adapter(
         &self,
         options: &RequestAdapterOptions<'_, '_>,