From 7910fd8059f361f48553c03d84c8e1410e94134e Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Wed, 3 Jul 2024 13:25:29 +0200 Subject: [PATCH] [wgpu-hal] require a `Surface` to be passed to `Instance.enumerate_adapters()` on WebGL2 Also makes wgpu's `enumerate_adapters()` native only. --- CHANGELOG.md | 10 ++++ tests/src/init.rs | 21 ++++++--- wgpu-core/src/instance.rs | 6 ++- wgpu-hal/examples/halmark/main.rs | 2 +- wgpu-hal/examples/ray-traced-triangle/main.rs | 2 +- wgpu-hal/src/dx12/instance.rs | 5 +- wgpu-hal/src/empty.rs | 5 +- wgpu-hal/src/gles/egl.rs | 5 +- wgpu-hal/src/gles/web.rs | 46 +++++++------------ wgpu-hal/src/gles/wgl.rs | 5 +- wgpu-hal/src/lib.rs | 6 ++- wgpu-hal/src/metal/mod.rs | 5 +- wgpu-hal/src/vulkan/instance.rs | 5 +- wgpu/src/backend/wgpu_core.rs | 1 + wgpu/src/lib.rs | 9 ++-- 15 files changed, 80 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 42a7f76cfa..3546f8e4a3 100644 --- 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 index 9a21c98471..f66f084891 100644 --- 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) { let instance = initialize_instance(); #[allow(unused_variables)] - let _surface: wgpu::Surface; + let surface: Option; let surface_guard: Option; - // 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,15 +62,17 @@ 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() @@ -76,7 +80,10 @@ pub async fn initialize_adapter(adapter_index: usize) -> (Instance, Adapter, Opt .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 index b2aad0662a..3cef19aedd 100644 --- 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 index 81474f233d..bd09a4e724 100644 --- a/wgpu-hal/examples/halmark/main.rs +++ b/wgpu-hal/examples/halmark/main.rs @@ -111,7 +111,7 @@ impl Example { }; 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 index cf0e146ec9..f27e3d067e 100644 --- a/wgpu-hal/examples/ray-traced-triangle/main.rs +++ b/wgpu-hal/examples/ray-traced-triangle/main.rs @@ -237,7 +237,7 @@ impl Example { }; 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 index 3c86d19f3c..4a4c6c6ff9 100644 --- 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> { + unsafe fn enumerate_adapters( + &self, + _surface_hint: Option<&super::Surface>, + ) -> Vec> { 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 index 65116199f2..227dce7ee0 100644 --- 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> { + unsafe fn enumerate_adapters( + &self, + _surface_hint: Option<&Context>, + ) -> Vec> { Vec::new() } } diff --git a/wgpu-hal/src/gles/egl.rs b/wgpu-hal/src/gles/egl.rs index 07cd8e835d..f35d697d5e 100644 --- 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> { + unsafe fn enumerate_adapters( + &self, + _surface_hint: Option<&Surface>, + ) -> Vec> { 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 index 081f7da5d1..a36710f648 100644 --- 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>, -} +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 { profiling::scope!("Init OpenGL (WebGL) Backend"); - Ok(Instance { - webgl2_context: Mutex::new(None), - }) + Ok(Instance) } - unsafe fn enumerate_adapters(&self) -> Vec> { - 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> { + 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 index 1111d98f83..c221b3e59d 100644 --- 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> { + unsafe fn enumerate_adapters( + &self, + _surface_hint: Option<&Surface>, + ) -> Vec> { unsafe { super::Adapter::expose(AdapterContext { inner: self.inner.clone(), diff --git a/wgpu-hal/src/lib.rs b/wgpu-hal/src/lib.rs index e5137bf754..ccc459c101 100644 --- 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<::Surface, InstanceError>; unsafe fn destroy_surface(&self, surface: ::Surface); - unsafe fn enumerate_adapters(&self) -> Vec>; + /// `surface_hint` is only used by the GLES backend targeting WebGL2 + unsafe fn enumerate_adapters( + &self, + surface_hint: Option<&::Surface>, + ) -> Vec>; } pub trait Surface: WasmNotSendSync { diff --git a/wgpu-hal/src/metal/mod.rs b/wgpu-hal/src/metal/mod.rs index 24f3fed809..177b02569a 100644 --- 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> { + unsafe fn enumerate_adapters( + &self, + _surface_hint: Option<&Surface>, + ) -> Vec> { let devices = metal::Device::all(); let mut adapters: Vec> = devices .into_iter() diff --git a/wgpu-hal/src/vulkan/instance.rs b/wgpu-hal/src/vulkan/instance.rs index 18acaeabb9..ec720f3788 100644 --- 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> { + unsafe fn enumerate_adapters( + &self, + _surface_hint: Option<&super::Surface>, + ) -> Vec> { 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 index ea4cc05888..dc9ee58cc9 100644 --- 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 { self.0 .enumerate_adapters(wgc::instance::AdapterInputs::Mask(backends, |_| None)) diff --git a/wgpu/src/lib.rs b/wgpu/src/lib.rs index 5eaec79eb2..b0d27e3ef7 100644 --- 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 { let context = Arc::clone(&self.context); self.context @@ -2391,7 +2388,7 @@ impl Instance { }) .collect() }) - .unwrap_or_default() + .unwrap() } /// Retrieves an [`Adapter`] which matches the given [`RequestAdapterOptions`]. @@ -2399,6 +2396,8 @@ impl Instance { /// 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<'_, '_>,