Skip to content

Conversation

@Jiawei-Shao
Copy link
Contributor

@Jiawei-Shao Jiawei-Shao commented May 10, 2022

This patch intends to clarify all the validations with
maxInterStageShaderComponents in WebGPU SPEC.

  • The total number of 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
      vertex output component.
  • The total amount of user-defined and built-in fragment input
    components should be no more than maxInterStageShaderComponents.
  • 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 #1962 for more details.

Fixes #1962
Fixes #2400
Closes #2763


Preview | Diff

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
@github-actions
Copy link
Contributor

Previews, as seen when this build job started (30fb065):
WebGPU | IDL
WGSL
Explainer

@github-actions
Copy link
Contributor

Previews, as seen when this build job started (7e3abfb):
WebGPU | IDL
WGSL
Explainer

@github-actions
Copy link
Contributor

Previews, as seen when this build job started (7641128):
WebGPU | IDL
WGSL
Explainer

@Jiawei-Shao Jiawei-Shao requested a review from kainino0x May 10, 2022 07:55
Copy link
Contributor

@Kangz Kangz left a 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
Copy link
Contributor

kainino0x commented May 11, 2022

FYI: #2400 considers refactoring maxInterStageShaderComponents=64 to maxInterStageShaderVariables=16. That's orthogonal though. I think your investigation disproves that we can do that. #2400 (comment)

Copy link
Contributor

@kainino0x kainino0x left a 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.

@Jiawei-Shao
Copy link
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!

@github-actions
Copy link
Contributor

github-actions bot commented Jun 1, 2022

Previews, as seen when this build job started (70ba205):
WebGPU | IDL
WGSL
Explainer

Copy link
Contributor

@kainino0x kainino0x left a comment

Choose a reason for hiding this comment

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

LGTM!

@kainino0x kainino0x changed the title Clarify all the validations with maxInterStageShaderComponents Add maxInterStageShaderVariables and inter-stage limit validation Jun 2, 2022
@kainino0x kainino0x enabled auto-merge (squash) June 2, 2022 03:46
@kainino0x kainino0x merged commit b487c44 into gpuweb:main Jun 2, 2022
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
Copy link
Contributor

github-actions bot commented Jun 2, 2022

Previews, as seen when this build job started (c836ff1):
WebGPU | IDL
WGSL
Explainer

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
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.

maxInterStageShaderComponents is not enough Should we constrain the location of user input-output stage variables WGSL

3 participants