2d array conversion rows vs columns mismatch?

[repi]

One issue I ran into when starting using glam for our stuff was this 2d array conversion function:

impl From<[[f32; 4]; 4]> for Mat4 {
    #[inline]
    fn from(m: [[f32; 4]; 4]) -> Self {
        Mat4 {
            x_axis: m[0].into(),
            y_axis: m[1].into(),
            z_axis: m[2].into(),
            w_axis: m[3].into(),
        }
    }
}

I wasn't expecting it to load the rows of the fixed 2d array into the columns and vice versa here. Think when converting from a 2d array that has a definition of rows and columns the most expected result would be to convert from that representation to the internal representation (which in glam is colum-major) for storage.

So how about changing this function to internally transpose the stores here?

Also have the impl From<[f32; 16]> for Mat4 and for that it makes sense store the data linearly directly as colum-major because there is no additional information about how the source is organized wrt to rows and vectors.

[bitshifter]

Maybe this is a bit too ambiguous. I'm not sure what I was thinking at the time when I wrote it, but the layout as it is loaded matches how it is stored in the Mat4 (e.g. treated as columns). Perhaps it would be better to replace this from imp with an explicit function to avoid any confusion? I think one of the other popular Rust libraries uses explicit functions rather than from, maybe it was nalgebra.

[repi]

Yeah I was thinking something similar also, have explicit load/store functions for different variants of this on the object instead, then one have a chance to document and name it based on how it works.

[bitshifter]

Naming is hard! So far I have:

pub fn Mat4::from_array_as_cols(m: [f32; 16]) -> Self
pub fn Mat4::from_arrays_as_cols(m: [[f32; 4]; 4]) -> Self
pub fn Mat4::to_array_as_cols(&self) -> [f32; 16]
pub fn Mat4::to_arrays_as_cols(&self) -> [[f32; 4]; 4]

Which convert from array data in column-major order to a Mat4 and vice-versa.

I'm not totally sold on this naming convention, happy to accept alternative suggestions.

Not sure I can deprecated the From trait implementations (I've added deprecations but they don't appear in docs or trigger warnings for using them) so might just have to delete them.

[repi]

Yeah I'm not fully sure what are good names for them either. But having that instead of the traits would be better at least.

And do think it is fine to nuke the trait functions now as the API is evolving (before a 1.0 stable release), with some heads up.

[bitshifter]

EmbarkStudios/rust-ecosystem#36 (comment) fixed in release 0.8.0.