Skip to content

Commit

Permalink
Compat: fix rgXXX format as storage texture issues (#3805)
Browse files Browse the repository at this point in the history
Spec changed so that usage of rgXXX format textures
as storage textures should generate a shader module
creation error.
  • Loading branch information
greggman authored Jun 21, 2024
1 parent 9f1f6db commit 85678b2
Show file tree
Hide file tree
Showing 10 changed files with 27 additions and 10 deletions.
5 changes: 3 additions & 2 deletions src/webgpu/format_info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1782,13 +1782,14 @@ export const kCompatModeUnsupportedStorageTextureFormats: readonly GPUTextureFor
export function isTextureFormatUsableAsStorageFormat(
format: GPUTextureFormat,
isCompatibilityMode: boolean
) {
): boolean {
if (isCompatibilityMode) {
if (kCompatModeUnsupportedStorageTextureFormats.indexOf(format) >= 0) {
return false;
}
}
return !!kTextureFormatInfo[format].color?.storage;
const info = kTextureFormatInfo[format];
return !!(info.color?.storage || info.depth?.storage || info.stencil?.storage);
}

export function isRegularTextureFormat(format: GPUTextureFormat) {
Expand Down
8 changes: 8 additions & 0 deletions src/webgpu/gpu_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,14 @@ export class GPUTestBase extends Fixture<GPUTestSubcaseBatchState> {
}
}

skipIfTextureFormatNotUsableAsStorageTexture(...formats: (GPUTextureFormat | undefined)[]) {
for (const format of formats) {
if (format && !isTextureFormatUsableAsStorageFormat(format, this.isCompatibility)) {
this.skip(`Texture with ${format} is not usable as a storage texture`);
}
}
}

/** Skips this test case if the `langFeature` is *not* supported. */
skipIfLanguageFeatureNotSupported(langFeature: WGSLLanguageFeature) {
if (!this.hasLanguageFeature(langFeature)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,8 @@ Validates the return type of ${builtin} is the expected type.
)
.fn(t => {
const { returnType, textureType, format } = t.params;
t.skipIfTextureFormatNotUsableAsStorageTexture(format);

const returnVarType = kValuesTypes[returnType];
const { returnType: returnRequiredType, hasLevelArg } =
kValidTextureDimensionParameterTypesForStorageTextures[textureType];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,8 @@ Validates that only incorrect coords arguments are rejected by ${builtin}
)
.fn(t => {
const { textureType, coordType, format, value } = t.params;
t.skipIfTextureFormatNotUsableAsStorageTexture(format);

const coordArgType = kValuesTypes[coordType];
const { coordsArgTypes, hasArrayIndexArg } =
kValidTextureLoadParameterTypesForStorageTextures[textureType];
Expand Down Expand Up @@ -307,6 +309,8 @@ Validates that only incorrect array_index arguments are rejected by ${builtin}
)
.fn(t => {
const { textureType, arrayIndexType, format, value } = t.params;
t.skipIfTextureFormatNotUsableAsStorageTexture(format);

const arrayIndexArgType = kValuesTypes[arrayIndexType];
const args = [arrayIndexArgType.create(value)];
const { coordsArgTypes, hasLevelArg } =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ Validates the return type of ${builtin} is the expected type.
)
.fn(t => {
const { returnType, textureType, format } = t.params;
t.skipIfTextureFormatNotUsableAsStorageTexture(format);

const returnVarType = kValuesTypes[returnType];

const varWGSL = returnVarType.toString();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,8 @@ Validates that only incorrect value arguments are rejected by ${builtin}
)
.fn(t => {
const { textureType, valueType, format, value } = t.params;
t.skipIfTextureFormatNotUsableAsStorageTexture(format);

const valueArgType = kValuesTypes[valueType];
const args = [valueArgType.create(value)];
const { coordsArgTypes, hasArrayIndexArg } = kValidTextureStoreParameterTypes[textureType];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ g.test('var_decl')
)
.fn(t => {
const { type, format, access } = t.params;
t.skipIfTextureFormatNotUsableAsStorageTexture(format.format);

const source = `@group(0) @binding(0) var t : ${type}<${format.format}, ${access}>;`;
const requiresFeature = access !== 'write';
t.expectCompileResult(t.hasLanguageFeature(kFeatureName) || !requiresFeature, source);
Expand Down
2 changes: 1 addition & 1 deletion src/webgpu/shader/validation/shader_io/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ export const kResourceEmitters = new Map<string, ResourceDeclarationEmitter>([
['texture_storage_1d', basicEmitter('texture_storage_1d<rgba8unorm, write>')],
['texture_storage_2d', basicEmitter('texture_storage_2d<rgba8sint, write>')],
['texture_storage_2d_array', basicEmitter('texture_storage_2d_array<r32uint, write>')],
['texture_storage_3d', basicEmitter('texture_storage_3d<rg32uint, write>')],
['texture_storage_3d', basicEmitter('texture_storage_3d<rgba32uint, write>')],
['texture_depth_2d', basicEmitter('texture_depth_2d')],
['texture_depth_2d_array', basicEmitter('texture_depth_2d_array')],
['texture_depth_cube', basicEmitter('texture_depth_cube')],
Expand Down
2 changes: 1 addition & 1 deletion src/webgpu/shader/validation/types/alias.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ const kTypes = [
'texture_storage_1d<r32sint, read_write>',
'texture_storage_1d<r32float, read>',
'texture_storage_2d<rgba16uint, write>',
'texture_storage_2d_array<rg32float, write>',
'texture_storage_2d_array<rgba32float, write>',
'texture_storage_3d<bgra8unorm, write>',
'texture_depth_2d',
'texture_depth_2d_array',
Expand Down
8 changes: 2 additions & 6 deletions src/webgpu/shader/validation/types/textures.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ g.test('sampled_texture_types')

g.test('external_sampled_texture_types')
.desc(
`Test that texture_extenal compiles and cannot specify address space
`Test that texture_external compiles and cannot specify address space
`
)
.fn(t => {
Expand All @@ -118,13 +118,9 @@ Besides, the shader compilation should always pass regardless of whether the for
)
.fn(t => {
const { format, access, comma } = t.params;
const info = kTextureFormatInfo[format];
// bgra8unorm is considered a valid storage format at shader compilation stage
const isFormatValid =
info.color?.storage ||
info.depth?.storage ||
info.stencil?.storage ||
format === 'bgra8unorm';
isTextureFormatUsableAsStorageFormat(format, t.isCompatibility) || format === 'bgra8unorm';
const isAccessValid = kAccessModes.includes(access);
const wgsl = `@group(0) @binding(0) var tex: texture_storage_2d<${format}, ${access}${comma}>;`;
t.expectCompileResult(isFormatValid && isAccessValid, wgsl);
Expand Down

0 comments on commit 85678b2

Please sign in to comment.