From e14e419cf67727abe03646e35b7460f96b9d5051 Mon Sep 17 00:00:00 2001 From: Kai Ninomiya Date: Wed, 15 Nov 2023 10:43:05 -0800 Subject: [PATCH] Updates for writeTimestamp removal (#3167) * Test writeTimestamp never works writeTimestamp has been removed from the spec so it should always TypeError. (Technically it shouldn't exist at all but per our error design this is sort of OK since it's the same thing you would get if writeTimestamp were behind a feature that's not enabled.) Passes on Chrome Canary, confirmed fails after enabling chrome://flags/#enable-webgpu-developer-features enabled (which causes it not to throw a TypeError). * Fix all the other tests * metadata renames * use skipIf --- .../features/query_types.spec.ts | 55 +++++++++++++++++-- .../encoding/encoder_open_state.spec.ts | 6 +- .../encoding/queries/general.spec.ts | 35 +++++++++--- .../queue/destroyed/query_set.spec.ts | 8 ++- .../state/device_lost/destroy.spec.ts | 6 +- src/webgpu/listing_meta.json | 8 +-- 6 files changed, 98 insertions(+), 20 deletions(-) diff --git a/src/webgpu/api/validation/capability_checks/features/query_types.spec.ts b/src/webgpu/api/validation/capability_checks/features/query_types.spec.ts index 8016252b1ede..f045848fb0bb 100644 --- a/src/webgpu/api/validation/capability_checks/features/query_types.spec.ts +++ b/src/webgpu/api/validation/capability_checks/features/query_types.spec.ts @@ -43,11 +43,13 @@ g.test('createQuerySet') }); }); -g.test('writeTimestamp') +g.test('timestamp') .desc( ` Tests that writing a timestamp throws a type error exception if the features don't contain 'timestamp-query'. + + TODO: writeTimestamp test is disabled since it's removed from the spec for now. ` ) .params(u => u.combine('featureContainsTimestampQuery', [false, true])) @@ -66,11 +68,52 @@ g.test('writeTimestamp') const querySet = t.device.createQuerySet({ type: featureContainsTimestampQuery ? 'timestamp' : 'occlusion', - count: 1, + count: 2, }); - const encoder = t.createEncoder('non-pass'); - t.shouldThrow(featureContainsTimestampQuery ? false : 'TypeError', () => { - encoder.encoder.writeTimestamp(querySet, 0); - }); + { + let expected = featureContainsTimestampQuery ? false : 'TypeError'; + // writeTimestamp no longer exists and this should always TypeError. + expected = 'TypeError'; + + const encoder = t.createEncoder('non-pass'); + t.shouldThrow(expected, () => { + encoder.encoder.writeTimestamp(querySet, 0); + }); + encoder.finish(); + } + + { + const encoder = t.createEncoder('non-pass'); + encoder.encoder + .beginComputePass({ + timestampWrites: { querySet, beginningOfPassWriteIndex: 0, endOfPassWriteIndex: 1 }, + }) + .end(); + t.expectValidationError(() => { + encoder.finish(); + }, !featureContainsTimestampQuery); + } + + { + const encoder = t.createEncoder('non-pass'); + const view = t + .trackForCleanup( + t.device.createTexture({ + size: [16, 16, 1], + format: 'rgba8unorm', + usage: GPUTextureUsage.RENDER_ATTACHMENT, + }) + ) + .createView(); + encoder.encoder + .beginRenderPass({ + colorAttachments: [{ view, loadOp: 'clear', storeOp: 'discard' }], + timestampWrites: { querySet, beginningOfPassWriteIndex: 0, endOfPassWriteIndex: 1 }, + }) + .end(); + t.expectValidationError(() => { + encoder.finish(); + }, !featureContainsTimestampQuery); + } }); 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 0d56222eedbb..66a00b2c3bf4 100644 --- a/src/webgpu/api/validation/encoding/encoder_open_state.spec.ts +++ b/src/webgpu/api/validation/encoding/encoder_open_state.spec.ts @@ -146,6 +146,8 @@ g.test('non_pass_commands') ` Test that functions of GPUCommandEncoder generate a validation error if the encoder is already finished. + + TODO: writeTimestamp is removed from the spec so it's skipped if it TypeErrors. ` ) .params(u => @@ -260,8 +262,10 @@ g.test('non_pass_commands') } break; case 'writeTimestamp': - { + try { encoder.writeTimestamp(querySet, 0); + } catch (ex) { + t.skipIf(ex instanceof TypeError, 'writeTimestamp is actually not available'); } break; case 'resolveQuerySet': diff --git a/src/webgpu/api/validation/encoding/queries/general.spec.ts b/src/webgpu/api/validation/encoding/queries/general.spec.ts index 0ed2352bfd1a..e529c3b5ff67 100644 --- a/src/webgpu/api/validation/encoding/queries/general.spec.ts +++ b/src/webgpu/api/validation/encoding/queries/general.spec.ts @@ -68,13 +68,15 @@ Tests that begin occlusion query with query index: encoder.validateFinish(t.params.queryIndex < 2); }); -g.test('timestamp_query,query_type_and_index') +g.test('writeTimestamp,query_type_and_index') .desc( ` Tests that write timestamp to all types of query set on all possible encoders: - type {occlusion, timestamp} - queryIndex {in, out of} range for GPUQuerySet - x= {non-pass} encoder + + TODO: writeTimestamp is removed from the spec so it's skipped if it TypeErrors. ` ) .params(u => @@ -101,15 +103,21 @@ Tests that write timestamp to all types of query set on all possible encoders: const querySet = createQuerySetWithType(t, type, count); const encoder = t.createEncoder('non-pass'); - encoder.encoder.writeTimestamp(querySet, queryIndex); + try { + encoder.encoder.writeTimestamp(querySet, queryIndex); + } catch (ex) { + t.skipIf(ex instanceof TypeError, 'writeTimestamp is actually not available'); + } encoder.validateFinish(type === 'timestamp' && queryIndex < count); }); -g.test('timestamp_query,invalid_query_set') +g.test('writeTimestamp,invalid_query_set') .desc( ` Tests that write timestamp to a invalid query set that failed during creation: - x= {non-pass} encoder + + TODO: writeTimestamp is removed from the spec so it's skipped if it TypeErrors. ` ) .paramsSubcasesOnly(u => u.combine('querySetState', ['valid', 'invalid'] as const)) @@ -125,12 +133,21 @@ Tests that write timestamp to a invalid query set that failed during creation: }); const encoder = t.createEncoder('non-pass'); - encoder.encoder.writeTimestamp(querySet, 0); + try { + encoder.encoder.writeTimestamp(querySet, 0); + } catch (ex) { + t.skipIf(ex instanceof TypeError, 'writeTimestamp is actually not available'); + } encoder.validateFinish(querySetState !== 'invalid'); }); -g.test('timestamp_query,device_mismatch') - .desc('Tests writeTimestamp cannot be called with a query set created from another device') +g.test('writeTimestamp,device_mismatch') + .desc( + `Tests writeTimestamp cannot be called with a query set created from another device + + TODO: writeTimestamp is removed from the spec so it's skipped if it TypeErrors. + ` + ) .paramsSubcasesOnly(u => u.combine('mismatched', [true, false])) .beforeAllSubcases(t => { t.selectDeviceForQueryTypeOrSkipTestCase('timestamp'); @@ -147,6 +164,10 @@ g.test('timestamp_query,device_mismatch') t.trackForCleanup(querySet); const encoder = t.createEncoder('non-pass'); - encoder.encoder.writeTimestamp(querySet, 0); + try { + encoder.encoder.writeTimestamp(querySet, 0); + } catch (ex) { + t.skipIf(ex instanceof TypeError, 'writeTimestamp is actually not available'); + } encoder.validateFinish(!mismatched); }); diff --git a/src/webgpu/api/validation/queue/destroyed/query_set.spec.ts b/src/webgpu/api/validation/queue/destroyed/query_set.spec.ts index 1d8adab7e836..b091e4855f59 100644 --- a/src/webgpu/api/validation/queue/destroyed/query_set.spec.ts +++ b/src/webgpu/api/validation/queue/destroyed/query_set.spec.ts @@ -29,6 +29,8 @@ g.test('writeTimestamp') ` Tests that use a destroyed query set in writeTimestamp on {non-pass, compute, render} encoder. - x= {destroyed, not destroyed (control case)} + + TODO: writeTimestamp is removed from the spec so it's skipped if it TypeErrors. ` ) .params(u => u.beginSubcases().combine('querySetState', ['valid', 'destroyed'] as const)) @@ -40,7 +42,11 @@ Tests that use a destroyed query set in writeTimestamp on {non-pass, compute, re }); const encoder = t.createEncoder('non-pass'); - encoder.encoder.writeTimestamp(querySet, 0); + try { + encoder.encoder.writeTimestamp(querySet, 0); + } catch (ex) { + t.skipIf(ex instanceof TypeError, 'writeTimestamp is actually not available'); + } encoder.validateFinishAndSubmitGivenState(t.params.querySetState); }); diff --git a/src/webgpu/api/validation/state/device_lost/destroy.spec.ts b/src/webgpu/api/validation/state/device_lost/destroy.spec.ts index df03427a0a34..56d53c98676c 100644 --- a/src/webgpu/api/validation/state/device_lost/destroy.spec.ts +++ b/src/webgpu/api/validation/state/device_lost/destroy.spec.ts @@ -899,7 +899,11 @@ Tests encoding and finishing a writeTimestamp command on destroyed device. const { type, stage, awaitLost } = t.params; const querySet = t.device.createQuerySet({ type, count: 2 }); await t.executeCommandsAfterDestroy(stage, awaitLost, 'non-pass', maker => { - maker.encoder.writeTimestamp(querySet, 0); + try { + maker.encoder.writeTimestamp(querySet, 0); + } catch (ex) { + t.skipIf(ex instanceof TypeError, 'writeTimestamp is actually not available'); + } return maker; }); }); diff --git a/src/webgpu/listing_meta.json b/src/webgpu/listing_meta.json index 5a1060805a06..e4e3e6bad37f 100644 --- a/src/webgpu/listing_meta.json +++ b/src/webgpu/listing_meta.json @@ -265,7 +265,7 @@ "webgpu:api,validation,buffer,mapping:unmap,state,mappingPending:*": { "subcaseMS": 22.951 }, "webgpu:api,validation,buffer,mapping:unmap,state,unmapped:*": { "subcaseMS": 74.200 }, "webgpu:api,validation,capability_checks,features,query_types:createQuerySet:*": { "subcaseMS": 10.451 }, - "webgpu:api,validation,capability_checks,features,query_types:writeTimestamp:*": { "subcaseMS": 1.200 }, + "webgpu:api,validation,capability_checks,features,query_types:timestamp:*": { "subcaseMS": 1.200 }, "webgpu:api,validation,capability_checks,features,texture_formats:canvas_configuration:*": { "subcaseMS": 4.339 }, "webgpu:api,validation,capability_checks,features,texture_formats:canvas_configuration_view_formats:*": { "subcaseMS": 4.522 }, "webgpu:api,validation,capability_checks,features,texture_formats:check_capability_guarantees:*": { "subcaseMS": 55.901 }, @@ -542,9 +542,9 @@ "webgpu:api,validation,encoding,queries,general:occlusion_query,invalid_query_set:*": { "subcaseMS": 1.651 }, "webgpu:api,validation,encoding,queries,general:occlusion_query,query_index:*": { "subcaseMS": 0.500 }, "webgpu:api,validation,encoding,queries,general:occlusion_query,query_type:*": { "subcaseMS": 4.702 }, - "webgpu:api,validation,encoding,queries,general:timestamp_query,device_mismatch:*": { "subcaseMS": 0.101 }, - "webgpu:api,validation,encoding,queries,general:timestamp_query,invalid_query_set:*": { "subcaseMS": 0.101 }, - "webgpu:api,validation,encoding,queries,general:timestamp_query,query_type_and_index:*": { "subcaseMS": 0.301 }, + "webgpu:api,validation,encoding,queries,general:writeTimestamp,device_mismatch:*": { "subcaseMS": 0.101 }, + "webgpu:api,validation,encoding,queries,general:writeTimestamp,invalid_query_set:*": { "subcaseMS": 0.101 }, + "webgpu:api,validation,encoding,queries,general:writeTimestamp,query_type_and_index:*": { "subcaseMS": 0.301 }, "webgpu:api,validation,encoding,queries,resolveQuerySet:destination_buffer_usage:*": { "subcaseMS": 16.050 }, "webgpu:api,validation,encoding,queries,resolveQuerySet:destination_offset_alignment:*": { "subcaseMS": 0.325 }, "webgpu:api,validation,encoding,queries,resolveQuerySet:first_query_and_query_count:*": { "subcaseMS": 0.250 },