Skip to content

Commit

Permalink
Properly handle the case where Navigator.gpu is undefined and WebGPU …
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
BGR360 committed Sep 15, 2024
1 parent d79ebc4 commit 2fac5e9
Show file tree
Hide file tree
Showing 4 changed files with 126 additions and 24 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
7 changes: 4 additions & 3 deletions wgpu/src/api/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down
96 changes: 75 additions & 21 deletions wgpu/src/backend/webgpu.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#![allow(clippy::type_complexity)]

mod defined_non_null_js_value;
mod ext_bindings;
mod webgpu_sys;

Expand All @@ -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.
//
Expand All @@ -38,7 +41,10 @@ unsafe impl<T> Send for Sendable<T> {}
#[cfg(send_sync)]
unsafe impl<T> Sync for Sendable<T> {}

pub(crate) struct ContextWebGpu(webgpu_sys::Gpu);
pub(crate) struct ContextWebGpu {
/// `None` if browser does not advertise support for WebGPU.
gpu: Option<DefinedNonNullJsValue<webgpu_sys::Gpu>>,
}
#[cfg(send_sync)]
unsafe impl Send for ContextWebGpu {}
#[cfg(send_sync)]
Expand Down Expand Up @@ -189,6 +195,36 @@ impl<F, M> MakeSendFuture<F, M> {
#[cfg(send_sync)]
unsafe impl<F, M> Send for MakeSendFuture<F, M> {}

/// Wraps a future that returns `Option<T>` and adds the ability to immediately
/// return None.
pub(crate) struct OptionFuture<F>(Option<F>);

impl<F: Future<Output = Option<T>>, T> Future for OptionFuture<F> {
type Output = Option<T>;

fn poll(self: Pin<&mut Self>, cx: &mut task::Context<'_>) -> Poll<Self::Output> {
// 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<F> OptionFuture<F> {
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;
Expand Down Expand Up @@ -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:
/// * <https://developer.mozilla.org/en-US/docs/Web/API/Navigator/gpu>
/// * <https://developer.mozilla.org/en-US/docs/Web/API/WorkerNavigator/gpu>
pub fn get_browser_gpu_property() -> Option<webgpu_sys::Gpu> {
pub fn get_browser_gpu_property(
) -> Result<Option<DefinedNonNullJsValue<webgpu_sys::Gpu>>, 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::<web_sys::Window>().navigator();
Some(ext_bindings::NavigatorGpu::gpu(&navigator))
ext_bindings::NavigatorGpu::gpu(&navigator)
} else if !global.worker().is_undefined() {
let navigator = global
.unchecked_into::<web_sys::WorkerGlobalScope>()
.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 {
Expand Down Expand Up @@ -1096,9 +1139,11 @@ impl crate::context::Context for ContextWebGpu {
type SubmissionIndexData = ();
type PipelineCacheData = ();

type RequestAdapterFuture = MakeSendFuture<
wasm_bindgen_futures::JsFuture,
fn(JsFutureResult) -> Option<Self::AdapterData>,
type RequestAdapterFuture = OptionFuture<
MakeSendFuture<
wasm_bindgen_futures::JsFuture,
fn(JsFutureResult) -> Option<Self::AdapterData>,
>,
>;
type RequestDeviceFuture = MakeSendFuture<
wasm_bindgen_futures::JsFuture,
Expand All @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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);
}
Expand Down
46 changes: 46 additions & 0 deletions wgpu/src/backend/webgpu/defined_non_null_js_value.rs
Original file line number Diff line number Diff line change
@@ -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>(T);

impl<T> DefinedNonNullJsValue<T>
where
T: AsRef<JsValue>,
{
pub fn new(value: T) -> Option<Self> {
if value.as_ref().is_undefined() || value.as_ref().is_null() {
None
} else {
Some(Self(value))
}
}
}

impl<T> Deref for DefinedNonNullJsValue<T> {
type Target = T;

fn deref(&self) -> &Self::Target {
&self.0
}
}

impl<T> DerefMut for DefinedNonNullJsValue<T> {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.0
}
}

impl<T> AsRef<T> for DefinedNonNullJsValue<T> {
fn as_ref(&self) -> &T {
&self.0
}
}

impl<T> AsMut<T> for DefinedNonNullJsValue<T> {
fn as_mut(&mut self) -> &mut T {
&mut self.0
}
}

0 comments on commit 2fac5e9

Please sign in to comment.