Add maxInterStageShaderVariables and inter-stage limit validation#2857
Merged
kainino0x merged 9 commits intogpuweb:mainfrom Jun 2, 2022
Merged
Add maxInterStageShaderVariables and inter-stage limit validation#2857kainino0x merged 9 commits intogpuweb:mainfrom
kainino0x merged 9 commits intogpuweb:mainfrom
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
Contributor
Kangz
approved these changes
May 10, 2022
Contributor
Contributor
Kangz
approved these changes
May 10, 2022
Contributor
Kangz
left a comment
Kangz
left a comment
There was a problem hiding this comment.
LGTM, the previous version was also fine FWIW.
Editors WDYT?
Contributor
|
|
kainino0x
reviewed
May 11, 2022
kainino0x
reviewed
May 11, 2022
kainino0x
reviewed
May 11, 2022
kainino0x
reviewed
May 12, 2022
Contributor
kainino0x
left a comment
kainino0x
left a comment
There was a problem hiding this comment.
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.
Contributor
Author
|
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! |
Contributor
kainino0x
changed the title
github-actions bot
added a commit
that referenced
this pull request
Jun 2, 2022
) SHA: b487c44 Reason: push, by @kainino0x Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Contributor
github-actions bot
added a commit
that referenced
this pull request
Jun 2, 2022
) SHA: b487c44 Reason: push, by @kainino0x Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions bot
added a commit
that referenced
this pull request
Jun 2, 2022
) SHA: b487c44 Reason: push, by @kainino0x Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
jdarpinian
pushed a commit
to jdarpinian/gpuweb
that referenced
this pull request
Aug 12, 2022
juj
added a commit
to juj/wasm_webgpu
that referenced
this pull request
Aug 18, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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