Additional test cases for encoder states (#4401)

Author: andyleiserson

src/webgpu/api/validation/encoding/encoder_open_state.spec.ts

@@ -275,14 +275,16 @@ g.test('render_pass_commands')
     Test that functions of GPURenderPassEncoder generate a validation error if the encoder or the
     pass is already finished.
 
-    - TODO: Consider testing: nothing before command, end before command, end+finish before command.
+    TODO(https://github.com/gpuweb/gpuweb/issues/5207): Resolve whether the error condition
+    \`finishBeforeCommand !== 'no'\` is correct, or should be changed to
+    \`finishBeforeCommand === 'encoder'\`.
   `
   )
   .params(u =>
     u
       .combine('command', kRenderPassEncoderCommands)
       .beginSubcases()
-      .combine('finishBeforeCommand', [false, true])
+      .combine('finishBeforeCommand', ['no', 'pass', 'encoder'])
   )
   .fn(t => {
     const { command, finishBeforeCommand } = t.params;
@@ -305,8 +307,10 @@ g.test('render_pass_commands')
 
     const bindGroup = t.createBindGroupForTest();
 
-    if (finishBeforeCommand) {
+    if (finishBeforeCommand !== 'no') {
       renderPass.end();
+    }
+    if (finishBeforeCommand === 'encoder') {
       encoder.finish();
     }
 
@@ -404,23 +408,23 @@ g.test('render_pass_commands')
           break;
         case 'pushDebugGroup':
           {
-            encoder.pushDebugGroup('group');
+            renderPass.pushDebugGroup('group');
           }
           break;
         case 'popDebugGroup':
           {
-            encoder.popDebugGroup();
+            renderPass.popDebugGroup();
           }
           break;
         case 'insertDebugMarker':
           {
-            encoder.insertDebugMarker('marker');
+            renderPass.insertDebugMarker('marker');
           }
           break;
         default:
           unreachable();
       }
-    }, finishBeforeCommand);
+    }, finishBeforeCommand !== 'no');
   });
 
 g.test('render_bundle_commands')
@@ -525,14 +529,16 @@ g.test('compute_pass_commands')
     Test that functions of GPUComputePassEncoder generate a validation error if the encoder or the
     pass is already finished.
 
-    - TODO: Consider testing: nothing before command, end before command, end+finish before command.
+    TODO(https://github.com/gpuweb/gpuweb/issues/5207): Resolve whether the error condition
+    \`finishBeforeCommand !== 'no'\` is correct, or should be changed to
+    \`finishBeforeCommand === 'encoder'\`.
   `
   )
   .params(u =>
     u
       .combine('command', kComputePassEncoderCommands)
       .beginSubcases()
-      .combine('finishBeforeCommand', [false, true])
+      .combine('finishBeforeCommand', ['no', 'pass', 'encoder'])
   )
   .fn(t => {
     const { command, finishBeforeCommand } = t.params;
@@ -549,8 +555,10 @@ g.test('compute_pass_commands')
 
     const bindGroup = t.createBindGroupForTest();
 
-    if (finishBeforeCommand) {
+    if (finishBeforeCommand !== 'no') {
       computePass.end();
+    }
+    if (finishBeforeCommand === 'encoder') {
       encoder.finish();
     }
 
@@ -594,5 +602,5 @@ g.test('compute_pass_commands')
         default:
           unreachable();
       }
-    }, finishBeforeCommand);
+    }, finishBeforeCommand !== 'no');
   });

src/webgpu/api/validation/encoding/encoder_state.spec.ts

