-
Notifications
You must be signed in to change notification settings - Fork 1k
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
[naga wgsl-in] vecN()
constructors and let
type conversions
#7367
base: trunk
Are you sure you want to change the base?
Conversation
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.
Love the links to the spec and the thorough test coverage 🥇
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.
Great, LGTM!
Signed-off-by: sagudev <[email protected]>
…ession Signed-off-by: sagudev <[email protected]>
Signed-off-by: sagudev <[email protected]>
Signed-off-by: sagudev <[email protected]>
* Support `vecN()` constructors (fixes gfx-rs#7356) * Apply automatic conversions to the initializer for `let` bindings
3e98cee
to
bae3612
Compare
Rebased to resolve conflicts. |
@andyleiserson, @sagudev: Should we just merge this, and close #7304? Or do we need to merge that first? |
I don't know if @jamienicol reviewed all the changes or just the ones that I added in this PR. And @jimblandy mentioned documenting |
I only reviewed your changes @andyleiserson |
Connections
Fix for #7356.
This pull request is on top of #7304, due to overlapping effects on naga output snapshots.
Description
Naga did not previously allow use of
vec4()
as a constructor, because it has no type information. However, the spec states that this constructor (andvec2
/vec3
) should return a vector of abstract ints. Depending on the context (e.g. the case ofvar v: vec4f = vec()
from the associated issue), the vector of abstract ints may then be converted to another type. While writing test cases for this, I noticed that we weren't applying automatic conversions to the initializer oflet
declarations, so I fixed that as well, changing the code for handlinglet
declarations to operate similarly to the code forvar
andconst
, which already handled conversions.Testing
Added several snapshot tests to exercise various related cases, plus a few cases where a conversion is not possible in
wgsl_errors
. Type conversion for let declarations also fixes the CTS testwebgpu:shader,validation,decl,let:type:case="array_sized"
.Squash or Rebase? Squash
Checklist
cargo fmt
.taplo format
.cargo clippy --tests
. If applicable, add:--target wasm32-unknown-unknown
cargo xtask test
to run tests.CHANGELOG.md
entry.