incorrect padding/alignment with vec3<f32> to glam::Vec3A

[EriKWDev]

the following wgsl struct

struct Job {
    position: vec3<f32>,
    direction: vec3<f32>,
    accum: vec3<f32>,
    depth: u32,
};

gets turned into this rust struct

#[repr(C, align(16))]
#[derive(Debug, PartialEq, Clone, Copy)]
pub struct Job {
    /// size: 12, offset: 0x0, type: `vec3<f32>`
    pub position: glam::Vec3A,
    /// size: 12, offset: 0x10, type: `vec3<f32>`
    pub direction: glam::Vec3A,
    /// size: 12, offset: 0x20, type: `vec3<f32>`
    pub accum: glam::Vec3A,
    pub _pad_accum: [u8; 0xC - core::mem::size_of::<glam::Vec3A>()],
    /// size: 4, offset: 0x2C, type: `u32`
    pub depth: u32,
}

which doesn't compile since 0xC - size_of::<Vec3A>() overflows.

0xC is 12 bytes but the size of a glam::Vec3A is 16 as can be seen inside glam:

#[derive(Clone, Copy)]
#[repr(transparent)]
pub struct Vec3A(pub(crate) __m128);

I think the choice of using Vec3A as opposed to the normal Vec3 in is a bit strange. In cases where we want vec3, vec3, vec3 in wgsl without any 'padding fields' in rust, Vec3A could work, but in my case above where I would've expected the depth: u32 to be inserted instead of padding, a Vec3A will not work

The normal glam::Vec3 has the following definition which I think is easier to reason about for the generator and would allow packing data in the padding as I'd expected

#[derive(Clone, Copy, PartialEq)]
#[cfg_attr(not(target_arch = "spirv"), repr(C))]
#[cfg_attr(target_arch = "spirv", repr(simd))]
pub struct Vec3 {
    pub x: f32,
    pub y: f32,
    pub z: f32,
}

I would've expected the following rust struct to be produced:

#[repr(C, align(16))]
#[derive(Debug, PartialEq, Clone, Copy)]
pub struct Job2 {
    pub position: glam::Vec3,
    pub pad_0: [u8; 4],
    pub direction: glam::Vec3,
    pub pad_1: [u8; 4],
    pub accum: glam::Vec3,
    pub depth: u32,
}

[Swoorup]

Are you using it as uniform buffer object or a storage buffer object? I'll have to recheck this later, but I think at the time I found that the alignment that came with glam::Vec3 was incorrect.

The const assertions are added to catch issues in cases where the codegen fails to properly align or use the correct sizes.

[EriKWDev]

I used it inside a read_write storage array, but I just tried read-only storage as well as a lone uniform and no matter which usage the same rust struct is generated which causes a compilation error

  _pad_accum: [0; 0xC - core::mem::size_of::<glam::Vec3A>()],
                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ attempt to compute `12_usize - 16_usize`, which would overflow

When writing these by hand I've always inserted a u32 after each glam::Vec3 unless there is something with the size of a u32 that could take the padding's place, such as in this case.

The static assertions checks the sizes

assert!(std::mem::size_of:: < glam::Vec3A > () == 16);
assert!(std::mem::align_of:: < glam::Vec3A > () == 16);

but in the generated comments it claims different sizes and uses the size '0xC' to generate padding which is 12, not 16. Seems like something is incorrect here or I have misunderstood how this all works

#[repr(C, align(16))]
#[derive(Debug, PartialEq, Clone, Copy)]
pub struct Job {
    /// size: 12, offset: 0x0, type: `vec3<f32>`
    pub position: glam::Vec3A,
    /// size: 12, offset: 0x10, type: `vec3<f32>`
    pub direction: glam::Vec3A,
    /// size: 12, offset: 0x20, type: `vec3<f32>`
    pub accum: glam::Vec3A,
    pub _pad_accum: [u8; 0xC - core::mem::size_of::<glam::Vec3A>()],
    /// size: 4, offset: 0x2C, type: `u32`
    pub depth: u32,
}

[EriKWDev]

Side-note: The types sent to the GPU maybe don't need to use these user-provided types at all. When padding is inserted a nice-to-have struct MyStructInit gets generated whose job it is to transform it into the padded struct.

Seems to me like the padded struct could just use regular 'bytemuck type map' like [f32; 3]:s and whatnot instead to guarantee the same layout always and then the MyStructInit -> MyStruct transformation could handle the different math-modules

[sytherax]

Hi @EriKWDev,

This has been fixed in wgsl_bindgen v0.20.0: https://crates.io/crates/wgsl_bindgen/0.20.0

We've replaced glam::Vec3A with glam::Vec3 and fixed the padding calculations as you suggested. Your WGSL struct now generates the correct Rust layout:

#[repr(C, align(16))]
#[derive(Debug, PartialEq, Clone, Copy)]
pub struct Job {
    pub position: glam::Vec3,
    pub _pad_position: [u8; 0x4],
    pub direction: glam::Vec3,
    pub _pad_direction: [u8; 0x4],
    pub accum: glam::Vec3,
    pub depth: u32,
}

This is a breaking change - you'll need to update code that was using Vec3A types.

Please try v0.20.0 and confirm it works for your use case.