wgpu icon indicating copy to clipboard operation
wgpu copied to clipboard

Naga mesh shader WGSL writer

Open Slightlyclueless opened this issue 1 month ago • 5 comments

Connections Based off of #8370

Description

Adds a WGSL writer for mesh shader functionality.

Testing TODO

Squash or Rebase? Squash

Checklist

  • [x] Run cargo fmt.
  • [x] Run taplo format.
  • [x] Run cargo clippy --tests.
  • [x] Run cargo xtask test to run tests.
  • [ ] If this contains user-facing changes, add a CHANGELOG.md entry.

Slightlyclueless avatar Nov 05 '25 23:11 Slightlyclueless

All changes are in naga/src/back/wgsl/writer.rs and naga/src/back/wgsl/to_wgsl.rs. Code is ugly and I absolutely want to clean it up before I mark as ready to merge

Dunno what's up with the PR history lol.

Slightlyclueless avatar Nov 05 '25 23:11 Slightlyclueless

First comments

  • Logic to detect mesh shader logic is incorrect. You also need to check for mesh shader builtins, per primitive attributes anywhere, task payload variables. This is because files might contain types/data that isn't used in any shaders
  • The generated task payload variable doesn't have a storage class
  • I don't understand why the shader stage, workgroup size, task payload come in different orders between mesh and task shaders
  • Mesh thing doesn't have the variable it needs (@mesh(variable))
  • Per primitive isn't written correctly
  • You will want to make sure that the snapshot is correctly written

inner-daemons avatar Nov 06 '25 04:11 inner-daemons

Logic to detect mesh shader logic is incorrect. You also need to check for mesh shader builtins, per primitive attributes anywhere, task payload variables. This is because files might contain types/data that isn't used in any shaders

I though so, but wasn't sure. Will change,

The generated task payload variable doesn't have a storage class

Ah yep, not sure how I missed that one.

I don't understand why the shader stage, workgroup size, task payload come in different orders between mesh and task shaders

Because I am a clutz, and didn't sanity check that after going "Yay it no longer crashes".

Mesh thing doesn't have the variable it needs (@mesh(variable))

Whoops, that's what I get for not looking at the correct spec document.

Per primitive isn't written correctly

Not sure what you mean by this?

You will want to make sure that the snapshot is correctly written

Again, not sure what you mean. I may just be being a bit thick though

Slightlyclueless avatar Nov 06 '25 18:11 Slightlyclueless

For the snapshot, you will want to go to naga/tests/in/wgsl/mesh-shader.toml and change to this line targets = "IR | ANALYSIS | WGSL"

Then run cargo xtask test snapshot before every commit. Snapshots are how WGPU tracks how shaders are written between commits, and how we check that naga continues to work on many shaders.

inner-daemons avatar Nov 06 '25 21:11 inner-daemons

Also feel free to mark this as no longer a draft

And just to manage expectations, this probably won't be merged until at least the 26th, as I don't have the power to do that so it'll have to have a second review (hopefully with fewer comments)

inner-daemons avatar Nov 17 '25 03:11 inner-daemons

Ci is mad

cwfitzgerald avatar Dec 12 '25 18:12 cwfitzgerald

uhhh, that's not any of my code causing CI to get mad...

Slightlyclueless avatar Dec 12 '25 23:12 Slightlyclueless