Conversation
|
Results for build job (at 509b754): +webgpu:api,operation,command_buffer,programmable,immediate:basic_execution:* - 39 cases, 39 subcases (~1/case)
+webgpu:api,operation,command_buffer,programmable,immediate:update_data:* - 3 cases, 3 subcases (~1/case)
+webgpu:api,operation,command_buffer,programmable,immediate:pipeline_switch:* - 2 cases, 2 subcases (~1/case)
+webgpu:api,operation,command_buffer,programmable,immediate:use_max_immediate_size:* - 3 cases, 3 subcases (~1/case)
+webgpu:api,operation,command_buffer,programmable,immediate:typed_array_arguments:* - 33 cases, 33 subcases (~1/case)
+webgpu:api,operation,command_buffer,programmable,immediate:multiple_updates_before_draw_or_dispatch:* - 3 cases, 3 subcases (~1/case)
+webgpu:api,operation,command_buffer,programmable,immediate:render_pass_and_bundle_mix:* - 1 cases, 1 subcases (~1/case)
+webgpu:api,operation,command_buffer,programmable,immediate:render_bundle_isolation:* - 1 cases, 1 subcases (~1/case)
-TOTAL: 285993 cases, 2344970 subcases
+TOTAL: 286078 cases, 2345055 subcases |
Implement operation tests for setImmediates in ComputePassEncoder, RenderPassEncoder, and RenderBundleEncoder. - Add basic execution tests for scalar, vector, and struct types. - Add tests for partial updates and multiple updates (using range verification). - Add tests for pipeline switching (no inheritance). - Add tests for large data (maxImmediateSize) with range verification. - Add tests for TypedArray arguments with offsets. - Add tests for mixing render pass and bundle execution.
834cd4a to
509b754
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .params(u => u.combine('encoderType', kProgrammableEncoderTypes)) | ||
| .fn(t => { | ||
| const { encoderType } = t.params; | ||
| const maxImmediateSize = t.device.limits.maxImmediateSize!; |
There was a problem hiding this comment.
The maxImmediateSize limit can be undefined even when supportsImmediateData() returns true. The test should check if maxImmediateSize is defined and skip the test if it's not, similar to how validation tests handle this case. Without this check, the test will fail with a TypeError when trying to divide undefined by 4.
| const maxImmediateSize = t.device.limits.maxImmediateSize!; | |
| const maxImmediateSize = t.device.limits.maxImmediateSize; | |
| if (maxImmediateSize === undefined) { | |
| t.skip('maxImmediateSize limit is undefined'); | |
| return; | |
| } |
| // Output size: 2 draws * 2 u32s * 4 bytes = 16 bytes. | ||
| const pipeline = createPipeline(t, 'render pass', wgslDecl, wgslUsage, 16) as GPURenderPipeline; |
There was a problem hiding this comment.
The immediateSize parameter should be 8 bytes (vec2<u32> = 2 * 4 bytes), not 16 bytes. The comment at line 761 confuses the output buffer size with the immediate data size. The immediate data structure is vec2<u32>, which only requires 8 bytes of immediate storage.
| // Output size: 2 draws * 2 u32s * 4 bytes = 16 bytes. | |
| const pipeline = createPipeline(t, 'render pass', wgslDecl, wgslUsage, 16) as GPURenderPipeline; | |
| // Output buffer size: 2 draws * 2 u32s * 4 bytes = 16 bytes. Immediate data (vec2<u32>) = 8 bytes. | |
| const pipeline = createPipeline(t, 'render pass', wgslDecl, wgslUsage, 8) as GPURenderPipeline; |
| { | ||
| binding: 0, | ||
| visibility: GPUShaderStage.COMPUTE | GPUShaderStage.FRAGMENT, | ||
| buffer: { type: 'storage' }, |
There was a problem hiding this comment.
The tests use storage buffers in fragment shaders but don't check if this is supported in compatibility mode. Similar tests in this directory (programmable_state_test.ts:18-34) include a check for maxStorageBuffersInFragmentStage in compatibility mode. Consider adding a similar check in the test initialization or as a beforeAllSubcases hook to skip tests when storage buffers in fragment stage are not available.
| const pipelineA = createPipeline(t, encoderType, wgslDecl, wgslUsage, 16); | ||
| const pipelineB = createPipeline(t, encoderType, wgslDecl, wgslUsage, 16); |
There was a problem hiding this comment.
The PR description mentions testing "pipeline switching (compatible and incompatible)" but the pipeline_switch test only tests compatible pipelines (both pipelineA and pipelineB have the same immediateSize of 16 bytes). Consider adding test cases for switching between pipelines with different immediate layouts to verify that immediate data is correctly reset or handled when switching to an incompatible pipeline.
Implement operation tests for setImmediates in ComputePassEncoder, RenderPassEncoder, and RenderBundleEncoder.
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.