-
Notifications
You must be signed in to change notification settings - Fork 101
Add new builtins to maxInterStageShaderVariables test #4554
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
Conversation
|
Results for build job (at 85f4ba2): -webgpu:api,validation,capability_checks,limits,maxInterStageShaderVariables:createRenderPipeline,at_over:* - 640 cases, 640 subcases (~1/case)
+webgpu:api,validation,capability_checks,limits,maxInterStageShaderVariables:createRenderPipeline,at_over:* - 1880 cases, 1880 subcases (~1/case)
-TOTAL: 284498 cases, 2342682 subcases
+TOTAL: 285738 cases, 2343922 subcases |
51b8675 to
c8d7632
Compare
|
It sounds like this renders #4541 obsolete? |
|
Well yea, kind of. Your other PR fixed the test so it counted 1 for each builtin. That pointed out that chrome was only counting one. When I went into the spec and saw 3 others were missing I started adding them but with 6 it felt like it needed a bigger refactor. I guess I missed your newer one. Sorry about that. Which one do you prefer? |
|
Hi Erich, I just wanted to apologize. I totally missed that you had more PRs up for this issue. Feel free to close this PR. |
The validation for maxInterStageShaderVariables requires checking primitive_index, subgroup_size, and subgroup_invocation_id. With this change chromium passes the latest CTS test: gpuweb/cts#4554 Bug: 474266022,474266023 Fixed: 474266022,474266023 Change-Id: I6a6a6964256b06687781dfb026f982fce83b8bac Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/284055 Commit-Queue: Gregg Tavares <gman@chromium.org> Reviewed-by: James Price <jrprice@google.com>
Co-authored-by: Gregg Tavares <github@greggman.com>
c012822 to
f7b1674
Compare
…put (#4555) * refactor: lower some vars. in `max_variables_count,input` * fix: correctly count fragment input deductions for built-ins in `max_variables_count,input` * refactor: `max_variables_count,input`: hoist `success` to hidden case param. * proposed refactor * Test primitive-index and subgroups too (taken from #4554) Co-authored-by: Gregg Tavares <github@greggman.com> * add missing enable directives --------- Co-authored-by: Erich Gubler <erichdongubler@gmail.com> Co-authored-by: Gregg Tavares <github@greggman.com>
src/webgpu/api/validation/capability_checks/limits/maxInterStageShaderVariables.spec.ts
Outdated
Show resolved
Hide resolved
src/webgpu/api/validation/capability_checks/limits/maxInterStageShaderVariables.spec.ts
Show resolved
Hide resolved
primitive_index, subgroup_id, subgroup_size were missing from the test.
58e63d3 to
85f4ba2
Compare
primitive_index, subgroup_id, subgroup_size were missing from the test.
note: Chromium doesn't currently count these. Bugs filed. Also, chromium doesn't currently count correctly for the pre-existing ones. Fixes in progress.
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.