From 374fe04f3773c2737f1d5c3d5fd96388edbb9153 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Beaufort?= Date: Fri, 28 Mar 2025 12:48:57 +0100 Subject: [PATCH 1/3] Add tests for copyBufferToBuffer overloads --- .../command_buffer/copyBufferToBuffer.spec.ts | 39 ++++++++++++++----- 1 file changed, 30 insertions(+), 9 deletions(-) diff --git a/src/webgpu/api/operation/command_buffer/copyBufferToBuffer.spec.ts b/src/webgpu/api/operation/command_buffer/copyBufferToBuffer.spec.ts index 746d17e518ad..c8794841473d 100644 --- a/src/webgpu/api/operation/command_buffer/copyBufferToBuffer.spec.ts +++ b/src/webgpu/api/operation/command_buffer/copyBufferToBuffer.spec.ts @@ -18,11 +18,20 @@ g.test('single') ) .paramsSubcasesOnly(u => u // - .combine('srcOffset', [0, 4, 8, 16]) - .combine('dstOffset', [0, 4, 8, 16]) - .combine('copySize', [0, 4, 8, 16]) - .expand('srcBufferSize', p => [p.srcOffset + p.copySize, p.srcOffset + p.copySize + 8]) - .expand('dstBufferSize', p => [p.dstOffset + p.copySize, p.dstOffset + p.copySize + 8]) + .combine('srcOffset', [0, 4, 8, 16, undefined]) + .combine('dstOffset', [0, 4, 8, 16, undefined]) + .unless( + p => (p.srcOffset === undefined || p.dstOffset === undefined) && p.srcOffset !== p.dstOffset + ) + .combine('copySize', [0, 4, 8, 16, undefined]) + .expand('srcBufferSize', p => [ + p.srcOffset ?? 0 + (p.copySize ?? 0), + p.srcOffset ?? 0 + (p.copySize ?? 0) + 8, + ]) + .expand('dstBufferSize', p => [ + p.dstOffset ?? 0 + (p.copySize ?? 0), + p.dstOffset ?? 0 + (p.copySize ?? 0) + 8, + ]) ) .fn(t => { const { srcOffset, dstOffset, copySize, srcBufferSize, dstBufferSize } = t.params; @@ -40,12 +49,24 @@ g.test('single') }); const encoder = t.device.createCommandEncoder(); - encoder.copyBufferToBuffer(src, srcOffset, dst, dstOffset, copySize); - t.device.queue.submit([encoder.finish()]); + if (srcOffset === undefined || dstOffset === undefined) { + encoder.copyBufferToBuffer(src, dst, copySize); + } else { + encoder.copyBufferToBuffer(src, srcOffset, dst, dstOffset, copySize); + } + + const expectedSrcOffset = srcOffset ?? 0; + const expectedDstOffset = dstOffset ?? 0; + const expectedCopySize = copySize ?? srcBufferSize - expectedSrcOffset; + + const isValid = dstBufferSize - expectedDstOffset >= expectedCopySize; + t.expectValidationError(() => { + t.device.queue.submit([encoder.finish()]); + }, !isValid); const expectedDstData = new Uint8Array(dstBufferSize); - for (let i = 0; i < copySize; ++i) { - expectedDstData[dstOffset + i] = srcData[srcOffset + i]; + for (let i = 0; i < expectedCopySize; ++i) { + expectedDstData[expectedDstOffset + i] = srcData[expectedSrcOffset + i]; } t.expectGPUBufferValuesEqual(dst, expectedDstData); From 4805520fcfb84e3636ba91fff370292403c8b7a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Beaufort?= Date: Mon, 31 Mar 2025 10:38:37 +0200 Subject: [PATCH 2/3] Update types + nit --- package-lock.json | 17 +++++++++-------- package.json | 2 +- .../command_buffer/copyBufferToBuffer.spec.ts | 8 ++++---- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/package-lock.json b/package-lock.json index 6bb6ff409512..5d96d2c6fe12 100644 --- a/package-lock.json +++ b/package-lock.json @@ -24,7 +24,7 @@ "@types/w3c-image-capture": "^1.0.10", "@typescript-eslint/eslint-plugin": "^6.9.1", "@typescript-eslint/parser": "^6.9.1", - "@webgpu/types": "^0.1.59", + "@webgpu/types": "^0.1.60", "ansi-colors": "4.1.3", "babel-plugin-add-header-comment": "^1.0.3", "babel-plugin-const-enum": "^1.2.0", @@ -1539,10 +1539,11 @@ "dev": true }, "node_modules/@webgpu/types": { - "version": "0.1.59", - "resolved": "https://registry.npmjs.org/@webgpu/types/-/types-0.1.59.tgz", - "integrity": "sha512-jZJ6ipNli+rn++/GAPqsZXfsgjx951wlCW7vNAg+oGdp0ZYidTOkbVTVeK2frzowuD5ch7MRz7leOEX1PMv43A==", - "dev": true + "version": "0.1.60", + "resolved": "https://registry.npmjs.org/@webgpu/types/-/types-0.1.60.tgz", + "integrity": "sha512-8B/tdfRFKdrnejqmvq95ogp8tf52oZ51p3f4QD5m5Paey/qlX4Rhhy5Y8tgFMi7Ms70HzcMMw3EQjH/jdhTwlA==", + "dev": true, + "license": "BSD-3-Clause" }, "node_modules/abbrev": { "version": "1.1.1", @@ -10076,9 +10077,9 @@ "dev": true }, "@webgpu/types": { - "version": "0.1.59", - "resolved": "https://registry.npmjs.org/@webgpu/types/-/types-0.1.59.tgz", - "integrity": "sha512-jZJ6ipNli+rn++/GAPqsZXfsgjx951wlCW7vNAg+oGdp0ZYidTOkbVTVeK2frzowuD5ch7MRz7leOEX1PMv43A==", + "version": "0.1.60", + "resolved": "https://registry.npmjs.org/@webgpu/types/-/types-0.1.60.tgz", + "integrity": "sha512-8B/tdfRFKdrnejqmvq95ogp8tf52oZ51p3f4QD5m5Paey/qlX4Rhhy5Y8tgFMi7Ms70HzcMMw3EQjH/jdhTwlA==", "dev": true }, "abbrev": { diff --git a/package.json b/package.json index 3192733b4ceb..426c1239e35a 100644 --- a/package.json +++ b/package.json @@ -50,7 +50,7 @@ "@types/w3c-image-capture": "^1.0.10", "@typescript-eslint/eslint-plugin": "^6.9.1", "@typescript-eslint/parser": "^6.9.1", - "@webgpu/types": "^0.1.59", + "@webgpu/types": "^0.1.60", "ansi-colors": "4.1.3", "babel-plugin-add-header-comment": "^1.0.3", "babel-plugin-const-enum": "^1.2.0", diff --git a/src/webgpu/api/operation/command_buffer/copyBufferToBuffer.spec.ts b/src/webgpu/api/operation/command_buffer/copyBufferToBuffer.spec.ts index c8794841473d..ef83ffa129dd 100644 --- a/src/webgpu/api/operation/command_buffer/copyBufferToBuffer.spec.ts +++ b/src/webgpu/api/operation/command_buffer/copyBufferToBuffer.spec.ts @@ -25,12 +25,12 @@ g.test('single') ) .combine('copySize', [0, 4, 8, 16, undefined]) .expand('srcBufferSize', p => [ - p.srcOffset ?? 0 + (p.copySize ?? 0), - p.srcOffset ?? 0 + (p.copySize ?? 0) + 8, + (p.srcOffset ?? 0) + (p.copySize ?? 0), + (p.srcOffset ?? 0) + (p.copySize ?? 0) + 8, ]) .expand('dstBufferSize', p => [ - p.dstOffset ?? 0 + (p.copySize ?? 0), - p.dstOffset ?? 0 + (p.copySize ?? 0) + 8, + (p.dstOffset ?? 0) + (p.copySize ?? 0), + (p.dstOffset ?? 0) + (p.copySize ?? 0) + 8, ]) ) .fn(t => { From aada5f9136fddcc4943460fb922dd0ce9075b611 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Beaufort?= Date: Wed, 2 Apr 2025 08:36:35 +0200 Subject: [PATCH 3/3] Address feedback --- .../command_buffer/copyBufferToBuffer.spec.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/webgpu/api/operation/command_buffer/copyBufferToBuffer.spec.ts b/src/webgpu/api/operation/command_buffer/copyBufferToBuffer.spec.ts index ef83ffa129dd..0fc202a8a145 100644 --- a/src/webgpu/api/operation/command_buffer/copyBufferToBuffer.spec.ts +++ b/src/webgpu/api/operation/command_buffer/copyBufferToBuffer.spec.ts @@ -1,6 +1,7 @@ export const description = 'copyBufferToBuffer operation tests'; import { makeTestGroup } from '../../../../common/framework/test_group.js'; +import { assert } from '../../../../common/util/util.js'; import { AllFeaturesMaxLimitsGPUTest } from '../../../gpu_test.js'; export const g = makeTestGroup(AllFeaturesMaxLimitsGPUTest); @@ -16,8 +17,11 @@ g.test('single') - covers the end of the dstBuffer - covers neither the beginning nor the end of the dstBuffer` ) - .paramsSubcasesOnly(u => + .params(u => u // + // Whether the test case requires the newly-added method signature or not. + .combine('newSig', [false, true]) + .beginSubcases() .combine('srcOffset', [0, 4, 8, 16, undefined]) .combine('dstOffset', [0, 4, 8, 16, undefined]) .unless( @@ -32,6 +36,11 @@ g.test('single') (p.dstOffset ?? 0) + (p.copySize ?? 0), (p.dstOffset ?? 0) + (p.copySize ?? 0) + 8, ]) + // Bifurcate the cases between newSig=false and newSig=true based on whether they need it. + .filter(p => { + const needsNewSignature = [p.srcOffset, p.dstOffset, p.copySize].includes(undefined); + return p.newSig === needsNewSignature; + }) ) .fn(t => { const { srcOffset, dstOffset, copySize, srcBufferSize, dstBufferSize } = t.params; @@ -50,6 +59,7 @@ g.test('single') const encoder = t.device.createCommandEncoder(); if (srcOffset === undefined || dstOffset === undefined) { + assert(srcOffset === undefined && dstOffset === undefined); encoder.copyBufferToBuffer(src, dst, copySize); } else { encoder.copyBufferToBuffer(src, srcOffset, dst, dstOffset, copySize);