From f2c617456699343dcb93634f1b690dc5950181e8 Mon Sep 17 00:00:00 2001 From: Gregg Tavares Date: Wed, 11 Dec 2024 15:35:51 +0900 Subject: [PATCH 1/3] Compat: adjust for 0 vert/frag storage buffers/textures Compat does allows supporting 0 storage buffers and storage textures in fragment shaders. Update some test for this situation. --- .../maxStorageBuffersPerShaderStage.spec.ts | 18 ++++++++ .../validation/layout_shader_compat.spec.ts | 43 ++++++++++++++++++- .../resource_compatibility.spec.ts | 33 +++++++++++++- src/webgpu/gpu_test.ts | 11 +++++ 4 files changed, 102 insertions(+), 3 deletions(-) diff --git a/src/webgpu/api/validation/capability_checks/limits/maxStorageBuffersPerShaderStage.spec.ts b/src/webgpu/api/validation/capability_checks/limits/maxStorageBuffersPerShaderStage.spec.ts index ee7c3a024630..05c737774c70 100644 --- a/src/webgpu/api/validation/capability_checks/limits/maxStorageBuffersPerShaderStage.spec.ts +++ b/src/webgpu/api/validation/capability_checks/limits/maxStorageBuffersPerShaderStage.spec.ts @@ -21,6 +21,8 @@ import { const kExtraLimits: LimitsRequest = { maxBindingsPerBindGroup: 'adapterLimit', maxBindGroups: 'adapterLimit', + maxStorageBuffersInFragmentStage: 'adapterLimit', + maxStorageBuffersInVertexStage: 'adapterLimit', }; const limit = 'maxStorageBuffersPerShaderStage'; @@ -165,6 +167,22 @@ g.test('createPipeline,at_over') `can not test ${testValue} bindings in same group because maxBindingsPerBindGroup = ${device.limits.maxBindingsPerBindGroup}` ); + t.skipIf( + (bindingCombination === 'fragment' || + bindingCombination === 'vertexAndFragmentWithPossibleVertexStageOverflow' || + bindingCombination === 'vertexAndFragmentWithPossibleFragmentStageOverflow') && + testValue > device.limits.maxStorageBuffersInFragmentStage, + `can not test ${testValue} bindings as it is more than maxStorageBuffersInFragmentStage(${device.limits.maxStorageBuffersInFragmentStage})` + ); + + t.skipIf( + (bindingCombination === 'vertex' || + bindingCombination === 'vertexAndFragmentWithPossibleVertexStageOverflow' || + bindingCombination === 'vertexAndFragmentWithPossibleFragmentStageOverflow') && + testValue > device.limits.maxStorageBuffersInVertexStage, + `can not test ${testValue} bindings as it is more than maxStorageBuffersInVertexStage(${device.limits.maxStorageBuffersInVertexStage})` + ); + const code = getPerStageWGSLForBindingCombination( bindingCombination, order, diff --git a/src/webgpu/api/validation/layout_shader_compat.spec.ts b/src/webgpu/api/validation/layout_shader_compat.spec.ts index 5ee16510c77a..79e4f7b2926e 100644 --- a/src/webgpu/api/validation/layout_shader_compat.spec.ts +++ b/src/webgpu/api/validation/layout_shader_compat.spec.ts @@ -12,6 +12,7 @@ import { ValidBindableResource, } from '../../capability_info.js'; import { GPUConst } from '../../constants.js'; +import { MaxLimitsTestMixin } from '../../gpu_test.js'; import { ValidationTest } from './validation_test.js'; @@ -156,7 +157,7 @@ const BindingResourceCompatibleWithShaderStages = function ( return true; }; -export const g = makeTestGroup(F); +export const g = makeTestGroup(MaxLimitsTestMixin(F)); g.test('pipeline_layout_shader_exact_match') .desc( @@ -195,6 +196,46 @@ g.test('pipeline_layout_shader_exact_match') isBindingStaticallyUsed, } = t.params; + if (t.isCompatibility) { + const bindingUsedWithVertexStage = + (shaderStageWithBinding & GPUShaderStage.VERTEX) !== 0 || + (pipelineLayoutVisibility & GPUShaderStage.VERTEX) !== 0; + const bindingUsedWithFragmentStage = + (shaderStageWithBinding & GPUShaderStage.FRAGMENT) !== 0 || + (pipelineLayoutVisibility & GPUShaderStage.FRAGMENT) !== 0; + const bindingIsStorageBuffer = + bindingInPipelineLayout === 'readonlyStorageBuf' || + bindingInPipelineLayout === 'storageBuf'; + const bindingIsStorageTexture = + bindingInPipelineLayout === 'readonlyStorageTex' || + bindingInPipelineLayout === 'readwriteStorageTex' || + bindingInPipelineLayout === 'writeonlyStorageTex'; + t.skipIf( + bindingUsedWithVertexStage && + bindingIsStorageBuffer && + t.device.limits.maxStorageBuffersInVertexStage === 0, + 'Storage buffers can not be used in vertex shaders because maxStorageBuffersInVertexStage === 0' + ); + t.skipIf( + bindingUsedWithVertexStage && + bindingIsStorageTexture && + t.device.limits.maxStorageTexturesInVertexStage === 0, + 'Storage textures can not be used in vertex shaders because maxStorageTexturesInVertexStage === 0' + ); + t.skipIf( + bindingUsedWithFragmentStage && + bindingIsStorageBuffer && + t.device.limits.maxStorageBuffersInFragmentStage === 0, + 'Storage buffers can not be used in fragment shaders because maxStorageBuffersInFragmentStage === 0' + ); + t.skipIf( + bindingUsedWithFragmentStage && + bindingIsStorageTexture && + t.device.limits.maxStorageTexturesInFragmentStage === 0, + 'Storage textures can not be used in fragment shaders because maxStorageTexturesInFragmentStage === 0' + ); + } + const layout = t.createPipelineLayout(bindingInPipelineLayout, pipelineLayoutVisibility); const bindResourceDeclaration = `@group(0) @binding(0) ${t.GetBindableResourceShaderDeclaration( bindingInShader diff --git a/src/webgpu/api/validation/render_pipeline/resource_compatibility.spec.ts b/src/webgpu/api/validation/render_pipeline/resource_compatibility.spec.ts index f14211482164..9406d30f9493 100644 --- a/src/webgpu/api/validation/render_pipeline/resource_compatibility.spec.ts +++ b/src/webgpu/api/validation/render_pipeline/resource_compatibility.spec.ts @@ -4,6 +4,7 @@ Tests for resource compatibility between pipeline layout and shader modules import { makeTestGroup } from '../../../../common/framework/test_group.js'; import { keysOf } from '../../../../common/util/data_tables.js'; +import { MaxLimitsTestMixin } from '../../../gpu_test.js'; import { kAPIResources, getWGSLShaderForResource, @@ -13,7 +14,7 @@ import { import { CreateRenderPipelineValidationTest } from './common.js'; -export const g = makeTestGroup(CreateRenderPipelineValidationTest); +export const g = makeTestGroup(MaxLimitsTestMixin(CreateRenderPipelineValidationTest)); g.test('resource_compatibility') .desc( @@ -53,8 +54,36 @@ g.test('resource_compatibility') ((wgslResource.buffer !== undefined && wgslResource.buffer.type === 'storage') || (wgslResource.storageTexture !== undefined && wgslResource.storageTexture.access !== 'read-only')), - 'Storage buffers and textures cannot be used in vertex shaders' + 'Read-Write Storage buffers and textures cannot be used in vertex shaders' ); + if (t.isCompatibility) { + t.skipIf( + t.params.stage === 'vertex' && + (apiResource.buffer?.type === 'storage' || + apiResource.buffer?.type === 'read-only-storage') && + t.device.limits.maxStorageBuffersInVertexStage === 0, + 'Storage buffers can not be used in vertex shaders because maxStorageBuffersInVertexStage === 0' + ); + t.skipIf( + t.params.stage === 'vertex' && + apiResource.storageTexture !== undefined && + t.device.limits.maxStorageTexturesInVertexStage === 0, + 'Storage textures can not be used in vertex shaders because maxStorageTexturesInVertexStage === 0' + ); + t.skipIf( + t.params.stage === 'fragment' && + (apiResource.buffer?.type === 'storage' || + apiResource.buffer?.type === 'read-only-storage') && + t.device.limits.maxStorageBuffersInFragmentStage === 0, + 'Storage buffers can not be used in fragment shaders because maxStorageBuffersInFragmentStage === 0' + ); + t.skipIf( + t.params.stage === 'fragment' && + apiResource.storageTexture !== undefined && + t.device.limits.maxStorageTexturesInFragmentStage === 0, + 'Storage textures can not be used in fragment shaders because maxStorageTexturesInFragmentStage === 0' + ); + } t.skipIfTextureViewDimensionNotSupported(wgslResource.texture?.viewDimension); const emptyVS = ` @vertex diff --git a/src/webgpu/gpu_test.ts b/src/webgpu/gpu_test.ts index ffc20f5f2c22..7ecd7c0befa0 100644 --- a/src/webgpu/gpu_test.ts +++ b/src/webgpu/gpu_test.ts @@ -67,6 +67,17 @@ import { import { createTextureFromTexelViews } from './util/texture.js'; import { reifyExtent3D, reifyOrigin3D } from './util/unions.js'; +// Declarations for WebGPU items we want tests for that are not yet officially part of the spec. +declare global { + // MAINTENANCE_TODO: remove once added to @webgpu/types + interface GPUSupportedLimits { + readonly maxStorageBuffersInFragmentStage: number; + readonly maxStorageTexturesInFragmentStage: number; + readonly maxStorageBuffersInVertexStage: number; + readonly maxStorageTexturesInVertexStage: number; + } +} + const devicePool = new DevicePool(); // MAINTENANCE_TODO: When DevicePool becomes able to provide multiple devices at once, use the From eaf74b9534a9b1402f1542a12be2da9a008dda03 Mon Sep 17 00:00:00 2001 From: Gregg Tavares Date: Thu, 12 Dec 2024 10:34:44 +0900 Subject: [PATCH 2/3] address comments --- .../limits/maxStorageBuffersPerShaderStage.spec.ts | 4 ++-- src/webgpu/gpu_test.ts | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/webgpu/api/validation/capability_checks/limits/maxStorageBuffersPerShaderStage.spec.ts b/src/webgpu/api/validation/capability_checks/limits/maxStorageBuffersPerShaderStage.spec.ts index 05c737774c70..38828eef261d 100644 --- a/src/webgpu/api/validation/capability_checks/limits/maxStorageBuffersPerShaderStage.spec.ts +++ b/src/webgpu/api/validation/capability_checks/limits/maxStorageBuffersPerShaderStage.spec.ts @@ -171,7 +171,7 @@ g.test('createPipeline,at_over') (bindingCombination === 'fragment' || bindingCombination === 'vertexAndFragmentWithPossibleVertexStageOverflow' || bindingCombination === 'vertexAndFragmentWithPossibleFragmentStageOverflow') && - testValue > device.limits.maxStorageBuffersInFragmentStage, + testValue > (device.limits.maxStorageBuffersInFragmentStage ?? actualLimit), `can not test ${testValue} bindings as it is more than maxStorageBuffersInFragmentStage(${device.limits.maxStorageBuffersInFragmentStage})` ); @@ -179,7 +179,7 @@ g.test('createPipeline,at_over') (bindingCombination === 'vertex' || bindingCombination === 'vertexAndFragmentWithPossibleVertexStageOverflow' || bindingCombination === 'vertexAndFragmentWithPossibleFragmentStageOverflow') && - testValue > device.limits.maxStorageBuffersInVertexStage, + testValue > (device.limits.maxStorageBuffersInVertexStage ?? actualLimit), `can not test ${testValue} bindings as it is more than maxStorageBuffersInVertexStage(${device.limits.maxStorageBuffersInVertexStage})` ); diff --git a/src/webgpu/gpu_test.ts b/src/webgpu/gpu_test.ts index 7ecd7c0befa0..53e4a1481791 100644 --- a/src/webgpu/gpu_test.ts +++ b/src/webgpu/gpu_test.ts @@ -71,10 +71,10 @@ import { reifyExtent3D, reifyOrigin3D } from './util/unions.js'; declare global { // MAINTENANCE_TODO: remove once added to @webgpu/types interface GPUSupportedLimits { - readonly maxStorageBuffersInFragmentStage: number; - readonly maxStorageTexturesInFragmentStage: number; - readonly maxStorageBuffersInVertexStage: number; - readonly maxStorageTexturesInVertexStage: number; + readonly maxStorageBuffersInFragmentStage?: number; + readonly maxStorageTexturesInFragmentStage?: number; + readonly maxStorageBuffersInVertexStage?: number; + readonly maxStorageTexturesInVertexStage?: number; } } From c427874282f78916b43e6af9017d9ed4968b63c0 Mon Sep 17 00:00:00 2001 From: Gregg Tavares Date: Thu, 12 Dec 2024 10:54:47 +0900 Subject: [PATCH 3/3] fix previous change --- .../maxStorageBuffersPerShaderStage.spec.ts | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/src/webgpu/api/validation/capability_checks/limits/maxStorageBuffersPerShaderStage.spec.ts b/src/webgpu/api/validation/capability_checks/limits/maxStorageBuffersPerShaderStage.spec.ts index 38828eef261d..60be6d30c1d2 100644 --- a/src/webgpu/api/validation/capability_checks/limits/maxStorageBuffersPerShaderStage.spec.ts +++ b/src/webgpu/api/validation/capability_checks/limits/maxStorageBuffersPerShaderStage.spec.ts @@ -167,21 +167,23 @@ g.test('createPipeline,at_over') `can not test ${testValue} bindings in same group because maxBindingsPerBindGroup = ${device.limits.maxBindingsPerBindGroup}` ); - t.skipIf( - (bindingCombination === 'fragment' || - bindingCombination === 'vertexAndFragmentWithPossibleVertexStageOverflow' || - bindingCombination === 'vertexAndFragmentWithPossibleFragmentStageOverflow') && - testValue > (device.limits.maxStorageBuffersInFragmentStage ?? actualLimit), - `can not test ${testValue} bindings as it is more than maxStorageBuffersInFragmentStage(${device.limits.maxStorageBuffersInFragmentStage})` - ); + if (t.isCompatibility) { + t.skipIf( + (bindingCombination === 'fragment' || + bindingCombination === 'vertexAndFragmentWithPossibleVertexStageOverflow' || + bindingCombination === 'vertexAndFragmentWithPossibleFragmentStageOverflow') && + testValue > device.limits.maxStorageBuffersInFragmentStage!, + `can not test ${testValue} bindings as it is more than maxStorageBuffersInFragmentStage(${device.limits.maxStorageBuffersInFragmentStage})` + ); - t.skipIf( - (bindingCombination === 'vertex' || - bindingCombination === 'vertexAndFragmentWithPossibleVertexStageOverflow' || - bindingCombination === 'vertexAndFragmentWithPossibleFragmentStageOverflow') && - testValue > (device.limits.maxStorageBuffersInVertexStage ?? actualLimit), - `can not test ${testValue} bindings as it is more than maxStorageBuffersInVertexStage(${device.limits.maxStorageBuffersInVertexStage})` - ); + t.skipIf( + (bindingCombination === 'vertex' || + bindingCombination === 'vertexAndFragmentWithPossibleVertexStageOverflow' || + bindingCombination === 'vertexAndFragmentWithPossibleFragmentStageOverflow') && + testValue > device.limits.maxStorageBuffersInVertexStage!, + `can not test ${testValue} bindings as it is more than maxStorageBuffersInVertexStage(${device.limits.maxStorageBuffersInVertexStage})` + ); + } const code = getPerStageWGSLForBindingCombination( bindingCombination,