Add details to InstanceError and CreateSurfaceError. (gfx-rs#4066)

Co-authored-by: Connor Fitzgerald <[email protected]>

CHANGELOG.md

@@ -70,9 +70,12 @@ By @Valaphee in [#3402](https://github.com/gfx-rs/wgpu/pull/3402)
 
 ### Changes
 
+#### General
+
 - Omit texture store bound checks since they are no-ops if out of bounds on all APIs. By @teoxoy in [#3975](https://github.com/gfx-rs/wgpu/pull/3975)
 - Validate `DownlevelFlags::READ_ONLY_DEPTH_STENCIL`. By @teoxoy in [#4031](https://github.com/gfx-rs/wgpu/pull/4031)
 - Add validation in accordance with WebGPU `setViewport` valid usage for `x`, `y` and `this.[[attachment_size]]`. By @James2022-rgb in [#4058](https://github.com/gfx-rs/wgpu/pull/4058)
+- `wgpu::CreateSurfaceError` now gives details of the failure, but no longer implements `PartialEq`. By @kpreid in [#4066](https://github.com/gfx-rs/wgpu/pull/4066)
 - Make `WGPU_POWER_PREF=none` a valid value. By @fornwall in [4076](https://github.com/gfx-rs/wgpu/pull/4076)
 
 #### Vulkan

tests/src/lib.rs

@@ -312,7 +312,7 @@ pub fn initialize_test(parameters: TestParameters, test_function: impl FnOnce(Te
         if #[cfg(any(not(target_arch = "wasm32"), target_os = "emscripten"))] {
             let canary_set = wgpu::hal::VALIDATION_CANARY.get_and_reset();
         } else {
-            let canary_set = _surface_guard.check_for_unreported_errors();
+            let canary_set = _surface_guard.unwrap().check_for_unreported_errors();
         }
     );
 
@@ -345,24 +345,18 @@ pub fn initialize_test(parameters: TestParameters, test_function: impl FnOnce(Te
     }
 }
 
-fn initialize_adapter() -> (Adapter, SurfaceGuard) {
-    let backends = wgpu::util::backend_bits_from_env().unwrap_or_else(Backends::all);
-    let dx12_shader_compiler = wgpu::util::dx12_shader_compiler_from_env().unwrap_or_default();
-    let gles_minor_version = wgpu::util::gles_minor_version_from_env().unwrap_or_default();
-    let instance = Instance::new(wgpu::InstanceDescriptor {
-        backends,
-        dx12_shader_compiler,
-        gles_minor_version,
-    });
-    let surface_guard;
+fn initialize_adapter() -> (Adapter, Option<SurfaceGuard>) {
+    let instance = initialize_instance();
+    let surface_guard: Option<SurfaceGuard>;
     let compatible_surface;
 
+    // Create a canvas iff we need a WebGL2RenderingContext to have a working device.
     #[cfg(not(all(
         target_arch = "wasm32",
         any(target_os = "emscripten", feature = "webgl")
     )))]
     {
-        surface_guard = SurfaceGuard {};
+        surface_guard = None;
         compatible_surface = None;
     }
     #[cfg(all(
@@ -398,7 +392,7 @@ fn initialize_adapter() -> (Adapter, SurfaceGuard) {
                 .expect("could not create surface from canvas")
         };
 
-        surface_guard = SurfaceGuard { canvas };
+        surface_guard = Some(SurfaceGuard { canvas });
 
         compatible_surface = Some(surface);
     }
@@ -413,12 +407,21 @@ fn initialize_adapter() -> (Adapter, SurfaceGuard) {
     (adapter, surface_guard)
 }
 
-struct SurfaceGuard {
-    #[cfg(all(
-        target_arch = "wasm32",
-        any(target_os = "emscripten", feature = "webgl")
-    ))]
-    canvas: web_sys::HtmlCanvasElement,
+pub fn initialize_instance() -> Instance {
+    let backends = wgpu::util::backend_bits_from_env().unwrap_or_else(Backends::all);
+    let dx12_shader_compiler = wgpu::util::dx12_shader_compiler_from_env().unwrap_or_default();
+    let gles_minor_version = wgpu::util::gles_minor_version_from_env().unwrap_or_default();
+    Instance::new(wgpu::InstanceDescriptor {
+        backends,
+        dx12_shader_compiler,
+        gles_minor_version,
+    })
+}
+
+// Public because it is used by tests of interacting with canvas
+pub struct SurfaceGuard {
+    #[cfg(target_arch = "wasm32")]
+    pub canvas: web_sys::HtmlCanvasElement,
 }
 
 impl SurfaceGuard {
@@ -452,11 +455,8 @@ impl Drop for SurfaceGuard {
     }
 }
 
-#[cfg(all(
-    target_arch = "wasm32",
-    any(target_os = "emscripten", feature = "webgl")
-))]
-fn create_html_canvas() -> web_sys::HtmlCanvasElement {
+#[cfg(target_arch = "wasm32")]
+pub fn create_html_canvas() -> web_sys::HtmlCanvasElement {
     use wasm_bindgen::JsCast;
 
     web_sys::window()

tests/tests/create_surface_error.rs

@@ -0,0 +1,28 @@
+//! 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
+/// include WebGL and WebGPU, but *not* Emscripten GLES.
+#[cfg(all(target_arch = "wasm32", not(target_os = "emscripten")))]
+#[wasm_bindgen_test::wasm_bindgen_test]
+fn canvas_get_context_returned_null() {
+    // Not using initialize_test() because that goes straight to creating the canvas for us.
+    let instance = wgpu_test::initialize_instance();
+    // Create canvas and cleanup on drop
+    let canvas_g = wgpu_test::SurfaceGuard {
+        canvas: wgpu_test::create_html_canvas(),
+    };
+    // Using a context id that is not "webgl2" or "webgpu" will render the canvas unusable by wgpu.
+    canvas_g.canvas.get_context("2d").unwrap();
+
+    #[allow(clippy::redundant_clone)] // false positive — can't and shouldn't move out.
+    let error = instance
+        .create_surface_from_canvas(canvas_g.canvas.clone())
+        .unwrap_err();
+
+    assert!(
+        error
+            .to_string()
+            .contains("canvas.getContext() returned null"),
+        "{error}"
+    );
+}

tests/tests/root.rs

@@ -10,6 +10,7 @@ mod buffer;
 mod buffer_copy;
 mod buffer_usages;
 mod clear_texture;
+mod create_surface_error;
 mod device;
 mod encoder;
 mod example_wgsl;

wgpu-hal/examples/halmark/main.rs

@@ -86,7 +86,7 @@ struct Example<A: hal::Api> {
 }
 
 impl<A: hal::Api> Example<A> {
-    fn init(window: &winit::window::Window) -> Result<Self, hal::InstanceError> {
+    fn init(window: &winit::window::Window) -> Result<Self, Box<dyn std::error::Error>> {
         let instance_desc = hal::InstanceDescriptor {
             name: "example",
             flags: if cfg!(debug_assertions) {
@@ -108,13 +108,13 @@ impl<A: hal::Api> Example<A> {
         let (adapter, capabilities) = unsafe {
             let mut adapters = instance.enumerate_adapters();
             if adapters.is_empty() {
-                return Err(hal::InstanceError);
+                return Err("no adapters found".into());
             }
             let exposed = adapters.swap_remove(0);
             (exposed.adapter, exposed.capabilities)
         };
-        let surface_caps =
-            unsafe { adapter.surface_capabilities(&surface) }.ok_or(hal::InstanceError)?;
+        let surface_caps = unsafe { adapter.surface_capabilities(&surface) }
+            .ok_or("failed to get surface capabilities")?;
         log::info!("Surface caps: {:#?}", surface_caps);
 
         let hal::OpenDevice { device, mut queue } = unsafe {

wgpu-hal/src/auxil/dxgi/factory.rs

@@ -96,7 +96,9 @@ pub fn create_factory(
     required_factory_type: DxgiFactoryType,
     instance_flags: crate::InstanceFlags,
 ) -> Result<(d3d12::DxgiLib, d3d12::DxgiFactory), crate::InstanceError> {
-    let lib_dxgi = d3d12::DxgiLib::new().map_err(|_| crate::InstanceError)?;
+    let lib_dxgi = d3d12::DxgiLib::new().map_err(|e| {
+        crate::InstanceError::with_source(String::from("failed to load dxgi.dll"), e)
+    })?;
 
     let mut factory_flags = d3d12::FactoryCreationFlags::empty();
 
@@ -128,18 +130,22 @@ pub fn create_factory(
             Ok(factory) => Some(factory),
             // We hard error here as we _should have_ been able to make a factory4 but couldn't.
             Err(err) => {
-                log::error!("Failed to create IDXGIFactory4: {}", err);
-                return Err(crate::InstanceError);
+                // err is a Cow<str>, not an Error implementor
+                return Err(crate::InstanceError::new(format!(
+                    "failed to create IDXGIFactory4: {err:?}"
+                )));
             }
         },
         // If we require factory4, hard error.
         Err(err) if required_factory_type == DxgiFactoryType::Factory4 => {
-            log::error!("IDXGIFactory1 creation function not found: {:?}", err);
-            return Err(crate::InstanceError);
+            return Err(crate::InstanceError::with_source(
+                String::from("IDXGIFactory1 creation function not found"),
+                err,
+            ));
         }
         // If we don't print it to info as all win7 will hit this case.
         Err(err) => {
-            log::info!("IDXGIFactory1 creation function not found: {:?}", err);
+            log::info!("IDXGIFactory1 creation function not found: {err:?}");
             None
         }
     };
@@ -153,8 +159,10 @@ pub fn create_factory(
             }
             // If we require factory6, hard error.
             Err(err) if required_factory_type == DxgiFactoryType::Factory6 => {
-                log::warn!("Failed to cast IDXGIFactory4 to IDXGIFactory6: {:?}", err);
-                return Err(crate::InstanceError);
+                // err is a Cow<str>, not an Error implementor
+                return Err(crate::InstanceError::new(format!(
+                    "failed to cast IDXGIFactory4 to IDXGIFactory6: {err:?}"
+                )));
             }
             // If we don't print it to info.
             Err(err) => {
@@ -169,14 +177,18 @@ pub fn create_factory(
         Ok(pair) => match pair.into_result() {
             Ok(factory) => factory,
             Err(err) => {
-                log::error!("Failed to create IDXGIFactory1: {}", err);
-                return Err(crate::InstanceError);
+                // err is a Cow<str>, not an Error implementor
+                return Err(crate::InstanceError::new(format!(
+                    "failed to create IDXGIFactory1: {err:?}"
+                )));
             }
         },
         // We always require at least factory1, so hard error
         Err(err) => {
-            log::error!("IDXGIFactory1 creation function not found: {:?}", err);
-            return Err(crate::InstanceError);
+            return Err(crate::InstanceError::with_source(
+                String::from("IDXGIFactory1 creation function not found"),
+                err,
+            ));
         }
     };
 
@@ -188,8 +200,10 @@ pub fn create_factory(
         }
         // If we require factory2, hard error.
         Err(err) if required_factory_type == DxgiFactoryType::Factory2 => {
-            log::warn!("Failed to cast IDXGIFactory1 to IDXGIFactory2: {:?}", err);
-            return Err(crate::InstanceError);
+            // err is a Cow<str>, not an Error implementor
+            return Err(crate::InstanceError::new(format!(
+                "failed to cast IDXGIFactory1 to IDXGIFactory2: {err:?}"
+            )));
         }
         // If we don't print it to info.
         Err(err) => {

wgpu-hal/src/dx11/instance.rs

@@ -8,10 +8,13 @@ impl crate::Instance<super::Api> for super::Instance {
         };
 
         if !enable_dx11 {
-            return Err(crate::InstanceError);
+            return Err(crate::InstanceError::new(String::from(
+                "DX11 support is unstable; set WGPU_UNSTABLE_DX11_BACKEND=1 to enable anyway",
+            )));
         }
 
-        let lib_d3d11 = super::library::D3D11Lib::new().ok_or(crate::InstanceError)?;
+        let lib_d3d11 = super::library::D3D11Lib::new()
+            .ok_or_else(|| crate::InstanceError::new(String::from("failed to load d3d11.dll")))?;
 
         let (lib_dxgi, factory) = auxil::dxgi::factory::create_factory(
             auxil::dxgi::factory::DxgiFactoryType::Factory1,

wgpu-hal/src/dx12/instance.rs

@@ -12,7 +12,9 @@ impl Drop for super::Instance {
 
 impl crate::Instance<super::Api> for super::Instance {
     unsafe fn init(desc: &crate::InstanceDescriptor) -> Result<Self, crate::InstanceError> {
-        let lib_main = d3d12::D3D12Lib::new().map_err(|_| crate::InstanceError)?;
+        let lib_main = d3d12::D3D12Lib::new().map_err(|e| {
+            crate::InstanceError::with_source(String::from("failed to load d3d12.dll"), e)
+        })?;
 
         if desc.flags.contains(crate::InstanceFlags::VALIDATION) {
             // Enable debug layer
@@ -95,7 +97,9 @@ impl crate::Instance<super::Api> for super::Instance {
                 supports_allow_tearing: self.supports_allow_tearing,
                 swap_chain: None,
             }),
-            _ => Err(crate::InstanceError),
+            _ => Err(crate::InstanceError::new(format!(
+                "window handle {window_handle:?} is not a Win32 handle"
+            ))),
         }
     }
     unsafe fn destroy_surface(&self, _surface: super::Surface) {

wgpu-hal/src/gles/adapter.rs

@@ -43,8 +43,9 @@ impl super::Adapter {
                     src = &src[pos + es_sig.len()..];
                 }
                 None => {
-                    log::warn!("ES not found in '{}'", src);
-                    return Err(crate::InstanceError);
+                    return Err(crate::InstanceError::new(format!(
+                        "OpenGL version {src:?} does not contain 'ES'"
+                    )));
                 }
             }
         };
@@ -86,10 +87,9 @@ impl super::Adapter {
                 },
                 minor,
             )),
-            _ => {
-                log::warn!("Unable to extract the version from '{}'", version);
-                Err(crate::InstanceError)
-            }
+            _ => Err(crate::InstanceError::new(format!(
+                "unable to extract OpenGL version from {version:?}"
+            ))),
         }
     }
 
@@ -975,27 +975,30 @@ mod tests {
 
     #[test]
     fn test_version_parse() {
-        let error = Err(crate::InstanceError);
-        assert_eq!(Adapter::parse_version("1"), error);
-        assert_eq!(Adapter::parse_version("1."), error);
-        assert_eq!(Adapter::parse_version("1 h3l1o. W0rld"), error);
-        assert_eq!(Adapter::parse_version("1. h3l1o. W0rld"), error);
-        assert_eq!(Adapter::parse_version("1.2.3"), error);
-        assert_eq!(Adapter::parse_version("OpenGL ES 3.1"), Ok((3, 1)));
+        Adapter::parse_version("1").unwrap_err();
+        Adapter::parse_version("1.").unwrap_err();
+        Adapter::parse_version("1 h3l1o. W0rld").unwrap_err();
+        Adapter::parse_version("1. h3l1o. W0rld").unwrap_err();
+        Adapter::parse_version("1.2.3").unwrap_err();
+
+        assert_eq!(Adapter::parse_version("OpenGL ES 3.1").unwrap(), (3, 1));
+        assert_eq!(
+            Adapter::parse_version("OpenGL ES 2.0 Google Nexus").unwrap(),
+            (2, 0)
+        );
+        assert_eq!(Adapter::parse_version("GLSL ES 1.1").unwrap(), (1, 1));
         assert_eq!(
-            Adapter::parse_version("OpenGL ES 2.0 Google Nexus"),
-            Ok((2, 0))
+            Adapter::parse_version("OpenGL ES GLSL ES 3.20").unwrap(),
+            (3, 2)
         );
-        assert_eq!(Adapter::parse_version("GLSL ES 1.1"), Ok((1, 1)));
-        assert_eq!(Adapter::parse_version("OpenGL ES GLSL ES 3.20"), Ok((3, 2)));
         assert_eq!(
             // WebGL 2.0 should parse as OpenGL ES 3.0
-            Adapter::parse_version("WebGL 2.0 (OpenGL ES 3.0 Chromium)"),
-            Ok((3, 0))
+            Adapter::parse_version("WebGL 2.0 (OpenGL ES 3.0 Chromium)").unwrap(),
+            (3, 0)
         );
         assert_eq!(
-            Adapter::parse_version("WebGL GLSL ES 3.00 (OpenGL ES GLSL ES 3.0 Chromium)"),
-            Ok((3, 0))
+            Adapter::parse_version("WebGL GLSL ES 3.00 (OpenGL ES GLSL ES 3.0 Chromium)").unwrap(),
+            (3, 0)
         );
     }
 }