[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.
gfx-rs · Feb 9, 2024 · 4af531c · 4af531c
1 parent 2382c8e
commit 4af531c
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 22 deletions.
diff --git 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
@@ -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());
+        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,
-        });
+            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);
+            pass.set_bind_group(0, &bind_group, &[]);
+            pass.set_pipeline(&pipeline);
+            pass.dispatch_workgroups(1, 1, 1);
 
-        wgpu_test::fail(&ctx.device, || {
             drop(pass);
+            let _ = encoder.finish();
         });
     });
diff --git 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(),
                 },