From 77cb042e2a7dc6478c96b9db0284c9b1f335bfad Mon Sep 17 00:00:00 2001 From: Kevin Reid Date: Mon, 28 Aug 2023 21:25:18 -0700 Subject: [PATCH] Add details to `RequestDeviceError`. --- CHANGELOG.md | 2 +- tests/src/lib.rs | 2 +- tests/tests/device.rs | 51 ++++++++++++++++++++ wgpu/src/backend/direct.rs | 3 +- wgpu/src/backend/web.rs | 4 +- wgpu/src/lib.rs | 95 ++++++++++++++++++++++++++++++++++++-- 6 files changed, 147 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index db91b89718..ad4c81d076 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -75,7 +75,7 @@ By @Valaphee in [#3402](https://github.com/gfx-rs/wgpu/pull/3402) - 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) +- `wgpu::CreateSurfaceError` and `wgpu::RequestDeviceError` now give details of the failure, but no longer implement `PartialEq` and cannot be constructed. By @kpreid in [#4066](https://github.com/gfx-rs/wgpu/pull/4066) and [#4145](https://github.com/gfx-rs/wgpu/pull/4145) - 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 236b353386..c506126708 100644 --- a/tests/src/lib.rs +++ b/tests/src/lib.rs @@ -449,7 +449,7 @@ pub fn initialize_test(parameters: TestParameters, test_function: impl FnOnce(Te } } -fn initialize_adapter() -> (Adapter, Option) { +pub fn initialize_adapter() -> (Adapter, Option) { let instance = initialize_instance(); let surface_guard: Option; let compatible_surface; diff --git a/tests/tests/device.rs b/tests/tests/device.rs index f43791f86e..7964f2afdb 100644 --- a/tests/tests/device.rs +++ b/tests/tests/device.rs @@ -40,3 +40,54 @@ fn device_mismatch() { }, ); } + +#[cfg(not(all(target_arch = "wasm32", not(target_os = "emscripten"))))] +#[test] +fn request_device_error_on_native() { + pollster::block_on(request_device_error_message()); +} + +/// Check that `RequestDeviceError`s produced have some diagnostic information. +/// +/// Note: this is a wasm *and* native test. On wasm it is run directly; on native, indirectly +#[wasm_bindgen_test::wasm_bindgen_test] +async fn request_device_error_message() { + // Not using initialize_test() because that doesn't let us catch the error + // nor .await anything + let (adapter, _surface_guard) = wgpu_test::initialize_adapter(); + + let device_error = adapter + .request_device( + &wgpu::DeviceDescriptor { + // Force a failure by requesting absurd limits. + features: wgpu::Features::all(), + limits: wgpu::Limits { + max_texture_dimension_1d: u32::MAX, + max_texture_dimension_2d: u32::MAX, + max_texture_dimension_3d: u32::MAX, + max_bind_groups: u32::MAX, + max_push_constant_size: u32::MAX, + ..Default::default() + }, + ..Default::default() + }, + None, + ) + .await + .unwrap_err(); + + let device_error = device_error.to_string(); + cfg_if::cfg_if! { + if #[cfg(all(target_arch = "wasm32", not(feature = "webgl")))] { + // On WebGPU, so the error we get will be from the browser WebGPU API. + // Per the WebGPU specification this should be a `TypeError` when features are not + // available, , + // and the stringification it goes through for Rust should put that in the message. + let expected = "TypeError"; + } else { + // This message appears whenever wgpu-core is used as the implementation. + let expected = "Unsupported features were requested: Features("; + } + } + assert!(device_error.contains(expected), "{device_error}"); +} diff --git a/wgpu/src/backend/direct.rs b/wgpu/src/backend/direct.rs index 8eec9adad5..2e15e295e8 100644 --- a/wgpu/src/backend/direct.rs +++ b/wgpu/src/backend/direct.rs @@ -622,8 +622,7 @@ impl crate::Context for Context { () )); if let Some(err) = error { - log::error!("Error in Adapter::request_device: {}", err); - return ready(Err(crate::RequestDeviceError)); + return ready(Err(err.into())); } let error_sink = Arc::new(Mutex::new(ErrorSinkRaw::new())); let device = Device { diff --git a/wgpu/src/backend/web.rs b/wgpu/src/backend/web.rs index d64bd8bcb1..2f83d50c55 100644 --- a/wgpu/src/backend/web.rs +++ b/wgpu/src/backend/web.rs @@ -812,7 +812,9 @@ fn future_request_device( (device_id, device_data, queue_id, queue_data) }) - .map_err(|_| crate::RequestDeviceError) + .map_err(|error_value| crate::RequestDeviceError { + inner: crate::RequestDeviceErrorKind::Web(error_value), + }) } fn future_pop_error_scope(result: JsFutureResult) -> Option { diff --git a/wgpu/src/lib.rs b/wgpu/src/lib.rs index 94345f1adb..19dc20120d 100644 --- a/wgpu/src/lib.rs +++ b/wgpu/src/lib.rs @@ -2738,18 +2738,103 @@ impl Drop for Device { } } -/// Requesting a device failed. -#[derive(Clone, PartialEq, Eq, Debug)] -pub struct RequestDeviceError; +/// Requesting a device from an [`Adapter`] failed. +#[derive(Clone, Debug)] +pub struct RequestDeviceError { + inner: RequestDeviceErrorKind, +} +#[derive(Clone, Debug)] +enum RequestDeviceErrorKind { + /// Error from [`wgpu_core`]. + // must match dependency cfg + #[cfg(any( + not(target_arch = "wasm32"), + feature = "webgl", + target_os = "emscripten" + ))] + Core(core::instance::RequestDeviceError), + + /// Error from web API that was called by `wgpu` to request a device. + /// + /// (This is currently never used by the webgl backend, but it could be.) + #[cfg(all( + target_arch = "wasm32", + not(any(target_os = "emscripten", feature = "webgl")) + ))] + Web(wasm_bindgen::JsValue), +} + +#[cfg(all( + feature = "fragile-send-sync-non-atomic-wasm", + not(target_feature = "atomics") +))] +unsafe impl Send for RequestDeviceErrorKind {} +#[cfg(all( + feature = "fragile-send-sync-non-atomic-wasm", + not(target_feature = "atomics") +))] +unsafe impl Sync for RequestDeviceErrorKind {} + +#[cfg(any( + not(target_arch = "wasm32"), + all( + feature = "fragile-send-sync-non-atomic-wasm", + not(target_feature = "atomics") + ) +))] static_assertions::assert_impl_all!(RequestDeviceError: Send, Sync); impl fmt::Display for RequestDeviceError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "Requesting a device failed") + match &self.inner { + #[cfg(any( + not(target_arch = "wasm32"), + feature = "webgl", + target_os = "emscripten" + ))] + RequestDeviceErrorKind::Core(error) => error.fmt(f), + #[cfg(all( + target_arch = "wasm32", + not(any(target_os = "emscripten", feature = "webgl")) + ))] + RequestDeviceErrorKind::Web(error_js_value) => { + // wasm-bindgen provides a reasonable error stringification via `Debug` impl + write!(f, "{error_js_value:?}") + } + } + } +} + +impl error::Error for RequestDeviceError { + fn source(&self) -> Option<&(dyn error::Error + 'static)> { + match &self.inner { + #[cfg(any( + not(target_arch = "wasm32"), + feature = "webgl", + target_os = "emscripten" + ))] + RequestDeviceErrorKind::Core(error) => error.source(), + #[cfg(all( + target_arch = "wasm32", + not(any(target_os = "emscripten", feature = "webgl")) + ))] + RequestDeviceErrorKind::Web(_) => None, + } } } -impl error::Error for RequestDeviceError {} +#[cfg(any( + not(target_arch = "wasm32"), + feature = "webgl", + target_os = "emscripten" +))] +impl From for RequestDeviceError { + fn from(error: core::instance::RequestDeviceError) -> Self { + Self { + inner: RequestDeviceErrorKind::Core(error), + } + } +} /// [`Instance::create_surface()`] or a related function failed. #[derive(Clone, Debug)]