Replacing #[repr(C)] for uniforms with a better alternative

[mtsr]

What problem does this solve or what need does it fill?

Memory layout in GLSL is diffent to #[repr(C)] that is currently used on lights, rects and sprites. This will break silently when you add an extra field or otherwise accidentally mess up layout.

See the GLSL specs.

What solution would you like?

Learn-WGPU discusses this here and mentions two crates that help solve this issue by deriving Uniform or AsStd140 or AsStd430:

• crevice
• glsl-layout

An initial look at both crates makes crevice look more attractive, because it has support for AsStd430 and mint types

What alternative(s) have you considered?

Manually verifying memory layout.

Additional context

[mtsr]

If there's agreement on a crate I can make a PR.

[Frizi]

Note that both of those crates introduce extra intermediary type that has correct layout, instead of modifying the layout of the struct directly. That means there is an extra conversion step when writing data to the GPU buffer. It won't be as fast as plain memcpy.

[mtsr]

Ok, that seems suboptimal indeed. Although I'd be interested to see how it actually compiles and how it performs. Because right now we also don't do a plain memcpy everywhere, because we're doing multiple writes of data from different components in some cases and creating an intermediate struct with combined data in other cases. And creating a temporary struct means doing one set of smaller copies and then one bigger memcpy, which might not necessarily perform much worse than what we already do.

But it seems safe to assume that if it is indeed costing performance we could do better with a custom derive that writes to a buffer directly.

In the meantime, do people agree that this alignment issue is worth solving? Is it bad enough that we want to use a potentially suboptimal solution for the time being to prevent errors cause by this? Even if we think Bevy developers are fine, we might want to consider end-users that write their own things.

[Frizi]

we could do better with a custom derive that writes to a buffer directly

I believe this is what crevice does, as its std140::Writer can read non-std140 vector of data and perform the writes directly to the GPU buffer in correct format, without extra allocations or buffering. So maybe it's actually fast in practice. Likely still bound by memory speed, so I think using it is definitely a great option.

One concern is the cost of this extra step in non-optimized builds, but this ship has probably already sailed :)

Of course none of that matters for non-vec types. When writing only few fields you don't want to use memcpy anyway, and speed is not an issue.

What I had in mind as an alternative is a macro that directly modifies the annotated struct by adding the padding fields, but this also has it's drawbacks. Namely, you have to choose one specific format (std140/std430). Also the inserted padding fields prevent usage of exhaustive struct initializers (because you would have to name the padding fields). So that leaves you with ugly ..Default::default() or a macro initializer that adds the padding fields (e.g. PointLight! { color: ... }).

So, I believe we should use crevice and not try to reinvent it until we have a good data-backed reason.

Also a wording nitpick. Whatever we choose, we don't technically get rid of #[repr(C)], as the generated structs also have that repr. So we aren't really "replacing" it, more like abstracting away.

[mockersf]

To consider, in addition of structs that are #[repr(C)], there are a few places where we write bytes manually and that may cause issues with different platforms.

For example, in #1798 I added an extra byte of padding to a vec3 to be std140 compatible, to help with WebGL2 support.

[james7132]

Closing this now that bevy_crevice is used for all of our rendering pipelines.