From 8fc3eb5b0506dc2bd7288aa10225f459f491ad89 Mon Sep 17 00:00:00 2001 From: Kevin Reid Date: Thu, 31 Aug 2023 22:48:31 -0700 Subject: [PATCH] Add details to `InstanceError` and `CreateSurfaceError`. (#4066) Co-authored-by: Connor Fitzgerald --- CHANGELOG.md | 3 ++ tests/src/lib.rs | 48 ++++++++--------- tests/tests/create_surface_error.rs | 28 ++++++++++ tests/tests/root.rs | 1 + wgpu-hal/examples/halmark/main.rs | 8 +-- wgpu-hal/src/auxil/dxgi/factory.rs | 42 ++++++++++----- wgpu-hal/src/dx11/instance.rs | 7 ++- wgpu-hal/src/dx12/instance.rs | 8 ++- wgpu-hal/src/gles/adapter.rs | 45 ++++++++-------- wgpu-hal/src/gles/egl.rs | 43 ++++++++++----- wgpu-hal/src/gles/web.rs | 16 +++--- wgpu-hal/src/lib.rs | 41 ++++++++++++-- wgpu-hal/src/metal/mod.rs | 4 +- wgpu-hal/src/vulkan/instance.rs | 67 +++++++++++++---------- wgpu/src/backend/direct.rs | 10 +--- wgpu/src/backend/web.rs | 17 ++++-- wgpu/src/lib.rs | 83 ++++++++++++++++++++++------- 17 files changed, 322 insertions(+), 149 deletions(-) create mode 100644 tests/tests/create_surface_error.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index c7c22fcb73..75e6554dcf 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/tests/src/lib.rs b/tests/src/lib.rs index fb57d2a5a8..1d741f1812 100644 --- a/tests/src/lib.rs +++ b/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) { + let instance = initialize_instance(); + let surface_guard: Option; 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() diff --git a/tests/tests/create_surface_error.rs b/tests/tests/create_surface_error.rs new file mode 100644 index 0000000000..f8962697ce --- /dev/null +++ b/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}" + ); +} diff --git a/tests/tests/root.rs b/tests/tests/root.rs index b376ab4981..25df8eda90 100644 --- a/tests/tests/root.rs +++ b/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; diff --git a/wgpu-hal/examples/halmark/main.rs b/wgpu-hal/examples/halmark/main.rs index c6b739bf17..5518cdaf4b 100644 --- a/wgpu-hal/examples/halmark/main.rs +++ b/wgpu-hal/examples/halmark/main.rs @@ -86,7 +86,7 @@ struct Example { } impl Example { - fn init(window: &winit::window::Window) -> Result { + fn init(window: &winit::window::Window) -> Result> { let instance_desc = hal::InstanceDescriptor { name: "example", flags: if cfg!(debug_assertions) { @@ -108,13 +108,13 @@ impl Example { 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 { diff --git a/wgpu-hal/src/auxil/dxgi/factory.rs b/wgpu-hal/src/auxil/dxgi/factory.rs index 123ca4933e..7ae6e745f0 100644 --- a/wgpu-hal/src/auxil/dxgi/factory.rs +++ b/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, 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, 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, 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, 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) => { diff --git a/wgpu-hal/src/dx11/instance.rs b/wgpu-hal/src/dx11/instance.rs index 1d8c2b51a2..e7a4e2e705 100644 --- a/wgpu-hal/src/dx11/instance.rs +++ b/wgpu-hal/src/dx11/instance.rs @@ -8,10 +8,13 @@ impl crate::Instance 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, diff --git a/wgpu-hal/src/dx12/instance.rs b/wgpu-hal/src/dx12/instance.rs index 208d2179f7..32d6f1690c 100644 --- a/wgpu-hal/src/dx12/instance.rs +++ b/wgpu-hal/src/dx12/instance.rs @@ -12,7 +12,9 @@ impl Drop for super::Instance { impl crate::Instance for super::Instance { unsafe fn init(desc: &crate::InstanceDescriptor) -> Result { - 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 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) { diff --git a/wgpu-hal/src/gles/adapter.rs b/wgpu-hal/src/gles/adapter.rs index 2c68961e39..348f62bc03 100644 --- a/wgpu-hal/src/gles/adapter.rs +++ b/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) ); } } diff --git a/wgpu-hal/src/gles/egl.rs b/wgpu-hal/src/gles/egl.rs index b904dffee9..d6d3d621f9 100644 --- a/wgpu-hal/src/gles/egl.rs +++ b/wgpu-hal/src/gles/egl.rs @@ -283,7 +283,10 @@ fn choose_config( } } - Err(crate::InstanceError) + // TODO: include diagnostic details that are currently logged + Err(crate::InstanceError::new(String::from( + "unable to find an acceptable EGL framebuffer configuration", + ))) } fn gl_debug_message_callback(source: u32, gltype: u32, id: u32, severity: u32, message: &str) { @@ -495,7 +498,12 @@ impl Inner { display: khronos_egl::Display, force_gles_minor_version: wgt::Gles3MinorVersion, ) -> Result { - let version = egl.initialize(display).map_err(|_| crate::InstanceError)?; + let version = egl.initialize(display).map_err(|e| { + crate::InstanceError::with_source( + String::from("failed to initialize EGL display connection"), + e, + ) + })?; let vendor = egl .query_string(Some(display), khronos_egl::VENDOR) .unwrap(); @@ -599,8 +607,10 @@ impl Inner { let context = match egl.create_context(display, config, None, &context_attributes) { Ok(context) => context, Err(e) => { - log::warn!("unable to create GLES 3.x context: {:?}", e); - return Err(crate::InstanceError); + return Err(crate::InstanceError::with_source( + String::from("unable to create GLES 3.x context"), + e, + )); } }; @@ -623,8 +633,10 @@ impl Inner { egl.create_pbuffer_surface(display, config, &attributes) .map(Some) .map_err(|e| { - log::warn!("Error in create_pbuffer_surface: {:?}", e); - crate::InstanceError + crate::InstanceError::with_source( + String::from("error in create_pbuffer_surface"), + e, + ) })? }; @@ -734,8 +746,10 @@ impl crate::Instance for Instance { let egl = match egl_result { Ok(egl) => Arc::new(egl), Err(e) => { - log::info!("Unable to open libEGL: {:?}", e); - return Err(crate::InstanceError); + return Err(crate::InstanceError::with_source( + String::from("unable to open libEGL"), + e, + )); } }; @@ -899,8 +913,9 @@ impl crate::Instance for Instance { }; if ret != 0 { - log::error!("Error returned from ANativeWindow_setBuffersGeometry"); - return Err(crate::InstanceError); + return Err(crate::InstanceError::new(format!( + "error {ret} returned from ANativeWindow_setBuffersGeometry", + ))); } } #[cfg(not(target_os = "emscripten"))] @@ -938,8 +953,7 @@ impl crate::Instance for Instance { Arc::clone(&inner.egl.instance), display, inner.force_gles_minor_version, - ) - .map_err(|_| crate::InstanceError)?; + )?; let old_inner = std::mem::replace(inner.deref_mut(), new_inner); inner.wl_display = Some(display_handle.display); @@ -950,8 +964,9 @@ impl crate::Instance for Instance { #[cfg(target_os = "emscripten")] (Rwh::Web(_), _) => {} other => { - log::error!("Unsupported window: {:?}", other); - return Err(crate::InstanceError); + return Err(crate::InstanceError::new(format!( + "unsupported window: {other:?}" + ))); } }; diff --git a/wgpu-hal/src/gles/web.rs b/wgpu-hal/src/gles/web.rs index 02cd6a3ecb..13bce85f84 100644 --- a/wgpu-hal/src/gles/web.rs +++ b/wgpu-hal/src/gles/web.rs @@ -66,14 +66,16 @@ impl Instance { // “not supported” could include “insufficient GPU resources” or “the GPU process // previously crashed”. So, we must return it as an `Err` since it could occur // for circumstances outside the application author's control. - return Err(crate::InstanceError); + return Err(crate::InstanceError::new(String::from( + "canvas.getContext() returned null; webgl2 not available or canvas already in use" + ))); } Err(js_error) => { // - // A thrown exception indicates misuse of the canvas state. Ideally we wouldn't - // panic in this case, but for now, `InstanceError` conveys no detail, so it - // is more informative to panic with a specific message. - panic!("canvas.getContext() threw {js_error:?}") + // A thrown exception indicates misuse of the canvas state. + return Err(crate::InstanceError::new(format!( + "canvas.getContext() threw exception {js_error:?}", + ))); } }; @@ -156,7 +158,9 @@ impl crate::Instance for Instance { self.create_surface_from_canvas(canvas) } else { - Err(crate::InstanceError) + Err(crate::InstanceError::new(format!( + "window handle {window_handle:?} is not a web handle" + ))) } } diff --git a/wgpu-hal/src/lib.rs b/wgpu-hal/src/lib.rs index 020c665709..4bff6b8d8f 100644 --- a/wgpu-hal/src/lib.rs +++ b/wgpu-hal/src/lib.rs @@ -90,7 +90,7 @@ use std::{ num::NonZeroU32, ops::{Range, RangeInclusive}, ptr::NonNull, - sync::atomic::AtomicBool, + sync::{atomic::AtomicBool, Arc}, }; use bitflags::bitflags; @@ -152,9 +152,42 @@ pub enum SurfaceError { Other(&'static str), } -#[derive(Clone, Debug, Eq, PartialEq, Error)] -#[error("Not supported")] -pub struct InstanceError; +/// Error occurring while trying to create an instance, or create a surface from an instance; +/// typically relating to the state of the underlying graphics API or hardware. +#[derive(Clone, Debug, Error)] +#[error("{message}")] +pub struct InstanceError { + /// These errors are very platform specific, so do not attempt to encode them as an enum. + /// + /// This message should describe the problem in sufficient detail to be useful for a + /// user-to-developer “why won't this work on my machine” bug report, and otherwise follow + /// . + message: String, + + /// Underlying error value, if any is available. + #[source] + source: Option>, +} + +impl InstanceError { + #[allow(dead_code)] // may be unused on some platforms + pub(crate) fn new(message: String) -> Self { + Self { + message, + source: None, + } + } + #[allow(dead_code)] // may be unused on some platforms + pub(crate) fn with_source( + message: String, + source: impl std::error::Error + Send + Sync + 'static, + ) -> Self { + Self { + message, + source: Some(Arc::new(source)), + } + } +} pub trait Api: Clone + Sized { type Instance: Instance; diff --git a/wgpu-hal/src/metal/mod.rs b/wgpu-hal/src/metal/mod.rs index 3a8ebc5570..76f57002ff 100644 --- a/wgpu-hal/src/metal/mod.rs +++ b/wgpu-hal/src/metal/mod.rs @@ -100,7 +100,9 @@ impl crate::Instance for Instance { raw_window_handle::RawWindowHandle::AppKit(handle) => Ok(unsafe { Surface::from_view(handle.ns_view, Some(&self.managed_metal_layer_delegate)) }), - _ => Err(crate::InstanceError), + _ => Err(crate::InstanceError::new(format!( + "window handle {window_handle:?} is not a Metal-compatible handle" + ))), } } diff --git a/wgpu-hal/src/vulkan/instance.rs b/wgpu-hal/src/vulkan/instance.rs index 4fa4a3e27d..18b141a070 100644 --- a/wgpu-hal/src/vulkan/instance.rs +++ b/wgpu-hal/src/vulkan/instance.rs @@ -194,8 +194,10 @@ impl super::Instance { let instance_extensions = entry .enumerate_instance_extension_properties(None) .map_err(|e| { - log::info!("enumerate_instance_extension_properties: {:?}", e); - crate::InstanceError + crate::InstanceError::with_source( + String::from("enumerate_instance_extension_properties() failed"), + e, + ) })?; // Check our extensions against the available extensions @@ -366,8 +368,9 @@ impl super::Instance { window: vk::Window, ) -> Result { if !self.shared.extensions.contains(&khr::XlibSurface::name()) { - log::warn!("Vulkan driver does not support VK_KHR_xlib_surface"); - return Err(crate::InstanceError); + return Err(crate::InstanceError::new(String::from( + "Vulkan driver does not support VK_KHR_xlib_surface", + ))); } let surface = { @@ -391,8 +394,9 @@ impl super::Instance { window: vk::xcb_window_t, ) -> Result { if !self.shared.extensions.contains(&khr::XcbSurface::name()) { - log::warn!("Vulkan driver does not support VK_KHR_xcb_surface"); - return Err(crate::InstanceError); + return Err(crate::InstanceError::new(String::from( + "Vulkan driver does not support VK_KHR_xcb_surface", + ))); } let surface = { @@ -420,8 +424,9 @@ impl super::Instance { .extensions .contains(&khr::WaylandSurface::name()) { - log::debug!("Vulkan driver does not support VK_KHR_wayland_surface"); - return Err(crate::InstanceError); + return Err(crate::InstanceError::new(String::from( + "Vulkan driver does not support VK_KHR_wayland_surface", + ))); } let surface = { @@ -447,8 +452,9 @@ impl super::Instance { .extensions .contains(&khr::AndroidSurface::name()) { - log::warn!("Vulkan driver does not support VK_KHR_android_surface"); - return Err(crate::InstanceError); + return Err(crate::InstanceError::new(String::from( + "Vulkan driver does not support VK_KHR_android_surface", + ))); } let surface = { @@ -470,8 +476,9 @@ impl super::Instance { hwnd: *mut c_void, ) -> Result { if !self.shared.extensions.contains(&khr::Win32Surface::name()) { - log::debug!("Vulkan driver does not support VK_KHR_win32_surface"); - return Err(crate::InstanceError); + return Err(crate::InstanceError::new(String::from( + "Vulkan driver does not support VK_KHR_win32_surface", + ))); } let surface = { @@ -496,8 +503,9 @@ impl super::Instance { view: *mut c_void, ) -> Result { if !self.shared.extensions.contains(&ext::MetalSurface::name()) { - log::warn!("Vulkan driver does not support VK_EXT_metal_surface"); - return Err(crate::InstanceError); + return Err(crate::InstanceError::new(String::from( + "Vulkan driver does not support VK_EXT_metal_surface", + ))); } let layer = unsafe { @@ -546,20 +554,18 @@ impl crate::Instance for super::Instance { unsafe fn init(desc: &crate::InstanceDescriptor) -> Result { use crate::auxil::cstr_from_bytes_until_nul; - let entry = match unsafe { ash::Entry::load() } { - Ok(entry) => entry, - Err(err) => { - log::info!("Missing Vulkan entry points: {:?}", err); - return Err(crate::InstanceError); - } - }; + let entry = unsafe { ash::Entry::load() }.map_err(|err| { + crate::InstanceError::with_source(String::from("missing Vulkan entry points"), err) + })?; let driver_api_version = match entry.try_enumerate_instance_version() { // Vulkan 1.1+ Ok(Some(version)) => version, Ok(None) => vk::API_VERSION_1_0, Err(err) => { - log::warn!("try_enumerate_instance_version: {:?}", err); - return Err(crate::InstanceError); + return Err(crate::InstanceError::with_source( + String::from("try_enumerate_instance_version() failed"), + err, + )); } }; @@ -590,7 +596,10 @@ impl crate::Instance for super::Instance { let instance_layers = entry.enumerate_instance_layer_properties().map_err(|e| { log::info!("enumerate_instance_layer_properties: {:?}", e); - crate::InstanceError + crate::InstanceError::with_source( + String::from("enumerate_instance_layer_properties() failed"), + e, + ) })?; fn find_layer<'layers>( @@ -682,8 +691,10 @@ impl crate::Instance for super::Instance { .enabled_extension_names(&str_pointers[layers.len()..]); unsafe { entry.create_instance(&create_info, None) }.map_err(|e| { - log::warn!("create_instance: {:?}", e); - crate::InstanceError + crate::InstanceError::with_source( + String::from("Entry::create_instance() failed"), + e, + ) })? }; @@ -739,7 +750,9 @@ impl crate::Instance for super::Instance { { self.create_surface_from_view(handle.ui_view) } - (_, _) => Err(crate::InstanceError), + (_, _) => Err(crate::InstanceError::new(format!( + "window handle {window_handle:?} is not a Vulkan-compatible handle" + ))), } } diff --git a/wgpu/src/backend/direct.rs b/wgpu/src/backend/direct.rs index fca1d80c3c..8eec9adad5 100644 --- a/wgpu/src/backend/direct.rs +++ b/wgpu/src/backend/direct.rs @@ -244,10 +244,7 @@ impl Context { &self, canvas: web_sys::HtmlCanvasElement, ) -> Result { - let id = self - .0 - .create_surface_webgl_canvas(canvas, ()) - .map_err(|hal::InstanceError| crate::CreateSurfaceError {})?; + let id = self.0.create_surface_webgl_canvas(canvas, ())?; Ok(Surface { id, configured_device: Mutex::default(), @@ -259,10 +256,7 @@ impl Context { &self, canvas: web_sys::OffscreenCanvas, ) -> Result { - let id = self - .0 - .create_surface_webgl_offscreen_canvas(canvas, ()) - .map_err(|hal::InstanceError| crate::CreateSurfaceError {})?; + let id = self.0.create_surface_webgl_offscreen_canvas(canvas, ())?; Ok(Surface { id, configured_device: Mutex::default(), diff --git a/wgpu/src/backend/web.rs b/wgpu/src/backend/web.rs index 1fc1c6683f..d64bd8bcb1 100644 --- a/wgpu/src/backend/web.rs +++ b/wgpu/src/backend/web.rs @@ -920,13 +920,22 @@ impl Context { // “not supported” could include “insufficient GPU resources” or “the GPU process // previously crashed”. So, we must return it as an `Err` since it could occur // for circumstances outside the application author's control. - return Err(crate::CreateSurfaceError {}); + return Err(crate::CreateSurfaceError { + inner: crate::CreateSurfaceErrorKind::Web( + String::from( + "canvas.getContext() returned null; webgpu not available or canvas already in use" + ) + ) + }); } Err(js_error) => { // - // A thrown exception indicates misuse of the canvas state. Ideally we wouldn't - // panic in this case ... TODO - panic!("canvas.getContext() threw {js_error:?}") + // A thrown exception indicates misuse of the canvas state. + return Err(crate::CreateSurfaceError { + inner: crate::CreateSurfaceErrorKind::Web(format!( + "canvas.getContext() threw exception {js_error:?}", + )), + }); } }; diff --git a/wgpu/src/lib.rs b/wgpu/src/lib.rs index 1c3e1a58b5..94345f1adb 100644 --- a/wgpu/src/lib.rs +++ b/wgpu/src/lib.rs @@ -15,8 +15,7 @@ mod macros; use std::{ any::Any, borrow::Cow, - error, - fmt::{Debug, Display}, + error, fmt, future::Future, marker::PhantomData, num::NonZeroU32, @@ -1700,8 +1699,8 @@ pub enum SurfaceError { } static_assertions::assert_impl_all!(SurfaceError: Send, Sync); -impl Display for SurfaceError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +impl fmt::Display for SurfaceError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{}", match self { Self::Timeout => "A timeout was encountered while trying to acquire the next frame", Self::Outdated => "The underlying surface has changed, and therefore the swap chain must be updated", @@ -2744,8 +2743,8 @@ impl Drop for Device { pub struct RequestDeviceError; static_assertions::assert_impl_all!(RequestDeviceError: Send, Sync); -impl Display for RequestDeviceError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +impl fmt::Display for RequestDeviceError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "Requesting a device failed") } } @@ -2753,28 +2752,76 @@ impl Display for RequestDeviceError { impl error::Error for RequestDeviceError {} /// [`Instance::create_surface()`] or a related function failed. -#[derive(Clone, PartialEq, Eq, Debug)] +#[derive(Clone, Debug)] #[non_exhaustive] pub struct CreateSurfaceError { - // TODO: Report diagnostic clues + inner: CreateSurfaceErrorKind, +} +#[derive(Clone, Debug)] +enum CreateSurfaceErrorKind { + /// Error from [`wgpu_hal`]. + #[cfg(any( + not(target_arch = "wasm32"), + target_os = "emscripten", + feature = "webgl" + ))] + // must match dependency cfg + Hal(hal::InstanceError), + + /// Error from WebGPU surface creation. + #[allow(dead_code)] // may be unused depending on target and features + Web(String), } static_assertions::assert_impl_all!(CreateSurfaceError: Send, Sync); -impl Display for CreateSurfaceError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "Creating a surface failed") +impl fmt::Display for CreateSurfaceError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match &self.inner { + #[cfg(any( + not(target_arch = "wasm32"), + target_os = "emscripten", + feature = "webgl" + ))] + CreateSurfaceErrorKind::Hal(e) => e.fmt(f), + CreateSurfaceErrorKind::Web(e) => e.fmt(f), + } } } -impl error::Error for CreateSurfaceError {} +impl error::Error for CreateSurfaceError { + fn source(&self) -> Option<&(dyn error::Error + 'static)> { + match &self.inner { + #[cfg(any( + not(target_arch = "wasm32"), + target_os = "emscripten", + feature = "webgl" + ))] + CreateSurfaceErrorKind::Hal(e) => e.source(), + CreateSurfaceErrorKind::Web(_) => None, + } + } +} + +#[cfg(any( + not(target_arch = "wasm32"), + target_os = "emscripten", + feature = "webgl" +))] +impl From for CreateSurfaceError { + fn from(e: hal::InstanceError) -> Self { + Self { + inner: CreateSurfaceErrorKind::Hal(e), + } + } +} /// Error occurred when trying to async map a buffer. #[derive(Clone, PartialEq, Eq, Debug)] pub struct BufferAsyncError; static_assertions::assert_impl_all!(BufferAsyncError: Send, Sync); -impl Display for BufferAsyncError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +impl fmt::Display for BufferAsyncError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "Error occurred when trying to async map a buffer") } } @@ -4849,8 +4896,8 @@ impl Clone for Id { impl Copy for Id {} #[cfg(feature = "expose-ids")] -impl Debug for Id { - fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { +impl fmt::Debug for Id { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { f.debug_tuple("Id").field(&self.0).finish() } } @@ -5150,8 +5197,8 @@ impl error::Error for Error { } } -impl Display for Error { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +impl fmt::Display for Error { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { Error::OutOfMemory { .. } => f.write_str("Out of Memory"), Error::Validation { description, .. } => f.write_str(description),