hadronized/luminance-rs

Use arrays instead of tuples for pixel types

kpreid opened this issue · 4 comments

kpreid commented

Idea for a small refinement, which I would propose be included the next time there's an incompatible API version for other reasons:

Change the Pixel::Encoding types from being tuples like (u8, u8, u8, u8) to arrays like [u8; 4].

This would simplify some componentwise color processing code — especially given Rust features min_const_generics and array_map. Additionally, it would be consistent with the types that are allowed for uniforms and vertex attributes (which include arrays up to 4 elements but not tuples).

kpreid commented

Using tuples may also be technically unsound as the layout of tuples is not guaranteed.

kpreid commented

It looks like the WebGL2 backend wasn't updated to support using arrays instead of tuples:

error[E0277]: the trait bound `[u8; 4]: luminance_webgl::webgl2::array_buffer::IntoArrayBuffer` is not satisfied
   --> all-is-cubes/src/lum/block_texture.rs:37:18
    |
37  |     pub texture: BlockTexture,
    |                  ^^^^^^^^^^^^ the trait `luminance_webgl::webgl2::array_buffer::IntoArrayBuffer` is not implemented for `[u8; 4]`
    | 
   ::: /Users/kpreid/.cargo/git/checkouts/luminance-rs-441601892339f541/352a90a/luminance/src/texture.rs:556:15
    |
556 |   B: ?Sized + TextureBackend<D, P>,
    |               -------------------- required by this bound in `luminance::texture::Texture`
    |
    = note: required because of the requirements on the impl of `luminance::backend::texture::Texture<luminance_front::texture::Dim3, NormRGBA8UI>` for `luminance_webgl::webgl2::WebGL2`

This is from testing against the fix/483-webgl-index-buffer-corruption branch. According to locally generated docs, IntoArrayBuffer is implemented for tuples but not for arrays.

hadronized commented

Ah that’s weird, I wonder why I didn’t get into that situation. Thanks!

robert-chiniquy commented

I just got hit with this one too. Looking fwd to 0.44 being published.