From 92e6d7ef4fbc29a373922bfedff8eacdf143a238 Mon Sep 17 00:00:00 2001 From: Kai Ninomiya Date: Thu, 13 Mar 2025 21:30:44 -0700 Subject: [PATCH 1/2] Add tests for mapping behavior around device.destroy() --- .../api/operation/buffers/map_detach.spec.ts | 4 + .../state/device_lost/destroy.spec.ts | 109 +++++++++++++++++- 2 files changed, 110 insertions(+), 3 deletions(-) diff --git a/src/webgpu/api/operation/buffers/map_detach.spec.ts b/src/webgpu/api/operation/buffers/map_detach.spec.ts index c7699007c845..5112b15c9de7 100644 --- a/src/webgpu/api/operation/buffers/map_detach.spec.ts +++ b/src/webgpu/api/operation/buffers/map_detach.spec.ts @@ -62,10 +62,13 @@ g.test('while_mapped') if (mapMode !== undefined) { if (mappedAtCreation) { + t.expect(buffer.mapState === 'mapped'); buffer.unmap(); } + t.expect(buffer.mapState === 'unmapped'); await buffer.mapAsync(mapMode); } + t.expect(buffer.mapState === 'mapped'); const arrayBuffer = buffer.getMappedRange(); const view = new Uint8Array(arrayBuffer); @@ -78,4 +81,5 @@ g.test('while_mapped') t.expect(arrayBuffer.byteLength === 0, 'ArrayBuffer should be detached'); t.expect(view.byteLength === 0, 'ArrayBufferView should be detached'); + t.expect(buffer.mapState === 'unmapped'); }); 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 065e842a111f..da218d807c90 100644 --- a/src/webgpu/api/validation/state/device_lost/destroy.spec.ts +++ b/src/webgpu/api/validation/state/device_lost/destroy.spec.ts @@ -21,6 +21,7 @@ import { kTextureUsageCopyInfo, kShaderStageKeys, } from '../../../../capability_info.js'; +import { GPUConst } from '../../../../constants.js'; import { getBlockInfoForColorTextureFormat, kCompressedTextureFormats, @@ -52,16 +53,15 @@ class DeviceDestroyTests extends AllFeaturesMaxLimitsValidationTest { * `fn` after the device is destroyed without any specific expectation. If `awaitLost` is true, we * also wait for device.lost to resolve before executing `fn` in the destroy case. */ - async executeAfterDestroy(fn: () => void, awaitLost: boolean): Promise { + async executeAfterDestroy(fn: () => void | Promise, awaitLost: boolean): Promise { this.expectDeviceLost('destroyed'); - this.expectValidationError(fn, false); this.device.destroy(); if (awaitLost) { const lostInfo = await this.device.lost; this.expect(lostInfo.reason === 'destroyed'); } - fn(); + await fn(); } /** @@ -146,6 +146,109 @@ Tests creating buffers on destroyed device. Tests valid combinations of: }, awaitLost); }); +g.test('mapping,mappedAtCreation') + .desc( + ` +Tests behavior of mappedAtCreation buffers when destroying the device (multiple times). +- Various usages +- Wait for .lost or not + ` + ) + .params(u => + u + .combine('usage', [ + GPUConst.BufferUsage.MAP_READ, + GPUConst.BufferUsage.MAP_WRITE, + GPUConst.BufferUsage.COPY_SRC, + ]) + .combine('awaitLost', [true, false]) + ) + .fn(async t => { + const { awaitLost, usage } = t.params; + + const b1 = t.createBufferTracked({ size: 16, usage, mappedAtCreation: true }); + t.expect(b1.mapState === 'mapped', 'b1 before destroy 1'); + + await t.executeAfterDestroy(() => { + // Destroy should have unmapped everything. + t.expect(b1.mapState === 'unmapped', 'b1 after destroy 1'); + // But unmap just in case, to reset state before continuing. + b1.unmap(); + + // mappedAtCreation should still work. + const b2 = t.createBufferTracked({ size: 16, usage, mappedAtCreation: true }); + t.expect(b2.mapState === 'mapped', 'b2 at creation after destroy 1'); + + // Destroying again should unmap. + t.device.destroy(); + t.expect(b2.mapState === 'unmapped', 'b2 after destroy 2'); + }, awaitLost); + }); + +g.test('mapping,mapAsync') + .desc( + ` +Tests behavior of mapAsync'd buffers when destroying the device. +- Various usages +- Wait for .lost or not + +TODO(https://github.com/gpuweb/gpuweb/issues/5101): Test which error we got at [1]. + ` + ) + .params(u => + u + .combine('usage', [ + GPUConst.BufferUsage.MAP_READ, + GPUConst.BufferUsage.MAP_WRITE, + GPUConst.BufferUsage.COPY_SRC, + ]) + .combine('awaitLost', [true, false]) + ) + .fn(async t => { + const { awaitLost, usage } = t.params; + + const b = t.createBufferTracked({ size: 16, usage }); + const mode = + usage === GPUBufferUsage.MAP_READ + ? GPUMapMode.READ + : usage === GPUBufferUsage.MAP_WRITE + ? GPUMapMode.WRITE + : 0; + if (mode) { + await b.mapAsync(mode); + t.expect(b.mapState === 'mapped', 'bAsync before destroy 1'); + } else { + t.expect(b.mapState === 'unmapped', 'bAsync before destroy 1'); + } + + await t.executeAfterDestroy(async () => { + // Destroy should have unmapped everything. + t.expect(b.mapState === 'unmapped', 'bAsync after destroy 1'); + // But unmap just in case, to reset state before continuing. + b.unmap(); + + // Check that mapping after destroy fails. + // Also check that if mapAsync fails validation, it still produces an OperationError. + try { + await b.mapAsync(mode); + t.expect(false, 'bAsync mapAsync after destroy 1 should reject'); + } catch (ex) { + if (mode) { + // The mapAsync call is valid except for the fact that the device is lost. + t.expect(ex instanceof DOMException && ex.name === 'AbortError'); + } else { + // The mapAsync call is also invalid for other reasons. + // [1] Test which error type we got. (And maybe switch to shouldReject().) + t.expect(ex instanceof DOMException); + } + } + const mapPromise = b.mapAsync(mode); + t.shouldReject('AbortError', mapPromise); + await mapPromise.catch(() => {}); + t.expect(b.mapState === 'unmapped', 'bAsync after mapAsync after destroy 1'); + }, awaitLost); + }); + g.test('createTexture,2d,uncompressed_format') .desc( ` From 0691911ac830b5bf6c3f772326e1a69ac51fc287 Mon Sep 17 00:00:00 2001 From: Kai Ninomiya Date: Fri, 21 Mar 2025 19:43:42 -0700 Subject: [PATCH 2/2] update to for spec 5101 (5102 still TBD) --- .../state/device_lost/destroy.spec.ts | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) 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 da218d807c90..b268b40a55be 100644 --- a/src/webgpu/api/validation/state/device_lost/destroy.spec.ts +++ b/src/webgpu/api/validation/state/device_lost/destroy.spec.ts @@ -191,8 +191,6 @@ g.test('mapping,mapAsync') Tests behavior of mapAsync'd buffers when destroying the device. - Various usages - Wait for .lost or not - -TODO(https://github.com/gpuweb/gpuweb/issues/5101): Test which error we got at [1]. ` ) .params(u => @@ -227,21 +225,7 @@ TODO(https://github.com/gpuweb/gpuweb/issues/5101): Test which error we got at [ // But unmap just in case, to reset state before continuing. b.unmap(); - // Check that mapping after destroy fails. - // Also check that if mapAsync fails validation, it still produces an OperationError. - try { - await b.mapAsync(mode); - t.expect(false, 'bAsync mapAsync after destroy 1 should reject'); - } catch (ex) { - if (mode) { - // The mapAsync call is valid except for the fact that the device is lost. - t.expect(ex instanceof DOMException && ex.name === 'AbortError'); - } else { - // The mapAsync call is also invalid for other reasons. - // [1] Test which error type we got. (And maybe switch to shouldReject().) - t.expect(ex instanceof DOMException); - } - } + // Check that mapping after destroy fails with AbortError. const mapPromise = b.mapAsync(mode); t.shouldReject('AbortError', mapPromise); await mapPromise.catch(() => {});