Skip to content

Override-expressions for Fixed Size Arrays#6654

Merged
teoxoy merged 21 commits intogfx-rs:trunkfrom
kentslaney:array-overrides
Dec 10, 2024
Merged

Override-expressions for Fixed Size Arrays#6654
teoxoy merged 21 commits intogfx-rs:trunkfrom
kentslaney:array-overrides

Conversation

@kentslaney
Copy link
Copy Markdown
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 a review from a team December 7, 2024 06:12
@kentslaney kentslaney requested a review from a team as a code owner December 7, 2024 06:12
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
@kentslaney kentslaney requested a review from teoxoy December 10, 2024 14:17
Copy link
Copy Markdown
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!

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