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

[naga hlsl-out] Handle additional cases of Cx2 matrices #7438

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

andyleiserson
Copy link
Contributor

Connections
Fixes #4423.

Description
Fixes the original issue in #4423, and a few more that I found while working on the fix. Because memory layouts differ between HLSL and WGSL in the case of C-column-by-2-row matrices, we store such matrices as C separate vectors. There is a comment in back::hlsl describing this in more detail. There are a lot of different kinds of accesses to these matrices that need to be adjusted in code generation, and we missed some:

  • Stores to struct in a storage buffer having a member m that is a Cx2 matrix, tried to obtain the value to be stored from member m in the source struct, but it is actually stored in C separate members m_0 ... m_<C-1>.
  • Loads from a Cx2 matrix in a uniform buffer tried to index into the source value using the subscript operator, but it is actually a __matCx2 struct with C separate members.
  • Stores to a struct-of-array-of-struct-of-matCx2 in a storage buffer tried to use write_value_type to declare a temporary for the value to be stored, but write_value_type does not support writing struct types.

The functional fixes are in the first commit. The second commit does some refactoring to use a helper method that I added in additional places, but it's just refactoring, and I'm not highly confident in any part of these changes, so I'd be fine to drop the refactoring commit if we decide it's not worth the risk.

Testing
Adds a snapshot test with a variety of Cx2 matrix cases.

Squash or Rebase?

If we keep the refactor commit, then rebase.

Checklist

  • Run cargo fmt.
  • Run taplo format.
  • Run cargo clippy --tests. If applicable, add:
    • --target wasm32-unknown-unknown
  • Run cargo xtask test to run tests.
  • If this contains user-facing changes, add a CHANGELOG.md entry.

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.

[hlsl-out] Stores of vec2-based matrix inside struct tries to access raw matrix directly
2 participants