-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Naga mesh shader SPIR-V writer #8456
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 mesh shader SPIR-V writer #8456
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.
Pull Request Overview
This PR adds WGSL mesh shader support to wgpu, enabling the use of mesh shaders on the Vulkan backend through naga's WGSL frontend and SPIR-V backend. The implementation introduces new shader stages (Task and Mesh), new built-ins, a task_payload address space, and updates the API to use a workgroup output variable approach instead of imperative setVertex/setPrimitive calls.
Key changes:
- Adds mesh shader capability validation in wgpu-core
- Implements WGSL parsing for mesh shader syntax including
@mesh,@task,@payload, and@per_primitiveattributes - Adds SPIR-V backend support for mesh shader emission
- Removes GLSL compilation in favor of WGSL for Vulkan mesh shaders
- Updates documentation and adds comprehensive test coverage
Reviewed Changes
Copilot reviewed 39 out of 51 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
wgpu-core/src/device/mod.rs |
Adds MESH_SHADER capability to validator based on feature flag |
naga/src/ir/mod.rs |
Adds new built-ins (VertexCount, Vertices, etc.) and restructures MeshStageInfo to include output_variable |
naga/src/front/wgsl/parse/mod.rs |
Implements parsing for mesh shader attributes and directives |
naga/src/valid/interface.rs |
Adds validation for mesh shader constraints and output types |
naga/src/back/spv/writer.rs |
Implements SPIR-V emission for mesh shaders including output variable setup |
tests/tests/wgpu-gpu/mesh_shader/mod.rs |
Replaces GLSL compilation with WGSL, removes MESH_DISABLED test |
tests/tests/wgpu-gpu/mesh_shader/shader.wgsl |
New WGSL test shader for mesh shaders |
docs/api-specs/mesh_shading.md |
Updates documentation to reflect new workgroup variable approach |
CHANGELOG.md |
Documents the new feature additions |
Comments suppressed due to low confidence (1)
naga/src/back/spv/writer.rs:1
- Using
NonMaxU32::new(0).unwrap()to create a null handle is fragile and unclear. Consider defining a constant likeNULL_TYPE_HANDLEor using a more explicit pattern to represent invalid/placeholder handles.
use alloc::{string::String, vec, vec::Vec};
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
4230264 to
79f93a3
Compare
|
Holdup what did I do |
4230264 to
18fa9a8
Compare
|
is this almost done ? |
|
Haha yeah its basically done. I merged trunk a while back and now Im getting errors I can't understand (see commit log) but I will no doubt figure it out. I think @cwfitzgerald will get to reviewing it on the 26th, but it'll probably arrive after that. |
|
@inner-daemons so its mainly you who is working on mesh shaders ? |
|
@amarhassam Yeah, I try to post updates to #7197 every now and then, and you can track the status there. I've had 2 people offer to write the HLSL writer and WGSL writer, but not much progress has been made on either I think. Otherwise everything is just me |
|
@cwfitzgerald All comments should be addressed, ready for round 2! (=2) |
cwfitzgerald
left a comment
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.
Some more comments - also tests need to pass.
|
I'll work on fixing the tests... at some point |
|
For future me to enjoy and ponder: |
|
I also have some AMD GPU code from windows that I'll upload at some point but its absolutely unreadable to my eyes. |
|
For future reference: I have taken the generated code with debug symbols and put it through SPIRV-Opt, I have put it through SPIRV-Cross and then back through glslc, and both times it still caused a bug in LLVMPipe. Not sure about AMD however. I have also narrowed it down to just being the mesh shader part. |
|
Update: got this to work by modifying the body of main() in the generated GLSL. Therefore, the interface is fine, and its just an issue with something goofy. |
|
These comments are spammy if anyone is listening so you don't need to I have narrowed it down to this little bit of code that breaks stuff even if the values are immediately rewritten: Notably, it has to be referenced by I'm beginning to believe this really is a mesa bug |
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.
Pull request overview
Copilot reviewed 27 out of 46 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cwfitzgerald
left a comment
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.
Absolutely unforgivable nit that I'll deal with tomorrow when sorting out the changelog.
| } | ||
| ``` | ||
|
|
||
| See other changes in this changelog for more information. |
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.
Credit yourself lmao
Connections
Works towards #7197
Builds on #8370
Description
Add a SPIR-V writer for mesh shaders
All major changes here are in the SPIR-V backend for naga and naga snapshots. Other "changes" are inherited from #8370
Testing
Mesh shader test WGSL is now written to SPIR-V as a snapshot. Mesh shader tests & example use naga-written spirv on vulkan backend.
Squash or Rebase?
Squash
Checklist
cargo fmt.taplo format.cargo clippy --tests. If applicable, add:--target wasm32-unknown-unknowncargo xtask testto run tests.CHANGELOG.mdentry.