Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds Metal Shading Language (MSL) writer support for mesh shaders, enabling WGSL mesh shaders to be automatically transpiled to MSL. This replaces the previous approach of using handwritten Metal shader files.
Changes:
- Implements MSL code generation for task and mesh shader entry points with proper Metal 3.0 syntax
- Refactors mesh shader detection into a shared
uses_mesh_shaders()function used across multiple backends - Enables Metal as a target backend for mesh shader tests and examples
Reviewed changes
Copilot reviewed 14 out of 19 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/tests/wgpu-gpu/mesh_shader/shader.metal | Deleted handwritten Metal shader, replaced with auto-generated code |
| tests/tests/wgpu-gpu/mesh_shader/mod.rs | Removed compile_msl function, now uses WGSL compilation for Metal backend |
| naga/tests/out/msl/wgsl-policy-mix.msl | Fixed ordering of private variable initialization before workgroup barrier |
| naga/tests/out/msl/wgsl-mesh-shader*.msl | New generated MSL output files for mesh shader tests |
| naga/tests/in/wgsl/mesh-shader*.toml | Added Metal as target and MSL version 3.0 requirement |
| naga/src/proc/mod.rs | Added uses_mesh_shaders() utility function for detecting mesh shader usage |
| naga/src/back/wgsl/writer.rs | Refactored to use new uses_mesh_shaders() function |
| naga/src/back/spv/writer.rs | Refactored mesh shader checking, moved logic to use shared function |
| naga/src/back/spv/mesh_shader.rs | Removed require_mesh_shaders() helper, consolidated into writer |
| naga/src/back/msl/writer.rs | Main implementation of mesh shader MSL code generation |
| naga/src/back/msl/mod.rs | Added Payload binding type, MeshOutput location mode, and UnsupportedMeshShader error |
| examples/features/src/mesh_shader/shader.metal | Deleted handwritten Metal shader |
| examples/features/src/mesh_shader/mod.rs | Removed compile_msl function, now uses WGSL for Metal |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@cwfitzgerald Thanks for the review, will get to it today. I didn't make it fully clear, but when you reviewed this PR many parts were derived from #8756 which is designed to land before this and the HLSL PR. I've moved all changes from all 3 PRs that could could conflict into that PR and then back here. So thats why there are some changes related to spv |
|
Alright, I've cleaned this up a tiny bit, since I had left some changes from the SPV fixes PR. |
Wumpf
left a comment
Wumpf
left a comment
There was a problem hiding this comment.
frankly, changes to msl/writer.rs are just a nightmare to review. The file is so insanely large, even with an agent I can hardly make sense of it. As so often it seems this is no individual's fault, but we have to make sure it gets better, so anything we can do to improve readability & maintainability we have to do. I believe a lof of the steps could be factored more cleanly
Importantly, the outputs of the test shaders look good I believe, so I think after a little bit of polish we can move forward with this.
wgpu-types/src/limits.rs
Outdated
| // M3 would up both of these to 1M | ||
| // New chips in the apple10 family are expected to up this limit to 4M. |
There was a problem hiding this comment.
So these are limits derived from M2 chips then? bit unclear
Also what do you mean they are "expected to"? That's is lacking a source and will age poorly
There was a problem hiding this comment.
Source is still metal feature set tables, its just that those devices haven't been released yet IIRC.
| } | ||
|
|
||
| pub(super) fn write_workgroup_variables_initialization( | ||
| pub(in super::super) fn write_workgroup_variables_initialization( |
There was a problem hiding this comment.
super::super? srsly? just move it out
There was a problem hiding this comment.
Move it out where? Just make in pub fn or?
| ] { | ||
| writeln!(self.out, "struct {out_name} {{")?; | ||
| let crate::TypeInner::Struct { ref members, .. } = module.types[struct_ty].inner else { | ||
| unreachable!() |
There was a problem hiding this comment.
takes a bit of mental load to reason through undocumented unreachables. Can we have some message telling me about how the output type has to be a struct?
And while we're here arent these always output_struct_ty? that would help as well
There was a problem hiding this comment.
What do you mean by that second part? What's output_struct_ty?
|
@Wumpf Mostly addressed |
inner-daemons
force-pushed
the
mesh-shading/msl-write
branch
from
698ba38 to
a1fe019
Compare
ChristopherBiscardi
left a comment
ChristopherBiscardi
left a comment
There was a problem hiding this comment.
While unfamiliar with a few of the intricacies of wgpu here, I have reviewed the code and not found anything obviously wrong.
Probably more interestingly, I ported Bevy to use this PR to run a couple of examples I've been working on using the current release's wgsl/vulkan support. The examples run as expected with no wgsl changes, using the wgsl I wrote targeting vulkan platforms, with this PR on an m1 max.
One example here is a visualization of a single task shader dispatching vec3u(u32(globals.time)) mesh shaders that draw a single cube at their grid position.
screenshot-2026-03-11-at-20.51.57-converted.mp4
The other is an in-progress version of this grass, which doesn't currently use task shaders.
screenshot-2026-03-11-at-20.54.34-converted.mp4
So this PR works as expected as far as my demo code/bevy is concerned.
Connections
Works towards #7197
Depends on #9099
Description
Adds support for mesh shaders to MSL writer.
Also adds a limit.
Testing
Snapshots & examples & other tests
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.