bytesPerBlock refactors; generalize validation,image_copy,buffer_related over aspects

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,

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,

src/webgpu/api/validation/image_copy/image_copy.ts

@@ -4,11 +4,28 @@ import {
   SizedTextureFormat,
   kTextureFormatInfo,
   isCompressedTextureFormat,
+  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 +140,7 @@ export class ImageCopyTest extends ValidationTest {
 
   testBuffer(
     buffer: GPUBuffer,
-    texture: GPUTexture,
+    texture: GPUImageCopyTexture,
     textureDataLayout: GPUImageDataLayout,
     size: GPUExtent3D,
     {
@@ -145,14 +162,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 +182,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 +261,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 +282,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,

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'
+        );
+      }
+    }
+  });

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: {