From 6ac148a71ac65ab3d3bebad3c74402544754c8fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Beaufort?= Date: Tue, 24 Sep 2024 15:06:11 +0200 Subject: [PATCH 1/8] Add validation tests for Multi-Draw Indirect --- .../cmds/render/indirect_multi_draw.spec.ts | 427 ++++++++++++++++++ .../encoding/cmds/render/types.d.ts | 17 + 2 files changed, 444 insertions(+) create mode 100644 src/webgpu/api/validation/encoding/cmds/render/indirect_multi_draw.spec.ts create mode 100644 src/webgpu/api/validation/encoding/cmds/render/types.d.ts diff --git a/src/webgpu/api/validation/encoding/cmds/render/indirect_multi_draw.spec.ts b/src/webgpu/api/validation/encoding/cmds/render/indirect_multi_draw.spec.ts new file mode 100644 index 000000000000..0cdb48cbe2a4 --- /dev/null +++ b/src/webgpu/api/validation/encoding/cmds/render/indirect_multi_draw.spec.ts @@ -0,0 +1,427 @@ +export const description = ` +Validation tests for multiDrawIndirect/multiDrawIndexedIndirect on render pass. +`; + +import { kUnitCaseParamsBuilder } from '../../../../../../common/framework/params_builder.js'; +import { makeTestGroup } from '../../../../../../common/framework/test_group.js'; +import { + GPUConst, + kMaxUnsignedLongValue, + kMaxUnsignedLongLongValue, +} from '../../../../../constants.js'; +import { kResourceStates } from '../../../../../gpu_test.js'; +import { ValidationTest } from '../../../validation_test.js'; + +const kIndirectMultiDrawTestParams = kUnitCaseParamsBuilder + .combine('indexed', [true, false] as const) + .combine('useDrawCountBuffer', [true, false] as const); + +class F extends ValidationTest { + makeIndexBuffer(): GPUBuffer { + return this.createBufferTracked({ + size: 16, + usage: GPUBufferUsage.INDEX, + }); + } +} + +export const g = makeTestGroup(F); + +g.test('buffers_state') + .desc( + ` +Tests indirect and draw count buffers must be valid. + ` + ) + .beforeAllSubcases(t => { + t.selectDeviceOrSkipTestCase('chromium-experimental-multi-draw-indirect' as GPUFeatureName); + }) + + .paramsSubcasesOnly( + kIndirectMultiDrawTestParams + .combine('indirectState', kResourceStates) + .combine('drawCountState', kResourceStates) + ) + .fn(t => { + const { indexed, indirectState, useDrawCountBuffer, drawCountState } = t.params; + const indirectBuffer = t.createBufferWithState(indirectState, { + size: 256, + usage: GPUBufferUsage.INDIRECT, + }); + const drawCountBuffer = useDrawCountBuffer + ? t.createBufferWithState(drawCountState, { + size: 256, + usage: GPUBufferUsage.INDIRECT, + }) + : undefined; + + const { encoder, validateFinishAndSubmit } = t.createEncoder('render pass'); + encoder.setPipeline(t.createNoOpRenderPipeline()); + if (indexed) { + encoder.setIndexBuffer(t.makeIndexBuffer(), 'uint32'); + encoder.multiDrawIndexedIndirect(indirectBuffer, 0, 1, drawCountBuffer); + } else { + encoder.multiDrawIndirect(indirectBuffer, 0, 1, drawCountBuffer); + } + + const shouldBeValid = + indirectState !== 'invalid' && (!drawCountBuffer || drawCountState !== 'invalid'); + const submitShouldSucceedIfValid = + indirectState !== 'destroyed' && (!drawCountBuffer || drawCountState !== 'destroyed'); + validateFinishAndSubmit(shouldBeValid, submitShouldSucceedIfValid); + }); + +g.test('buffers,device_mismatch') + .desc( + 'Tests multiDraw(Indexed)Indirect cannot be called with buffers created from another device' + ) + .paramsSubcasesOnly(kIndirectMultiDrawTestParams.combine('mismatched', [true, false])) + .beforeAllSubcases(t => { + t.selectDeviceOrSkipTestCase('chromium-experimental-multi-draw-indirect' as GPUFeatureName); + t.selectMismatchedDeviceOrSkipTestCase(undefined); + }) + .fn(t => { + const { indexed, useDrawCountBuffer, mismatched } = t.params; + + const sourceDevice = mismatched ? t.mismatchedDevice : t.device; + + const indirectBuffer = t.trackForCleanup( + sourceDevice.createBuffer({ + size: 256, + usage: GPUBufferUsage.INDIRECT, + }) + ); + const drawCountBuffer = useDrawCountBuffer + ? t.trackForCleanup( + sourceDevice.createBuffer({ + size: 256, + usage: GPUBufferUsage.INDIRECT, + }) + ) + : undefined; + + const { encoder, validateFinish } = t.createEncoder('render pass'); + encoder.setPipeline(t.createNoOpRenderPipeline()); + if (indexed) { + encoder.setIndexBuffer(t.makeIndexBuffer(), 'uint32'); + encoder.multiDrawIndexedIndirect(indirectBuffer, 0, 1, drawCountBuffer); + } else { + encoder.multiDrawIndirect(indirectBuffer, 0, 1, drawCountBuffer); + } + validateFinish(!mismatched); + }); + +g.test('indirect_buffer_usage') + .desc( + ` +Tests indirect and draw count buffers must have 'Indirect' usage. + ` + ) + .beforeAllSubcases(t => { + t.selectDeviceOrSkipTestCase('chromium-experimental-multi-draw-indirect' as GPUFeatureName); + }) + .paramsSubcasesOnly( + kIndirectMultiDrawTestParams + .combine('indirectUsage', [ + GPUConst.BufferUsage.INDIRECT, + GPUConst.BufferUsage.VERTEX, + GPUConst.BufferUsage.VERTEX | GPUConst.BufferUsage.INDIRECT, + ] as const) + .combine('drawCountUsage', [ + GPUConst.BufferUsage.INDIRECT, + GPUConst.BufferUsage.VERTEX, + GPUConst.BufferUsage.VERTEX | GPUConst.BufferUsage.INDIRECT, + ] as const) + ) + .fn(t => { + const { indexed, indirectUsage, useDrawCountBuffer, drawCountUsage } = t.params; + + const indirectBuffer = t.createBufferTracked({ + size: 256, + usage: indirectUsage, + }); + const drawCountBuffer = useDrawCountBuffer + ? t.createBufferTracked({ + size: 256, + usage: drawCountUsage, + }) + : undefined; + + const { encoder, validateFinish } = t.createEncoder('render pass'); + encoder.setPipeline(t.createNoOpRenderPipeline()); + if (indexed) { + encoder.setIndexBuffer(t.makeIndexBuffer(), 'uint32'); + encoder.multiDrawIndexedIndirect(indirectBuffer, 0, 1, drawCountBuffer); + } else { + encoder.multiDrawIndirect(indirectBuffer, 0, 1, drawCountBuffer); + } + const shouldSucceed = + (indirectUsage & GPUBufferUsage.INDIRECT) !== 0 && + (!drawCountBuffer || drawCountUsage & GPUBufferUsage.INDIRECT) !== 0; + validateFinish(shouldSucceed); + }); + +g.test('offsets_alignment') + .desc( + ` +Tests indirect and draw count offsets must be a multiple of 4. + ` + ) + .beforeAllSubcases(t => { + t.selectDeviceOrSkipTestCase('chromium-experimental-multi-draw-indirect' as GPUFeatureName); + }) + .paramsSubcasesOnly( + kIndirectMultiDrawTestParams + .combine('indirectOffset', [0, 2, 4] as const) + .combine('drawCountOffset', [0, 2, 4] as const) + ) + .fn(t => { + const { indexed, indirectOffset, useDrawCountBuffer, drawCountOffset } = t.params; + + const indirectBuffer = t.createBufferTracked({ + size: 256, + usage: GPUBufferUsage.INDIRECT, + }); + const drawCountBuffer = useDrawCountBuffer + ? t.createBufferTracked({ + size: 256, + usage: GPUBufferUsage.INDIRECT, + }) + : undefined; + + const { encoder, validateFinish } = t.createEncoder('render pass'); + encoder.setPipeline(t.createNoOpRenderPipeline()); + if (indexed) { + encoder.setIndexBuffer(t.makeIndexBuffer(), 'uint32'); + encoder.multiDrawIndexedIndirect( + indirectBuffer, + indirectOffset, + 1, + drawCountBuffer, + drawCountOffset + ); + } else { + encoder.multiDrawIndirect( + indirectBuffer, + indirectOffset, + 1, + drawCountBuffer, + drawCountOffset + ); + } + + // We need to figure out if https://github.com/gpuweb/gpuweb/pull/2315/files#r1773031950 applies. + validateFinish(indirectOffset % 4 === 0 && (!useDrawCountBuffer || drawCountOffset % 4 === 0)); + }); + +g.test('indirect_offset_oob') + .desc( + ` +Tests multi indirect draw calls with various offsets and buffer sizes. + ` + ) + .beforeAllSubcases(t => { + t.selectDeviceOrSkipTestCase('chromium-experimental-multi-draw-indirect' as GPUFeatureName); + }) + .params(u => + u.expandWithParams(() => { + return [ + // In bounds + { bufferSize: 4, indirectOffset: 0, _valid: true }, + // In bounds, bigger buffer + { bufferSize: 7, indirectOffset: 0, _valid: true }, + // In bounds, bigger buffer, positive offset + { bufferSize: 8, indirectOffset: 4, _valid: true }, + // In bounds for maxDrawCount + { bufferSize: 8, indirectOffset: 0, maxDrawCount: 2, _valid: true }, + // In bounds with drawCountBuffer + { bufferSize: 4, indirectOffset: 0, useDrawCountBuffer: true, _valid: true }, + // In bounds with drawCountBuffer, bigger buffer + { + bufferSize: 7, + indirectOffset: 0, + drawCountOffset: 6 * Uint32Array.BYTES_PER_ELEMENT, + useDrawCountBuffer: true, + _valid: true, + }, + // In bounds, non-multiple of 4 offsets + { bufferSize: 5, indirectOffset: 1, _valid: false }, + { bufferSize: 5, indirectOffset: 2, _valid: false }, + { + bufferSize: 5, + indirectOffset: 0, + drawCountOffset: 1, + useDrawCountBuffer: true, + _valid: false, + }, + { + bufferSize: 5, + indirectOffset: 0, + drawCountOffset: 2, + useDrawCountBuffer: true, + _valid: false, + }, + // Out of bounds, buffer too small + { bufferSize: 3, indirectOffset: 0, _valid: false }, + // Out of bounds, index too big + { bufferSize: 4, indirectOffset: 1 * Uint32Array.BYTES_PER_ELEMENT, _valid: false }, + // Out of bounds, index past buffer + { bufferSize: 4, indirectOffset: 5 * Uint32Array.BYTES_PER_ELEMENT, _valid: false }, + // Out of bounds, too small for maxDrawCount + { bufferSize: 7, indirectOffset: 0, drawCountOffset: 0, maxDrawCount: 2, _valid: false }, + // Out of bounds, offset too big for drawCountBuffer + { + bufferSize: 4, + indirectOffset: 0, + drawCountOffset: 4 * Uint32Array.BYTES_PER_ELEMENT, + useDrawCountBuffer: true, + _valid: false, + }, + // Out of bounds, index + size of command overflows + // uint64_t offset = std::numeric_limits::max(); + { bufferSize: 7, indirectOffset: kMaxUnsignedLongLongValue, _valid: false }, + // Out of bounds, index + size of command overflows with drawCountBuffer + { + bufferSize: 7, + indirectOffset: 0, + drawCountOffset: kMaxUnsignedLongLongValue, + useDrawCountBuffer: true, + _valid: false, + }, + // Out of bounds, maxDrawCount = kMaxUnsignedLongValue + { bufferSize: 7, indirectOffset: 0, maxDrawCount: kMaxUnsignedLongValue, _valid: false }, + ]; + }) + ) + .fn(t => { + const { + bufferSize, + indirectOffset, + drawCountOffset = 0, + maxDrawCount = 1, + useDrawCountBuffer = false, + _valid, + } = t.params; + + const indirectBuffer = t.createBufferTracked({ + size: bufferSize * Uint32Array.BYTES_PER_ELEMENT, + usage: GPUBufferUsage.INDIRECT, + }); + + const { encoder, validateFinish } = t.createEncoder('render pass'); + encoder.setPipeline(t.createNoOpRenderPipeline()); + encoder.multiDrawIndirect( + indirectBuffer, + indirectOffset, + maxDrawCount, + useDrawCountBuffer ? indirectBuffer : undefined, + drawCountOffset + ); + + validateFinish(_valid); + }); + +g.test('indexed_indirect_offset_oob') + .desc( + ` +Tests multi indexed indirect draw calls with various offsets and buffer sizes. + ` + ) + .beforeAllSubcases(t => { + t.selectDeviceOrSkipTestCase('chromium-experimental-multi-draw-indirect' as GPUFeatureName); + }) + .params(u => + u.expandWithParams(() => { + return [ + // In bounds + { bufferSize: 5, indirectOffset: 0, _valid: true }, + // In bounds, bigger buffer + { bufferSize: 9, indirectOffset: 0, _valid: true }, + // In bounds, bigger buffer, positive offset + { bufferSize: 10, indirectOffset: 5 * Uint32Array.BYTES_PER_ELEMENT, _valid: true }, + // In bounds with drawCountBuffer + { bufferSize: 5, indirectOffset: 0, useDrawCountBuffer: true, _valid: true }, + // In bounds with drawCountBuffer, bigger buffer + { + bufferSize: 6, + indirectOffset: 0, + drawCountOffset: 5 * Uint32Array.BYTES_PER_ELEMENT, + useDrawCountBuffer: true, + _valid: true, + }, + // In bounds, non-multiple of 4 offsets + { bufferSize: 6, indirectOffset: 1, _valid: false }, + { bufferSize: 6, indirectOffset: 2, _valid: false }, + { + bufferSize: 6, + indirectOffset: 0, + drawCountOffset: 1, + useDrawCountBuffer: true, + _valid: false, + }, + { + bufferSize: 6, + indirectOffset: 0, + drawCountOffset: 2, + useDrawCountBuffer: true, + _valid: false, + }, + // Out of bounds, buffer too small + { bufferSize: 4, indirectOffset: 0, _valid: false }, + // Out of bounds, index too big + { bufferSize: 5, indirectOffset: 1 * Uint32Array.BYTES_PER_ELEMENT, _valid: false }, + // Out of bounds, index past buffer + { bufferSize: 5, indirectOffset: 5 * Uint32Array.BYTES_PER_ELEMENT, _valid: false }, + // Out of bounds, too small for maxDrawCount + { bufferSize: 5, indirectOffset: 0, drawCountOffset: 0, maxDrawCount: 2, _valid: false }, + // Out of bounds, offset too big for drawCountBuffer + { + bufferSize: 5, + indirectOffset: 0, + drawCountOffset: 5 * Uint32Array.BYTES_PER_ELEMENT, + useDrawCountBuffer: true, + _valid: false, + }, + // Out of bounds, index + size of command overflows + { bufferSize: 10, indirectOffset: kMaxUnsignedLongLongValue, _valid: false }, + // Out of bounds, index + size of command overflows with drawCountBuffer + { + bufferSize: 10, + indirectOffset: 0, + drawCountOffset: kMaxUnsignedLongLongValue, + useDrawCountBuffer: true, + _valid: false, + }, + // Out of bounds, maxDrawCount = kMaxUnsignedLongValue + { bufferSize: 5, indirectOffset: 0, maxDrawCount: kMaxUnsignedLongValue, _valid: false }, + ]; + }) + ) + .fn(t => { + const { + bufferSize, + indirectOffset, + drawCountOffset = 0, + maxDrawCount = 1, + useDrawCountBuffer = false, + _valid, + } = t.params; + + const indirectBuffer = t.createBufferTracked({ + size: bufferSize * Uint32Array.BYTES_PER_ELEMENT, + usage: GPUBufferUsage.INDIRECT, + }); + + const { encoder, validateFinish } = t.createEncoder('render pass'); + encoder.setPipeline(t.createNoOpRenderPipeline()); + encoder.setIndexBuffer(t.makeIndexBuffer(), 'uint32'); + encoder.multiDrawIndexedIndirect( + indirectBuffer, + indirectOffset, + maxDrawCount, + useDrawCountBuffer ? indirectBuffer : undefined, + drawCountOffset + ); + + validateFinish(_valid); + }); diff --git a/src/webgpu/api/validation/encoding/cmds/render/types.d.ts b/src/webgpu/api/validation/encoding/cmds/render/types.d.ts new file mode 100644 index 000000000000..7aed233b8c5d --- /dev/null +++ b/src/webgpu/api/validation/encoding/cmds/render/types.d.ts @@ -0,0 +1,17 @@ +// This can be removed once https://github.com/gpuweb/gpuweb/pull/2315 is merged. +interface GPURenderCommandsMixin { + multiDrawIndirect( + indirectBuffer: GPUBuffer, + indirectOffset: GPUSize64, + maxDrawCount: GPUSize32, + drawCountBuffer?: GPUBuffer, + drawCountOffset?: GPUSize64 + ): undefined; + multiDrawIndexedIndirect( + indirectBuffer: GPUBuffer, + indirectOffset: GPUSize64, + maxDrawCount: GPUSize32, + drawCountBuffer?: GPUBuffer, + drawCountOffset?: GPUSize64 + ): undefined; +} From 82789180a7ff0bafd221e9043ae9a02a0c4e8dfa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Beaufort?= Date: Wed, 6 Nov 2024 11:15:33 +0100 Subject: [PATCH 2/8] Address first pass of feedback --- .../cmds/render/indirect_multi_draw.spec.ts | 262 +++++++++--------- 1 file changed, 129 insertions(+), 133 deletions(-) diff --git a/src/webgpu/api/validation/encoding/cmds/render/indirect_multi_draw.spec.ts b/src/webgpu/api/validation/encoding/cmds/render/indirect_multi_draw.spec.ts index 0cdb48cbe2a4..65741b7bf5bb 100644 --- a/src/webgpu/api/validation/encoding/cmds/render/indirect_multi_draw.spec.ts +++ b/src/webgpu/api/validation/encoding/cmds/render/indirect_multi_draw.spec.ts @@ -224,74 +224,72 @@ Tests multi indirect draw calls with various offsets and buffer sizes. t.selectDeviceOrSkipTestCase('chromium-experimental-multi-draw-indirect' as GPUFeatureName); }) .params(u => - u.expandWithParams(() => { - return [ - // In bounds - { bufferSize: 4, indirectOffset: 0, _valid: true }, - // In bounds, bigger buffer - { bufferSize: 7, indirectOffset: 0, _valid: true }, - // In bounds, bigger buffer, positive offset - { bufferSize: 8, indirectOffset: 4, _valid: true }, - // In bounds for maxDrawCount - { bufferSize: 8, indirectOffset: 0, maxDrawCount: 2, _valid: true }, - // In bounds with drawCountBuffer - { bufferSize: 4, indirectOffset: 0, useDrawCountBuffer: true, _valid: true }, - // In bounds with drawCountBuffer, bigger buffer - { - bufferSize: 7, - indirectOffset: 0, - drawCountOffset: 6 * Uint32Array.BYTES_PER_ELEMENT, - useDrawCountBuffer: true, - _valid: true, - }, - // In bounds, non-multiple of 4 offsets - { bufferSize: 5, indirectOffset: 1, _valid: false }, - { bufferSize: 5, indirectOffset: 2, _valid: false }, - { - bufferSize: 5, - indirectOffset: 0, - drawCountOffset: 1, - useDrawCountBuffer: true, - _valid: false, - }, - { - bufferSize: 5, - indirectOffset: 0, - drawCountOffset: 2, - useDrawCountBuffer: true, - _valid: false, - }, - // Out of bounds, buffer too small - { bufferSize: 3, indirectOffset: 0, _valid: false }, - // Out of bounds, index too big - { bufferSize: 4, indirectOffset: 1 * Uint32Array.BYTES_PER_ELEMENT, _valid: false }, - // Out of bounds, index past buffer - { bufferSize: 4, indirectOffset: 5 * Uint32Array.BYTES_PER_ELEMENT, _valid: false }, - // Out of bounds, too small for maxDrawCount - { bufferSize: 7, indirectOffset: 0, drawCountOffset: 0, maxDrawCount: 2, _valid: false }, - // Out of bounds, offset too big for drawCountBuffer - { - bufferSize: 4, - indirectOffset: 0, - drawCountOffset: 4 * Uint32Array.BYTES_PER_ELEMENT, - useDrawCountBuffer: true, - _valid: false, - }, - // Out of bounds, index + size of command overflows - // uint64_t offset = std::numeric_limits::max(); - { bufferSize: 7, indirectOffset: kMaxUnsignedLongLongValue, _valid: false }, - // Out of bounds, index + size of command overflows with drawCountBuffer - { - bufferSize: 7, - indirectOffset: 0, - drawCountOffset: kMaxUnsignedLongLongValue, - useDrawCountBuffer: true, - _valid: false, - }, - // Out of bounds, maxDrawCount = kMaxUnsignedLongValue - { bufferSize: 7, indirectOffset: 0, maxDrawCount: kMaxUnsignedLongValue, _valid: false }, - ]; - }) + u.combineWithParams([ + // In bounds + { bufferSize: 4, indirectOffset: 0, _valid: true }, + // In bounds, bigger buffer + { bufferSize: 7, indirectOffset: 0, _valid: true }, + // In bounds, bigger buffer, positive offset + { bufferSize: 8, indirectOffset: 4, _valid: true }, + // In bounds for maxDrawCount + { bufferSize: 8, indirectOffset: 0, maxDrawCount: 2, _valid: true }, + // In bounds with drawCountBuffer + { bufferSize: 4, indirectOffset: 0, useDrawCountBuffer: true, _valid: true }, + // In bounds with drawCountBuffer, bigger buffer + { + bufferSize: 7, + indirectOffset: 0, + drawCountOffset: 6 * Uint32Array.BYTES_PER_ELEMENT, + useDrawCountBuffer: true, + _valid: true, + }, + // In bounds, non-multiple of 4 offsets + { bufferSize: 5, indirectOffset: 1, _valid: false }, + { bufferSize: 5, indirectOffset: 2, _valid: false }, + { + bufferSize: 5, + indirectOffset: 0, + drawCountOffset: 1, + useDrawCountBuffer: true, + _valid: false, + }, + { + bufferSize: 5, + indirectOffset: 0, + drawCountOffset: 2, + useDrawCountBuffer: true, + _valid: false, + }, + // Out of bounds, buffer too small + { bufferSize: 3, indirectOffset: 0, _valid: false }, + // Out of bounds, index too big + { bufferSize: 4, indirectOffset: 1 * Uint32Array.BYTES_PER_ELEMENT, _valid: false }, + // Out of bounds, index past buffer + { bufferSize: 4, indirectOffset: 5 * Uint32Array.BYTES_PER_ELEMENT, _valid: false }, + // Out of bounds, too small for maxDrawCount + { bufferSize: 7, indirectOffset: 0, drawCountOffset: 0, maxDrawCount: 2, _valid: false }, + // Out of bounds, offset too big for drawCountBuffer + { + bufferSize: 4, + indirectOffset: 0, + drawCountOffset: 4 * Uint32Array.BYTES_PER_ELEMENT, + useDrawCountBuffer: true, + _valid: false, + }, + // Out of bounds, index + size of command overflows + // uint64_t offset = std::numeric_limits::max(); + { bufferSize: 7, indirectOffset: kMaxUnsignedLongLongValue, _valid: false }, + // Out of bounds, index + size of command overflows with drawCountBuffer + { + bufferSize: 7, + indirectOffset: 0, + drawCountOffset: kMaxUnsignedLongLongValue, + useDrawCountBuffer: true, + _valid: false, + }, + // Out of bounds, maxDrawCount = kMaxUnsignedLongValue + { bufferSize: 7, indirectOffset: 0, maxDrawCount: kMaxUnsignedLongValue, _valid: false }, + ]) ) .fn(t => { const { @@ -331,71 +329,69 @@ Tests multi indexed indirect draw calls with various offsets and buffer sizes. t.selectDeviceOrSkipTestCase('chromium-experimental-multi-draw-indirect' as GPUFeatureName); }) .params(u => - u.expandWithParams(() => { - return [ - // In bounds - { bufferSize: 5, indirectOffset: 0, _valid: true }, - // In bounds, bigger buffer - { bufferSize: 9, indirectOffset: 0, _valid: true }, - // In bounds, bigger buffer, positive offset - { bufferSize: 10, indirectOffset: 5 * Uint32Array.BYTES_PER_ELEMENT, _valid: true }, - // In bounds with drawCountBuffer - { bufferSize: 5, indirectOffset: 0, useDrawCountBuffer: true, _valid: true }, - // In bounds with drawCountBuffer, bigger buffer - { - bufferSize: 6, - indirectOffset: 0, - drawCountOffset: 5 * Uint32Array.BYTES_PER_ELEMENT, - useDrawCountBuffer: true, - _valid: true, - }, - // In bounds, non-multiple of 4 offsets - { bufferSize: 6, indirectOffset: 1, _valid: false }, - { bufferSize: 6, indirectOffset: 2, _valid: false }, - { - bufferSize: 6, - indirectOffset: 0, - drawCountOffset: 1, - useDrawCountBuffer: true, - _valid: false, - }, - { - bufferSize: 6, - indirectOffset: 0, - drawCountOffset: 2, - useDrawCountBuffer: true, - _valid: false, - }, - // Out of bounds, buffer too small - { bufferSize: 4, indirectOffset: 0, _valid: false }, - // Out of bounds, index too big - { bufferSize: 5, indirectOffset: 1 * Uint32Array.BYTES_PER_ELEMENT, _valid: false }, - // Out of bounds, index past buffer - { bufferSize: 5, indirectOffset: 5 * Uint32Array.BYTES_PER_ELEMENT, _valid: false }, - // Out of bounds, too small for maxDrawCount - { bufferSize: 5, indirectOffset: 0, drawCountOffset: 0, maxDrawCount: 2, _valid: false }, - // Out of bounds, offset too big for drawCountBuffer - { - bufferSize: 5, - indirectOffset: 0, - drawCountOffset: 5 * Uint32Array.BYTES_PER_ELEMENT, - useDrawCountBuffer: true, - _valid: false, - }, - // Out of bounds, index + size of command overflows - { bufferSize: 10, indirectOffset: kMaxUnsignedLongLongValue, _valid: false }, - // Out of bounds, index + size of command overflows with drawCountBuffer - { - bufferSize: 10, - indirectOffset: 0, - drawCountOffset: kMaxUnsignedLongLongValue, - useDrawCountBuffer: true, - _valid: false, - }, - // Out of bounds, maxDrawCount = kMaxUnsignedLongValue - { bufferSize: 5, indirectOffset: 0, maxDrawCount: kMaxUnsignedLongValue, _valid: false }, - ]; - }) + u.combineWithParams([ + // In bounds + { bufferSize: 5, indirectOffset: 0, _valid: true }, + // In bounds, bigger buffer + { bufferSize: 9, indirectOffset: 0, _valid: true }, + // In bounds, bigger buffer, positive offset + { bufferSize: 10, indirectOffset: 5 * Uint32Array.BYTES_PER_ELEMENT, _valid: true }, + // In bounds with drawCountBuffer + { bufferSize: 5, indirectOffset: 0, useDrawCountBuffer: true, _valid: true }, + // In bounds with drawCountBuffer, bigger buffer + { + bufferSize: 6, + indirectOffset: 0, + drawCountOffset: 5 * Uint32Array.BYTES_PER_ELEMENT, + useDrawCountBuffer: true, + _valid: true, + }, + // In bounds, non-multiple of 4 offsets + { bufferSize: 6, indirectOffset: 1, _valid: false }, + { bufferSize: 6, indirectOffset: 2, _valid: false }, + { + bufferSize: 6, + indirectOffset: 0, + drawCountOffset: 1, + useDrawCountBuffer: true, + _valid: false, + }, + { + bufferSize: 6, + indirectOffset: 0, + drawCountOffset: 2, + useDrawCountBuffer: true, + _valid: false, + }, + // Out of bounds, buffer too small + { bufferSize: 4, indirectOffset: 0, _valid: false }, + // Out of bounds, index too big + { bufferSize: 5, indirectOffset: 1 * Uint32Array.BYTES_PER_ELEMENT, _valid: false }, + // Out of bounds, index past buffer + { bufferSize: 5, indirectOffset: 5 * Uint32Array.BYTES_PER_ELEMENT, _valid: false }, + // Out of bounds, too small for maxDrawCount + { bufferSize: 5, indirectOffset: 0, drawCountOffset: 0, maxDrawCount: 2, _valid: false }, + // Out of bounds, offset too big for drawCountBuffer + { + bufferSize: 5, + indirectOffset: 0, + drawCountOffset: 5 * Uint32Array.BYTES_PER_ELEMENT, + useDrawCountBuffer: true, + _valid: false, + }, + // Out of bounds, index + size of command overflows + { bufferSize: 10, indirectOffset: kMaxUnsignedLongLongValue, _valid: false }, + // Out of bounds, index + size of command overflows with drawCountBuffer + { + bufferSize: 10, + indirectOffset: 0, + drawCountOffset: kMaxUnsignedLongLongValue, + useDrawCountBuffer: true, + _valid: false, + }, + // Out of bounds, maxDrawCount = kMaxUnsignedLongValue + { bufferSize: 5, indirectOffset: 0, maxDrawCount: kMaxUnsignedLongValue, _valid: false }, + ]) ) .fn(t => { const { From 49d5487fefa07d531c3e557c71cee51feb87806d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Beaufort?= Date: Fri, 8 Nov 2024 13:59:52 +0100 Subject: [PATCH 3/8] Address second part of feedback --- .../cmds/render/indirect_multi_draw.spec.ts | 308 ++++++++---------- .../encoding/cmds/render/types.d.ts | 4 +- .../encoding/encoder_open_state.spec.ts | 18 + 3 files changed, 151 insertions(+), 179 deletions(-) diff --git a/src/webgpu/api/validation/encoding/cmds/render/indirect_multi_draw.spec.ts b/src/webgpu/api/validation/encoding/cmds/render/indirect_multi_draw.spec.ts index 65741b7bf5bb..79b17f943aed 100644 --- a/src/webgpu/api/validation/encoding/cmds/render/indirect_multi_draw.spec.ts +++ b/src/webgpu/api/validation/encoding/cmds/render/indirect_multi_draw.spec.ts @@ -214,187 +214,131 @@ Tests indirect and draw count offsets must be a multiple of 4. validateFinish(indirectOffset % 4 === 0 && (!useDrawCountBuffer || drawCountOffset % 4 === 0)); }); +interface IndirectOffsetOobCase { + indirectOffset: number; + bufferSize: number; + useDrawCountBuffer?: boolean; + drawCountOffset?: number; + maxDrawCount?: number; + _valid: boolean; +} + g.test('indirect_offset_oob') .desc( ` Tests multi indirect draw calls with various offsets and buffer sizes. +- (indirect offset, b.size) is + - (0, 0) + - (0, min size) (control case) + - (0, min size + 1) (control case) + - (0, min size) with drawCountBuffer (control case) + - (b.size / 2, b.size) with doubled b.size (control case) + - (0, min size) with drawCountBuffer and drawCountOffset in bounds (control case) + - (0, min size - 1) + - (0, min size - min alignment) + - (min alignment +/- 1, min size + min alignment) + - (0, min size + min alignment) with drawCountBuffer and drawCountOffset not a multiple of 4 + - (1, min size) index too big + - (min size + min alignment, min size) index past buffer + - (0, min size) with maxDrawCount = 2 + - (0, min size) with drawCountOffset = min size + - (kMaxUnsignedLongLongValue, min size) + - (0, min size) with drawCountOffset = kMaxUnsignedLongLongValue + - (0, min size) with maxDrawCount = kMaxUnsignedLongValue +- min size = indirect draw parameters size +- x =(multiDrawIndirect, multiDrawIndexedIndirect) ` ) .beforeAllSubcases(t => { t.selectDeviceOrSkipTestCase('chromium-experimental-multi-draw-indirect' as GPUFeatureName); }) - .params(u => - u.combineWithParams([ - // In bounds - { bufferSize: 4, indirectOffset: 0, _valid: true }, - // In bounds, bigger buffer - { bufferSize: 7, indirectOffset: 0, _valid: true }, - // In bounds, bigger buffer, positive offset - { bufferSize: 8, indirectOffset: 4, _valid: true }, - // In bounds for maxDrawCount - { bufferSize: 8, indirectOffset: 0, maxDrawCount: 2, _valid: true }, - // In bounds with drawCountBuffer - { bufferSize: 4, indirectOffset: 0, useDrawCountBuffer: true, _valid: true }, - // In bounds with drawCountBuffer, bigger buffer - { - bufferSize: 7, - indirectOffset: 0, - drawCountOffset: 6 * Uint32Array.BYTES_PER_ELEMENT, - useDrawCountBuffer: true, - _valid: true, - }, - // In bounds, non-multiple of 4 offsets - { bufferSize: 5, indirectOffset: 1, _valid: false }, - { bufferSize: 5, indirectOffset: 2, _valid: false }, - { - bufferSize: 5, - indirectOffset: 0, - drawCountOffset: 1, - useDrawCountBuffer: true, - _valid: false, - }, - { - bufferSize: 5, - indirectOffset: 0, - drawCountOffset: 2, - useDrawCountBuffer: true, - _valid: false, - }, - // Out of bounds, buffer too small - { bufferSize: 3, indirectOffset: 0, _valid: false }, - // Out of bounds, index too big - { bufferSize: 4, indirectOffset: 1 * Uint32Array.BYTES_PER_ELEMENT, _valid: false }, - // Out of bounds, index past buffer - { bufferSize: 4, indirectOffset: 5 * Uint32Array.BYTES_PER_ELEMENT, _valid: false }, - // Out of bounds, too small for maxDrawCount - { bufferSize: 7, indirectOffset: 0, drawCountOffset: 0, maxDrawCount: 2, _valid: false }, - // Out of bounds, offset too big for drawCountBuffer - { - bufferSize: 4, - indirectOffset: 0, - drawCountOffset: 4 * Uint32Array.BYTES_PER_ELEMENT, - useDrawCountBuffer: true, - _valid: false, - }, - // Out of bounds, index + size of command overflows - // uint64_t offset = std::numeric_limits::max(); - { bufferSize: 7, indirectOffset: kMaxUnsignedLongLongValue, _valid: false }, - // Out of bounds, index + size of command overflows with drawCountBuffer - { - bufferSize: 7, - indirectOffset: 0, - drawCountOffset: kMaxUnsignedLongLongValue, - useDrawCountBuffer: true, - _valid: false, - }, - // Out of bounds, maxDrawCount = kMaxUnsignedLongValue - { bufferSize: 7, indirectOffset: 0, maxDrawCount: kMaxUnsignedLongValue, _valid: false }, - ]) - ) - .fn(t => { - const { - bufferSize, - indirectOffset, - drawCountOffset = 0, - maxDrawCount = 1, - useDrawCountBuffer = false, - _valid, - } = t.params; - - const indirectBuffer = t.createBufferTracked({ - size: bufferSize * Uint32Array.BYTES_PER_ELEMENT, - usage: GPUBufferUsage.INDIRECT, - }); - - const { encoder, validateFinish } = t.createEncoder('render pass'); - encoder.setPipeline(t.createNoOpRenderPipeline()); - encoder.multiDrawIndirect( - indirectBuffer, - indirectOffset, - maxDrawCount, - useDrawCountBuffer ? indirectBuffer : undefined, - drawCountOffset - ); - - validateFinish(_valid); - }); -g.test('indexed_indirect_offset_oob') - .desc( - ` -Tests multi indexed indirect draw calls with various offsets and buffer sizes. - ` - ) - .beforeAllSubcases(t => { - t.selectDeviceOrSkipTestCase('chromium-experimental-multi-draw-indirect' as GPUFeatureName); - }) - .params(u => - u.combineWithParams([ - // In bounds - { bufferSize: 5, indirectOffset: 0, _valid: true }, - // In bounds, bigger buffer - { bufferSize: 9, indirectOffset: 0, _valid: true }, - // In bounds, bigger buffer, positive offset - { bufferSize: 10, indirectOffset: 5 * Uint32Array.BYTES_PER_ELEMENT, _valid: true }, - // In bounds with drawCountBuffer - { bufferSize: 5, indirectOffset: 0, useDrawCountBuffer: true, _valid: true }, - // In bounds with drawCountBuffer, bigger buffer - { - bufferSize: 6, - indirectOffset: 0, - drawCountOffset: 5 * Uint32Array.BYTES_PER_ELEMENT, - useDrawCountBuffer: true, - _valid: true, - }, - // In bounds, non-multiple of 4 offsets - { bufferSize: 6, indirectOffset: 1, _valid: false }, - { bufferSize: 6, indirectOffset: 2, _valid: false }, - { - bufferSize: 6, - indirectOffset: 0, - drawCountOffset: 1, - useDrawCountBuffer: true, - _valid: false, - }, - { - bufferSize: 6, - indirectOffset: 0, - drawCountOffset: 2, - useDrawCountBuffer: true, - _valid: false, - }, - // Out of bounds, buffer too small - { bufferSize: 4, indirectOffset: 0, _valid: false }, - // Out of bounds, index too big - { bufferSize: 5, indirectOffset: 1 * Uint32Array.BYTES_PER_ELEMENT, _valid: false }, - // Out of bounds, index past buffer - { bufferSize: 5, indirectOffset: 5 * Uint32Array.BYTES_PER_ELEMENT, _valid: false }, - // Out of bounds, too small for maxDrawCount - { bufferSize: 5, indirectOffset: 0, drawCountOffset: 0, maxDrawCount: 2, _valid: false }, - // Out of bounds, offset too big for drawCountBuffer - { - bufferSize: 5, - indirectOffset: 0, - drawCountOffset: 5 * Uint32Array.BYTES_PER_ELEMENT, - useDrawCountBuffer: true, - _valid: false, - }, - // Out of bounds, index + size of command overflows - { bufferSize: 10, indirectOffset: kMaxUnsignedLongLongValue, _valid: false }, - // Out of bounds, index + size of command overflows with drawCountBuffer - { - bufferSize: 10, - indirectOffset: 0, - drawCountOffset: kMaxUnsignedLongLongValue, - useDrawCountBuffer: true, - _valid: false, - }, - // Out of bounds, maxDrawCount = kMaxUnsignedLongValue - { bufferSize: 5, indirectOffset: 0, maxDrawCount: kMaxUnsignedLongValue, _valid: false }, - ]) + .paramsSubcasesOnly(u => + u.combine('indexed', [true, false] as const).expandWithParams(p => { + const indirectParamsSize = p.indexed ? 20 : 16; + return [ + { indirectOffset: 0, bufferSize: 0, _valid: false }, + // In bounds + { indirectOffset: 0, bufferSize: indirectParamsSize, _valid: true }, + { indirectOffset: 0, bufferSize: indirectParamsSize + 1, _valid: true }, + // In bounds with drawCountBuffer + { + indirectOffset: 0, + bufferSize: indirectParamsSize, + useDrawCountBuffer: true, + _valid: true, + }, + // In bounds, bigger buffer, positive offset + { indirectOffset: indirectParamsSize, bufferSize: indirectParamsSize * 2, _valid: true }, + // In bounds with drawCountBuffer, bigger buffer + { + indirectOffset: 0, + bufferSize: indirectParamsSize, + useDrawCountBuffer: true, + drawCountOffset: indirectParamsSize - 4, + _valid: true, + }, + // Out of bounds, buffer too small + { indirectOffset: 0, bufferSize: indirectParamsSize - 1, _valid: false }, + { indirectOffset: 0, bufferSize: indirectParamsSize - 4, _valid: false }, + // In bounds, non-multiple of 4 offsets + { indirectOffset: 3, bufferSize: indirectParamsSize + 4, _valid: false }, + { indirectOffset: 5, bufferSize: indirectParamsSize + 4, _valid: false }, + { + indirectOffset: 0, + bufferSize: indirectParamsSize + 4, + useDrawCountBuffer: true, + drawCountOffset: 1, + _valid: false, + }, + { + indirectOffset: 0, + bufferSize: indirectParamsSize + 4, + useDrawCountBuffer: true, + drawCountOffset: 2, + _valid: false, + }, + // Out of bounds, index too big + { indirectOffset: 4, bufferSize: indirectParamsSize, _valid: false }, + // Out of bounds, index past buffer + { indirectOffset: indirectParamsSize + 4, bufferSize: indirectParamsSize, _valid: false }, + // Out of bounds, too small for maxDrawCount + { indirectOffset: 0, bufferSize: indirectParamsSize, maxDrawCount: 2, _valid: false }, + // Out of bounds, offset too big for drawCountBuffer + { + indirectOffset: 0, + bufferSize: indirectParamsSize, + useDrawCountBuffer: true, + drawCountOffset: indirectParamsSize, + _valid: false, + }, + // Out of bounds, index + size of command overflows + { + indirectOffset: kMaxUnsignedLongLongValue, + bufferSize: indirectParamsSize, + _valid: false, + }, + // Out of bounds, index + size of command overflows with drawCountBuffer + { + indirectOffset: 0, + bufferSize: indirectParamsSize, + useDrawCountBuffer: true, + drawCountOffset: kMaxUnsignedLongLongValue, + _valid: false, + }, + // Out of bounds, maxDrawCount = kMaxUnsignedLongValue + { + indirectOffset: 0, + bufferSize: indirectParamsSize, + maxDrawCount: kMaxUnsignedLongValue, + _valid: false, + }, + ] as const; + }) ) .fn(t => { const { + indexed, bufferSize, indirectOffset, drawCountOffset = 0, @@ -404,20 +348,30 @@ Tests multi indexed indirect draw calls with various offsets and buffer sizes. } = t.params; const indirectBuffer = t.createBufferTracked({ - size: bufferSize * Uint32Array.BYTES_PER_ELEMENT, + size: bufferSize, usage: GPUBufferUsage.INDIRECT, }); const { encoder, validateFinish } = t.createEncoder('render pass'); encoder.setPipeline(t.createNoOpRenderPipeline()); - encoder.setIndexBuffer(t.makeIndexBuffer(), 'uint32'); - encoder.multiDrawIndexedIndirect( - indirectBuffer, - indirectOffset, - maxDrawCount, - useDrawCountBuffer ? indirectBuffer : undefined, - drawCountOffset - ); + if (indexed) { + encoder.setIndexBuffer(t.makeIndexBuffer(), 'uint32'); + encoder.multiDrawIndexedIndirect( + indirectBuffer, + indirectOffset, + maxDrawCount, + useDrawCountBuffer ? indirectBuffer : undefined, + drawCountOffset + ); + } else { + encoder.multiDrawIndirect( + indirectBuffer, + indirectOffset, + maxDrawCount, + useDrawCountBuffer ? indirectBuffer : undefined, + drawCountOffset + ); + } validateFinish(_valid); }); diff --git a/src/webgpu/api/validation/encoding/cmds/render/types.d.ts b/src/webgpu/api/validation/encoding/cmds/render/types.d.ts index 7aed233b8c5d..dafc517260ca 100644 --- a/src/webgpu/api/validation/encoding/cmds/render/types.d.ts +++ b/src/webgpu/api/validation/encoding/cmds/render/types.d.ts @@ -1,5 +1,5 @@ -// This can be removed once https://github.com/gpuweb/gpuweb/pull/2315 is merged. -interface GPURenderCommandsMixin { +// MAINTENANCE_TODO: Remove once https://github.com/gpuweb/gpuweb/pull/2315 is merged. +interface GPURenderPassEncoder { multiDrawIndirect( indirectBuffer: GPUBuffer, indirectOffset: GPUSize64, 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 f6e9d4230d5a..c32935790d27 100644 --- a/src/webgpu/api/validation/encoding/encoder_open_state.spec.ts +++ b/src/webgpu/api/validation/encoding/encoder_open_state.spec.ts @@ -101,6 +101,8 @@ const kRenderPassEncoderCommandInfo: { pushDebugGroup: {}, popDebugGroup: {}, insertDebugMarker: {}, + multiDrawIndirect: {}, + multiDrawIndexedIndirect: {}, }; const kRenderPassEncoderCommands = keysOf(kRenderPassEncoderCommandInfo); @@ -298,6 +300,12 @@ g.test('render_pass_commands') .beginSubcases() .combine('finishBeforeCommand', [false, true]) ) + .beforeAllSubcases(t => { + const { command } = t.params; + if (command === 'multiDrawIndirect' || command === 'multiDrawIndexedIndirect') { + t.selectDeviceOrSkipTestCase('chromium-experimental-multi-draw-indirect' as GPUFeatureName); + } + }) .fn(t => { const { command, finishBeforeCommand } = t.params; @@ -414,6 +422,16 @@ g.test('render_pass_commands') encoder.insertDebugMarker('marker'); } break; + case 'multiDrawIndirect': + { + renderPass.multiDrawIndirect(buffer, 0, 1); + } + break; + case 'multiDrawIndexedIndirect': + { + renderPass.multiDrawIndexedIndirect(buffer, 0, 1); + } + break; default: unreachable(); } From ac9bf64b4a577154875ca5ce620179f357b1f728 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Beaufort?= Date: Fri, 8 Nov 2024 14:49:04 +0100 Subject: [PATCH 4/8] Remove types.d.ts --- .../cmds/render/indirect_multi_draw.spec.ts | 30 ++++++++++++------- .../encoding/cmds/render/types.d.ts | 17 ----------- .../encoding/encoder_open_state.spec.ts | 12 ++++++-- 3 files changed, 29 insertions(+), 30 deletions(-) delete mode 100644 src/webgpu/api/validation/encoding/cmds/render/types.d.ts diff --git a/src/webgpu/api/validation/encoding/cmds/render/indirect_multi_draw.spec.ts b/src/webgpu/api/validation/encoding/cmds/render/indirect_multi_draw.spec.ts index 79b17f943aed..a27ca63e8c14 100644 --- a/src/webgpu/api/validation/encoding/cmds/render/indirect_multi_draw.spec.ts +++ b/src/webgpu/api/validation/encoding/cmds/render/indirect_multi_draw.spec.ts @@ -59,9 +59,11 @@ Tests indirect and draw count buffers must be valid. encoder.setPipeline(t.createNoOpRenderPipeline()); if (indexed) { encoder.setIndexBuffer(t.makeIndexBuffer(), 'uint32'); - encoder.multiDrawIndexedIndirect(indirectBuffer, 0, 1, drawCountBuffer); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (encoder as any).multiDrawIndexedIndirect(indirectBuffer, 0, 1, drawCountBuffer); } else { - encoder.multiDrawIndirect(indirectBuffer, 0, 1, drawCountBuffer); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (encoder as any).multiDrawIndirect(indirectBuffer, 0, 1, drawCountBuffer); } const shouldBeValid = @@ -104,9 +106,11 @@ g.test('buffers,device_mismatch') encoder.setPipeline(t.createNoOpRenderPipeline()); if (indexed) { encoder.setIndexBuffer(t.makeIndexBuffer(), 'uint32'); - encoder.multiDrawIndexedIndirect(indirectBuffer, 0, 1, drawCountBuffer); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (encoder as any).multiDrawIndexedIndirect(indirectBuffer, 0, 1, drawCountBuffer); } else { - encoder.multiDrawIndirect(indirectBuffer, 0, 1, drawCountBuffer); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (encoder as any).multiDrawIndirect(indirectBuffer, 0, 1, drawCountBuffer); } validateFinish(!mismatched); }); @@ -151,9 +155,11 @@ Tests indirect and draw count buffers must have 'Indirect' usage. encoder.setPipeline(t.createNoOpRenderPipeline()); if (indexed) { encoder.setIndexBuffer(t.makeIndexBuffer(), 'uint32'); - encoder.multiDrawIndexedIndirect(indirectBuffer, 0, 1, drawCountBuffer); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (encoder as any).multiDrawIndexedIndirect(indirectBuffer, 0, 1, drawCountBuffer); } else { - encoder.multiDrawIndirect(indirectBuffer, 0, 1, drawCountBuffer); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (encoder as any).multiDrawIndirect(indirectBuffer, 0, 1, drawCountBuffer); } const shouldSucceed = (indirectUsage & GPUBufferUsage.INDIRECT) !== 0 && @@ -193,7 +199,8 @@ Tests indirect and draw count offsets must be a multiple of 4. encoder.setPipeline(t.createNoOpRenderPipeline()); if (indexed) { encoder.setIndexBuffer(t.makeIndexBuffer(), 'uint32'); - encoder.multiDrawIndexedIndirect( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (encoder as any).multiDrawIndexedIndirect( indirectBuffer, indirectOffset, 1, @@ -201,7 +208,8 @@ Tests indirect and draw count offsets must be a multiple of 4. drawCountOffset ); } else { - encoder.multiDrawIndirect( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (encoder as any).multiDrawIndirect( indirectBuffer, indirectOffset, 1, @@ -356,7 +364,8 @@ Tests multi indirect draw calls with various offsets and buffer sizes. encoder.setPipeline(t.createNoOpRenderPipeline()); if (indexed) { encoder.setIndexBuffer(t.makeIndexBuffer(), 'uint32'); - encoder.multiDrawIndexedIndirect( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (encoder as any).multiDrawIndexedIndirect( indirectBuffer, indirectOffset, maxDrawCount, @@ -364,7 +373,8 @@ Tests multi indirect draw calls with various offsets and buffer sizes. drawCountOffset ); } else { - encoder.multiDrawIndirect( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (encoder as any).multiDrawIndirect( indirectBuffer, indirectOffset, maxDrawCount, diff --git a/src/webgpu/api/validation/encoding/cmds/render/types.d.ts b/src/webgpu/api/validation/encoding/cmds/render/types.d.ts deleted file mode 100644 index dafc517260ca..000000000000 --- a/src/webgpu/api/validation/encoding/cmds/render/types.d.ts +++ /dev/null @@ -1,17 +0,0 @@ -// MAINTENANCE_TODO: Remove once https://github.com/gpuweb/gpuweb/pull/2315 is merged. -interface GPURenderPassEncoder { - multiDrawIndirect( - indirectBuffer: GPUBuffer, - indirectOffset: GPUSize64, - maxDrawCount: GPUSize32, - drawCountBuffer?: GPUBuffer, - drawCountOffset?: GPUSize64 - ): undefined; - multiDrawIndexedIndirect( - indirectBuffer: GPUBuffer, - indirectOffset: GPUSize64, - maxDrawCount: GPUSize32, - drawCountBuffer?: GPUBuffer, - drawCountOffset?: GPUSize64 - ): undefined; -} 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 c32935790d27..9f29289753e2 100644 --- a/src/webgpu/api/validation/encoding/encoder_open_state.spec.ts +++ b/src/webgpu/api/validation/encoding/encoder_open_state.spec.ts @@ -79,7 +79,11 @@ const kEncoderCommandInfo: { }; const kEncoderCommands = keysOf(kEncoderCommandInfo); -type RenderPassEncoderCommands = keyof Omit; +// MAINTENANCE_TODO: Remove multiDrawIndirect and multiDrawIndexedIndirect once https://github.com/gpuweb/gpuweb/pull/2315 is merged. +type RenderPassEncoderCommands = + | keyof Omit + | 'multiDrawIndirect' + | 'multiDrawIndexedIndirect'; const kRenderPassEncoderCommandInfo: { readonly [k in RenderPassEncoderCommands]: {}; } = { @@ -424,12 +428,14 @@ g.test('render_pass_commands') break; case 'multiDrawIndirect': { - renderPass.multiDrawIndirect(buffer, 0, 1); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (renderPass as any).multiDrawIndirect(buffer, 0, 1); } break; case 'multiDrawIndexedIndirect': { - renderPass.multiDrawIndexedIndirect(buffer, 0, 1); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (renderPass as any).multiDrawIndexedIndirect(buffer, 0, 1); } break; default: From fa24773e63ac2bc3611c353aca0935591cde7b94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Beaufort?= Date: Wed, 20 Nov 2024 15:49:59 +0100 Subject: [PATCH 5/8] Address feedback from Kai --- .../cmds/render/indirect_multi_draw.spec.ts | 73 ++++++++++--------- .../encoding/encoder_open_state.spec.ts | 28 +++---- 2 files changed, 52 insertions(+), 49 deletions(-) diff --git a/src/webgpu/api/validation/encoding/cmds/render/indirect_multi_draw.spec.ts b/src/webgpu/api/validation/encoding/cmds/render/indirect_multi_draw.spec.ts index a27ca63e8c14..c7a9a4010039 100644 --- a/src/webgpu/api/validation/encoding/cmds/render/indirect_multi_draw.spec.ts +++ b/src/webgpu/api/validation/encoding/cmds/render/indirect_multi_draw.spec.ts @@ -41,6 +41,15 @@ Tests indirect and draw count buffers must be valid. kIndirectMultiDrawTestParams .combine('indirectState', kResourceStates) .combine('drawCountState', kResourceStates) + // drawCountState only matters if useDrawCountBuffer=true + .filter(p => p.useDrawCountBuffer || p.drawCountState === 'valid') + // Filter out a few unnecessary cases that would hit two errors in the same API call + .filter( + p => + p.indirectState === 'valid' || + p.drawCountState === 'valid' || + p.indirectState !== p.drawCountState + ) ) .fn(t => { const { indexed, indirectState, useDrawCountBuffer, drawCountState } = t.params; @@ -77,25 +86,32 @@ g.test('buffers,device_mismatch') .desc( 'Tests multiDraw(Indexed)Indirect cannot be called with buffers created from another device' ) - .paramsSubcasesOnly(kIndirectMultiDrawTestParams.combine('mismatched', [true, false])) + .paramsSubcasesOnly( + kIndirectMultiDrawTestParams.combineWithParams([ + { indirectMismatched: false, drawCountMismatched: false }, // control case + { indirectMismatched: true, drawCountMismatched: false }, + { indirectMismatched: false, drawCountMismatched: true }, + ]) + ) .beforeAllSubcases(t => { t.selectDeviceOrSkipTestCase('chromium-experimental-multi-draw-indirect' as GPUFeatureName); t.selectMismatchedDeviceOrSkipTestCase(undefined); }) .fn(t => { - const { indexed, useDrawCountBuffer, mismatched } = t.params; + const { indexed, useDrawCountBuffer, indirectMismatched, drawCountMismatched } = t.params; - const sourceDevice = mismatched ? t.mismatchedDevice : t.device; + const indirectDevice = indirectMismatched ? t.mismatchedDevice : t.device; + const drawCountDevice = drawCountMismatched ? t.mismatchedDevice : t.device; const indirectBuffer = t.trackForCleanup( - sourceDevice.createBuffer({ + indirectDevice.createBuffer({ size: 256, usage: GPUBufferUsage.INDIRECT, }) ); const drawCountBuffer = useDrawCountBuffer ? t.trackForCleanup( - sourceDevice.createBuffer({ + drawCountDevice.createBuffer({ size: 256, usage: GPUBufferUsage.INDIRECT, }) @@ -112,7 +128,7 @@ g.test('buffers,device_mismatch') // eslint-disable-next-line @typescript-eslint/no-explicit-any (encoder as any).multiDrawIndirect(indirectBuffer, 0, 1, drawCountBuffer); } - validateFinish(!mismatched); + validateFinish(!indirectMismatched && (!useDrawCountBuffer || !drawCountMismatched)); }); g.test('indirect_buffer_usage') @@ -177,9 +193,17 @@ Tests indirect and draw count offsets must be a multiple of 4. t.selectDeviceOrSkipTestCase('chromium-experimental-multi-draw-indirect' as GPUFeatureName); }) .paramsSubcasesOnly( - kIndirectMultiDrawTestParams - .combine('indirectOffset', [0, 2, 4] as const) - .combine('drawCountOffset', [0, 2, 4] as const) + kIndirectMultiDrawTestParams.combineWithParams([ + // Valid + { indirectOffset: 0, drawCountOffset: 0 }, + { indirectOffset: 4, drawCountOffset: 0 }, + { indirectOffset: 0, drawCountOffset: 4 }, + // Invalid + { indirectOffset: 2, drawCountOffset: 0 }, + { indirectOffset: 6, drawCountOffset: 0 }, + { indirectOffset: 0, drawCountOffset: 2 }, + { indirectOffset: 0, drawCountOffset: 6 }, + ] as const) ) .fn(t => { const { indexed, indirectOffset, useDrawCountBuffer, drawCountOffset } = t.params; @@ -231,36 +255,12 @@ interface IndirectOffsetOobCase { _valid: boolean; } -g.test('indirect_offset_oob') +g.test('offsets_and_buffer_sizes') .desc( ` -Tests multi indirect draw calls with various offsets and buffer sizes. -- (indirect offset, b.size) is - - (0, 0) - - (0, min size) (control case) - - (0, min size + 1) (control case) - - (0, min size) with drawCountBuffer (control case) - - (b.size / 2, b.size) with doubled b.size (control case) - - (0, min size) with drawCountBuffer and drawCountOffset in bounds (control case) - - (0, min size - 1) - - (0, min size - min alignment) - - (min alignment +/- 1, min size + min alignment) - - (0, min size + min alignment) with drawCountBuffer and drawCountOffset not a multiple of 4 - - (1, min size) index too big - - (min size + min alignment, min size) index past buffer - - (0, min size) with maxDrawCount = 2 - - (0, min size) with drawCountOffset = min size - - (kMaxUnsignedLongLongValue, min size) - - (0, min size) with drawCountOffset = kMaxUnsignedLongLongValue - - (0, min size) with maxDrawCount = kMaxUnsignedLongValue -- min size = indirect draw parameters size -- x =(multiDrawIndirect, multiDrawIndexedIndirect) +Tests multi indirect draw calls with various indirect offsets, buffer sizes, draw count offsets, and draw count buffer sizes. ` ) - .beforeAllSubcases(t => { - t.selectDeviceOrSkipTestCase('chromium-experimental-multi-draw-indirect' as GPUFeatureName); - }) - .paramsSubcasesOnly(u => u.combine('indexed', [true, false] as const).expandWithParams(p => { const indirectParamsSize = p.indexed ? 20 : 16; @@ -344,6 +344,9 @@ Tests multi indirect draw calls with various offsets and buffer sizes. ] as const; }) ) + .beforeAllSubcases(t => { + t.selectDeviceOrSkipTestCase('chromium-experimental-multi-draw-indirect' as GPUFeatureName); + }) .fn(t => { const { indexed, 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 9f29289753e2..7e22ca5de888 100644 --- a/src/webgpu/api/validation/encoding/encoder_open_state.spec.ts +++ b/src/webgpu/api/validation/encoding/encoder_open_state.spec.ts @@ -91,6 +91,8 @@ const kRenderPassEncoderCommandInfo: { drawIndexed: {}, drawIndexedIndirect: {}, drawIndirect: {}, + multiDrawIndexedIndirect: {}, + multiDrawIndirect: {}, setIndexBuffer: {}, setBindGroup: {}, setVertexBuffer: {}, @@ -105,8 +107,6 @@ const kRenderPassEncoderCommandInfo: { pushDebugGroup: {}, popDebugGroup: {}, insertDebugMarker: {}, - multiDrawIndirect: {}, - multiDrawIndexedIndirect: {}, }; const kRenderPassEncoderCommands = keysOf(kRenderPassEncoderCommandInfo); @@ -358,6 +358,18 @@ g.test('render_pass_commands') renderPass.drawIndexedIndirect(buffer, 0); } break; + case 'multiDrawIndirect': + { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (renderPass as any).multiDrawIndirect(buffer, 0, 1); + } + break; + case 'multiDrawIndexedIndirect': + { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (renderPass as any).multiDrawIndexedIndirect(buffer, 0, 1); + } + break; case 'setBindGroup': { renderPass.setBindGroup(0, bindGroup); @@ -426,18 +438,6 @@ g.test('render_pass_commands') encoder.insertDebugMarker('marker'); } break; - case 'multiDrawIndirect': - { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (renderPass as any).multiDrawIndirect(buffer, 0, 1); - } - break; - case 'multiDrawIndexedIndirect': - { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (renderPass as any).multiDrawIndexedIndirect(buffer, 0, 1); - } - break; default: unreachable(); } From cf08f24cf5c90aba55d54ea4b94c2c9f044c8a21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Beaufort?= Date: Thu, 21 Nov 2024 11:05:17 +0100 Subject: [PATCH 6/8] Address feedback | part 2 --- .../cmds/render/indirect_multi_draw.spec.ts | 177 ++++++++++-------- 1 file changed, 98 insertions(+), 79 deletions(-) diff --git a/src/webgpu/api/validation/encoding/cmds/render/indirect_multi_draw.spec.ts b/src/webgpu/api/validation/encoding/cmds/render/indirect_multi_draw.spec.ts index c7a9a4010039..58652f666995 100644 --- a/src/webgpu/api/validation/encoding/cmds/render/indirect_multi_draw.spec.ts +++ b/src/webgpu/api/validation/encoding/cmds/render/indirect_multi_draw.spec.ts @@ -87,11 +87,14 @@ g.test('buffers,device_mismatch') 'Tests multiDraw(Indexed)Indirect cannot be called with buffers created from another device' ) .paramsSubcasesOnly( - kIndirectMultiDrawTestParams.combineWithParams([ - { indirectMismatched: false, drawCountMismatched: false }, // control case - { indirectMismatched: true, drawCountMismatched: false }, - { indirectMismatched: false, drawCountMismatched: true }, - ]) + kIndirectMultiDrawTestParams + .combineWithParams([ + { indirectMismatched: false, drawCountMismatched: false }, // control case + { indirectMismatched: true, drawCountMismatched: false }, + { indirectMismatched: false, drawCountMismatched: true }, + ]) + // drawCountMismatched only matters if useDrawCountBuffer=true + .filter(p => p.useDrawCountBuffer || !p.drawCountMismatched) ) .beforeAllSubcases(t => { t.selectDeviceOrSkipTestCase('chromium-experimental-multi-draw-indirect' as GPUFeatureName); @@ -128,7 +131,7 @@ g.test('buffers,device_mismatch') // eslint-disable-next-line @typescript-eslint/no-explicit-any (encoder as any).multiDrawIndirect(indirectBuffer, 0, 1, drawCountBuffer); } - validateFinish(!indirectMismatched && (!useDrawCountBuffer || !drawCountMismatched)); + validateFinish(!indirectMismatched && !drawCountMismatched); }); g.test('indirect_buffer_usage') @@ -246,101 +249,131 @@ Tests indirect and draw count offsets must be a multiple of 4. validateFinish(indirectOffset % 4 === 0 && (!useDrawCountBuffer || drawCountOffset % 4 === 0)); }); -interface IndirectOffsetOobCase { - indirectOffset: number; - bufferSize: number; - useDrawCountBuffer?: boolean; - drawCountOffset?: number; - maxDrawCount?: number; - _valid: boolean; -} +g.test('indirect_buffer_OOB') + .desc( + ` +Tests multi indirect draw calls with various indirect offsets and buffer sizes without draw count buffer. +` + ) + .paramsSubcasesOnly(u => + u.combine('indexed', [true, false] as const).expandWithParams(p => { + const indirectParamsSize = p.indexed ? 20 : 16; + return [ + { indirectOffset: 0, bufferSize: 0, maxDrawCount: 1, _valid: false }, + // In bounds + { indirectOffset: 0, bufferSize: indirectParamsSize, maxDrawCount: 1, _valid: true }, + { indirectOffset: 0, bufferSize: indirectParamsSize + 1, maxDrawCount: 1, _valid: true }, + // In bounds, bigger buffer, positive offset + { + indirectOffset: indirectParamsSize, + bufferSize: indirectParamsSize * 2, + maxDrawCount: 1, + _valid: true, + }, + // Out of bounds, buffer too small + { indirectOffset: 0, bufferSize: indirectParamsSize - 1, maxDrawCount: 1, _valid: false }, + { indirectOffset: 0, bufferSize: indirectParamsSize - 4, maxDrawCount: 1, _valid: false }, + // In bounds, non-multiple of 4 offsets + { indirectOffset: 3, bufferSize: indirectParamsSize + 4, maxDrawCount: 1, _valid: false }, + { indirectOffset: 5, bufferSize: indirectParamsSize + 4, maxDrawCount: 1, _valid: false }, + // Out of bounds, index too big + { indirectOffset: 4, bufferSize: indirectParamsSize, maxDrawCount: 1, _valid: false }, + // Out of bounds, index past buffer + { + indirectOffset: indirectParamsSize + 4, + bufferSize: indirectParamsSize, + maxDrawCount: 1, + _valid: false, + }, + // Out of bounds, too small for maxDrawCount + { indirectOffset: 0, bufferSize: indirectParamsSize, maxDrawCount: 2, _valid: false }, + // Out of bounds, index + size of command overflows + { + indirectOffset: kMaxUnsignedLongLongValue, + bufferSize: indirectParamsSize, + maxDrawCount: 1, + _valid: false, + }, + // Out of bounds, maxDrawCount = kMaxUnsignedLongValue + { + indirectOffset: 0, + bufferSize: indirectParamsSize, + maxDrawCount: kMaxUnsignedLongValue, + _valid: false, + }, + ] as const; + }) + ) + .beforeAllSubcases(t => { + t.selectDeviceOrSkipTestCase('chromium-experimental-multi-draw-indirect' as GPUFeatureName); + }) + .fn(t => { + const { indexed, indirectOffset, bufferSize, maxDrawCount, _valid } = t.params; + + const indirectBuffer = t.createBufferTracked({ + size: bufferSize, + usage: GPUBufferUsage.INDIRECT, + }); + + const { encoder, validateFinish } = t.createEncoder('render pass'); + encoder.setPipeline(t.createNoOpRenderPipeline()); + if (indexed) { + encoder.setIndexBuffer(t.makeIndexBuffer(), 'uint32'); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (encoder as any).multiDrawIndexedIndirect(indirectBuffer, indirectOffset, maxDrawCount); + } else { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (encoder as any).multiDrawIndirect(indirectBuffer, indirectOffset, maxDrawCount); + } + + validateFinish(_valid); + }); -g.test('offsets_and_buffer_sizes') +g.test('draw_count_buffer_OOB') .desc( ` -Tests multi indirect draw calls with various indirect offsets, buffer sizes, draw count offsets, and draw count buffer sizes. +Tests multi indirect draw calls with various draw count offsets, and draw count buffer sizes. ` ) .paramsSubcasesOnly(u => - u.combine('indexed', [true, false] as const).expandWithParams(p => { + u.combine('indexed', [true, false] as const).expandWithParams(p => { const indirectParamsSize = p.indexed ? 20 : 16; return [ - { indirectOffset: 0, bufferSize: 0, _valid: false }, // In bounds - { indirectOffset: 0, bufferSize: indirectParamsSize, _valid: true }, - { indirectOffset: 0, bufferSize: indirectParamsSize + 1, _valid: true }, - // In bounds with drawCountBuffer { - indirectOffset: 0, bufferSize: indirectParamsSize, - useDrawCountBuffer: true, + drawCountOffset: 0, _valid: true, }, - // In bounds, bigger buffer, positive offset - { indirectOffset: indirectParamsSize, bufferSize: indirectParamsSize * 2, _valid: true }, - // In bounds with drawCountBuffer, bigger buffer + // In bounds with bigger buffer { - indirectOffset: 0, bufferSize: indirectParamsSize, - useDrawCountBuffer: true, drawCountOffset: indirectParamsSize - 4, _valid: true, }, - // Out of bounds, buffer too small - { indirectOffset: 0, bufferSize: indirectParamsSize - 1, _valid: false }, - { indirectOffset: 0, bufferSize: indirectParamsSize - 4, _valid: false }, // In bounds, non-multiple of 4 offsets - { indirectOffset: 3, bufferSize: indirectParamsSize + 4, _valid: false }, - { indirectOffset: 5, bufferSize: indirectParamsSize + 4, _valid: false }, { - indirectOffset: 0, bufferSize: indirectParamsSize + 4, - useDrawCountBuffer: true, drawCountOffset: 1, _valid: false, }, { - indirectOffset: 0, bufferSize: indirectParamsSize + 4, - useDrawCountBuffer: true, drawCountOffset: 2, _valid: false, }, - // Out of bounds, index too big - { indirectOffset: 4, bufferSize: indirectParamsSize, _valid: false }, - // Out of bounds, index past buffer - { indirectOffset: indirectParamsSize + 4, bufferSize: indirectParamsSize, _valid: false }, - // Out of bounds, too small for maxDrawCount - { indirectOffset: 0, bufferSize: indirectParamsSize, maxDrawCount: 2, _valid: false }, // Out of bounds, offset too big for drawCountBuffer { - indirectOffset: 0, bufferSize: indirectParamsSize, - useDrawCountBuffer: true, drawCountOffset: indirectParamsSize, _valid: false, }, - // Out of bounds, index + size of command overflows - { - indirectOffset: kMaxUnsignedLongLongValue, - bufferSize: indirectParamsSize, - _valid: false, - }, // Out of bounds, index + size of command overflows with drawCountBuffer { - indirectOffset: 0, bufferSize: indirectParamsSize, - useDrawCountBuffer: true, drawCountOffset: kMaxUnsignedLongLongValue, _valid: false, }, - // Out of bounds, maxDrawCount = kMaxUnsignedLongValue - { - indirectOffset: 0, - bufferSize: indirectParamsSize, - maxDrawCount: kMaxUnsignedLongValue, - _valid: false, - }, ] as const; }) ) @@ -348,15 +381,7 @@ Tests multi indirect draw calls with various indirect offsets, buffer sizes, dra t.selectDeviceOrSkipTestCase('chromium-experimental-multi-draw-indirect' as GPUFeatureName); }) .fn(t => { - const { - indexed, - bufferSize, - indirectOffset, - drawCountOffset = 0, - maxDrawCount = 1, - useDrawCountBuffer = false, - _valid, - } = t.params; + const { indexed, bufferSize, drawCountOffset, _valid } = t.params; const indirectBuffer = t.createBufferTracked({ size: bufferSize, @@ -370,20 +395,14 @@ Tests multi indirect draw calls with various indirect offsets, buffer sizes, dra // eslint-disable-next-line @typescript-eslint/no-explicit-any (encoder as any).multiDrawIndexedIndirect( indirectBuffer, - indirectOffset, - maxDrawCount, - useDrawCountBuffer ? indirectBuffer : undefined, + 0, + 1, + indirectBuffer, drawCountOffset ); } else { // eslint-disable-next-line @typescript-eslint/no-explicit-any - (encoder as any).multiDrawIndirect( - indirectBuffer, - indirectOffset, - maxDrawCount, - useDrawCountBuffer ? indirectBuffer : undefined, - drawCountOffset - ); + (encoder as any).multiDrawIndirect(indirectBuffer, 0, 1, indirectBuffer, drawCountOffset); } validateFinish(_valid); From 233ef0e46b709f6b1aba16ffcbcc829e06ff0d96 Mon Sep 17 00:00:00 2001 From: Kai Ninomiya Date: Fri, 22 Nov 2024 17:25:41 -0800 Subject: [PATCH 7/8] address comments about indirectBuffer_range --- .../cmds/render/indirect_multi_draw.spec.ts | 90 ++++++++----------- 1 file changed, 38 insertions(+), 52 deletions(-) diff --git a/src/webgpu/api/validation/encoding/cmds/render/indirect_multi_draw.spec.ts b/src/webgpu/api/validation/encoding/cmds/render/indirect_multi_draw.spec.ts index 58652f666995..4590a66a90e6 100644 --- a/src/webgpu/api/validation/encoding/cmds/render/indirect_multi_draw.spec.ts +++ b/src/webgpu/api/validation/encoding/cmds/render/indirect_multi_draw.spec.ts @@ -249,66 +249,49 @@ Tests indirect and draw count offsets must be a multiple of 4. validateFinish(indirectOffset % 4 === 0 && (!useDrawCountBuffer || drawCountOffset % 4 === 0)); }); -g.test('indirect_buffer_OOB') +g.test('indirectBuffer_range') .desc( ` Tests multi indirect draw calls with various indirect offsets and buffer sizes without draw count buffer. ` ) .paramsSubcasesOnly(u => - u.combine('indexed', [true, false] as const).expandWithParams(p => { - const indirectParamsSize = p.indexed ? 20 : 16; - return [ - { indirectOffset: 0, bufferSize: 0, maxDrawCount: 1, _valid: false }, - // In bounds - { indirectOffset: 0, bufferSize: indirectParamsSize, maxDrawCount: 1, _valid: true }, - { indirectOffset: 0, bufferSize: indirectParamsSize + 1, maxDrawCount: 1, _valid: true }, - // In bounds, bigger buffer, positive offset - { - indirectOffset: indirectParamsSize, - bufferSize: indirectParamsSize * 2, - maxDrawCount: 1, - _valid: true, - }, - // Out of bounds, buffer too small - { indirectOffset: 0, bufferSize: indirectParamsSize - 1, maxDrawCount: 1, _valid: false }, - { indirectOffset: 0, bufferSize: indirectParamsSize - 4, maxDrawCount: 1, _valid: false }, - // In bounds, non-multiple of 4 offsets - { indirectOffset: 3, bufferSize: indirectParamsSize + 4, maxDrawCount: 1, _valid: false }, - { indirectOffset: 5, bufferSize: indirectParamsSize + 4, maxDrawCount: 1, _valid: false }, - // Out of bounds, index too big - { indirectOffset: 4, bufferSize: indirectParamsSize, maxDrawCount: 1, _valid: false }, - // Out of bounds, index past buffer - { - indirectOffset: indirectParamsSize + 4, - bufferSize: indirectParamsSize, - maxDrawCount: 1, - _valid: false, - }, - // Out of bounds, too small for maxDrawCount - { indirectOffset: 0, bufferSize: indirectParamsSize, maxDrawCount: 2, _valid: false }, - // Out of bounds, index + size of command overflows - { - indirectOffset: kMaxUnsignedLongLongValue, - bufferSize: indirectParamsSize, - maxDrawCount: 1, - _valid: false, - }, - // Out of bounds, maxDrawCount = kMaxUnsignedLongValue - { - indirectOffset: 0, - bufferSize: indirectParamsSize, - maxDrawCount: kMaxUnsignedLongValue, - _valid: false, - }, - ] as const; - }) + u + .combine('indexed', [true, false] as const) // + .expandWithParams(function* (p) { + const drawParamsSize = p.indexed ? 20 : 16; + + // Simple OOB test cases, using a delta from the exact required size + for (const { maxDrawCount, offset } of [ + { maxDrawCount: 1, offset: 0 }, + { maxDrawCount: 1, offset: 4 }, + { maxDrawCount: 1, offset: drawParamsSize + 4 }, + { maxDrawCount: 2, offset: 0 }, + { maxDrawCount: 6, offset: drawParamsSize }, + ]) { + const exactRequiredSize = offset + maxDrawCount * drawParamsSize; + yield { offset, maxDrawCount, bufferSize: exactRequiredSize }; + yield { offset, maxDrawCount, bufferSize: exactRequiredSize - 1 }; + } + + // Additional test cases + // - Buffer size is 0 + yield { offset: 0, maxDrawCount: 1, bufferSize: 0 }; + // - Buffer size is unaligned (OK) + yield { offset: 0, maxDrawCount: 1, bufferSize: drawParamsSize + 1 }; + // - In-bounds, but non-multiple of 4 offset + yield { offset: 2, maxDrawCount: 1, bufferSize: drawParamsSize + 8 }; + yield { offset: 6, maxDrawCount: 1, bufferSize: drawParamsSize + 8 }; + // - Out of bounds, (offset + (drawParamsSize * maxDrawCount)) may overflow + yield { offset: kMaxUnsignedLongLongValue, maxDrawCount: 1, bufferSize: 1024 }; + yield { offset: 0, maxDrawCount: kMaxUnsignedLongValue, bufferSize: 1024 }; + }) ) .beforeAllSubcases(t => { t.selectDeviceOrSkipTestCase('chromium-experimental-multi-draw-indirect' as GPUFeatureName); }) .fn(t => { - const { indexed, indirectOffset, bufferSize, maxDrawCount, _valid } = t.params; + const { indexed, offset, maxDrawCount, bufferSize } = t.params; const indirectBuffer = t.createBufferTracked({ size: bufferSize, @@ -320,13 +303,16 @@ Tests multi indirect draw calls with various indirect offsets and buffer sizes w if (indexed) { encoder.setIndexBuffer(t.makeIndexBuffer(), 'uint32'); // eslint-disable-next-line @typescript-eslint/no-explicit-any - (encoder as any).multiDrawIndexedIndirect(indirectBuffer, indirectOffset, maxDrawCount); + (encoder as any).multiDrawIndexedIndirect(indirectBuffer, offset, maxDrawCount); } else { // eslint-disable-next-line @typescript-eslint/no-explicit-any - (encoder as any).multiDrawIndirect(indirectBuffer, indirectOffset, maxDrawCount); + (encoder as any).multiDrawIndirect(indirectBuffer, offset, maxDrawCount); } - validateFinish(_valid); + const paramsSize = indexed ? 20 : 16; + const exactRequiredSize = offset + maxDrawCount * paramsSize; + const valid = offset % 4 === 0 && bufferSize >= exactRequiredSize; + validateFinish(valid); }); g.test('draw_count_buffer_OOB') From fa5df1ed64c39015037653fc3ab17cb25461a3e1 Mon Sep 17 00:00:00 2001 From: Kai Ninomiya Date: Fri, 22 Nov 2024 17:44:29 -0800 Subject: [PATCH 8/8] address comments about drawCountBuffer_range --- .../cmds/render/indirect_multi_draw.spec.ts | 71 ++++++------------- 1 file changed, 22 insertions(+), 49 deletions(-) diff --git a/src/webgpu/api/validation/encoding/cmds/render/indirect_multi_draw.spec.ts b/src/webgpu/api/validation/encoding/cmds/render/indirect_multi_draw.spec.ts index 4590a66a90e6..f2ad688f8938 100644 --- a/src/webgpu/api/validation/encoding/cmds/render/indirect_multi_draw.spec.ts +++ b/src/webgpu/api/validation/encoding/cmds/render/indirect_multi_draw.spec.ts @@ -315,61 +315,38 @@ Tests multi indirect draw calls with various indirect offsets and buffer sizes w validateFinish(valid); }); -g.test('draw_count_buffer_OOB') +g.test('drawCountBuffer_range') .desc( ` Tests multi indirect draw calls with various draw count offsets, and draw count buffer sizes. ` ) .paramsSubcasesOnly(u => - u.combine('indexed', [true, false] as const).expandWithParams(p => { - const indirectParamsSize = p.indexed ? 20 : 16; - return [ + u + .combine('indexed', [true, false] as const) // + .combineWithParams([ // In bounds - { - bufferSize: indirectParamsSize, - drawCountOffset: 0, - _valid: true, - }, - // In bounds with bigger buffer - { - bufferSize: indirectParamsSize, - drawCountOffset: indirectParamsSize - 4, - _valid: true, - }, - // In bounds, non-multiple of 4 offsets - { - bufferSize: indirectParamsSize + 4, - drawCountOffset: 1, - _valid: false, - }, - { - bufferSize: indirectParamsSize + 4, - drawCountOffset: 2, - _valid: false, - }, + { offset: 0, bufferSize: 4 }, + { offset: 0, bufferSize: 5 }, + // In bounds, but non-multiple of 4 offset + { offset: 2, bufferSize: 8 }, // Out of bounds, offset too big for drawCountBuffer - { - bufferSize: indirectParamsSize, - drawCountOffset: indirectParamsSize, - _valid: false, - }, - // Out of bounds, index + size of command overflows with drawCountBuffer - { - bufferSize: indirectParamsSize, - drawCountOffset: kMaxUnsignedLongLongValue, - _valid: false, - }, - ] as const; - }) + { offset: 4, bufferSize: 7 }, + // Out of bounds, (offset + kDrawCountSize) may overflow + { offset: kMaxUnsignedLongLongValue, bufferSize: 1024 }, + ]) ) .beforeAllSubcases(t => { t.selectDeviceOrSkipTestCase('chromium-experimental-multi-draw-indirect' as GPUFeatureName); }) .fn(t => { - const { indexed, bufferSize, drawCountOffset, _valid } = t.params; + const { indexed, bufferSize, offset } = t.params; const indirectBuffer = t.createBufferTracked({ + size: indexed ? 20 : 16, + usage: GPUBufferUsage.INDIRECT, + }); + const drawCountBuffer = t.createBufferTracked({ size: bufferSize, usage: GPUBufferUsage.INDIRECT, }); @@ -379,17 +356,13 @@ Tests multi indirect draw calls with various draw count offsets, and draw count if (indexed) { encoder.setIndexBuffer(t.makeIndexBuffer(), 'uint32'); // eslint-disable-next-line @typescript-eslint/no-explicit-any - (encoder as any).multiDrawIndexedIndirect( - indirectBuffer, - 0, - 1, - indirectBuffer, - drawCountOffset - ); + (encoder as any).multiDrawIndexedIndirect(indirectBuffer, 0, 1, drawCountBuffer, offset); } else { // eslint-disable-next-line @typescript-eslint/no-explicit-any - (encoder as any).multiDrawIndirect(indirectBuffer, 0, 1, indirectBuffer, drawCountOffset); + (encoder as any).multiDrawIndirect(indirectBuffer, 0, 1, drawCountBuffer, offset); } - validateFinish(_valid); + const kDrawCountSize = 4; + const valid = offset % 4 === 0 && offset + kDrawCountSize <= bufferSize; + validateFinish(valid); });