Operation tests for setImmediates#4558
Conversation
|
Results for build job (at 6ffc1ce): +webgpu:api,operation,command_buffer,programmable,immediate:basic_execution:* - 51 cases, 51 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:* - 4 cases, 4 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:* - 36 cases, 216 subcases (~6/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: 280650 cases, 2321809 subcases
+TOTAL: 280752 cases, 2322091 subcases |
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.
2c527ae to
ffe4c7c
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 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
4c8a817 to
7e4e8d8
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 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
7e4e8d8 to
2e3521f
Compare
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.
2e3521f to
6f16cf5
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 no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Softly ping for review. |
|
The operation tests seem to have good coverage. They became much simpler when the group decided to add validation that all immediates are set instead of defaulting to 0. |
Kangz
left a comment
There was a problem hiding this comment.
Wow Github's code review UI is really annoying to use.
|
I think I've resolved comments. PTAL, thanks ! |
| if (!this.isCompatibility) { | ||
| return; | ||
| } | ||
| const needsStorageBuffersInFragmentStage = |
There was a problem hiding this comment.
If I understand correctly, this test is skipped on devices that don't support storage buffers in fragment shaders. But, those device do support immediates meaning this test fails to test immediates in fragment shaders on those device.
Should the test be refactored not to use storage buffers in fragment shaders? Many other tests were refactored to do this.
There was a problem hiding this comment.
Fair point. I think we need to refactor this to cover more devices. Is there anyone working a uniform refactor for all test cases or we could do it seperately.
There was a problem hiding this comment.
I think I refactored all exsiting tests to not require storage buffers in fragment shaders (except for the tests that are specifically testing storage buffers in fragment shaders 😛) If there are any left I'm happy to fix them.
…sInFragmentStage greggman@ pointed out that the previous immediates operation tests relies on StorageBuffersInFragmentStage, which will reduce the cover rate of the devices. (gpuweb#4558 (comment)) Remove usage of StorageBuffersInFragmentStage and directly rendering result out to do operation test
…sInFragmentStage greggman@ pointed out that the previous immediates operation tests relies on StorageBuffersInFragmentStage, which will reduce the cover rate of the devices. (gpuweb#4558 (comment)) Remove usage of StorageBuffersInFragmentStage and directly rendering result out to do operation test
…sInFragmentStage greggman@ pointed out that the previous immediates operation tests relies on StorageBuffersInFragmentStage, which will reduce the cover rate of the devices. (gpuweb#4558 (comment)) Remove usage of StorageBuffersInFragmentStage and directly rendering result out to do operation test
…sInFragmentStage greggman@ pointed out that the previous immediates operation tests relies on StorageBuffersInFragmentStage, which will reduce the cover rate of the devices. (gpuweb#4558 (comment)) Remove usage of StorageBuffersInFragmentStage and directly rendering result out to do operation test
…sInFragmentStage (#4619) greggman@ pointed out that the previous immediates operation tests relies on StorageBuffersInFragmentStage, which will reduce the cover rate of the devices. (#4558 (comment)) Remove usage of StorageBuffersInFragmentStage and directly rendering result out to do operation test
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.