Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing serialization support for Mat2/Mat3/Mat4 #17

Closed
repi opened this issue Sep 13, 2019 · 4 comments · Fixed by #18
Closed

Missing serialization support for Mat2/Mat3/Mat4 #17

repi opened this issue Sep 13, 2019 · 4 comments · Fixed by #18

Comments

@repi
Copy link
Contributor

repi commented Sep 13, 2019

Was experimenting with glam as a potential replacement for us for nalgebra-glm and one of the first issues I ran into was that the Mat2, Mat3 and Mat4 types was missing serde serialization support.

Currently it is only implemented for Vec2, Vec3, Vec4 and Quat in glam_serde.rs but likely shouldn't be too hard to add support for matrix types as well?.

@bitshifter
Copy link
Owner

It should be pretty easy. I'm traveling at the moment so might not get a chance to take a look until I get back home in a weeks time.

@repi
Copy link
Contributor Author

repi commented Sep 13, 2019

np, thanks for the quick response. I'll give it a stab

@bitshifter
Copy link
Owner

bitshifter commented Sep 14, 2019

Actually maybe it's not as straight forward as I thought, I thought these might just be able to use #[derive(Serialize, Deserialize)] but for consistency between the different types it might be better to implement these traits. I've started a branch at https://github.com/bitshifter/glam-rs/tree/matn_serde but it's not really useful yet.

Do you know what nalgebra_glm matrix types look like when they're serde json serialized?

I'm thinking either [m1, m2, m3, m4] or [[m1, m2],[m3,m4]] for a Mat2. The latter is more verbose but more obviously a matrix. I tend to try match what the equivalent of

#[derive(Serialize, Deserialize)]
struct Mat2(Vec2, Vec2)

would be serialized as, which is probably the latter.

@repi
Copy link
Contributor Author

repi commented Sep 14, 2019

In my implementation I implemented the serialization explicit and serialized the matrices as flat arrays, thought it was a bit cleaner and potentially faster with less verbose output.

Checked up on how nalgebra-glm was serializing it and it was also doing it as a flat array of values instead of tuples:

Code:

   let m2 = glm::mat2(1.0, 2.0, 3.0, 4.0);
   let m3 = glm::mat3(1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0);
   let m4 = glm::mat4(1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0, 10.0, 11.0, 12.0, 13.0, 14.0, 15.0, 16.0);
   println!("mat2: {}", serde_json::to_string(&m2).unwrap());
   println!("mat3: {}", serde_json::to_string(&m3).unwrap());
   println!("mat4: {}", serde_json::to_string(&m4).unwrap());

Output:

mat2: [1.0,3.0,2.0,4.0]
mat3: [1.0,4.0,7.0,2.0,5.0,8.0,3.0,6.0,9.0]
mat4: [1.0,5.0,9.0,13.0,2.0,6.0,10.0,14.0,3.0,7.0,11.0,15.0,4.0,8.0,12.0,16.0]

So I think that is the way to go.

I'll bring over your tests to my branch as well and do a PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants