Add per-vertex attribute support to vulkan, spir-v and wgsl.#8821
Add per-vertex attribute support to vulkan, spir-v and wgsl.#8821cwfitzgerald merged 12 commits intogfx-rs:trunkfrom
Conversation
08941d5 to
7b76314
Compare
|
I may work on the HLSL and MSL support myself at a later date. I'll be reviewing this PR, since I've examined the spec and talked about this briefly with @atlv24. |
inner-daemons
left a comment
There was a problem hiding this comment.
Broadly looks good. Some comments for now, plus I want to think through the validation later to make sure you didn't miss anything.
There was a problem hiding this comment.
Pull request overview
This PR adds support for per-vertex attributes in fragment shaders, enabling access to raw per-vertex values within a triangle. The feature leverages Vulkan's VK_KHR_fragment_shader_barycentric extension and is implemented for WGSL input and SPIR-V/WGSL output only, with HLSL and MSL support deferred for future work.
Key Changes:
- Added
SHADER_PER_VERTEXfeature flag to wgpu-types - Integrated the feature with Vulkan's fragment shader barycentric extension in wgpu-hal
- Extended naga's IR with
PerVertexinterpolation mode and added validation for per-vertex fragment inputs (must be array of length 3)
Reviewed changes
Copilot reviewed 19 out of 21 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| wgpu-types/src/features.rs | Added SHADER_PER_VERTEX feature flag with Vulkan-only support annotation |
| wgpu-hal/src/vulkan/adapter.rs | Connected feature flag to VK_KHR_fragment_shader_barycentric extension and FragmentBarycentricKHR capability |
| wgpu-core/src/device/mod.rs | Mapped wgpu feature to naga validator capability |
| naga/src/ir/mod.rs | Added PerVertex variant to Interpolation enum |
| naga/src/front/wgsl/parse/conv.rs | Added WGSL parser support for per_vertex interpolation keyword |
| naga/src/front/spv/mod.rs | Added SPIR-V frontend support for PerVertexKHR decoration |
| naga/src/common/wgsl/to_wgsl.rs | Added WGSL output support for per_vertex interpolation |
| naga/src/back/spv/writer.rs | Added SPIR-V backend support with PerVertexKHR decoration and capability requirement |
| naga/src/back/hlsl/conv.rs | Added unreachable case for PerVertex (HLSL not yet supported) |
| naga/src/back/glsl/conv.rs | Added unreachable case for PerVertex (GLSL not yet supported) |
| naga/src/valid/mod.rs | Added PER_VERTEX capability flag |
| naga/src/valid/interface.rs | Added validation logic ensuring per-vertex inputs are fragment-only, array of length 3, and require proper capability |
| naga/tests/naga/validation.rs | Updated interpolation validation tests to include PerVertex |
| naga/tests/in/wgsl/per-vertex.wgsl | Added naga test shader for per-vertex feature |
| naga/tests/in/wgsl/per-vertex.toml | Test configuration restricting to WGSL and SPIRV targets |
| naga/tests/out/wgsl/wgsl-per-vertex.wgsl | Expected WGSL output for naga test |
| naga/tests/out/spv/wgsl-per-vertex.spvasm | Expected SPIR-V output for naga test |
| tests/tests/wgpu-gpu/per_vertex/per_vertex.wgsl | Integration test shader demonstrating per-vertex attributes |
| tests/tests/wgpu-gpu/per_vertex/mod.rs | Integration test rendering triangle strip and validating per-vertex values |
| tests/tests/wgpu-gpu/main.rs | Registered per_vertex test module |
| CHANGELOG.md | Added changelog entry for the new feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Could we make the texture a little bit bigger just to be perfectly clear with whats going on? Maybe just like 16x16 or something
There was a problem hiding this comment.
I believe the point of the other tests using a small texture is to be able to reason about it pixel by pixel mentally. With that many pixels, you'd be having to reason about 256 fragment shader invocations mentally which is a bit much. It would also make the assert huge and unreadable with 1024 byte literals
There was a problem hiding this comment.
I agree that this size is perfect, the diagram really helps!
cwfitzgerald
left a comment
There was a problem hiding this comment.
Code looks good!
Some smaller nits, plus could we add a test in wgsl_error to make sure that any per_vertex stuff gets properly validated out if the feature isn't there (very important for firefox). This would also be a good place to test the other validation, but this is less important.
There was a problem hiding this comment.
I agree that this size is perfect, the diagram really helps!
Connections
#8591
Description
HLSL and MSL support are possible, but hard. I want to land Vulkan support and worry about the rest later.
Testing
wgpu-gpu test and naga test.
Squash or Rebase?
squish
Checklist
cargo fmt.taplo format.cargo clippy --tests. If applicable, add:--target wasm32-unknown-unknowncargo xtask testto run tests.CHANGELOG.mdentry.