From 4af531cf6972574cc7ea68b33a4529c231f5cc57 Mon Sep 17 00:00:00 2001 From: Jim Blandy Date: Thu, 8 Feb 2024 19:08:01 -0800 Subject: [PATCH] [wgpu-core] Compute minimum binding size correctly for arrays. (#5222) * [wgpu-core] Add tests for minimum binding size validation. * [wgpu-core] Compute minimum binding size correctly for arrays. 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. --- CHANGELOG.md | 1 + tests/tests/buffer.rs | 35 ++++++++++++++--------------------- wgpu-core/src/validation.rs | 12 +++++++++--- 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 56877adcdf..0b09444407 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/tests/tests/buffer.rs b/tests/tests/buffer.rs index a1ae9c5559..23f244c249 100644 --- a/tests/tests/buffer.rs +++ b/tests/tests/buffer.rs @@ -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 @@ -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 @@ -318,18 +310,19 @@ static MINIMUM_BUFFER_BINDING_SIZE_DISPATCH: GpuTestConfiguration = GpuTestConfi }], }); - let mut encoder = ctx.device.create_command_encoder(&Default::default()); - - let mut pass = encoder.begin_compute_pass(&wgpu::ComputePassDescriptor { - label: None, - timestamp_writes: None, - }); - - pass.set_bind_group(0, &bind_group, &[]); - pass.set_pipeline(&pipeline); - pass.dispatch_workgroups(1, 1, 1); - wgpu_test::fail(&ctx.device, || { + let mut encoder = ctx.device.create_command_encoder(&Default::default()); + + let mut pass = encoder.begin_compute_pass(&wgpu::ComputePassDescriptor { + label: None, + timestamp_writes: None, + }); + + pass.set_bind_group(0, &bind_group, &[]); + pass.set_pipeline(&pipeline); + pass.dispatch_workgroups(1, 1, 1); + drop(pass); + let _ = encoder.finish(); }); }); diff --git a/wgpu-core/src/validation.rs b/wgpu-core/src/validation.rs index a0947ae83f..994a6cd523 100644 --- a/wgpu-core/src/validation.rs +++ b/wgpu-core/src/validation.rs @@ -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(), },