-
Notifications
You must be signed in to change notification settings - Fork 101
Add immediate size validation cases for pipeline creation #4573
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Results for build job (at 2127eae): +webgpu:api,validation,compute_pipeline:pipeline_creation_immediate_size_mismatch:* - 12 cases, 12 subcases (~1/case)
+webgpu:api,validation,render_pipeline,misc:pipeline_creation_immediate_size_mismatch:* - 18 cases, 18 subcases (~1/case)
-TOTAL: 285744 cases, 2344468 subcases
+TOTAL: 285774 cases, 2344498 subcases |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds validation test cases for immediate data size checks during pipeline creation. The tests verify that pipeline creation fails when shaders use immediate data larger than the immediateSize specified in the pipeline layout or larger than maxImmediateSize when using automatic layout.
Changes:
- Added helper function
supportsImmediateDatato check browser support for immediate data - Added
pipeline_creation_immediate_size_mismatchtest for compute pipelines - Added
pipeline_creation_immediate_size_mismatchtest for render pipelines
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| src/common/util/util.ts | Added supportsImmediateData helper function to detect immediate data feature support |
| src/webgpu/api/validation/compute_pipeline.spec.ts | Added test to validate immediate size checks during compute pipeline creation |
| src/webgpu/api/validation/render_pipeline/misc.spec.ts | Added test to validate immediate size checks during render pipeline creation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| g.test('pipeline_creation_immediate_size_mismatch') | ||
| .desc( | ||
| ` | ||
| Validate that creating a pipeline fails if the shader uses immediate data | ||
| larger than the immediateSize specified in the pipeline layout, or larger than | ||
| maxImmediateSize if layout is 'auto'. | ||
| Also validates that using less or equal size is allowed. | ||
| ` | ||
| ) | ||
| .params(u => | ||
| u.combineWithParams([ | ||
| { vertexSize: 16, fragmentSize: 16, layoutSize: 16 }, // Equal | ||
| { vertexSize: 12, fragmentSize: 12, layoutSize: 16 }, // Shader smaller | ||
| { vertexSize: 20, fragmentSize: 20, layoutSize: 16 }, // Shader larger (small diff) | ||
| { vertexSize: 32, fragmentSize: 32, layoutSize: 16 }, // Shader larger | ||
| { vertexSize: 'max+4', fragmentSize: 0, layoutSize: 'auto' }, // Vertex > Limit | ||
| { vertexSize: 0, fragmentSize: 'max+4', layoutSize: 'auto' }, // Fragment > Limit | ||
| { vertexSize: 'max', fragmentSize: 0, layoutSize: 'auto' }, // Vertex = Limit (Control) | ||
| { vertexSize: 0, fragmentSize: 'max', layoutSize: 'auto' }, // Fragment = Limit (Control) | ||
| { vertexSize: 'max', fragmentSize: 'max', layoutSize: 'auto' }, // Both at Limit (Control) | ||
| ] as const) | ||
| ) | ||
| .fn(t => { | ||
| if (!supportsImmediateData(t.device)) { | ||
| t.skip('Immediate data not supported'); | ||
| } | ||
| const { vertexSize, fragmentSize, layoutSize } = t.params; | ||
|
|
||
| const maxImmediateSize = t.device.limits.maxImmediateSize; | ||
| if (maxImmediateSize === undefined) { | ||
| t.skip('maxImmediateSize not supported'); | ||
| } | ||
|
|
||
| const resolveSize = (sizeDescriptor: number | string) => { | ||
| if (typeof sizeDescriptor === 'number') return sizeDescriptor; | ||
| if (sizeDescriptor === 'max') return maxImmediateSize; | ||
| if (sizeDescriptor === 'max+4') return maxImmediateSize + 4; | ||
| return 0; | ||
| }; | ||
|
|
||
| const resolvedVertexImmediateSize = resolveSize(vertexSize); | ||
| const resolvedFragmentImmediateSize = resolveSize(fragmentSize); | ||
| const varSize = Math.max(resolvedVertexImmediateSize, resolvedFragmentImmediateSize); | ||
|
|
||
| const code = ` | ||
| var<immediate> data: array<u32, ${varSize / 4}>; | ||
| fn use_v() { ${resolvedVertexImmediateSize > 0 ? '_ = data[0];' : ''} } | ||
| fn use_f() { ${resolvedFragmentImmediateSize > 0 ? '_ = data[0];' : ''} } | ||
| @vertex fn main_vertex() -> @builtin(position) vec4<f32> { use_v(); return vec4<f32>(0.0, 0.0, 0.0, 1.0); } | ||
| @fragment fn main_fragment() -> @location(0) vec4<f32> { use_f(); return vec4<f32>(0.0, 1.0, 0.0, 1.0); } | ||
| `; | ||
|
|
||
| let layout: GPUPipelineLayout | 'auto'; | ||
| let validSize: number; | ||
|
|
||
| if (layoutSize === 'auto') { | ||
| layout = 'auto'; | ||
| validSize = maxImmediateSize; | ||
| } else { | ||
| layout = t.device.createPipelineLayout({ | ||
| bindGroupLayouts: [], | ||
| immediateSize: layoutSize as number, | ||
| }); | ||
| validSize = layoutSize as number; | ||
| } | ||
|
|
||
| const shouldError = varSize > validSize; | ||
|
|
||
| t.expectValidationError(() => { | ||
| t.device.createRenderPipeline({ | ||
| layout, | ||
| vertex: { | ||
| module: t.device.createShaderModule({ code }), | ||
| }, | ||
| fragment: { | ||
| module: t.device.createShaderModule({ code }), | ||
| targets: [{ format: 'rgba8unorm' }], | ||
| }, | ||
| }); | ||
| }, shouldError); | ||
| }); |
Copilot
AI
Jan 21, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test should validate both synchronous and asynchronous pipeline creation. Add an isAsync parameter to match the pattern used by other pipeline creation tests in this file. The params should include .combine('isAsync', [true, false]) and the test should use vtu.doCreateRenderPipelineTest helper with the isAsync parameter instead of directly calling createRenderPipeline.
| module: t.device.createShaderModule({ code }), | ||
| }, | ||
| fragment: { | ||
| module: t.device.createShaderModule({ code }), |
Copilot
AI
Jan 21, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The shader defines entry points named main_vertex and main_fragment, but the pipeline descriptor doesn't specify entryPoint for either the vertex or fragment stages. According to the WebGPU specification, the default entry point name is "main", so the pipeline creation will fail with an entry point not found error instead of testing immediate data size validation.
Add entryPoint: 'main_vertex' to the vertex stage and entryPoint: 'main_fragment' to the fragment stage in the pipeline descriptor.
| module: t.device.createShaderModule({ code }), | |
| }, | |
| fragment: { | |
| module: t.device.createShaderModule({ code }), | |
| module: t.device.createShaderModule({ code }), | |
| entryPoint: 'main_vertex', | |
| }, | |
| fragment: { | |
| module: t.device.createShaderModule({ code }), | |
| entryPoint: 'main_fragment', |
| ] as const) | ||
| ) | ||
| .fn(t => { | ||
| if (!supportsImmediateData(t.device)) { |
Copilot
AI
Jan 21, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The supportsImmediateData function expects a GPU parameter but is being called with t.device which is a GPUDevice. This will cause a runtime error since GPUDevice does not have a wgslLanguageFeatures property.
The function should be called with the GPU object obtained via getGPU(t.rec) instead. Import getGPU from the navigator_gpu module and change the call to supportsImmediateData(getGPU(t.rec)).
| ] as const) | ||
| ) | ||
| .fn(t => { | ||
| if (!supportsImmediateData(t.device)) { |
Copilot
AI
Jan 21, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The supportsImmediateData function expects a GPU parameter but is being called with t.device which is a GPUDevice. This will cause a runtime error since GPUDevice does not have a wgslLanguageFeatures property.
The function should be called with the GPU object obtained via getGPU(t.rec) instead. Import getGPU from the navigator_gpu module and change the call to supportsImmediateData(getGPU(t.rec)).
| const maxImmediateSize = t.device.limits.maxImmediateSize; | ||
|
|
||
| let actualLayout: GPUPipelineLayout | 'auto'; | ||
| let validSize: number; | ||
|
|
||
| if (layoutSize === 'auto') { | ||
| actualLayout = 'auto'; | ||
| // checked above | ||
| validSize = maxImmediateSize!; |
Copilot
AI
Jan 21, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The maxImmediateSize limit is accessed without checking if it's undefined. While there's a comment on line 849 saying "checked above", the only check above is the supportsImmediateData check which doesn't guarantee that maxImmediateSize is defined. Add a check similar to the render pipeline test: if (maxImmediateSize === undefined) { t.skip('maxImmediateSize not supported'); }
| g.test('pipeline_creation_immediate_size_mismatch') | ||
| .desc( | ||
| ` | ||
| Validate that creating a pipeline fails if the shader uses immediate data | ||
| larger than the immediateSize specified in the pipeline layout, or larger than | ||
| maxImmediateSize if layout is 'auto'. | ||
| Also validates that using less or equal size is allowed. | ||
| ` | ||
| ) | ||
| .params(u => | ||
| u.combineWithParams([ | ||
| { shaderSize: 16, layoutSize: 16 }, // Equal | ||
| { shaderSize: 12, layoutSize: 16 }, // Shader smaller | ||
| { shaderSize: 20, layoutSize: 16 }, // Shader larger (small diff) | ||
| { shaderSize: 32, layoutSize: 16 }, // Shader larger | ||
| { shaderSize: 'max', layoutSize: 'auto' }, // Shader equal to limit (auto layout) | ||
| { shaderSize: 'max+4', layoutSize: 'auto' }, // Shader larger than limit (auto layout) | ||
| ] as const) | ||
| ) | ||
| .fn(t => { | ||
| if (!supportsImmediateData(t.device)) { | ||
| t.skip('Immediate data not supported'); | ||
| } | ||
|
|
||
| const { shaderSize, layoutSize } = t.params; | ||
|
|
||
| const maxImmediateSize = t.device.limits.maxImmediateSize; | ||
|
|
||
| let actualLayout: GPUPipelineLayout | 'auto'; | ||
| let validSize: number; | ||
|
|
||
| if (layoutSize === 'auto') { | ||
| actualLayout = 'auto'; | ||
| // checked above | ||
| validSize = maxImmediateSize!; | ||
| } else { | ||
| actualLayout = t.device.createPipelineLayout({ | ||
| bindGroupLayouts: [], | ||
| immediateSize: layoutSize as number, | ||
| }); | ||
| validSize = layoutSize as number; | ||
| } | ||
|
|
||
| let actualShaderSize: number; | ||
| if (shaderSize === 'max') { | ||
| actualShaderSize = validSize; | ||
| } else if (shaderSize === 'max+4') { | ||
| actualShaderSize = validSize + 4; | ||
| } else { | ||
| actualShaderSize = shaderSize as number; | ||
| } | ||
|
|
||
| const code = ` | ||
| var<immediate> data: array<u32, ${actualShaderSize / 4}>; | ||
| fn use() { _ = data[0]; } | ||
| @compute @workgroup_size(1) fn main_compute() { use(); } | ||
| `; | ||
|
|
||
| const shouldError = actualShaderSize > validSize; | ||
|
|
||
| t.expectValidationError(() => { | ||
| t.device.createComputePipeline({ | ||
| layout: actualLayout, | ||
| compute: { | ||
| module: t.device.createShaderModule({ code }), | ||
| entryPoint: 'main_compute', | ||
| }, | ||
| }); | ||
| }, shouldError); | ||
| }); |
Copilot
AI
Jan 21, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test should validate both synchronous and asynchronous pipeline creation. Add an isAsync parameter to match the pattern used by other pipeline creation tests in this file. The params should include .combine('isAsync', [true, false]) and the test should use vtu.doCreateComputePipelineTest helper with the isAsync parameter instead of directly calling createComputePipeline.
Add cases to validate immediate size checks during both compute and render pipeline creation.
cf15532 to
2127eae
Compare
Add cases to validate immediate size checks during both compute and render pipeline creation.
Issue: #
Requirements for PR author:
.unimplemented()./** documented */and new helper files are found inhelper_index.txt.Requirements for reviewer sign-off:
When landing this PR, be sure to make any necessary issue status updates.