From 2fac5e983e0f0d33f127bde04c2c3b0621faa685 Mon Sep 17 00:00:00 2001 From: Ben Reeves Date: Sun, 15 Sep 2024 03:07:40 -0500 Subject: [PATCH] Properly handle the case where Navigator.gpu is undefined and WebGPU is the only compiled backend (#6197) * Properly handle the case where `Navigator.gpu` is undefined and WebGPU is the only compiled backend. Previously, `Instance::request_adapter` would invoke a wasm binding with an undefined arg0, thus crashing the program. Now it will cleanly return `None` instead. Fixes #6196. * Fix typo in `Instance::new` doc comment. * Add note to CHANGELOG.md * Introduce `DefinedNonNullJsValue` type. * Assert definedness of self.gpu in surface_get_capabilities. * Use DefinedNonNullJsValue in signature of get_browser_gpu_property(). * Clarify meaning of gpu field with a comment. --- CHANGELOG.md | 1 + wgpu/src/api/instance.rs | 7 +- wgpu/src/backend/webgpu.rs | 96 +++++++++++++++---- .../webgpu/defined_non_null_js_value.rs | 46 +++++++++ 4 files changed, 126 insertions(+), 24 deletions(-) create mode 100644 wgpu/src/backend/webgpu/defined_non_null_js_value.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 35355b178..a647a0662 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -88,6 +88,7 @@ By @bradwerth [#6216](https://github.com/gfx-rs/wgpu/pull/6216). ### Bug Fixes - Fix incorrect hlsl image output type conversion. By @atlv24 in [#6123](https://github.com/gfx-rs/wgpu/pull/6123) +- Fix JS `TypeError` exception in `Instance::request_adapter` when browser doesn't support WebGPU but `wgpu` not compiled with `webgl` support. By @bgr360 in [#6197](https://github.com/gfx-rs/wgpu/pull/6197). #### Naga diff --git a/wgpu/src/api/instance.rs b/wgpu/src/api/instance.rs index 33a7b0f64..af6775b86 100644 --- a/wgpu/src/api/instance.rs +++ b/wgpu/src/api/instance.rs @@ -95,7 +95,7 @@ impl Instance { /// [`Backends::BROWSER_WEBGPU`] takes a special role: /// If it is set and WebGPU support is detected, this instance will *only* be able to create /// WebGPU adapters. If you instead want to force use of WebGL, either - /// disable the `webgpu` compile-time feature or do add the [`Backends::BROWSER_WEBGPU`] + /// disable the `webgpu` compile-time feature or don't add the [`Backends::BROWSER_WEBGPU`] /// flag to the the `instance_desc`'s `backends` field. /// If it is set and WebGPU support is *not* detected, the instance will use wgpu-core /// to create adapters. Meaning that if the `webgl` feature is enabled, it is able to create @@ -118,8 +118,9 @@ impl Instance { { let is_only_available_backend = !cfg!(wgpu_core); let requested_webgpu = _instance_desc.backends.contains(Backends::BROWSER_WEBGPU); - let support_webgpu = - crate::backend::get_browser_gpu_property().map_or(false, |gpu| !gpu.is_undefined()); + let support_webgpu = crate::backend::get_browser_gpu_property() + .map(|maybe_gpu| maybe_gpu.is_some()) + .unwrap_or(false); if is_only_available_backend || (requested_webgpu && support_webgpu) { return Self { diff --git a/wgpu/src/backend/webgpu.rs b/wgpu/src/backend/webgpu.rs index 7646085f7..e982300e7 100644 --- a/wgpu/src/backend/webgpu.rs +++ b/wgpu/src/backend/webgpu.rs @@ -1,5 +1,6 @@ #![allow(clippy::type_complexity)] +mod defined_non_null_js_value; mod ext_bindings; mod webgpu_sys; @@ -22,6 +23,8 @@ use crate::{ CompilationInfo, SurfaceTargetUnsafe, UncapturedErrorHandler, }; +use defined_non_null_js_value::DefinedNonNullJsValue; + // We need to make a wrapper for some of the handle types returned by the web backend to make them // implement `Send` and `Sync` to match native. // @@ -38,7 +41,10 @@ unsafe impl Send for Sendable {} #[cfg(send_sync)] unsafe impl Sync for Sendable {} -pub(crate) struct ContextWebGpu(webgpu_sys::Gpu); +pub(crate) struct ContextWebGpu { + /// `None` if browser does not advertise support for WebGPU. + gpu: Option>, +} #[cfg(send_sync)] unsafe impl Send for ContextWebGpu {} #[cfg(send_sync)] @@ -189,6 +195,36 @@ impl MakeSendFuture { #[cfg(send_sync)] unsafe impl Send for MakeSendFuture {} +/// Wraps a future that returns `Option` and adds the ability to immediately +/// return None. +pub(crate) struct OptionFuture(Option); + +impl>, T> Future for OptionFuture { + type Output = Option; + + fn poll(self: Pin<&mut Self>, cx: &mut task::Context<'_>) -> Poll { + // This is safe because we have no Drop implementation to violate the Pin requirements and + // do not provide any means of moving the inner future. + unsafe { + let this = self.get_unchecked_mut(); + match &mut this.0 { + Some(future) => Pin::new_unchecked(future).poll(cx), + None => task::Poll::Ready(None), + } + } + } +} + +impl OptionFuture { + fn some(future: F) -> Self { + Self(Some(future)) + } + + fn none() -> Self { + Self(None) + } +} + fn map_texture_format(texture_format: wgt::TextureFormat) -> webgpu_sys::GpuTextureFormat { use webgpu_sys::GpuTextureFormat as tf; use wgt::TextureFormat; @@ -1046,27 +1082,34 @@ pub enum Canvas { Offscreen(web_sys::OffscreenCanvas), } -/// Returns the browsers gpu object or `None` if the current context is neither the main thread nor a dedicated worker. +#[derive(Debug, Clone, Copy)] +pub struct BrowserGpuPropertyInaccessible; + +/// Returns the browser's gpu object or `Err(BrowserGpuPropertyInaccessible)` if +/// the current context is neither the main thread nor a dedicated worker. /// -/// If WebGPU is not supported, the Gpu property is `undefined` (but *not* necessarily `None`). +/// If WebGPU is not supported, the Gpu property is `undefined`, and so this +/// function will return `Ok(None)`. /// /// See: /// * /// * -pub fn get_browser_gpu_property() -> Option { +pub fn get_browser_gpu_property( +) -> Result>, BrowserGpuPropertyInaccessible> { let global: Global = js_sys::global().unchecked_into(); - if !global.window().is_undefined() { + let maybe_undefined_gpu: webgpu_sys::Gpu = if !global.window().is_undefined() { let navigator = global.unchecked_into::().navigator(); - Some(ext_bindings::NavigatorGpu::gpu(&navigator)) + ext_bindings::NavigatorGpu::gpu(&navigator) } else if !global.worker().is_undefined() { let navigator = global .unchecked_into::() .navigator(); - Some(ext_bindings::NavigatorGpu::gpu(&navigator)) + ext_bindings::NavigatorGpu::gpu(&navigator) } else { - None - } + return Err(BrowserGpuPropertyInaccessible); + }; + Ok(DefinedNonNullJsValue::new(maybe_undefined_gpu)) } impl crate::context::Context for ContextWebGpu { @@ -1096,9 +1139,11 @@ impl crate::context::Context for ContextWebGpu { type SubmissionIndexData = (); type PipelineCacheData = (); - type RequestAdapterFuture = MakeSendFuture< - wasm_bindgen_futures::JsFuture, - fn(JsFutureResult) -> Option, + type RequestAdapterFuture = OptionFuture< + MakeSendFuture< + wasm_bindgen_futures::JsFuture, + fn(JsFutureResult) -> Option, + >, >; type RequestDeviceFuture = MakeSendFuture< wasm_bindgen_futures::JsFuture, @@ -1115,12 +1160,13 @@ impl crate::context::Context for ContextWebGpu { >; fn init(_instance_desc: wgt::InstanceDescriptor) -> Self { - let Some(gpu) = get_browser_gpu_property() else { + let Ok(gpu) = get_browser_gpu_property() else { panic!( "Accessing the GPU is only supported on the main thread or from a dedicated worker" ); }; - ContextWebGpu(gpu) + + ContextWebGpu { gpu } } unsafe fn instance_create_surface( @@ -1190,12 +1236,16 @@ impl crate::context::Context for ContextWebGpu { if let Some(mapped_pref) = mapped_power_preference { mapped_options.power_preference(mapped_pref); } - let adapter_promise = self.0.request_adapter_with_options(&mapped_options); - - MakeSendFuture::new( - wasm_bindgen_futures::JsFuture::from(adapter_promise), - future_request_adapter, - ) + if let Some(gpu) = &self.gpu { + let adapter_promise = gpu.request_adapter_with_options(&mapped_options); + OptionFuture::some(MakeSendFuture::new( + wasm_bindgen_futures::JsFuture::from(adapter_promise), + future_request_adapter, + )) + } else { + // Gpu is undefined; WebGPU is not supported in this browser. + OptionFuture::none() + } } fn adapter_request_device( @@ -1316,7 +1366,11 @@ impl crate::context::Context for ContextWebGpu { let mut mapped_formats = formats.iter().map(|format| map_texture_format(*format)); // Preferred canvas format will only be either "rgba8unorm" or "bgra8unorm". // https://www.w3.org/TR/webgpu/#dom-gpu-getpreferredcanvasformat - let preferred_format = self.0.get_preferred_canvas_format(); + let gpu = self + .gpu + .as_ref() + .expect("Caller could not have created an adapter if gpu is undefined."); + let preferred_format = gpu.get_preferred_canvas_format(); if let Some(index) = mapped_formats.position(|format| format == preferred_format) { formats.swap(0, index); } diff --git a/wgpu/src/backend/webgpu/defined_non_null_js_value.rs b/wgpu/src/backend/webgpu/defined_non_null_js_value.rs new file mode 100644 index 000000000..fc5a8737e --- /dev/null +++ b/wgpu/src/backend/webgpu/defined_non_null_js_value.rs @@ -0,0 +1,46 @@ +use std::ops::{Deref, DerefMut}; + +use wasm_bindgen::JsValue; + +/// Derefs to a [`JsValue`] that's known not to be `undefined` or `null`. +#[derive(Debug)] +pub struct DefinedNonNullJsValue(T); + +impl DefinedNonNullJsValue +where + T: AsRef, +{ + pub fn new(value: T) -> Option { + if value.as_ref().is_undefined() || value.as_ref().is_null() { + None + } else { + Some(Self(value)) + } + } +} + +impl Deref for DefinedNonNullJsValue { + type Target = T; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl DerefMut for DefinedNonNullJsValue { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } +} + +impl AsRef for DefinedNonNullJsValue { + fn as_ref(&self) -> &T { + &self.0 + } +} + +impl AsMut for DefinedNonNullJsValue { + fn as_mut(&mut self) -> &mut T { + &mut self.0 + } +}