@@ -51,6 +51,9 @@ g.test('pass_end_invalid_order')
     `
   Test that beginning a {compute,render} pass before ending the previous {compute,render} pass
   causes an error.
+
+  TODO(https://github.com/gpuweb/gpuweb/issues/5207): Resolve whether a validation error
+  should be raised immediately if '!firstPassEnd && endPasses = [1, 0]'.
   `
   )
   .params(u =>
@@ -95,7 +98,13 @@ g.test('call_after_successful_finish')
   .desc(`Test that encoding command after a successful finish generates a validation error.`)
   .params(u =>
     u
-      .combine('callCmd', ['beginComputePass', 'beginRenderPass', 'insertDebugMarker'])
+      .combine('callCmd', [
+        'beginComputePass',
+        'beginRenderPass',
+        'finishAndSubmitFirst',
+        'finishAndSubmitSecond',
+        'insertDebugMarker',
+      ])
       .beginSubcases()
       .combine('prePassType', ['compute', 'render', 'no-op'])
       .combine('IsEncoderFinished', [false, true])
@@ -112,8 +121,9 @@ g.test('call_after_successful_finish')
       pass.end();
     }
 
+    let buffer;
     if (IsEncoderFinished) {
-      encoder.finish();
+      buffer = encoder.finish();
     }
 
     switch (callCmd) {
@@ -126,6 +136,9 @@ g.test('call_after_successful_finish')
           t.expectValidationError(() => {
             pass.end();
           }, IsEncoderFinished);
+          if (buffer) {
+            t.device.queue.submit([buffer]);
+          }
         }
         break;
       case 'beginRenderPass':
@@ -137,24 +150,49 @@ g.test('call_after_successful_finish')
           t.expectValidationError(() => {
             pass.end();
           }, IsEncoderFinished);
+          if (buffer) {
+            t.device.queue.submit([buffer]);
+          }
+        }
+        break;
+      case 'finishAndSubmitFirst':
+        t.expectValidationError(() => {
+          encoder.finish();
+        }, IsEncoderFinished);
+        if (buffer) {
+          t.device.queue.submit([buffer]);
+        }
+        break;
+      case 'finishAndSubmitSecond':
+        {
+          let secondBuffer: GPUCommandBuffer;
+          t.expectValidationError(() => {
+            secondBuffer = encoder.finish();
+          }, IsEncoderFinished);
+          t.expectValidationError(() => {
+            t.device.queue.submit([secondBuffer]);
+          }, IsEncoderFinished);
         }
         break;
       case 'insertDebugMarker':
         t.expectValidationError(() => {
           encoder.insertDebugMarker('');
         }, IsEncoderFinished);
+        if (buffer) {
+          t.device.queue.submit([buffer]);
+        }
         break;
     }
 
-    if (!IsEncoderFinished) {
+    if (!IsEncoderFinished && !callCmd.startsWith('finish')) {
       encoder.finish();
     }
   });
 
 g.test('pass_end_none')
   .desc(
     `
-  Test that ending a {compute,render} pass without ending the passes generates a validation error.
+  Test that finishing an encoder without ending a child {compute,render} pass generates a validation error.
   `
   )
   .paramsSubcasesOnly(u => u.combine('passType', ['compute', 'render']).combine('endCount', [0, 1]))
@@ -247,3 +285,49 @@ g.test('pass_end_twice,render_pass_invalid')
       encoder.finish();
     });
   });
+
+g.test('pass_begin_invalid_encoder')
+  .desc(
+    `
+  Test that {compute,render} passes can still be opened on an invalid encoder.
+  `
+  )
+  .params(u =>
+    u
+      .combine('pass0Type', ['compute', 'render'])
+      .combine('pass1Type', ['compute', 'render'])
+      .beginSubcases()
+      .combine('firstPassInvalid', [false, true])
+  )
+  .fn(t => {
+    const { pass0Type, pass1Type, firstPassInvalid } = t.params;
+
+    const view = t.createAttachmentTextureView();
+
+    const encoder = t.device.createCommandEncoder();
+
+    let firstPass;
+    if (pass0Type === 'compute') {
+      firstPass = encoder.beginComputePass();
+    } else {
+      firstPass = t.beginRenderPass(encoder, view);
+    }
+
+    if (firstPassInvalid) {
+      // Popping an empty debug group stack invalidates the pass.
+      firstPass.popDebugGroup();
+    }
+
+    // Ending an invalid pass invalidates the encoder
+    firstPass.end();
+
+    // Passes can still be opened on an invalid encoder
+    const secondPass =
+      pass1Type === 'compute' ? encoder.beginComputePass() : t.beginRenderPass(encoder, view);
+
+    secondPass.end();
+
+    t.expectValidationError(() => {
+      encoder.finish();
+    }, firstPassInvalid);
+  });