Skip to content

Commit

Permalink
Invoke a DeviceLostClosure immediately if set on an invalid device.
Browse files Browse the repository at this point in the history
To make the device invalid, this defines an explicit, test-only method
make_invalid. It also modifies calls that expect to always retrieve a
valid device.
  • Loading branch information
bradwerth committed Mar 7, 2024
1 parent 5162fd4 commit 1bf4f79
Show file tree
Hide file tree
Showing 8 changed files with 78 additions and 6 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ Bottom level categories:

### Major Changes

#### Vendored WebGPU Bindings from `web_sys`
#### Vendored WebGPU Bindings from `web_sys`

**`--cfg=web_sys_unstable_apis` is no longer needed in your `RUSTFLAGS` to compile for WebGPU!!!**

Expand Down Expand Up @@ -138,6 +138,7 @@ By @cwfitzgerald in [#5325](https://github.com/gfx-rs/wgpu/pull/5325).
- Fix behavior of integer `clamp` when `min` argument > `max` argument. By @cwfitzgerald in [#5300](https://github.com/gfx-rs/wgpu/pull/5300).
- Fix missing validation for `Device::clear_buffer` where `offset + size buffer.size` was not checked when `size` was omitted. By @ErichDonGubler in [#5282](https://github.com/gfx-rs/wgpu/pull/5282).
- Fix linking when targeting android. By @ashdnazg in [#5326](https://github.com/gfx-rs/wgpu/pull/5326).
- Failing to set the device lost closure will call the closure before returning. By @bradwerth in [#5358](https://github.com/gfx-rs/wgpu/pull/5358).

#### glsl-in

Expand Down
31 changes: 31 additions & 0 deletions tests/tests/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,37 @@ static DEVICE_DROP_THEN_LOST: GpuTestConfiguration = GpuTestConfiguration::new()
);
});

#[gpu_test]
static DEVICE_INVALID_THEN_SET_LOST_CALLBACK: GpuTestConfiguration = GpuTestConfiguration::new()
.parameters(TestParameters::default().expect_fail(FailureCase::webgl2()))
.run_sync(|ctx| {
// This test checks that when the device is invalid, a subsequent call
// to set the device lost callback will immediately call the callback.
// Invalidating the device is done via a testing-only method. Fails on
// webgl because webgl doesn't implement make_invalid.

// Make the device invalid.
ctx.device.make_invalid();

let was_called = std::sync::Arc::<std::sync::atomic::AtomicBool>::new(false.into());

// Set a LoseDeviceCallback on the device.
let was_called_clone = was_called.clone();
let callback = Box::new(move |reason, _m| {
was_called_clone.store(true, std::sync::atomic::Ordering::SeqCst);
assert!(
matches!(reason, wgt::DeviceLostReason::DeviceInvalid),
"Device lost info reason should match DeviceLostReason::DeviceInvalid."
);
});
ctx.device.set_device_lost_callback(callback);

assert!(
was_called.load(std::sync::atomic::Ordering::SeqCst),
"Device lost callback should have been called."
);
});

#[gpu_test]
static DEVICE_LOST_REPLACED_CALLBACK: GpuTestConfiguration = GpuTestConfiguration::new()
.parameters(TestParameters::default())
Expand Down
17 changes: 16 additions & 1 deletion wgpu-core/src/device/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2224,6 +2224,15 @@ impl Global {
}
}

// This is a test-only function to force the device into an
// invalid state by inserting an error value in its place in
// the registry.
pub fn device_make_invalid<A: HalApi>(&self, device_id: DeviceId) {
let hub = A::hub(self);
hub.devices
.force_replace_with_error(device_id, "Made invalid.");
}

pub fn device_drop<A: HalApi>(&self, device_id: DeviceId) {
profiling::scope!("Device::drop");
api_log!("Device::drop {device_id:?}");
Expand Down Expand Up @@ -2259,7 +2268,7 @@ impl Global {
) {
let hub = A::hub(self);

if let Ok(device) = hub.devices.get(device_id) {
if let Ok(Some(device)) = hub.devices.try_get(device_id) {
let mut life_tracker = device.lock_life();
if let Some(existing_closure) = life_tracker.device_lost_closure.take() {
// It's important to not hold the lock while calling the closure.
Expand All @@ -2268,6 +2277,12 @@ impl Global {
life_tracker = device.lock_life();
}
life_tracker.device_lost_closure = Some(device_lost_closure);
} else {
// No device? Okay. Just like we have to call any existing closure
// before we drop it, we need to call this closure before we exit
// this function, because there's no device that is ever going to
// call it.
device_lost_closure.call(DeviceLostReason::DeviceInvalid, "".to_string());
}
}

Expand Down
6 changes: 6 additions & 0 deletions wgpu-types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7190,4 +7190,10 @@ pub enum DeviceLostReason {
/// exactly once before it is dropped, which helps with managing the
/// memory owned by the callback.
ReplacedCallback = 3,
/// When setting the callback, but the device is already invalid
///
/// As above, when the callback is provided, wgpu guarantees that it
/// will eventually be called. If the device is already invalid, wgpu
/// will call the callback immediately, with this reason.
DeviceInvalid = 4,
}
4 changes: 4 additions & 0 deletions wgpu/src/backend/webgpu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1948,6 +1948,10 @@ impl crate::context::Context for ContextWebGpu {
create_identified(device_data.0.create_render_bundle_encoder(&mapped_desc))
}

fn device_make_invalid(&self, _device: &Self::DeviceId, _device_data: &Self::DeviceData) {
// Unimplemented
}

fn device_drop(&self, _device: &Self::DeviceId, _device_data: &Self::DeviceData) {
// Device is dropped automatically
}
Expand Down
10 changes: 6 additions & 4 deletions wgpu/src/backend/wgpu_core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1346,14 +1346,16 @@ impl crate::Context for ContextWgpuCore {
Err(e) => panic!("Error in Device::create_render_bundle_encoder: {e}"),
}
}
fn device_make_invalid(&self, device: &Self::DeviceId, _device_data: &Self::DeviceData) {
wgc::gfx_select!(device => self.0.device_make_invalid(*device));
}
#[cfg_attr(not(any(native, Emscripten)), allow(unused))]
fn device_drop(&self, device: &Self::DeviceId, _device_data: &Self::DeviceData) {
#[cfg(any(native, Emscripten))]
{
match wgc::gfx_select!(device => self.0.device_poll(*device, wgt::Maintain::wait())) {
Ok(_) => {}
Err(err) => self.handle_error_fatal(err, "Device::drop"),
}
// Call device_poll, but don't check for errors. We have to use its
// return value, but we just drop it.
let _ = wgc::gfx_select!(device => self.0.device_poll(*device, wgt::Maintain::wait()));
wgc::gfx_select!(device => self.0.device_drop(*device));
}
}
Expand Down
8 changes: 8 additions & 0 deletions wgpu/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@ pub trait Context: Debug + WasmNotSendSync + Sized {
device_data: &Self::DeviceData,
desc: &RenderBundleEncoderDescriptor<'_>,
) -> (Self::RenderBundleEncoderId, Self::RenderBundleEncoderData);
fn device_make_invalid(&self, device: &Self::DeviceId, device_data: &Self::DeviceData);
fn device_drop(&self, device: &Self::DeviceId, device_data: &Self::DeviceData);
fn device_set_device_lost_callback(
&self,
Expand Down Expand Up @@ -1293,6 +1294,7 @@ pub(crate) trait DynContext: Debug + WasmNotSendSync {
device_data: &crate::Data,
desc: &RenderBundleEncoderDescriptor<'_>,
) -> (ObjectId, Box<crate::Data>);
fn device_make_invalid(&self, device: &ObjectId, device_data: &crate::Data);
fn device_drop(&self, device: &ObjectId, device_data: &crate::Data);
fn device_set_device_lost_callback(
&self,
Expand Down Expand Up @@ -2350,6 +2352,12 @@ where
(render_bundle_encoder.into(), Box::new(data) as _)
}

fn device_make_invalid(&self, device: &ObjectId, device_data: &crate::Data) {
let device = <T::DeviceId>::from(*device);
let device_data = downcast_ref(device_data);
Context::device_make_invalid(self, &device, device_data)
}

fn device_drop(&self, device: &ObjectId, device_data: &crate::Data) {
let device = <T::DeviceId>::from(*device);
let device_data = downcast_ref(device_data);
Expand Down
5 changes: 5 additions & 0 deletions wgpu/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2703,6 +2703,11 @@ impl Device {
Box::new(callback),
)
}

/// Test-only function to make this device invalid.
pub fn make_invalid(&self) {
DynContext::device_make_invalid(&*self.context, &self.id, self.data.as_ref())
}
}

impl Drop for Device {
Expand Down

0 comments on commit 1bf4f79

Please sign in to comment.