From f9c4a451f5ac24e440fb7aa6287a4a27c0234e30 Mon Sep 17 00:00:00 2001 From: Connor Fitzgerald Date: Wed, 14 Dec 2022 19:43:04 -0500 Subject: [PATCH 1/2] Improve dynamic offset binding errors --- wgpu-core/src/binding_model.rs | 53 ++++++++++++++++++++++++++++---- wgpu-core/src/command/compute.rs | 2 +- wgpu-core/src/command/render.rs | 2 +- wgpu-core/src/device/mod.rs | 3 ++ 4 files changed, 52 insertions(+), 8 deletions(-) diff --git a/wgpu-core/src/binding_model.rs b/wgpu-core/src/binding_model.rs index 5feef3bab4..d64d563b25 100644 --- a/wgpu-core/src/binding_model.rs +++ b/wgpu-core/src/binding_model.rs @@ -683,23 +683,56 @@ pub enum BindingResource<'a> { #[derive(Clone, Debug, Error)] pub enum BindError { - #[error("number of dynamic offsets ({actual}) doesn't match the number of dynamic bindings in the bind group layout ({expected})")] - MismatchedDynamicOffsetCount { actual: usize, expected: usize }, #[error( - "dynamic binding at index {idx}: offset {offset} does not respect device's requested `{limit_name}` limit {alignment}" + "Bind group {group} expects {expected} dynamic offset{s0}. However {actual} dynamic offset{s1} were provided.", + s0 = if *.expected >= 2 { "s" } else { "" }, + s1 = if *.actual >= 2 { "s" } else { "" }, + )] + MismatchedDynamicOffsetCount { + group: u8, + actual: usize, + expected: usize, + }, + #[error( + "Dynamic binding index {idx} (targeting bind group {group}, binding {binding}) with value {offset}, does not respect device's requested `{limit_name}` limit: {alignment}" )] UnalignedDynamicBinding { idx: usize, + group: u8, + binding: u32, offset: u32, alignment: u32, limit_name: &'static str, }, - #[error("dynamic binding at index {idx} with offset {offset} would overrun the buffer (limit: {max})")] - DynamicBindingOutOfBounds { idx: usize, offset: u32, max: u64 }, + #[error( + "Dynamic binding offset index {idx} with offset {offset} would overrun the buffer bound to bind group {group} -> binding {binding}. \ + Buffer size is {buffer_size} bytes, the binding binds bytes {binding_range:?}, meaning the maximum the binding can be offset is {maximum_dynamic_offset} bytes", + )] + DynamicBindingOutOfBounds { + idx: usize, + group: u8, + binding: u32, + offset: u32, + buffer_size: wgt::BufferAddress, + binding_range: Range, + maximum_dynamic_offset: wgt::BufferAddress, + }, } #[derive(Debug)] pub struct BindGroupDynamicBindingData { + /// The index of the binding. + /// + /// Used for more descriptive errors. + pub(crate) binding_idx: u32, + /// The size of the buffer. + /// + /// Used for more descriptive errors. + pub(crate) buffer_size: wgt::BufferAddress, + /// The range that the binding covers. + /// + /// Used for more descriptive errors. + pub(crate) binding_range: Range, /// The maximum value the dynamic offset can have before running off the end of the buffer. pub(crate) maximum_dynamic_offset: wgt::BufferAddress, /// The binding type. @@ -739,11 +772,13 @@ pub struct BindGroup { impl BindGroup { pub(crate) fn validate_dynamic_bindings( &self, + bind_group_index: u8, offsets: &[wgt::DynamicOffset], limits: &wgt::Limits, ) -> Result<(), BindError> { if self.dynamic_binding_info.len() != offsets.len() { return Err(BindError::MismatchedDynamicOffsetCount { + group: bind_group_index, expected: self.dynamic_binding_info.len(), actual: offsets.len(), }); @@ -758,6 +793,8 @@ impl BindGroup { let (alignment, limit_name) = buffer_binding_type_alignment(limits, info.binding_type); if offset as wgt::BufferAddress % alignment as u64 != 0 { return Err(BindError::UnalignedDynamicBinding { + group: bind_group_index, + binding: info.binding_idx, idx, offset, alignment, @@ -767,9 +804,13 @@ impl BindGroup { if offset as wgt::BufferAddress > info.maximum_dynamic_offset { return Err(BindError::DynamicBindingOutOfBounds { + group: bind_group_index, + binding: info.binding_idx, idx, offset, - max: info.maximum_dynamic_offset, + buffer_size: info.buffer_size, + binding_range: info.binding_range.clone(), + maximum_dynamic_offset: info.maximum_dynamic_offset, }); } } diff --git a/wgpu-core/src/command/compute.rs b/wgpu-core/src/command/compute.rs index c228519595..dec6211a73 100644 --- a/wgpu-core/src/command/compute.rs +++ b/wgpu-core/src/command/compute.rs @@ -428,7 +428,7 @@ impl Global { .ok_or(ComputePassErrorInner::InvalidBindGroup(bind_group_id)) .map_pass_err(scope)?; bind_group - .validate_dynamic_bindings(&temp_offsets, &cmd_buf.limits) + .validate_dynamic_bindings(index, &temp_offsets, &cmd_buf.limits) .map_pass_err(scope)?; cmd_buf.buffer_memory_init_actions.extend( diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 09af0bbe6a..04e27ec062 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -1238,7 +1238,7 @@ impl Global { .ok_or(RenderCommandError::InvalidBindGroup(bind_group_id)) .map_pass_err(scope)?; bind_group - .validate_dynamic_bindings(&temp_offsets, &cmd_buf.limits) + .validate_dynamic_bindings(index, &temp_offsets, &cmd_buf.limits) .map_pass_err(scope)?; // merge the resource tracker in diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index ef3d8ee8af..cdfc0bea4f 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -1732,6 +1732,9 @@ impl Device { // Record binding info for validating dynamic offsets if dynamic { dynamic_binding_info.push(binding_model::BindGroupDynamicBindingData { + binding_idx: binding, + buffer_size: buffer.size, + binding_range: bb.offset..bind_end, maximum_dynamic_offset: buffer.size - bind_end, binding_type: binding_ty, }); From 93c9b0ba9ea788d5b843471334c96851d7844a7d Mon Sep 17 00:00:00 2001 From: Connor Fitzgerald Date: Wed, 14 Dec 2022 19:45:56 -0500 Subject: [PATCH 2/2] Changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bf53a3d2e9..d5830e1bb3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -158,6 +158,7 @@ Additionally `Surface::get_default_config` now returns an Option and returns Non - Make `RenderPassCompatibilityError` and `CreateShaderModuleError` not so huge. By @jimblandy in (#3226)[https://github.com/gfx-rs/wgpu/pull/3226] - Check for invalid bitflag bits in wgpu-core and allow them to be captured/replayed by @nical in (#3229)[https://github.com/gfx-rs/wgpu/pull/3229] - Evaluate `gfx_select!`'s `#[cfg]` conditions at the right time. By @jimblandy in [#3253](https://github.com/gfx-rs/wgpu/pull/3253) +- Improve error messages when binding bind group with dynamic offsets. By @cwfitzgerald in [#3294](https://github.com/gfx-rs/wgpu/pull/3294) #### WebGPU @@ -193,7 +194,7 @@ Additionally `Surface::get_default_config` now returns an Option and returns Non - Use cargo 1.64 workspace inheritance feature. By @jinleili in [#3107](https://github.com/gfx-rs/wgpu/pull/3107) - Move `ResourceMetadata` into its own module. By @jimblandy in [#3213](https://github.com/gfx-rs/wgpu/pull/3213) - Add WebAssembly testing infrastructure. By @haraldreingruber in [#3238](https://github.com/gfx-rs/wgpu/pull/3238) -- Error message when you forget to use cargo-nextest. By @cwfitzgerald in [#]() +- Error message when you forget to use cargo-nextest. By @cwfitzgerald in [#3293](https://github.com/gfx-rs/wgpu/pull/3293) #### Vulkan