Skip to content

Commit

Permalink
Updates for writeTimestamp removal (#3167)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
kainino0x authored Nov 15, 2023
1 parent d3e4c53 commit e14e419
Show file tree
Hide file tree
Showing 6 changed files with 98 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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]))
Expand All @@ -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);
}
});
Original file line number Diff line number Diff line change
Expand Up @@ -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 =>
Expand Down Expand Up @@ -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':
Expand Down
35 changes: 28 additions & 7 deletions src/webgpu/api/validation/encoding/queries/general.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 =>
Expand All @@ -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))
Expand All @@ -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');
Expand All @@ -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);
});
8 changes: 7 additions & 1 deletion src/webgpu/api/validation/queue/destroyed/query_set.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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);
});

Expand Down
6 changes: 5 additions & 1 deletion src/webgpu/api/validation/state/device_lost/destroy.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
});
Expand Down
8 changes: 4 additions & 4 deletions src/webgpu/listing_meta.json
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
Expand Down Expand Up @@ -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 },
Expand Down

0 comments on commit e14e419

Please sign in to comment.