Skip to content

Conversation

clarfonthey
Copy link
Contributor

@clarfonthey clarfonthey commented Sep 20, 2025

Description
Right now, include_spirv! cannot be used to directly define a static variable with the shader due to the wgpu::util::make_spirv_raw function not being const. This updates the macro to use an alternative implementation compatible with the current MSRV policy.

Note that the make_spirv_raw function is still not const because it accepts arbitrary input which may not be aligned, and thus allocates a new buffer for unaligned input. Instead, we use a separate function for the macros which can guarantee the alignment of the bytes being included.

Testing
Previously, this macro was not tested at all, and actually, include_spirv_raw had broken due to a change in the definition of ShaderModuleDescriptorPassthrough. Now, both include_spirv and include_spirv_raw are tested with a tiny "shader" file (it just contains the magic number + 0x11223344) to verify that the macros actually include a valid struct, and a proper test for the functionality is added to a wgpu-include-spirv test crate.

Squash or rebase? Squash.

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.


assert_eq!(
words[0], MAGIC_NUMBER,
"wrong magic word {:x}. Make sure you are using a binary SPIRV file.",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, we lose the ability to output the magic number here, since formatting doesn't work in constants. While there are workarounds that work with the current MSRV like the const_panic crate, I decided that this isn't a massive loss and it was better to not add more dependencies.

Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

Looking great, thanks for adding all those tests!
Albeit, see comment, to keep in line with the rest I think we should arrange this differently

@clarfonthey
Copy link
Contributor Author

The latest commit removes the test binary and splits the tests across the macros and util modules. Hopefully this is good to merge, but if you have any additional feedback, let me know.

@clarfonthey clarfonthey requested a review from Wumpf September 28, 2025 02:41
Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

nice!

@Wumpf Wumpf merged commit 7902357 into gfx-rs:trunk Oct 4, 2025
41 checks passed
sharmajai pushed a commit to sharmajai/wgpu that referenced this pull request Oct 12, 2025
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.

2 participants