Handle unsupported surfaces more gracefully

The Vulkan backend in particular panicked when creating a surface for which the window handle is not supported. Per discussion on Matrix, the surface should instead not be usable on that device. Getting the hal surface in wgpu-core may now return `None` to correctly represent those semantics.
gfx-rs · Sep 28, 2022 · b8b05c3 · b8b05c3
1 parent b65ebb4
commit b8b05c3
Show file tree

Hide file tree

Showing 10 changed files with 110 additions and 77 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -100,6 +100,8 @@ SurfaceConfiguration {
   violates Vulkan valid usage rules `VUID-StandaloneSpirv-Flat-06202`
   and `VUID-StandaloneSpirv-Flat-04744`. By @jimblandy in
   [#3008](https://github.com/gfx-rs/wgpu/pull/3008)
+- Fix bug where the Vulkan backend would panic when using a supported window and display handle but the
+  dependent extensions are not available by @i509VCB in [#3054](https://github.com/gfx-rs/wgpu/pull/3054).
 
 #### Gles
 - Report vendor id for Mesa and Apple GPUs. By @i509VCB [#3036](https://github.com/gfx-rs/wgpu/pull/3036)

diff --git a/Cargo.lock b/Cargo.lock
diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs
@@ -5186,7 +5186,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
             let caps = unsafe {
                 let suf = A::get_surface(surface);
                 let adapter = &adapter_guard[device.adapter_id.value];
-                match adapter.raw.adapter.surface_capabilities(&suf.raw) {
+                match adapter.raw.adapter.surface_capabilities(&suf.unwrap().raw) {
                     Some(caps) => caps,
                     None => break E::UnsupportedQueueFamily,
                 }
@@ -5214,6 +5214,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
 
             match unsafe {
                 A::get_surface_mut(surface)
+                    .unwrap()
                     .raw
                     .configure(&device.raw, &hal_config)
             } {

diff --git a/wgpu-core/src/hub.rs b/wgpu-core/src/hub.rs
@@ -1007,7 +1007,7 @@ impl<A: HalApi, F: GlobalIdentityHandlerFactory> Hub<A, F> {
                     let device = &devices[present.device_id.value];
                     let suf = A::get_surface_mut(surface);
                     unsafe {
-                        suf.raw.unconfigure(&device.raw);
+                        suf.unwrap().raw.unconfigure(&device.raw);
                         //TODO: we could destroy the surface here
                     }
                 }
@@ -1238,8 +1238,8 @@ pub trait HalApi: hal::Api {
     fn create_instance_from_hal(name: &str, hal_instance: Self::Instance) -> Instance;
     fn instance_as_hal(instance: &Instance) -> Option<&Self::Instance>;
     fn hub<G: GlobalIdentityHandlerFactory>(global: &Global<G>) -> &Hub<Self, G>;
-    fn get_surface(surface: &Surface) -> &HalSurface<Self>;
-    fn get_surface_mut(surface: &mut Surface) -> &mut HalSurface<Self>;
+    fn get_surface(surface: &Surface) -> Option<&HalSurface<Self>>;
+    fn get_surface_mut(surface: &mut Surface) -> Option<&mut HalSurface<Self>>;
 }
 
 impl HalApi for hal::api::Empty {
@@ -1253,10 +1253,10 @@ impl HalApi for hal::api::Empty {
     fn hub<G: GlobalIdentityHandlerFactory>(_: &Global<G>) -> &Hub<Self, G> {
         unimplemented!("called empty api")
     }
-    fn get_surface(_: &Surface) -> &HalSurface<Self> {
+    fn get_surface(_: &Surface) -> Option<&HalSurface<Self>> {
         unimplemented!("called empty api")
     }
-    fn get_surface_mut(_: &mut Surface) -> &mut HalSurface<Self> {
+    fn get_surface_mut(_: &mut Surface) -> Option<&mut HalSurface<Self>> {
         unimplemented!("called empty api")
     }
 }
@@ -1277,11 +1277,11 @@ impl HalApi for hal::api::Vulkan {
     fn hub<G: GlobalIdentityHandlerFactory>(global: &Global<G>) -> &Hub<Self, G> {
         &global.hubs.vulkan
     }
-    fn get_surface(surface: &Surface) -> &HalSurface<Self> {
-        surface.vulkan.as_ref().unwrap()
+    fn get_surface(surface: &Surface) -> Option<&HalSurface<Self>> {
+        surface.vulkan.as_ref()
     }
-    fn get_surface_mut(surface: &mut Surface) -> &mut HalSurface<Self> {
-        surface.vulkan.as_mut().unwrap()
+    fn get_surface_mut(surface: &mut Surface) -> Option<&mut HalSurface<Self>> {
+        surface.vulkan.as_mut()
     }
 }
 
@@ -1301,11 +1301,11 @@ impl HalApi for hal::api::Metal {
     fn hub<G: GlobalIdentityHandlerFactory>(global: &Global<G>) -> &Hub<Self, G> {
         &global.hubs.metal
     }
-    fn get_surface(surface: &Surface) -> &HalSurface<Self> {
-        surface.metal.as_ref().unwrap()
+    fn get_surface(surface: &Surface) -> Option<&HalSurface<Self>> {
+        surface.metal.as_ref()
     }
-    fn get_surface_mut(surface: &mut Surface) -> &mut HalSurface<Self> {
-        surface.metal.as_mut().unwrap()
+    fn get_surface_mut(surface: &mut Surface) -> Option<&mut HalSurface<Self>> {
+        surface.metal.as_mut()
     }
 }
 
@@ -1325,11 +1325,11 @@ impl HalApi for hal::api::Dx12 {
     fn hub<G: GlobalIdentityHandlerFactory>(global: &Global<G>) -> &Hub<Self, G> {
         &global.hubs.dx12
     }
-    fn get_surface(surface: &Surface) -> &HalSurface<Self> {
-        surface.dx12.as_ref().unwrap()
+    fn get_surface(surface: &Surface) -> Option<&HalSurface<Self>> {
+        surface.dx12.as_ref()
     }
-    fn get_surface_mut(surface: &mut Surface) -> &mut HalSurface<Self> {
-        surface.dx12.as_mut().unwrap()
+    fn get_surface_mut(surface: &mut Surface) -> Option<&mut HalSurface<Self>> {
+        surface.dx12.as_mut()
     }
 }
 
@@ -1349,11 +1349,11 @@ impl HalApi for hal::api::Dx11 {
     fn hub<G: GlobalIdentityHandlerFactory>(global: &Global<G>) -> &Hub<Self, G> {
         &global.hubs.dx11
     }
-    fn get_surface(surface: &Surface) -> &HalSurface<Self> {
-        surface.dx11.as_ref().unwrap()
+    fn get_surface(surface: &Surface) -> Option<&HalSurface<Self>> {
+        surface.dx11.as_ref()
     }
-    fn get_surface_mut(surface: &mut Surface) -> &mut HalSurface<Self> {
-        surface.dx11.as_mut().unwrap()
+    fn get_surface_mut(surface: &mut Surface) -> Option<&mut HalSurface<Self>> {
+        surface.dx11.as_mut()
     }
 }
 
@@ -1374,11 +1374,11 @@ impl HalApi for hal::api::Gles {
     fn hub<G: GlobalIdentityHandlerFactory>(global: &Global<G>) -> &Hub<Self, G> {
         &global.hubs.gl
     }
-    fn get_surface(surface: &Surface) -> &HalSurface<Self> {
-        surface.gl.as_ref().unwrap()
+    fn get_surface(surface: &Surface) -> Option<&HalSurface<Self>> {
+        surface.gl.as_ref()
     }
-    fn get_surface_mut(surface: &mut Surface) -> &mut HalSurface<Self> {
-        surface.gl.as_mut().unwrap()
+    fn get_surface_mut(surface: &mut Surface) -> Option<&mut HalSurface<Self>> {
+        surface.gl.as_mut()
     }
 }
 

diff --git a/wgpu-core/src/instance.rs b/wgpu-core/src/instance.rs
@@ -183,14 +183,14 @@ impl Surface {
         &self,
         adapter: &Adapter<A>,
     ) -> Result<SurfaceCapabilities, GetSurfaceSupportError> {
-        let suf = A::get_surface(self);
+        let suf = A::get_surface(self).ok_or(GetSurfaceSupportError::Unsupported)?;
         profiling::scope!("surface_capabilities");
         let caps = unsafe {
             adapter
                 .raw
                 .adapter
                 .surface_capabilities(&suf.raw)
-                .ok_or(GetSurfaceSupportError::UnsupportedQueueFamily)?
+                .ok_or(GetSurfaceSupportError::Unsupported)?
         };
 
         Ok(caps)
@@ -212,7 +212,15 @@ impl<A: HalApi> Adapter<A> {
 
     pub fn is_surface_supported(&self, surface: &Surface) -> bool {
         let suf = A::get_surface(surface);
-        unsafe { self.raw.adapter.surface_capabilities(&suf.raw) }.is_some()
+
+        // If get_surface returns None, then the API does not advertise support for the surface.
+        //
+        // This could occur if the user is running their app on Wayland but Vulkan does not support
+        // VK_KHR_wayland_surface.
+        match suf {
+            Some(suf) => unsafe { self.raw.adapter.surface_capabilities(&suf.raw) }.is_some(),
+            None => false,
+        }
     }
 
     pub(crate) fn get_texture_format_features(
@@ -372,8 +380,8 @@ pub enum GetSurfaceSupportError {
     InvalidAdapter,
     #[error("invalid surface")]
     InvalidSurface,
-    #[error("surface does not support the adapter's queue family")]
-    UnsupportedQueueFamily,
+    #[error("surface is not supported by the adapter")]
+    Unsupported,
 }
 
 #[derive(Clone, Debug, Error)]
@@ -721,9 +729,14 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
                         adapters.retain(|exposed| exposed.info.device_type == wgt::DeviceType::Cpu);
                     }
                     if let Some(surface) = compatible_surface {
-                        let suf_raw = &A::get_surface(surface).raw;
+                        let surface = &A::get_surface(surface);
                         adapters.retain(|exposed| unsafe {
-                            exposed.adapter.surface_capabilities(suf_raw).is_some()
+                            // If the surface does not exist for this backend, then the surface is not supported.
+                            surface.is_some()
+                                && exposed
+                                    .adapter
+                                    .surface_capabilities(&surface.unwrap().raw)
+                                    .is_some()
                         });
                     }
                     device_types.extend(adapters.iter().map(|ad| ad.info.device_type));

diff --git a/wgpu-core/src/present.rs b/wgpu-core/src/present.rs
@@ -134,7 +134,8 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
 
         let suf = A::get_surface_mut(surface);
         let (texture_id, status) = match unsafe {
-            suf.raw
+            suf.unwrap()
+                .raw
                 .acquire_texture(Some(std::time::Duration::from_millis(
                     FRAME_TIMEOUT_MS as u64,
                 )))
@@ -311,10 +312,10 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
                             Err(hal::SurfaceError::Lost)
                         } else if !has_work {
                             log::error!("No work has been submitted for this frame");
-                            unsafe { suf.raw.discard_texture(raw) };
+                            unsafe { suf.unwrap().raw.discard_texture(raw) };
                             Err(hal::SurfaceError::Outdated)
                         } else {
-                            unsafe { device.queue.present(&mut suf.raw, raw) }
+                            unsafe { device.queue.present(&mut suf.unwrap().raw, raw) }
                         }
                     }
                     resource::TextureInner::Native { .. } => unreachable!(),
@@ -387,7 +388,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
                         has_work: _,
                     } => {
                         if surface_id == parent_id.0 {
-                            unsafe { suf.raw.discard_texture(raw) };
+                            unsafe { suf.unwrap().raw.discard_texture(raw) };
                         } else {
                             log::warn!("Surface texture is outdated");
                         }

diff --git a/wgpu-hal/src/gles/web.rs b/wgpu-hal/src/gles/web.rs
@@ -123,7 +123,7 @@ impl crate::Instance<super::Api> for Instance {
 
             self.create_surface_from_canvas(&canvas)
         } else {
-            unreachable!()
+            Err(crate::InstanceError)
         }
     }