DescriptorInit error on unaligned shader uniforms

[icefoxen]

When creating a pipeline defined with:

gfx_defines!{
    vertex Vertex {
        pos: [f32; 2] = "a_Pos",
        uv: [f32; 2] = "a_Uv",
    }

    constant Transform {
        transform: [[f32; 4];4] = "u_Transform",
    }

    // Values that are different for each rect.
    constant RectProperties {
        src: [f32; 4] = "u_Src",
        rotation: f32 = "u_Rotation",
        dest: [f32; 2] = "u_Dest",
        scale: [f32;2] = "u_Scale",
        offset: [f32;2] = "u_Offset",
        shear: [f32;2] = "u_Shear",
    }

    pipeline pipe {
        ...
    }
}

My shader:

#version 150 core

in vec2 a_Pos;
in vec2 a_Uv;

uniform Transform {
    mat4 u_Transform;
};

uniform RectProperties {
    vec4 u_Src;
    float u_Rotation;
    vec2 u_Dest;
    vec2 u_Scale;
    vec2 u_Offset;
    vec2 u_Shear;
};

out vec2 v_Uv;

void main() {
    v_Uv = a_Uv;
    gl_Position = vec4((a_Pos * u_Scale) + u_Dest, 0.0, 1.0) * u_Transform;
}

When I create a pipeline with Factory::create_pipeline_simple() I get: DescriptorInit(ConstantBuffer("RectProperties", Some(Offset("u_Dest", 24))))

When I move rotation to the end of the uniform list it works, so I expect this is the driver refusing to handle non-aligned types. Can this either be padded automatically, or at least detected and warned against?

Thanks.

[kvark]

Thanks for filing the issue!
Please note that our master branch has better errors now for this, see #1182.

When I move rotation to the end of the uniform list it works, so I expect this is the driver refusing to handle non-aligned types.

The problem is Rust and GLSL align the data differently. This is kinda expected, although I completely agree that if we can force them to be the same, we totally should (like #[repr(glsl)]).

Can this either be padded automatically, or at least detected and warned against?

I don't think the macros can pad automatically or even warn about this at compile time.
A custom derive though, should at least be able to warn. I'll see if #1183 can be extended to check for a particular alignment.

[theor]

Question about that: if I'm not mistaken, alignment and padding is implementation dependent (see Memory Layout section here: https://www.khronos.org/opengl/wiki/Interface_Block_(GLSL)#Layout_query). I think that means macros are not a good fit, as it might work on the build machine but not on another one running the binary, right ?

Edit: Looking at the factory.update_sub_buffer gl impl, it uses glBufferSubData to update the whole buffer at once. I think in that case, we should do it field by field if necessary (ie. flag the code/gl mismatch). https://www.khronos.org/registry/OpenGL-Refpages/gl4/html/glBufferSubData.xhtml

[kvark]

@theor

if I'm not mistaken, alignment and padding is implementation dependent (see Memory Layout section here

There are defined layouts in GLSL: std140 and std430, and users are encouraged to use them.

I think that means macros are not a good fit, as it might work on the build machine but not on another one running the binary, right ?

Yes, totally agreed. See #1183 for our possible solution to avoid macro-generated structs.

I think in that case, we should do it field by field if necessary

That would be slower. I'd like to avoid calling the API too often.