Metal shader translation does not align `vec3`s to match std140

[kpreid]

See later updates as this original description is an incorrect diagnosis.

My application worked OK under the Metal backend but mis-shaded under the WebGL backend. I made the problem disappear by modifying my camera uniform struct to avoid lone f32s in favor of vec4<f32>s.

My understanding is that this is because WebGL uniforms expect “std140” layout which pads every field up to a multiple of 16 bytes (the size of a vec4<f32>). (My own knowledge of GL is limited here, as I never used structs in uniforms rather than individual scalar/vector/matrix uniforms before starting to use wgpu, so I could be missing some nuance.)

It seems to me that, to support portability between backends, wgpu should either:

• do whatever code transformation is necessary to effectively not have std140 padding on the WebGL backend (probably not a good idea), or
• consider it a MissingDownlevelFlags error when a shader has a struct used as a uniform which has padding under std140 that would not be present on other backends. (In particular, DownlevelFlags::BUFFER_BINDINGS_NOT_16_BYTE_ALIGNED probably applies to the same situations; maybe it should be renamed and expanded to cover this?)

[kpreid]

P.S. Another helpful feature that could be built on the ability to inspect/validate shaders' struct declarations is checking that a given Rust struct declaration's layout is compatible with the shader's, which would be useful for both uniform buffers and vertex buffers.

[kpreid]

From chat, I'm informed that I misunderstood std140 layout rules, and this might be a conformance problem under Metal rather than WebGL (if so, it would be that it's padding vec3<f32>s rather than letting f32s fall after them). The changes I made to my structs can be seen here: kpreid/all-is-cubes@e4c0571#diff-5996010e9b3212ff24d1b44569cd3939db0603b203f63be3f60405d983d1db7b

[kpreid]

Further investigation (thanks to cwfitzgerald and Lucien on Rust Game Development Discord) determined that the bug is actually in the Metal shader translation.

WGSL                                                               Metal

struct ShaderSpaceCamera {                                         struct ShaderSpaceCamera {
    @location(0) projection: mat4x4<f32>,                              metal::float4x4 projection;
    @location(1) view_matrix: mat4x4<f32>,                             metal::float4x4 view_matrix;
    @location(2) view_position: vec3<f32>,                             metal::float3 view_position;
    @location(3) light_lookup_offset_and_option: vec4<i32>,            metal::int4 light_lookup_offset_and_option;
    @location(4) fog_color: vec3<f32>,                                 metal::float3 fog_color;
    @location(5) fog_mode_blend: f32,                                  float fog_mode_blend;
    @location(6) fog_distance: f32,                                    float fog_distance;
    @location(7) exposure: f32,                                        float exposure;
    @location(8) _padding: f32,                                        float _padding;
};                                                                 };

metal::float3 has size of 16 and align of 16. A closer translation is metal::packed_float3, which has size of 12 and align of 4. However, since std140 specifies the alignment, but not the size, of vec3s to be 4 element sizes, further measures will be needed to ensure the alignment lands correctly.

(The above struct actually demonstrates this: per std140, there is padding between view_position and light_lookup_offset_and_option, because light_lookup_offset_and_option has an alignment of 16 bytes, but there is not padding between fog_color and fog_mode_blend, because fog_mode_blend's alignment is only 4 bytes.)

[teoxoy]

This is caused by

https://github.com/gfx-rs/naga/blob/350171ed2f45612386cc91a0ee9ff79d918939a9/src/back/msl/writer.rs#L398-L402

which was introduced as a fix for gfx-rs/naga#1297.

Considering we now interact with packed_vec3s differently (cast them only when we need to)

https://github.com/gfx-rs/naga/blob/350171ed2f45612386cc91a0ee9ff79d918939a9/src/back/msl/writer.rs#L1526-L1537

I think we can remove the check and see if it causes any issues.

Btw, you could remove the @location(x) attributes since I saw you aren't using the struct as an input/output for vertex/fragment stages.