-
Notifications
You must be signed in to change notification settings - Fork 353
Add maxInterStageShaderVariables and inter-stage limit validation #2857
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
Add maxInterStageShaderVariables and inter-stage limit validation #2857
Conversation
This patch intends to clarify all the validations with
maxInterStageShaderComponents in WebGPU SPEC.
- The total amount of both user-defined and built-in vertex output
components should be no more than `maxInterStageShaderComponents`.
- If the primitive type is `point-list`, the The total amount of
both user-defined and built-in vertex output components should
be no more than (`maxInterStageShaderComponents - 1`) as on
Vulkan `PointSize` should always be declared and consume 1
component.
- The total amount of both user-defined and built-in fragment input
components should be no more than `maxInterStageShaderComponents`.
- The default value of `maxInterStageShaderComponents` should be 64.
- The `location` of each vertex output and fragment input variables
must be less than `maxInterStageShaderComponents / 4` as is required
by Vulkan SPEC.
You can read the discussions in gpuweb#1962 for more details.
fixes: gpuweb#1962, gpuweb#2763
Kangz
left a comment
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.
LGTM, the previous version was also fine FWIW.
Editors WDYT?
|
|
kainino0x
left a comment
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.
about the title: this is is more than a clarification - it redefines what maxInterStageShaderComponents means (from just user-defined variables, to all variables).
The changes seem fine, but I don't feel I understand the underlying constraints enough yet - this is much deeper than editorial. Let's discuss on #1962.
|
Hi @kainino0x I've updated this PR based on #1962 (comment). For the details about how to compute both limits on each backend, I suggest we add them in another PR. PTAL, thanks! |
kainino0x
left a comment
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.
LGTM!
) SHA: b487c44 Reason: push, by @kainino0x Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
) SHA: b487c44 Reason: push, by @kainino0x Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
) SHA: b487c44 Reason: push, by @kainino0x Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This patch intends to clarify all the validations with
maxInterStageShaderComponents in WebGPU SPEC.
components should be no more than
maxInterStageShaderComponents.point-list, the The total amount ofboth user-defined and built-in vertex output components should
be no more than (
maxInterStageShaderComponents - 1) as onVulkan
PointSizeshould always be declared and consume 1vertex output component.
components should be no more than
maxInterStageShaderComponents.locationof each vertex output and fragment input variablesmust be less than
maxInterStageShaderComponents / 4as is requiredby Vulkan SPEC.
You can read the discussions in #1962 for more details.
Fixes #1962
Fixes #2400
Closes #2763
Preview | Diff