From ee9381f4000b3a5c81aee0ae6d14d3eceacb6ca3 Mon Sep 17 00:00:00 2001 From: Erich Gubler Date: Tue, 2 May 2023 14:58:48 -0400 Subject: [PATCH] fix(wgpu-core)!: use `u32` indices for bind group layouts everywhere The short description of this bug/fix pair is that, for some reason, we accept `u32` bind indices in the `wgpu` and `wgpu-core` APIs, narrow to `u8`, and then use `usize` for bind group indices in `wgpu-core`'s internals. This artificially narrows the range of bind group indices, and seems incorrect. Fix this by using `u32`s all the way until we convert to `usize`s. AFAIK, no supported platforms for `wgpu` have a `usize` whose size is less than `u32`'s. For further details, see [Mozilla's Bugzilla] and [`wgpu`'s Matrix chat]. [Mozilla's Bugzilla]: https://bugzilla.mozilla.org/show_bug.cgi?id=1813705#c8 [`wgpu`'s Matrix chat]: https://matrix.to/#/!FZyQrssSlHEZqrYcOb:matrix.org/$0wv-PJgyTTZ7LHtEhzFmAouNYa4VMY0MzVzW0n5J394?via=matrix.org&via=mozilla.org&via=kde.org --- CHANGELOG.md | 4 ++++ wgpu-core/src/binding_model.rs | 8 ++++---- wgpu-core/src/command/bundle.rs | 10 +++++----- wgpu-core/src/command/compute.rs | 10 +++++----- wgpu-core/src/command/draw.rs | 4 ++-- wgpu-core/src/command/render.rs | 6 +++--- 6 files changed, 23 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 49048f2f4ce..1c584613bc9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,10 @@ Bottom level categories: ## Unreleased +### Bug Fixes + +- Use `u32`s internally for bind group indices, rather than `u8`. By @ErichDonGubler in [#3743](https://github.com/gfx-rs/wgpu/pull/3743). + ## v0.16.0 (2023-04-19) ### Major changes diff --git a/wgpu-core/src/binding_model.rs b/wgpu-core/src/binding_model.rs index 5c63ac43ff4..97f56095f40 100644 --- a/wgpu-core/src/binding_model.rs +++ b/wgpu-core/src/binding_model.rs @@ -697,7 +697,7 @@ pub enum BindError { s1 = if *.actual >= 2 { "s" } else { "" }, )] MismatchedDynamicOffsetCount { - group: u8, + group: u32, actual: usize, expected: usize, }, @@ -706,7 +706,7 @@ pub enum BindError { )] UnalignedDynamicBinding { idx: usize, - group: u8, + group: u32, binding: u32, offset: u32, alignment: u32, @@ -718,7 +718,7 @@ pub enum BindError { )] DynamicBindingOutOfBounds { idx: usize, - group: u8, + group: u32, binding: u32, offset: u32, buffer_size: wgt::BufferAddress, @@ -780,7 +780,7 @@ pub struct BindGroup { impl BindGroup { pub(crate) fn validate_dynamic_bindings( &self, - bind_group_index: u8, + bind_group_index: u32, offsets: &[wgt::DynamicOffset], limits: &wgt::Limits, ) -> Result<(), BindError> { diff --git a/wgpu-core/src/command/bundle.rs b/wgpu-core/src/command/bundle.rs index fa95ff87fe2..34d7e84cf49 100644 --- a/wgpu-core/src/command/bundle.rs +++ b/wgpu-core/src/command/bundle.rs @@ -302,7 +302,7 @@ impl RenderBundleEncoder { .map_pass_err(scope)?; let max_bind_groups = device.limits.max_bind_groups; - if (index as u32) >= max_bind_groups { + if index >= max_bind_groups { return Err(RenderCommandError::BindGroupIndexOutOfRange { index, max: max_bind_groups, @@ -784,7 +784,7 @@ impl RenderBundle { unsafe { raw.set_bind_group( &pipeline_layout_guard[pipeline_layout_id.unwrap()].raw, - index as u32, + index, &bind_group.raw, &offsets[..num_dynamic_offsets as usize], ) @@ -1222,7 +1222,7 @@ impl State { fn set_bind_group( &mut self, - slot: u8, + slot: u32, bind_group_id: id::BindGroupId, layout_id: id::Valid, dynamic_offsets: Range, @@ -1365,7 +1365,7 @@ impl State { contents.is_dirty = false; let offsets = &contents.dynamic_offsets; return Some(RenderCommand::SetBindGroup { - index: i as u8, + index: i.try_into().unwrap(), bind_group_id: contents.bind_group_id, num_dynamic_offsets: (offsets.end - offsets.start) as u8, }); @@ -1469,7 +1469,7 @@ pub mod bundle_ffi { } bundle.base.commands.push(RenderCommand::SetBindGroup { - index: index.try_into().unwrap(), + index, num_dynamic_offsets: offset_length.try_into().unwrap(), bind_group_id, }); diff --git a/wgpu-core/src/command/compute.rs b/wgpu-core/src/command/compute.rs index ce40b8b79fb..d5b514f1948 100644 --- a/wgpu-core/src/command/compute.rs +++ b/wgpu-core/src/command/compute.rs @@ -38,7 +38,7 @@ use std::{fmt, mem, str}; )] pub enum ComputeCommand { SetBindGroup { - index: u8, + index: u32, num_dynamic_offsets: u8, bind_group_id: id::BindGroupId, }, @@ -163,7 +163,7 @@ pub enum ComputePassErrorInner { #[error("Bind group {0:?} is invalid")] InvalidBindGroup(id::BindGroupId), #[error("Bind group index {index} is greater than the device's requested `max_bind_group` limit {max}")] - BindGroupIndexOutOfRange { index: u8, max: u32 }, + BindGroupIndexOutOfRange { index: u32, max: u32 }, #[error("Compute pipeline {0:?} is invalid")] InvalidPipeline(id::ComputePipelineId), #[error("QuerySet {0:?} is invalid")] @@ -407,7 +407,7 @@ impl Global { let scope = PassErrorScope::SetBindGroup(bind_group_id); let max_bind_groups = cmd_buf.limits.max_bind_groups; - if (index as u32) >= max_bind_groups { + if index >= max_bind_groups { return Err(ComputePassErrorInner::BindGroupIndexOutOfRange { index, max: max_bind_groups, @@ -464,7 +464,7 @@ impl Global { unsafe { raw.set_bind_group( pipeline_layout, - index as u32 + i as u32, + index + i as u32, raw_bg, &e.dynamic_offsets, ); @@ -816,7 +816,7 @@ pub mod compute_ffi { } pass.base.commands.push(ComputeCommand::SetBindGroup { - index: index.try_into().unwrap(), + index, num_dynamic_offsets: offset_length.try_into().unwrap(), bind_group_id, }); diff --git a/wgpu-core/src/command/draw.rs b/wgpu-core/src/command/draw.rs index 26d2849e269..b629ffaba0e 100644 --- a/wgpu-core/src/command/draw.rs +++ b/wgpu-core/src/command/draw.rs @@ -66,7 +66,7 @@ pub enum RenderCommandError { #[error("Render bundle {0:?} is invalid")] InvalidRenderBundle(id::RenderBundleId), #[error("Bind group index {index} is greater than the device's requested `max_bind_group` limit {max}")] - BindGroupIndexOutOfRange { index: u8, max: u32 }, + BindGroupIndexOutOfRange { index: u32, max: u32 }, #[error("Dynamic buffer offset {0} does not respect device's requested `{1}` limit {2}")] UnalignedBufferOffset(u64, &'static str, u32), #[error("Number of buffer offsets ({actual}) does not match the number of dynamic bindings ({expected})")] @@ -148,7 +148,7 @@ pub struct Rect { )] pub enum RenderCommand { SetBindGroup { - index: u8, + index: u32, num_dynamic_offsets: u8, bind_group_id: id::BindGroupId, }, diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 0570ae8a0a6..b38e79c4b47 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -1305,7 +1305,7 @@ impl Global { } => { let scope = PassErrorScope::SetBindGroup(bind_group_id); let max_bind_groups = device.limits.max_bind_groups; - if (index as u32) >= max_bind_groups { + if index >= max_bind_groups { return Err(RenderCommandError::BindGroupIndexOutOfRange { index, max: max_bind_groups, @@ -1372,7 +1372,7 @@ impl Global { unsafe { raw.set_bind_group( pipeline_layout, - index as u32 + i as u32, + index + i as u32, raw_bg, &e.dynamic_offsets, ); @@ -2225,7 +2225,7 @@ pub mod render_ffi { } pass.base.commands.push(RenderCommand::SetBindGroup { - index: index.try_into().unwrap(), + index, num_dynamic_offsets: offset_length.try_into().unwrap(), bind_group_id, });