So, we are talking about set_push_constants. Although it's possible to cast the slices from and to each other, I would like to point out that with a slice of u8 you can prepare the data with bytes; and it is much cleaner if you want to dynamically assign values. Currently we have to cast the types for multiple times, and as I found there are already people got lost in the wild. Since &[u8] is generally recognized as a type for buffer, I would suggest using &[u8] instead of &[u32] to pass the push constant buffer. If it's acceptable, I would be happily opening a PR for this. :)
The reason is explained in the documentation for that function: "Data size must be a multiple of 4 and must be aligned to the 4s, so we take an array of u32. For example, with an offset of 4 and an array of [u32; 3], that will write to the range of 4..16."
If you are unfamiliar with size and alignment I suggest reading this: https://doc.rust-lang.org/reference/type-layout.html
Maybe we should add a helper function to take &[u8] like we do for shaders?
I understand there are alignment requirements for the input data, but IMO the library so commonly targeting should be aware of the ecosystem also serving the developers. bytes is currently the most broadly used library in the Rust ecosystem to fill buffers and it's common sense buffers are typed by a minimally addressable unit, which is byte. Every buffer in the standard library use bytes. Also, in the documentation we too describe the rules using byte offsets instead of word offsets, while the parameter is not in bytes but 32-bit words. It can also lead to confusion to the ones you want to protect.
It make sense to ensure the alignment from a type-inference level, but what wgpu currently encourages me is to extensively use unsafe functions like transmute. That is not what a Rust library should attempt to, and it does not protect me from violating the alignment requirements too.
I think you are making a strong point, and it seems like it would be a good change to make.
Want to also hear from @cwfitzgerald for they introduced push constants and may have a feeling about this.
So I think this is a reasonable argument, and actually the requirement is nonsensical from the beginning, so please, go ahead!
So when implementing push constants, I basically copied vulkan/gfx-hal's interface and made it safe. There is also precedent for having push constants be u32s, DX12 root constants are represented as 64 u32s. Additionally, the lib we use in the examples (and I myself use) bytemuck, converts from T to any array of primitives easily and just as safely as converting to u8.
All of this is rendered irrelevant however, as we just explicitly copy the contents of the push constants into our own u32 buffer internally. We can very easily adjust wgpu-core's push constant function to take [u8] and reconstruct u32's internally using u32::from_ne_bytes. This would mean the user doesn't actually have an alignment requirement. We could potentially also be very nice and deal with an array size that isn't a multiple of 4, but that'd be awfully nice of us :P
This is done now
Most helpful comment
So I think this is a reasonable argument, and actually the requirement is nonsensical from the beginning, so please, go ahead!
So when implementing push constants, I basically copied vulkan/gfx-hal's interface and made it safe. There is also precedent for having push constants be
u32s, DX12 root constants are represented as 64 u32s. Additionally, the lib we use in the examples (and I myself use)bytemuck, converts from T to any array of primitives easily and just as safely as converting to u8.All of this is rendered irrelevant however, as we just explicitly copy the contents of the push constants into our own
u32buffer internally. We can very easily adjustwgpu-core's push constant function to take[u8]and reconstruct u32's internally usingu32::from_ne_bytes. This would mean the user doesn't actually have an alignment requirement. We could potentially also be very nice and deal with an array size that isn't a multiple of 4, but that'd be awfully nice of us :P