From 58f9d5dbdf3dc087ea11bd403909f068022af7cd Mon Sep 17 00:00:00 2001
From: Greggman <github@greggman.com>
Date: Tue, 31 Dec 2024 10:49:59 +0900
Subject: [PATCH] Compat: refactor setBindGroup for 0 storage buffers in frag
 stage. (#4125)

Most of these tests didn't seem to require storage buffers
to test what they were testing so I switched them to uniform buffers.

One of them used both storage and uniform buffers so I added more
subcases so that it tests with both storage buffers and uniform
buffers in compute stage only (this is for devices that don't support
storage buffers in fragment stage). Then both compute+fragment stages
like it was, but this case is skipped if there are no storage buffers
in fragment shaders. I also added a case were it uses 2 uniform buffers.

It's not clear what it's testing needs to buffers to be of different
types.
---
 .../encoding/cmds/setBindGroup.spec.ts        | 42 ++++++++++++-------
 1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/src/webgpu/api/validation/encoding/cmds/setBindGroup.spec.ts b/src/webgpu/api/validation/encoding/cmds/setBindGroup.spec.ts
index 2bbcc2b8bb21..e392b718825e 100644
--- a/src/webgpu/api/validation/encoding/cmds/setBindGroup.spec.ts
+++ b/src/webgpu/api/validation/encoding/cmds/setBindGroup.spec.ts
@@ -16,7 +16,8 @@ import {
   kBufferBindingTypes,
   kMinDynamicBufferOffsetAlignment,
 } from '../../../../capability_info.js';
-import { kResourceStates, ResourceState } from '../../../../gpu_test.js';
+import { GPUConst } from '../../../../constants.js';
+import { kResourceStates, MaxLimitsTestMixin, ResourceState } from '../../../../gpu_test.js';
 import {
   kProgrammableEncoderTypes,
   ProgrammableEncoderType,
@@ -53,7 +54,7 @@ class F extends ValidationTest {
         return {
           buffer: this.createBufferWithState(state, {
             size: 4,
-            usage: GPUBufferUsage.STORAGE,
+            usage: GPUBufferUsage.UNIFORM,
           }),
         };
       default:
@@ -80,7 +81,7 @@ class F extends ValidationTest {
       entries: indices.map(binding => ({
         binding,
         visibility: this.encoderTypeToStageFlag(encoderType),
-        ...(resourceType === 'buffer' ? { buffer: { type: 'storage' } } : { texture: {} }),
+        ...(resourceType === 'buffer' ? { buffer: { type: 'uniform' } } : { texture: {} }),
       })),
     });
     const bindGroup = this.device.createBindGroup({
@@ -101,7 +102,7 @@ class F extends ValidationTest {
   }
 }
 
-export const g = makeTestGroup(F);
+export const g = makeTestGroup(MaxLimitsTestMixin(F));
 
 g.test('state_and_binding_index')
   .desc('Tests that setBindGroup correctly handles {valid, invalid, destroyed} bindGroups.')
@@ -153,7 +154,7 @@ g.test('bind_group,device_mismatch')
     const buffer = t.trackForCleanup(
       sourceDevice.createBuffer({
         size: 4,
-        usage: GPUBufferUsage.STORAGE,
+        usage: GPUBufferUsage.UNIFORM,
       })
     );
 
@@ -162,7 +163,7 @@ g.test('bind_group,device_mismatch')
         {
           binding: 0,
           visibility: t.encoderTypeToStageFlag(encoderType),
-          buffer: { type: 'storage', hasDynamicOffset: useU32Array },
+          buffer: { type: 'uniform', hasDynamicOffset: useU32Array },
         },
       ],
     });
@@ -224,15 +225,28 @@ g.test('dynamic_offsets_match_expectations_in_pass_encoder')
         { dynamicOffsets: [0, 0xffffffff], _success: false },
       ])
       .combine('useU32array', [false, true])
+      .beginSubcases()
+      .combine('visibility', [
+        GPUConst.ShaderStage.COMPUTE,
+        GPUConst.ShaderStage.COMPUTE | GPUConst.ShaderStage.FRAGMENT,
+      ] as const)
+      .combine('useStorage', [false, true] as const)
   )
   .fn(t => {
+    const { visibility, useStorage } = t.params;
+    t.skipIf(
+      t.isCompatibility &&
+        (visibility & GPUShaderStage.FRAGMENT) !== 0 &&
+        !(t.device.limits.maxStorageBuffersInFragmentStage! >= 1),
+      `maxStorageBuffersInFragmentStage${t.device.limits.maxStorageBuffersInFragmentStage} < 1`
+    );
     const kBindingSize = 12;
 
     const bindGroupLayout = t.device.createBindGroupLayout({
       entries: [
         {
           binding: 0,
-          visibility: GPUShaderStage.COMPUTE | GPUShaderStage.FRAGMENT,
+          visibility,
           buffer: {
             type: 'uniform',
             hasDynamicOffset: true,
@@ -240,9 +254,9 @@ g.test('dynamic_offsets_match_expectations_in_pass_encoder')
         },
         {
           binding: 1,
-          visibility: GPUShaderStage.COMPUTE | GPUShaderStage.FRAGMENT,
+          visibility,
           buffer: {
-            type: 'storage',
+            type: useStorage ? 'storage' : 'uniform',
             hasDynamicOffset: true,
           },
         },
@@ -254,9 +268,9 @@ g.test('dynamic_offsets_match_expectations_in_pass_encoder')
       usage: GPUBufferUsage.UNIFORM,
     });
 
-    const storageBuffer = t.createBufferTracked({
+    const storageOrUniformBuffer = t.createBufferTracked({
       size: 2 * kMinDynamicBufferOffsetAlignment + 8,
-      usage: GPUBufferUsage.STORAGE,
+      usage: useStorage ? GPUBufferUsage.STORAGE : GPUBufferUsage.UNIFORM,
     });
 
     const bindGroup = t.device.createBindGroup({
@@ -272,7 +286,7 @@ g.test('dynamic_offsets_match_expectations_in_pass_encoder')
         {
           binding: 1,
           resource: {
-            buffer: storageBuffer,
+            buffer: storageOrUniformBuffer,
             size: kBindingSize,
           },
         },
@@ -335,7 +349,7 @@ g.test('u32array_start_and_length')
         binding: i,
         visibility: GPUShaderStage.FRAGMENT,
         buffer: {
-          type: 'storage',
+          type: 'uniform',
           hasDynamicOffset: true,
         },
       })),
@@ -348,7 +362,7 @@ g.test('u32array_start_and_length')
         resource: {
           buffer: t.createBufferWithState('valid', {
             size: kBindingSize,
-            usage: GPUBufferUsage.STORAGE,
+            usage: GPUBufferUsage.UNIFORM,
           }),
           size: kBindingSize,
         },