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

Mesh shaders #44

Merged
merged 8 commits into from
Dec 25, 2024
Merged

Mesh shaders #44

merged 8 commits into from
Dec 25, 2024

Conversation

Firestar99
Copy link
Member

@Firestar99 Firestar99 commented Oct 26, 2024

Depends on #43

Diff without dependent PR: Firestar99/rust-gpu-new@update-rspirv...Firestar99:rust-gpu-new:mesh-shaders

changelog:

- [PR#44](https://github.com/Rust-GPU/rust-gpu/pull/44) added support for mesh and task shaders

@LegNeato
Copy link
Collaborator

It would be good to have a test, changelog entry, and an example for this. Might be good to do a blog post too!

@Firestar99
Copy link
Member Author

Plenty of tests already exist in this PR and I added a changelog entry in the description. I can make you a quick example shader later.

@Firestar99
Copy link
Member Author

There are two vendor-specific issues I know of with regards to "payloads" (sending data from task to mesh shader):

  • AMD on Windows assumes the payload is always a struct, even if the spec says it could be anything like an uint. It not being a struct will segfault the driver on pipeline creation. If you define a struct with just a single primitive member, like a uint, rust-gpu will "inline" the struct and make the payload be a uint. Workaround: add a second dummy member.
  • RenderDoc suffers from the same issue as above, and will display something like "shader likely compiled by some old weird dxc version"
  • RenderDoc assumes the struct for the payload is never constructed, only it's fields written to. For it's debugging, it will inject a new field into said struct to emit some debug info. If you just assign fields to that struct it's fine, but it won't fixup the OpConstruct statements to make a new struct, resulting in invalid spirv and RenderDoc complaining. Rust-gpu merges structs with the same members, making it rather easy to trip onto this one.

Suggested fix: Wrap all task shader payloads in a separate struct that is not optimized away.

@LegNeato
Copy link
Collaborator

Plenty of tests already exist in this PR and I added a changelog entry in the description. I can make you a quick example shader later.

Oh weird, they didn't show up on mobile!

@LegNeato
Copy link
Collaborator

@eddyb or @schell , I don't know anything about mesh shaders...can either of you review?

@Firestar99
Copy link
Member Author

I think nobody but me knows about mesh shaders...
I wouldn't worry too much about correctness of the mesh shader stuff, I've tested it plenty so hopefully all issues have already been caught.

@LegNeato
Copy link
Collaborator

Let's land this and iterate if needed, it has been sitting too long.

Copy link
Collaborator

@LegNeato LegNeato left a comment

Choose a reason for hiding this comment

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

:shipit:

@LegNeato LegNeato added this pull request to the merge queue Dec 25, 2024
Merged via the queue into Rust-GPU:main with commit bfa63c1 Dec 25, 2024
7 checks passed
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.

3 participants