From eca04f59db2ecb26db84c5ca7e1d7bf864ffa48a Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Tue, 11 Oct 2022 19:40:30 +0200 Subject: [PATCH] Make some errors not fatal (#3094) --- CHANGELOG.md | 1 + wgpu-core/src/hub.rs | 3 +- wgpu/src/backend/direct.rs | 17 ++++-------- wgpu/tests/common/mod.rs | 18 ++++++++++++ wgpu/tests/resource_error.rs | 53 ++++++++++++++++++++++++++++++++++++ wgpu/tests/root.rs | 1 + 6 files changed, 79 insertions(+), 14 deletions(-) create mode 100644 wgpu/tests/resource_error.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f0675f0e2..622fbb48fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,7 @@ Bottom level categories: - Bother to free the `hal::Api::CommandBuffer` when a `wgpu_core::command::CommandEncoder` is dropped. By @jimblandy in [#3069](https://github.com/gfx-rs/wgpu/pull/3069). - Fixed the mipmap example by adding the missing WRITE_TIMESTAMP_INSIDE_PASSES feature. By @Olaroll in [#3081](https://github.com/gfx-rs/wgpu/pull/3081). +- Avoid panicking in some interactions with invalid resources by @nical in (#3094)[https://github.com/gfx-rs/wgpu/pull/3094] #### WebGPU - Use `log` instead of `println` in hello example by @JolifantoBambla in [#2858](https://github.com/gfx-rs/wgpu/pull/2858) diff --git a/wgpu-core/src/hub.rs b/wgpu-core/src/hub.rs index 8e256e7c32..2c586c930e 100644 --- a/wgpu-core/src/hub.rs +++ b/wgpu-core/src/hub.rs @@ -358,9 +358,8 @@ impl Storage { let (index, epoch, _) = id.unzip(); let (result, storage_epoch) = match self.map.get_mut(index as usize) { Some(&mut Element::Occupied(ref mut v, epoch)) => (Ok(v), epoch), - Some(&mut Element::Vacant) => panic!("{}[{}] does not exist", self.kind, index), + Some(&mut Element::Vacant) | None => panic!("{}[{}] does not exist", self.kind, index), Some(&mut Element::Error(epoch, ..)) => (Err(InvalidId), epoch), - None => return Err(InvalidId), }; assert_eq!( epoch, storage_epoch, diff --git a/wgpu/src/backend/direct.rs b/wgpu/src/backend/direct.rs index 93b313205d..3d33e2c768 100644 --- a/wgpu/src/backend/direct.rs +++ b/wgpu/src/backend/direct.rs @@ -1790,22 +1790,18 @@ impl crate::Context for Context { } fn buffer_destroy(&self, buffer: &Self::BufferId) { + // Per spec, no error to report. Even calling destroy multiple times is valid. let global = &self.0; - match wgc::gfx_select!(buffer.id => global.buffer_destroy(buffer.id)) { - Ok(()) => (), - Err(err) => self.handle_error_fatal(err, "Buffer::destroy"), - } + let _ = wgc::gfx_select!(buffer.id => global.buffer_destroy(buffer.id)); } fn buffer_drop(&self, buffer: &Self::BufferId) { let global = &self.0; wgc::gfx_select!(buffer.id => global.buffer_drop(buffer.id, false)) } fn texture_destroy(&self, texture: &Self::TextureId) { + // Per spec, no error to report. Even calling destroy multiple times is valid. let global = &self.0; - match wgc::gfx_select!(texture.id => global.texture_destroy(texture.id)) { - Ok(()) => (), - Err(err) => self.handle_error_fatal(err, "Texture::destroy"), - } + let _ = wgc::gfx_select!(texture.id => global.texture_destroy(texture.id)); } fn texture_drop(&self, texture: &Self::TextureId) { let global = &self.0; @@ -1813,10 +1809,7 @@ impl crate::Context for Context { } fn texture_view_drop(&self, texture_view: &Self::TextureViewId) { let global = &self.0; - match wgc::gfx_select!(*texture_view => global.texture_view_drop(*texture_view, false)) { - Ok(()) => (), - Err(err) => self.handle_error_fatal(err, "TextureView::drop"), - } + let _ = wgc::gfx_select!(*texture_view => global.texture_view_drop(*texture_view, false)); } fn sampler_drop(&self, sampler: &Self::SamplerId) { let global = &self.0; diff --git a/wgpu/tests/common/mod.rs b/wgpu/tests/common/mod.rs index b0df54d2f1..9b12f46726 100644 --- a/wgpu/tests/common/mod.rs +++ b/wgpu/tests/common/mod.rs @@ -307,3 +307,21 @@ pub fn initialize_test(parameters: TestParameters, test_function: impl FnOnce(Te panic!("UNEXPECTED TEST FAILURE DUE TO {}", failure_cause) } } + +// Run some code in an error scope and assert that validation fails. +pub fn fail(device: &wgpu::Device, callback: impl FnOnce() -> T) -> T { + device.push_error_scope(wgpu::ErrorFilter::Validation); + let result = callback(); + assert!(pollster::block_on(device.pop_error_scope()).is_some()); + + result +} + +// Run some code in an error scope and assert that validation succeeds. +pub fn valid(device: &wgpu::Device, callback: impl FnOnce() -> T) -> T { + device.push_error_scope(wgpu::ErrorFilter::Validation); + let result = callback(); + assert!(pollster::block_on(device.pop_error_scope()).is_none()); + + result +} diff --git a/wgpu/tests/resource_error.rs b/wgpu/tests/resource_error.rs new file mode 100644 index 0000000000..81d50e5800 --- /dev/null +++ b/wgpu/tests/resource_error.rs @@ -0,0 +1,53 @@ +use crate::common::{fail, initialize_test, valid, TestParameters}; + +#[test] +fn bad_buffer() { + // Create a buffer with bad parameters and call a few methods. + // Validation should fail but there should be not panic. + initialize_test(TestParameters::default(), |ctx| { + let buffer = fail(&ctx.device, || { + ctx.device.create_buffer(&wgpu::BufferDescriptor { + label: None, + size: 99999999, + usage: wgpu::BufferUsages::MAP_READ | wgpu::BufferUsages::STORAGE, + mapped_at_creation: false, + }) + }); + + fail(&ctx.device, || { + buffer.slice(..).map_async(wgpu::MapMode::Write, |_| {}) + }); + fail(&ctx.device, || buffer.unmap()); + valid(&ctx.device, || buffer.destroy()); + valid(&ctx.device, || buffer.destroy()); + }); +} + +#[test] +fn bad_texture() { + // Create a texture with bad parameters and call a few methods. + // Validation should fail but there should be not panic. + initialize_test(TestParameters::default(), |ctx| { + let texture = fail(&ctx.device, || { + ctx.device.create_texture(&wgpu::TextureDescriptor { + label: None, + size: wgpu::Extent3d { + width: 0, + height: 12345678, + depth_or_array_layers: 9001, + }, + mip_level_count: 2000, + sample_count: 27, + dimension: wgpu::TextureDimension::D2, + format: wgpu::TextureFormat::Rgba8UnormSrgb, + usage: wgpu::TextureUsages::all(), + }) + }); + + fail(&ctx.device, || { + let _ = texture.create_view(&wgpu::TextureViewDescriptor::default()); + }); + valid(&ctx.device, || texture.destroy()); + valid(&ctx.device, || texture.destroy()); + }); +} diff --git a/wgpu/tests/root.rs b/wgpu/tests/root.rs index 9151d18e9e..9f5068b723 100644 --- a/wgpu/tests/root.rs +++ b/wgpu/tests/root.rs @@ -10,6 +10,7 @@ mod example_wgsl; mod instance; mod poll; mod resource_descriptor_accessor; +mod resource_error; mod shader_primitive_index; mod texture_bounds; mod vertex_indices;