wgsl-bindgen icon indicating copy to clipboard operation
wgsl-bindgen copied to clipboard

Builtin vertex input fields appear in generated Rust structure

Open Aern-do opened this issue 1 year ago • 8 comments

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 avatar Jul 19 '24 20:07 Aern-do

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

Aern-do avatar Jul 19 '24 20:07 Aern-do

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 avatar Jul 20 '24 03:07 Swoorup

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

Swoorup avatar Jul 20 '24 03:07 Swoorup

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.

Aern-do avatar Jul 20 '24 07:07 Aern-do

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 avatar Jul 20 '24 09:07 Swoorup

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.

Swoorup avatar Jul 27 '24 16:07 Swoorup

Disclaimer

I don't know if this is the right place to put this (it does seem to be related), if you want to I can move this into a seperate issue:

Issue

For some reason the binding for this:

struct VertexInput {
    @builtin(vertex_index) vertex_index: u32,
};

is this:

#[repr(C)]
#[derive(Debug, PartialEq, Clone, Copy, encase::ShaderType)]
pub struct VertexInput {
    pub vertex_index: [u8; 0x4],
}

which is a bit weird, but wouldn't really be a problem, but encase doesn't implement ShaderSize for u8's, so this is a compiler error!

jullanggit avatar Dec 17 '24 21:12 jullanggit

Disclaimer

I don't know if this is the right place to put this (it does seem to be related), if you want to I can move this into a seperate issue:

Issue

For some reason the binding for this:

struct VertexInput {
    @builtin(vertex_index) vertex_index: u32,
};

is this:

#[repr(C)]
#[derive(Debug, PartialEq, Clone, Copy, encase::ShaderType)]
pub struct VertexInput {
    pub vertex_index: [u8; 0x4],
}

which is a bit weird, but wouldn't really be a problem, but encase doesn't implement ShaderSize for u8's, so this is a compiler error!

This is a separate issue. I haven't given much attention to encase but can look into it.

Swoorup avatar Dec 22 '24 03:12 Swoorup

Fixed by https://github.com/Swoorup/wgsl-bindgen/pull/67

Also alignment is correct as is, see demo files for example:

  • https://github.com/Swoorup/wgsl-bindgen/blob/main/example/shaders/vec3_vertex_demo.wgsl
  • https://github.com/Swoorup/wgsl-bindgen/blob/main/example/src/demos/vec3_vertex_demo.rs

Swoorup avatar Jun 28 '25 03:06 Swoorup

Closing as fixed in https://crates.io/crates/wgsl_bindgen/0.19.1

Swoorup avatar Jun 28 '25 03:06 Swoorup