Skip to content

Conversation

@kainino0x
Copy link
Collaborator

@kainino0x kainino0x commented Jan 9, 2026

I took Erich's test fix #4546, then tried to tweak the original structure of the test so it's clear by construction that it actually tests what it's supposed to test.

Issue: #4538

See also #4539.


Requirements for PR author:

  • All missing test coverage is tracked with "TODO" or .unimplemented().
  • New helpers are /** documented */ and new helper files are found in helper_index.txt.
  • Test behaves as expected in a WebGPU implementation. (If not passing, explain above.)
  • Test have be tested with compatibility mode validation enabled and behave as expected. (If not passing, explain above.)

Requirements for reviewer sign-off:

  • Tests are properly located.
  • Test descriptions are accurate and complete.
  • Tests provide complete coverage (including validation control cases). Missing coverage MUST be covered by TODOs.
  • Tests avoid over-parameterization (see case count report).

When landing this PR, be sure to make any necessary issue status updates.

@github-actions
Copy link

github-actions bot commented Jan 9, 2026

Results for build job (at dee62c8):

-webgpu:api,validation,render_pipeline,inter_stage:max_variables_count,input:* - 8 cases, 8 subcases (~1/case)
+webgpu:api,validation,render_pipeline,inter_stage:max_variables_count,input:* - 6 cases, 12 subcases (~2/case)
-TOTAL: 284500 cases, 2342678 subcases
+TOTAL: 284498 cases, 2342682 subcases

@greggman
Copy link
Contributor

greggman commented Jan 9, 2026

DOH! Sorry, I missed that there was already a PR up for this. Choose which ever one you prefer.

@ErichDonGubler

@kainino0x
Copy link
Collaborator Author

kainino0x commented Jan 9, 2026

@greggman The duplicates are #4541 and #4554 for capability_checks,limits. This one and its ancestor #4546 are for a separate test render_pipeline,inter_stage

@greggman
Copy link
Contributor

greggman commented Jan 9, 2026

#4554 includes this as well, and includes the extensions. I can discard it, I can split it (it's updating 2 files), or you can review that. Even if this one is merged we will need the changes from #4554 afterwards for the new builtins.

@kainino0x
Copy link
Collaborator Author

Oh sorry, missed that that PR had two files!

Co-authored-by: Gregg Tavares <github@greggman.com>
@kainino0x
Copy link
Collaborator Author

Pulled in the extra optional-feature builtins from #4554

@greggman greggman self-requested a review January 10, 2026 00:47
@greggman
Copy link
Contributor

greggman commented Jan 10, 2026

I removed the changes to inter_stage.spec.ts from #4554

Copy link
Contributor

@greggman greggman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually, you'll need to add the enabled stuff to the WGSL or these will fail. Here's my previous PR for reference

@kainino0x
Copy link
Collaborator Author

kainino0x commented Jan 10, 2026

actually, you'll need to add the enabled stuff to the WGSL or these will fail. Here's my previous PR for reference

Ahh whoops, thank you for catching that! Totally forgot to copy that over. (I didn't test against fixed chromium, I probably should. Seems like the fix will be in Canary very soon so I'll just wait until then and verify this.)

@kainino0x kainino0x requested a review from greggman January 13, 2026 06:24
@kainino0x kainino0x enabled auto-merge (squash) January 13, 2026 23:34
@kainino0x kainino0x merged commit d1ef6a8 into gpuweb:main Jan 13, 2026
1 check passed
@kainino0x kainino0x deleted the fragment_input_deductions-in-max_variables_count branch January 14, 2026 03:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

maxInterStageShaderVariables should be decremented for _each_ fragment input built-in, not just once

3 participants