Swoorup / wgsl-bindgen

Generate typesafe Rust bindings for wgsl shaders in wgpu
https://crates.io/crates/wgsl_bindgen
MIT License
34 stars 7 forks source link

Builtin vertex input fields appear in generated Rust structure #34

Open Aern-do opened 4 months ago

Aern-do commented 4 months ago

I have this structure in a shader used as vertex shader input

struct VertexInput {
    @location(0) position: vec3<f32>,
    @location(1) texture_id: u32,

    @builtin(vertex_index) vertex_index: u32
}

@vertex
fn vs_main(in: VertexInput) -> VertexOutput { ... }

In the generated Rust code, the structure still contains the builtin field vertex_index

#[repr(C)]
#[derive(Debug, PartialEq, Clone, Copy)]
pub struct VertexInput {
    pub position: [f32; 4],
    pub texture_id: u32,
    pub vertex_index: u32,
}

However, vertex attributes do not have this field, and it appears only in the structure itself

pub const VERTEX_ATTRIBUTES: [wgpu::VertexAttribute; 2] = [
    wgpu::VertexAttribute {
        format: wgpu::VertexFormat::Float32x3,
        offset: std::mem::offset_of!(VertexInput, position) as u64,
        shader_location: 0,
    },
    wgpu::VertexAttribute {
        format: wgpu::VertexFormat::Uint32,
        offset: std::mem::offset_of!(VertexInput, texture_id) as u64,
        shader_location: 1,
    },
];
Aern-do commented 4 months ago

Also, in the generated structure, the position field is [f32; 4] instead of [f32; 3]. I'm not sure if this is correct behavior or not

Swoorup commented 4 months ago

However, vertex attributes do not have this field, and it appears only in the structure itself

The vertex_index you are specifying is built in and doesn't need to be specified.

Also, in the generated structure, the position field is [f32; 4] instead of [f32; 3]. I'm not sure if this is correct behavior or not

That is the correct behaviour, it is automatically adding the padding bytes in this case. You can provide your own types, it will still ensure (through const assertions that position is 16 bytes in length).

Swoorup commented 4 months ago

See https://www.w3.org/TR/WGSL/#alignment-and-size

Aern-do commented 4 months ago

The vertex_index you are specifying is built in and doesn't need to be specified.

I understand that this is the intended behavior. However, the vertex_index should not be included in the generated structure, since it is not a user input.

Swoorup commented 4 months ago

The vertex_index you are specifying is built in and doesn't need to be specified.

I understand that this is the intended behavior. However, the vertex_index should not be included in the generated structure, since it is not a user input.

Ah you're right. My caffeine is weak today. 😅

Swoorup commented 3 months ago

On second thought, generating vertex_index isn't necessary a bug. In places where this is used in eg: storage buffer, it will be treated as padding field, and the gpu fills this for you. I have made a change where constructors and Init structs will zero out this field.

The other part about the position being 16 bytes instead of 12 is likely a bug. Alignments are likely handled differently in vertex attrib. I'll need to play around with a working example to test that however.