Add per-vertex stuff #8591
Add per-vertex stuff #8591
Conversation
|
My references: Vulkan |
inner-daemons
left a comment
inner-daemons
left a comment
There was a problem hiding this comment.
I am leaving a bunch of comments here for my future self and because I want to have a better understanding of the code before I adopt it myself.
There was a problem hiding this comment.
I don't think we need to be worrying about GLSL output, especially if this is only for Vulkan accepted GLSL
| "invariant" => TokenValue::Invariant, | ||
| "flat" => TokenValue::Interpolation(crate::Interpolation::Flat), | ||
| "noperspective" => TokenValue::Interpolation(crate::Interpolation::Linear), | ||
| "pervertexEXT" => TokenValue::Interpolation(crate::Interpolation::PerVertex), |
There was a problem hiding this comment.
I'm not sure that we need to be parsing this for GLSL. If its super simple then why not, but otherwise I'll probably ignore or remove it.
| #[error("The `PER_VERTEX` capability must be enabled to use per-vertex fragment inputs.")] | ||
| PerVertexNotAllowed, |
There was a problem hiding this comment.
This should probably just be a MissingCapability error
| #[cfg_attr(feature = "deserialize", derive(serde::Deserialize))] | ||
| #[derive(Clone, Copy, Debug, Eq, PartialEq)] | ||
| pub struct Capabilities: u32 { | ||
| pub struct Capabilities: u64 { |
There was a problem hiding this comment.
I've been waiting for the day!
| .parameters( | ||
| TestParameters::default() | ||
| .test_features_limits() | ||
| .features(wgpu::Features::SHADER_BARYCENTRICS), |
There was a problem hiding this comment.
Probably also needs the other feature
| ) | ||
| .run_async(barycentric); | ||
|
|
||
| async fn barycentric(ctx: TestingContext) { |
There was a problem hiding this comment.
This functino is quite long. In particular, I've been advised for some of my own tests to avoid vertex buffers and just generate the vertices in the fragment shader.
There was a problem hiding this comment.
its based off of the barycentric test (and i forgot to rename it, its copy pasta lol) which in turn is based off of the triangle indexing test. Generating vertices in the shader would make the shader more complicated, idk.
| // These tests render two triangles to a 2x2 render target. The first triangle | ||
| // in the vertex buffer covers the bottom-left pixel, the second triangle | ||
| // covers the top-right pixel. | ||
| // XY layout of the render target, with two triangles: | ||
| // | ||
| // (-1,1) (0,1) (1,1) | ||
| // +-------+-------+ | ||
| // | |o-----o| | ||
| // | | \ / | | ||
| // | | \ / | | ||
| // | | o | | ||
| // (-1,0) +-------+-------+ (1,0) | ||
| // | o | | | ||
| // | / \ | | | ||
| // | / \ | | | ||
| // |o-----o| | | ||
| // +-------+-------+ | ||
| // (-1,-1) (0,-1) (1,-1) | ||
| // | ||
| // The fragment shader outputs color based on per-vertex position: | ||
| // | ||
| // return vec4(z[0], z[1], z[2], 1.0); | ||
| // |
There was a problem hiding this comment.
Is the reason for 2 triangles related to which vertex comes in index 1 or something? I'm not sure why we need 2
There was a problem hiding this comment.
One should be fine, i just wanted to have more assertion on the ordering of the per-vertex attributes being consistent between platforms. It is something i am a bit worried about
There was a problem hiding this comment.
Maybe a triangle strip topology or something would be a better test?
2 participants
Connections
CC @atlv24 for doing the work so far, this is based off their branch here.
Description
Adds per-vertex stuff to the fragment shader as a follow-up to barycentric coordinates, allowing you to do your own stuff for interpolations.
Testing
Explain how this change is tested.
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.