Skip to content

Commit

Permalink
Improve dynamic offset binding errors (#3294)
Browse files Browse the repository at this point in the history
  • Loading branch information
cwfitzgerald authored Dec 15, 2022
1 parent 2480eff commit 3ce5ca8
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 9 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down
53 changes: 47 additions & 6 deletions wgpu-core/src/binding_model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<wgt::BufferAddress>,
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<wgt::BufferAddress>,
/// 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.
Expand Down Expand Up @@ -739,11 +772,13 @@ pub struct BindGroup<A: HalApi> {
impl<A: HalApi> BindGroup<A> {
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(),
});
Expand All @@ -758,6 +793,8 @@ impl<A: HalApi> BindGroup<A> {
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,
Expand All @@ -767,9 +804,13 @@ impl<A: HalApi> BindGroup<A> {

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,
});
}
}
Expand Down
2 changes: 1 addition & 1 deletion wgpu-core/src/command/compute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
.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(
Expand Down
2 changes: 1 addition & 1 deletion wgpu-core/src/command/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1238,7 +1238,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
.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
Expand Down
3 changes: 3 additions & 0 deletions wgpu-core/src/device/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1732,6 +1732,9 @@ impl<A: HalApi> Device<A> {
// 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,
});
Expand Down

0 comments on commit 3ce5ca8

Please sign in to comment.