Skip to content
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

Override-expressions for Fixed Size Arrays #6654

Merged
merged 21 commits into from
Dec 10, 2024

Conversation

kentslaney
Copy link
Contributor

@kentslaney kentslaney commented Dec 3, 2024

This pull request is still a work in progress, but is open to avoid others repeating work that's already been done.

Connections
Resolves #5315

Description
The WebGPU spec allows for arrays to have a fixed size specified by an override-expression. This PR implements this by adding a Pending state to array sizes, which gets resolved with the rest of the override-expressions.

Testing
This change is tested by checking that a 1d spiral is correctly stored.

Checklist

  • Run cargo fmt.
  • Run taplo format.
  • Run cargo clippy. If applicable, add:
  • Run cargo xtask test to run tests.
  • Add change to CHANGELOG.md. See simple instructions inside file.

@kentslaney kentslaney marked this pull request as ready for review December 7, 2024 06:12
@kentslaney kentslaney requested review from a team as code owners December 7, 2024 06:12
Copy link
Member

@teoxoy teoxoy left a comment

Choose a reason for hiding this comment

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

We need to validate that arrays of this form are only declared on workgroup variables. They also can't be nested. I think this can be done via a new TypeFlags implementing the spec's distinction between "creation-fixed footprint" and "fixed footprint".

see https://www.w3.org/TR/WGSL/#fixed-footprint-types & https://www.w3.org/TR/WGSL/#example-workgroup-variables-sized-by-override

@teoxoy
Copy link
Member

teoxoy commented Dec 9, 2024

There is also this note in the spec:

Note: To qualify for type-equivalency, any override expression that is not a const expression must be an identifier. See Workgroup variables sized by overridable constants

I think to make this work we'd have to make ArraySize::Pending hold a new enum that is either a Handle<Override> or Handle<Expression>.

@kentslaney
Copy link
Contributor Author

kentslaney commented Dec 9, 2024

I think to make this work we'd have to make ArraySize::Pending hold a new enum that is either a Handle<Override> or Handle<Expression>.

You were right, that was much easier, sorry for the force push

@kentslaney kentslaney requested a review from teoxoy December 10, 2024 06:25
naga/src/valid/type.rs Show resolved Hide resolved
naga/src/valid/type.rs Outdated Show resolved Hide resolved
@kentslaney kentslaney requested a review from teoxoy December 10, 2024 14:17
Copy link
Member

@teoxoy teoxoy left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

@teoxoy teoxoy merged commit 5bec461 into gfx-rs:trunk Dec 10, 2024
27 checks passed
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.

Allow override-expressions in element count of fixed-size arrays
2 participants