From ac3cd915ffdd0e9be7f83d15a3ba6ff7cddc45a1 Mon Sep 17 00:00:00 2001
From: Greggman <github@greggman.com>
Date: Fri, 17 Jan 2025 03:45:29 +0900
Subject: [PATCH] Compat: Refactor draw test for 0 storage buffers (#4149)

Refactored one test to only optionally use a storage buffer.
Refactored another test to be skipped if no storage buffers in
fragment shaders.

It's possible the 2nd test is not needed as it seems similar
things are tested in src/webgpu/api/operation/vertex_state/correctness.spec.ts.

It's also possible the first test's case of seeing that the fragment
shader is not called for certain cases is also tested elsewhere.
---
 .../api/operation/rendering/draw.spec.ts      | 53 ++++++++++++-------
 1 file changed, 34 insertions(+), 19 deletions(-)

diff --git a/src/webgpu/api/operation/rendering/draw.spec.ts b/src/webgpu/api/operation/rendering/draw.spec.ts
index b5df5297795a..17e6dacf9b48 100644
--- a/src/webgpu/api/operation/rendering/draw.spec.ts
+++ b/src/webgpu/api/operation/rendering/draw.spec.ts
@@ -39,6 +39,8 @@ class DrawTest extends TextureTestMixin(MaxLimitsTestMixin(GPUTest)) {
       baseVertex: opts.baseVertex ?? 0,
     };
 
+    const haveStorageBuffersInFragmentStage =
+      !this.isCompatibility || this.device.limits.maxStorageBuffersInFragmentStage! > 0;
     const renderTargetSize = [72, 36];
 
     // The test will split up the render target into a grid where triangles of
@@ -101,7 +103,7 @@ struct Output {
 @group(0) @binding(0) var<storage, read_write> output : Output;
 
 @fragment fn frag_main() -> @location(0) vec4<f32> {
-  output.value = 1u;
+  ${haveStorageBuffersInFragmentStage ? 'output.value = 1u;' : ''}
   return vec4<f32>(0.0, 1.0, 0.0, 1.0);
 }
 `,
@@ -136,22 +138,26 @@ struct Output {
       },
     });
 
-    const resultBuffer = this.createBufferTracked({
-      size: Uint32Array.BYTES_PER_ELEMENT,
-      usage: GPUBufferUsage.STORAGE | GPUBufferUsage.COPY_SRC,
-    });
-
-    const resultBindGroup = this.device.createBindGroup({
-      layout: pipeline.getBindGroupLayout(0),
-      entries: [
-        {
-          binding: 0,
-          resource: {
-            buffer: resultBuffer,
-          },
-        },
-      ],
-    });
+    const resultBuffer = haveStorageBuffersInFragmentStage
+      ? this.createBufferTracked({
+          size: Uint32Array.BYTES_PER_ELEMENT,
+          usage: GPUBufferUsage.STORAGE | GPUBufferUsage.COPY_SRC,
+        })
+      : null;
+
+    const resultBindGroup = haveStorageBuffersInFragmentStage
+      ? this.device.createBindGroup({
+          layout: pipeline.getBindGroupLayout(0),
+          entries: [
+            {
+              binding: 0,
+              resource: {
+                buffer: resultBuffer!,
+              },
+            },
+          ],
+        })
+      : null;
 
     const commandEncoder = this.device.createCommandEncoder();
     const renderPass = commandEncoder.beginRenderPass({
@@ -279,7 +285,9 @@ struct Output {
 
     const didDraw = defaulted.count && defaulted.instanceCount;
 
-    this.expectGPUBufferValuesEqual(resultBuffer, new Uint32Array([didDraw ? 1 : 0]));
+    if (resultBuffer) {
+      this.expectGPUBufferValuesEqual(resultBuffer, new Uint32Array([didDraw ? 1 : 0]));
+    }
 
     const baseVertexCount = defaulted.baseVertex ?? 0;
     const pixelComparisons: PerPixelComparison<Uint8Array>[] = [];
@@ -321,7 +329,7 @@ Increasing the |first| param should skip some of the beginning triangles on the
 Increasing the |first_instance| param should skip of the beginning triangles on the vertical axis.
 The vertex buffer contains two sets of disjoint triangles, and base_vertex is used to select the second set.
 The test checks that the center of all of the expected triangles is drawn, and the others are empty.
-The fragment shader also writes out to a storage buffer. If the draw is zero-sized, check that no value is written.
+The fragment shader also writes out to a storage buffer if it can. If the draw is zero-sized, check that no value is written.
 
 Params:
   - first= {0, 3} - either the firstVertex or firstIndex
@@ -428,6 +436,13 @@ g.test('vertex_attributes,basic')
       .unless(p => p.step_mode === 'mixed' && p.vertex_buffer_count <= 1)
   )
   .fn(t => {
+    // MAINTENANCE_TODO: refactor this test so it doesn't need a storage buffer OR
+    // consider removing it. It's possible the tests in src/webgpu/api/operation/vertex_state/correctness.spec.ts
+    // already test this.
+    t.skipIf(
+      t.isCompatibility && !(t.device.limits.maxStorageBuffersInFragmentStage! > 0),
+      `maxStorageBuffersInFragmentStage(${t.device.limits.maxStorageBuffersInFragmentStage}) is 0`
+    );
     const vertexCount = 4;
     const instanceCount = 4;