diff --git a/src/webgpu/api/validation/encoding/encoder_open_state.spec.ts b/src/webgpu/api/validation/encoding/encoder_open_state.spec.ts index 6e9181142cdc..06ddc88a0576 100644 --- a/src/webgpu/api/validation/encoding/encoder_open_state.spec.ts +++ b/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'); }); diff --git a/src/webgpu/api/validation/encoding/encoder_state.spec.ts b/src/webgpu/api/validation/encoding/encoder_state.spec.ts index 3c3cde00c48d..40e746511d18 100644 --- a/src/webgpu/api/validation/encoding/encoder_state.spec.ts +++ b/src/webgpu/api/validation/encoding/encoder_state.spec.ts @@ -19,6 +19,7 @@ TODO: import { makeTestGroup } from '../../../../common/framework/test_group.js'; import { objectEquals } from '../../../../common/util/util.js'; import { AllFeaturesMaxLimitsGPUTest } from '../../../gpu_test.js'; +import * as vtu from '../validation_test_utils.js'; class F extends AllFeaturesMaxLimitsGPUTest { beginRenderPass(commandEncoder: GPUCommandEncoder, view: GPUTextureView): GPURenderPassEncoder { @@ -51,6 +52,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 +99,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 +122,9 @@ g.test('call_after_successful_finish') pass.end(); } + let buffer; if (IsEncoderFinished) { - encoder.finish(); + buffer = encoder.finish(); } switch (callCmd) { @@ -126,6 +137,9 @@ g.test('call_after_successful_finish') t.expectValidationError(() => { pass.end(); }, IsEncoderFinished); + if (buffer) { + t.device.queue.submit([buffer]); + } } break; case 'beginRenderPass': @@ -137,16 +151,41 @@ 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(); } }); @@ -154,7 +193,7 @@ g.test('call_after_successful_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 +286,85 @@ 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]) + ) + .beforeAllSubcases(t => t.usesMismatchedDevice()) + .fn(t => { + t.skipIfDeviceDoesNotSupportQueryType('timestamp'); + + const { pass0Type, pass1Type, firstPassInvalid } = t.params; + + const view = t.createAttachmentTextureView(); + const mismatchedTexture = vtu.getDeviceMismatchedRenderTexture(t, 4); + const mismatchedView = mismatchedTexture.createView(); + + const querySet = t.trackForCleanup( + t.device.createQuerySet({ + type: 'timestamp', + count: 2, + }) + ); + + const timestampWrites = { + querySet, + beginningOfPassWriteIndex: 0, + }; + + const descriptor = { + timestampWrites, + }; + + const mismatchedQuerySet = t.trackForCleanup( + t.mismatchedDevice.createQuerySet({ + type: 'timestamp', + count: 2, + }) + ); + + const mismatchedTimestampWrites = { + querySet: mismatchedQuerySet, + beginningOfPassWriteIndex: 0, + }; + + const mismatchedDescriptor = { + timestampWrites: mismatchedTimestampWrites, + }; + + const encoder = t.device.createCommandEncoder(); + + let firstPass; + if (pass0Type === 'compute') { + firstPass = firstPassInvalid + ? encoder.beginComputePass(mismatchedDescriptor) + : encoder.beginComputePass(descriptor); + } else { + firstPass = firstPassInvalid + ? t.beginRenderPass(encoder, mismatchedView) + : t.beginRenderPass(encoder, view); + } + + // 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); + });