From cc2dfd6275b456243ee94ce27364a4906e3024cc Mon Sep 17 00:00:00 2001 From: Kai Ninomiya Date: Mon, 11 Mar 2024 18:23:50 -0700 Subject: [PATCH 1/4] Refactor more easy cases of bytesPerBlock --- .../storage_texture/read_write.spec.ts | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/webgpu/api/operation/storage_texture/read_write.spec.ts b/src/webgpu/api/operation/storage_texture/read_write.spec.ts index 9eb04b2b458f..eb3da84681ca 100644 --- a/src/webgpu/api/operation/storage_texture/read_write.spec.ts +++ b/src/webgpu/api/operation/storage_texture/read_write.spec.ts @@ -8,7 +8,11 @@ TODO: import { makeTestGroup } from '../../../../common/framework/test_group.js'; import { assert, unreachable } from '../../../../common/util/util.js'; import { kTextureDimensions } from '../../../capability_info.js'; -import { kColorTextureFormats, kTextureFormatInfo } from '../../../format_info.js'; +import { + ColorTextureFormat, + kColorTextureFormats, + kTextureFormatInfo, +} from '../../../format_info.js'; import { GPUTest } from '../../../gpu_test.js'; import { align } from '../../../util/math.js'; @@ -17,10 +21,8 @@ type ShaderStageForReadWriteStorageTexture = (typeof kShaderStagesForReadWriteStorageTexture)[number]; class F extends GPUTest { - GetInitialData(storageTexture: GPUTexture): ArrayBuffer { - const format = storageTexture.format; - const bytesPerBlock = kTextureFormatInfo[format].bytesPerBlock; - assert(bytesPerBlock !== undefined); + GetInitialData(format: ColorTextureFormat, storageTexture: GPUTexture): ArrayBuffer { + const bytesPerBlock = kTextureFormatInfo[format].color.bytes; const width = storageTexture.width; const height = storageTexture.height; @@ -64,11 +66,11 @@ class F extends GPUTest { GetExpectedData( shaderStage: ShaderStageForReadWriteStorageTexture, + format: ColorTextureFormat, storageTexture: GPUTexture, initialData: ArrayBuffer ): ArrayBuffer { - const format = storageTexture.format; - const bytesPerBlock = kTextureFormatInfo[format].bytesPerBlock; + const bytesPerBlock = kTextureFormatInfo[format].color.bytes; assert(bytesPerBlock !== undefined); const width = storageTexture.width; @@ -310,7 +312,7 @@ g.test('basic') .params(u => u .combine('format', kColorTextureFormats) - .filter(p => kTextureFormatInfo[p.format].color?.readWriteStorage === true) + .filter(p => kTextureFormatInfo[p.format].color.readWriteStorage === true) .combine('shaderStage', kShaderStagesForReadWriteStorageTexture) .combine('textureDimension', kTextureDimensions) .combine('depthOrArrayLayers', [1, 2] as const) @@ -335,8 +337,8 @@ g.test('basic') }); t.trackForCleanup(storageTexture); - const bytesPerBlock = kTextureFormatInfo[format].bytesPerBlock; - const initialData = t.GetInitialData(storageTexture); + const bytesPerBlock = kTextureFormatInfo[format].color.bytes; + const initialData = t.GetInitialData(format, storageTexture); t.queue.writeTexture( { texture: storageTexture }, initialData, @@ -351,7 +353,7 @@ g.test('basic') t.RecordCommandsToTransform(t.device, shaderStage, commandEncoder, storageTexture); - const expectedData = t.GetExpectedData(shaderStage, storageTexture, initialData); + const expectedData = t.GetExpectedData(shaderStage, format, storageTexture, initialData); const readbackBuffer = t.device.createBuffer({ size: expectedData.byteLength, usage: GPUBufferUsage.COPY_SRC | GPUBufferUsage.COPY_DST, From c5a2ecf5950bd85962df4390c2f0d523e6661e43 Mon Sep 17 00:00:00 2001 From: Kai Ninomiya Date: Mon, 11 Mar 2024 12:48:57 -0700 Subject: [PATCH 2/4] Add unit tests for resolvePerAspectFormat --- src/webgpu/format_info.spec.ts | 34 ++++++++++++++++++++++++++++++++++ src/webgpu/format_info.ts | 2 +- 2 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 src/webgpu/format_info.spec.ts diff --git a/src/webgpu/format_info.spec.ts b/src/webgpu/format_info.spec.ts new file mode 100644 index 000000000000..340694317055 --- /dev/null +++ b/src/webgpu/format_info.spec.ts @@ -0,0 +1,34 @@ +export const description = ` +Unittests for helpers in format_info.ts. +`; + +import { Fixture } from '../common/framework/fixture.js'; +import { makeTestGroup } from '../common/framework/test_group.js'; +import { assert, objectEquals } from '../common/util/util.js'; + +import { kDepthStencilFormats, kTextureFormatInfo, resolvePerAspectFormat } from './format_info.js'; + +export const g = makeTestGroup(Fixture); + +g.test('resolvePerAspectFormat') + .desc( + `Test that resolvePerAspectFormat works and kTextureFormatInfo contains identical + information for the combined and separate versions of that aspect.` + ) + .fn(t => { + for (const format of kDepthStencilFormats) { + const info = kTextureFormatInfo[format]; + if (info.depth) { + const depthFormatInfo = kTextureFormatInfo[resolvePerAspectFormat(format, 'depth-only')]; + assert(objectEquals(info.depth, depthFormatInfo.depth), 'Error in texture format table'); + } + if (info.stencil) { + const stencilFormatInfo = + kTextureFormatInfo[resolvePerAspectFormat(format, 'stencil-only')]; + assert( + objectEquals(info.stencil, stencilFormatInfo.stencil), + 'Error in texture format table' + ); + } + } + }); diff --git a/src/webgpu/format_info.ts b/src/webgpu/format_info.ts index be1320d3cdad..1c3473c0487e 100644 --- a/src/webgpu/format_info.ts +++ b/src/webgpu/format_info.ts @@ -1,5 +1,5 @@ import { keysOf } from '../common/util/data_tables.js'; -import { assert, unreachable } from '../common/util/util.js'; +import { assert, objectEquals, unreachable } from '../common/util/util.js'; import { align } from './util/math.js'; import { ImageCopyType } from './util/texture/layout.js'; From ca16d2002c4fac1a37d1a76f92f6b9c8c7203eb5 Mon Sep 17 00:00:00 2001 From: Kai Ninomiya Date: Mon, 11 Mar 2024 20:00:14 -0700 Subject: [PATCH 3/4] Generalize validation,image_copy,buffer_related over aspects - Test a few depth/stencil formats that weren't in kSizedTextureFormats (depth24plus-stencil8 stencil, depth32float-stencil8 depth+stencil) - Parameterize over aspect (depth-only/stencil-only/all) - Reduce some overparameterization to make up for it (net ~3x faster) Removes a use of the deprecated .bytesPerBlock. --- .../image_copy/buffer_related.spec.ts | 65 ++++++++++++------- .../api/validation/image_copy/image_copy.ts | 48 ++++++++++++-- src/webgpu/format_info.ts | 25 +++++++ 3 files changed, 108 insertions(+), 30 deletions(-) diff --git a/src/webgpu/api/validation/image_copy/buffer_related.spec.ts b/src/webgpu/api/validation/image_copy/buffer_related.spec.ts index 6952e3734781..f43ff05331ad 100644 --- a/src/webgpu/api/validation/image_copy/buffer_related.spec.ts +++ b/src/webgpu/api/validation/image_copy/buffer_related.spec.ts @@ -4,14 +4,14 @@ import { makeTestGroup } from '../../../../common/framework/test_group.js'; import { kTextureDimensions } from '../../../capability_info.js'; import { GPUConst } from '../../../constants.js'; import { - kSizedTextureFormats, + aspectParamsForTextureFormat, kTextureFormatInfo, textureDimensionAndFormatCompatible, } from '../../../format_info.js'; import { kResourceStates } from '../../../gpu_test.js'; import { kImageCopyTypes } from '../../../util/texture/layout.js'; -import { ImageCopyTest, formatCopyableWithMethod } from './image_copy.js'; +import { ImageCopyTest, aspectCopyableWithMethod, kReducedFormatsList } from './image_copy.js'; export const g = makeTestGroup(ImageCopyTest); @@ -50,7 +50,7 @@ Test that the buffer must be valid and not destroyed. t.testBuffer( buffer, - texture, + { texture }, { bytesPerRow: 0 }, { width: 0, height: 0, depthOrArrayLayers: 0 }, { dataSize: 16, method, success, submit } @@ -86,7 +86,7 @@ g.test('buffer,device_mismatch') // Expect success in both finish and submit, or validation error in finish t.testBuffer( buffer, - texture, + { texture }, { bytesPerRow: 0 }, { width: 0, height: 0, depthOrArrayLayers: 0 }, { dataSize: 16, method, success, submit: success } @@ -135,7 +135,7 @@ TODO update such that it tests // Expect success in both finish and submit, or validation error in finish t.testBuffer( buffer, - texture, + { texture }, { bytesPerRow: 0 }, { width: 0, height: 0, depthOrArrayLayers: 0 }, { dataSize: 16, method, success, submit: success } @@ -148,38 +148,48 @@ g.test('bytes_per_row_alignment') Test that bytesPerRow must be a multiple of 256 for CopyB2T and CopyT2B if it is required. - for all copy methods between linear data and textures - for all texture dimensions -- for all sized formats. -- for various bytesPerRow aligned to 256 or not -- for various number of blocks rows copied +- for all sized (format,aspect) pairs, except for a bunch of redundant "regular" color formats +- for various bytesPerRow (aligned to 256 or not) +- for various number of blocks rows copied (0 or not) ` ) .params(u => u // .combine('method', kImageCopyTypes) - .combine('format', kSizedTextureFormats) - .filter(formatCopyableWithMethod) .combine('dimension', kTextureDimensions) + .combine('format', kReducedFormatsList) + .expandWithParams(p => aspectParamsForTextureFormat(p.format)) + .filter(p => aspectCopyableWithMethod(p.format, p._aspectKey, p.method)) .filter(({ dimension, format }) => textureDimensionAndFormatCompatible(dimension, format)) .beginSubcases() - .combine('bytesPerRow', [undefined, 0, 1, 255, 256, 257, 512]) - .combine('copyHeightInBlocks', [0, 1, 2, 3]) + .combineWithParams([ + // valid: height<=1 + { copyHeightInBlocks: 0, bytesPerRow: 1 }, + { copyHeightInBlocks: 0, bytesPerRow: undefined }, + { copyHeightInBlocks: 1, bytesPerRow: 1 }, + { copyHeightInBlocks: 1, bytesPerRow: undefined }, + // valid: multiples of 256 + { copyHeightInBlocks: 2, bytesPerRow: 0 }, + { copyHeightInBlocks: 2, bytesPerRow: 512 }, + { copyHeightInBlocks: 2, bytesPerRow: 256 }, + // valid only for writeTexture + { copyHeightInBlocks: 2, bytesPerRow: 128 }, + { copyHeightInBlocks: 2, bytesPerRow: 1 }, // with 1-byte formats + { copyHeightInBlocks: 2, bytesPerRow: 2 }, // with <=2-byte formats + { copyHeightInBlocks: 2, bytesPerRow: 8 }, // with <=8-byte formats + { copyHeightInBlocks: 2, bytesPerRow: 32 }, // with <=32-byte formats + ]) .expand('_textureHeightInBlocks', p => [ p.copyHeightInBlocks === 0 ? 1 : p.copyHeightInBlocks, ]) .unless(p => p.dimension === '1d' && p.copyHeightInBlocks > 1) // Depth/stencil format copies must copy the whole subresource. - .unless(p => { - const info = kTextureFormatInfo[p.format]; - return ( - (!!info.depth || !!info.stencil) && p.copyHeightInBlocks !== p._textureHeightInBlocks - ); - }) + .unless(p => p._aspectKey !== 'color' && p.copyHeightInBlocks !== p._textureHeightInBlocks) // bytesPerRow must be specified and it must be equal or greater than the bytes size of each row if we are copying multiple rows. // Note that we are copying one single block on each row in this test. .filter( - ({ format, bytesPerRow, copyHeightInBlocks }) => - (bytesPerRow === undefined && copyHeightInBlocks <= 1) || - (bytesPerRow !== undefined && bytesPerRow >= kTextureFormatInfo[format].bytesPerBlock) + ({ format, _aspectKey, bytesPerRow }) => + bytesPerRow === undefined || bytesPerRow >= kTextureFormatInfo[format][_aspectKey]!.bytes! ) ) .beforeAllSubcases(t => { @@ -188,8 +198,15 @@ Test that bytesPerRow must be a multiple of 256 for CopyB2T and CopyT2B if it is t.selectDeviceOrSkipTestCase(info.feature); }) .fn(t => { - const { method, dimension, format, bytesPerRow, copyHeightInBlocks, _textureHeightInBlocks } = - t.params; + const { + method, + dimension, + format, + aspect, + bytesPerRow, + copyHeightInBlocks, + _textureHeightInBlocks, + } = t.params; const info = kTextureFormatInfo[format]; @@ -217,7 +234,7 @@ Test that bytesPerRow must be a multiple of 256 for CopyB2T and CopyT2B if it is const copySize = [info.blockWidth, copyHeightInBlocks * info.blockHeight, 1]; // Expect success in both finish and submit, or validation error in finish - t.testBuffer(buffer, texture, { bytesPerRow }, copySize, { + t.testBuffer(buffer, { texture, aspect }, { bytesPerRow }, copySize, { dataSize: 512 * 8 * 16, method, success, diff --git a/src/webgpu/api/validation/image_copy/image_copy.ts b/src/webgpu/api/validation/image_copy/image_copy.ts index 0290046675ed..d41e1d7decdb 100644 --- a/src/webgpu/api/validation/image_copy/image_copy.ts +++ b/src/webgpu/api/validation/image_copy/image_copy.ts @@ -4,11 +4,29 @@ import { SizedTextureFormat, kTextureFormatInfo, isCompressedTextureFormat, + resolvePerAspectFormat, + kCompressedTextureFormats, + kDepthStencilFormats, } from '../../../format_info.js'; import { align } from '../../../util/math.js'; import { ImageCopyType } from '../../../util/texture/layout.js'; import { ValidationTest } from '../validation_test.js'; +export const kReducedFormatsList = [ + 'r8unorm', + 'rg8uint', + 'rgba8snorm', + 'r16sint', + 'rg16float', + 'rgba16float', + 'r32float', + 'rg32float', + 'rgba32float', + 'rgb9e5ufloat', + ...kCompressedTextureFormats, + ...kDepthStencilFormats, +] as const; + export class ImageCopyTest extends ValidationTest { testRun( textureCopyView: GPUImageCopyTexture, @@ -123,7 +141,7 @@ export class ImageCopyTest extends ValidationTest { testBuffer( buffer: GPUBuffer, - texture: GPUTexture, + texture: GPUImageCopyTexture, textureDataLayout: GPUImageDataLayout, size: GPUExtent3D, { @@ -145,14 +163,14 @@ export class ImageCopyTest extends ValidationTest { const data = new Uint8Array(dataSize); this.expectValidationError(() => { - this.device.queue.writeTexture({ texture }, data, textureDataLayout, size); + this.device.queue.writeTexture(texture, data, textureDataLayout, size); }, !success); break; } case 'CopyB2T': { const { encoder, validateFinish, validateFinishAndSubmit } = this.createEncoder('non-pass'); - encoder.copyBufferToTexture({ buffer, ...textureDataLayout }, { texture }, size); + encoder.copyBufferToTexture({ buffer, ...textureDataLayout }, texture, size); if (submit) { // validation error is expected to come from the submit and encoding should succeed @@ -165,13 +183,13 @@ export class ImageCopyTest extends ValidationTest { break; } case 'CopyT2B': { - if (this.isCompatibility && isCompressedTextureFormat(texture.format)) { + if (this.isCompatibility && isCompressedTextureFormat(texture.texture.format)) { this.skip( 'copyTextureToBuffer is not supported for compressed texture formats in compatibility mode.' ); } const { encoder, validateFinish, validateFinishAndSubmit } = this.createEncoder('non-pass'); - encoder.copyTextureToBuffer({ texture }, { buffer, ...textureDataLayout }, size); + encoder.copyTextureToBuffer(texture, { buffer, ...textureDataLayout }, size); if (submit) { // validation error is expected to come from the submit and encoding should succeed @@ -244,7 +262,11 @@ export function texelBlockAlignmentTestExpanderForValueToCoordinate({ } } -// This is a helper function used for filtering test parameters +/** + * This is a helper function used for filtering test parameters. + * + * @deprecated Prefer `aspectCopyableWithMethod` if possible. + */ export function formatCopyableWithMethod({ format, method }: WithFormatAndMethod): boolean { const info = kTextureFormatInfo[format]; if (info.depth || info.stencil) { @@ -261,6 +283,20 @@ export function formatCopyableWithMethod({ format, method }: WithFormatAndMethod } } +/** True iff the (format,aspect) pair can be copied with this method. */ +export function aspectCopyableWithMethod( + format: GPUTextureFormat, + aspect: 'color' | 'depth' | 'stencil', + method: ImageCopyType +) { + const info = kTextureFormatInfo[format][aspect]!; + if (method === 'CopyT2B') { + return info.copySrc; + } else { + return info.copyDst; + } +} + // This is a helper function used for filtering test parameters export function getACopyableAspectWithMethod({ format, diff --git a/src/webgpu/format_info.ts b/src/webgpu/format_info.ts index 1c3473c0487e..1becd9cbb68d 100644 --- a/src/webgpu/format_info.ts +++ b/src/webgpu/format_info.ts @@ -1558,6 +1558,31 @@ export const kValidTextureFormatsForCopyE2T = [ // Other related stuff // +/** + * Pass this to `expandWithParams` to expand a format list into a format+aspect list. + * + * In some cases, `aspect` will be unset, invoking the default (which is always `"all"`). + */ +export function* aspectParamsForTextureFormat(format: GPUTextureFormat): Generator<{ + /** `GPUTextureAspect | undefined` to pass to WebGPU. */ + aspect?: 'depth-only' | 'stencil-only'; + /** Key for the aspect in the info table: `kTextureFormatInfo[format][_aspectKey]`. */ + _aspectKey: 'color' | 'depth' | 'stencil'; +}> { + const info = kTextureFormatInfo[format]; + if (info.color?.bytes) { + yield { _aspectKey: 'color' } as const; + } + if (info.depth?.bytes) { + yield { aspect: 'depth-only', _aspectKey: 'depth' } as const; + if (!info.stencil) yield { _aspectKey: 'depth' } as const; + } + if (info.stencil?.bytes) { + yield { aspect: 'stencil-only', _aspectKey: 'stencil' } as const; + if (!info.depth) yield { _aspectKey: 'stencil' } as const; + } +} + const kDepthStencilFormatCapabilityInBufferTextureCopy = { // kUnsizedDepthStencilFormats depth24plus: { From 3d71fd30c57f6e8e1a4507007da7b9fde99279f7 Mon Sep 17 00:00:00 2001 From: Kai Ninomiya Date: Wed, 13 Mar 2024 11:22:32 -0700 Subject: [PATCH 4/4] lint --- src/webgpu/api/validation/image_copy/image_copy.ts | 1 - src/webgpu/format_info.ts | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/webgpu/api/validation/image_copy/image_copy.ts b/src/webgpu/api/validation/image_copy/image_copy.ts index d41e1d7decdb..09f8971bd846 100644 --- a/src/webgpu/api/validation/image_copy/image_copy.ts +++ b/src/webgpu/api/validation/image_copy/image_copy.ts @@ -4,7 +4,6 @@ import { SizedTextureFormat, kTextureFormatInfo, isCompressedTextureFormat, - resolvePerAspectFormat, kCompressedTextureFormats, kDepthStencilFormats, } from '../../../format_info.js'; diff --git a/src/webgpu/format_info.ts b/src/webgpu/format_info.ts index 1becd9cbb68d..2c72e05f8ab5 100644 --- a/src/webgpu/format_info.ts +++ b/src/webgpu/format_info.ts @@ -1,5 +1,5 @@ import { keysOf } from '../common/util/data_tables.js'; -import { assert, objectEquals, unreachable } from '../common/util/util.js'; +import { assert, unreachable } from '../common/util/util.js'; import { align } from './util/math.js'; import { ImageCopyType } from './util/texture/layout.js';