Skip to content

Commit

Permalink
[wgpu-core] Compute minimum binding size correctly for arrays.
Browse files Browse the repository at this point in the history
In early versions of WGSL, `storage` or `uniform` global variables had
to be either structs or runtime-sized arrays. This rule was relaxed,
and now globals can have any type; Naga automatically wraps such
variables in structs when required by the backend shading language.

Under the old rules, whenever wgpu-core saw a `storage` or `uniform`
global variable with an array type, it could assume it was a
runtime-sized array, and take the stride as the minimum binding size.
Under the new rules, wgpu-core must consider fixed-sized and
runtime-sized arrays separately.
  • Loading branch information
jimblandy committed Feb 8, 2024
1 parent 219a2eb commit 3194089
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ Bottom level categories:
- Fix the validation of vertex and index ranges. By @nical in [#5144](https://github.com/gfx-rs/wgpu/pull/5144) and [#5156](https://github.com/gfx-rs/wgpu/pull/5156)
- Device lost callbacks are invoked when replaced and when global is dropped. By @bradwerth in [#5168](https://github.com/gfx-rs/wgpu/pull/5168)
- Fix panic when creating a surface while no backend is available. By @wumpf [#5166](https://github.com/gfx-rs/wgpu/pull/5166)
- Correctly compute minimum buffer size for array-typed `storage` and `uniform` vars. By @jimblandy [#5222](https://github.com/gfx-rs/wgpu/pull/5222)

#### WGL

Expand Down
12 changes: 2 additions & 10 deletions tests/tests/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,11 +174,7 @@ static MAP_OFFSET: GpuTestConfiguration = GpuTestConfiguration::new().run_async(
/// 16 for that variable's group/index. Pipeline creation should fail.
#[gpu_test]
static MINIMUM_BUFFER_BINDING_SIZE_LAYOUT: GpuTestConfiguration = GpuTestConfiguration::new()
.parameters(
TestParameters::default()
.test_features_limits()
.skip(FailureCase::always()), // https://github.com/gfx-rs/wgpu/issues/5219
)
.parameters(TestParameters::default().test_features_limits())
.run_sync(|ctx| {
// Create a shader module that statically uses a storage buffer.
let shader_module = ctx
Expand Down Expand Up @@ -242,11 +238,7 @@ static MINIMUM_BUFFER_BINDING_SIZE_LAYOUT: GpuTestConfiguration = GpuTestConfigu
/// binding. Command recording should fail.
#[gpu_test]
static MINIMUM_BUFFER_BINDING_SIZE_DISPATCH: GpuTestConfiguration = GpuTestConfiguration::new()
.parameters(
TestParameters::default()
.test_features_limits()
.skip(FailureCase::always()), // https://github.com/gfx-rs/wgpu/issues/5219
)
.parameters(TestParameters::default().test_features_limits())
.run_sync(|ctx| {
// This test tries to use a bindgroup layout with a
// min_binding_size of 16 to an index whose WGSL type requires 32
Expand Down
12 changes: 9 additions & 3 deletions wgpu-core/src/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -892,9 +892,15 @@ impl Interface {
class,
},
naga::TypeInner::Sampler { comparison } => ResourceType::Sampler { comparison },
naga::TypeInner::Array { stride, .. } => ResourceType::Buffer {
size: wgt::BufferSize::new(stride as u64).unwrap(),
},
naga::TypeInner::Array { stride, size, .. } => {
let size = match size {
naga::ArraySize::Constant(size) => size.get() * stride,
naga::ArraySize::Dynamic => stride,
};
ResourceType::Buffer {
size: wgt::BufferSize::new(size as u64).unwrap(),
}
}
ref other => ResourceType::Buffer {
size: wgt::BufferSize::new(other.size(module.to_ctx()) as u64).unwrap(),
},
Expand Down

0 comments on commit 3194089

Please sign in to comment.