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

Fix Quaternion memory layout #50

Merged
merged 1 commit into from
Dec 27, 2019
Merged

Fix Quaternion memory layout #50

merged 1 commit into from
Dec 27, 2019

Conversation

aloucks
Copy link
Contributor

@aloucks aloucks commented Dec 26, 2019

The Quaternion memory layout is now [x, y, z, w] (i.e. scalar at the end). This is consistent with the Into trait implementation and also consistent with other libraries (such as GLM) and the GLTF spec.

Fixes #49

The Quaternion memory layout is now `[x, y, z, w]` (i.e. scalar at the
end). This is consistent with the `Into` trait implementation and also
consistent with other libraries (such as GLM) and the GLTF spec.

Fixes kvark#49
Copy link
Owner

@kvark kvark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!
I wonder if this can be considered a breaking change...
bors r+

@aloucks
Copy link
Contributor Author

aloucks commented Dec 27, 2019

@kvark I don't think bors is hooked up (based on your previous comments on another PR)

@kvark kvark merged commit 8e33991 into kvark:master Dec 27, 2019
@Ralith
Copy link
Contributor

Ralith commented Jan 6, 2020

I wonder if this can be considered a breaking change...

I think this is a breaking change iff sufficiently strong guarantees are asserted by the documentation to permit nontrivial sound transmutes. There's a reasonable argument that #[repr(C)], which shows up in rustdoc, constitutes such a guarantee. On the other hand, I suspect nobody will be affected, and it'd be nice to not break everything.

This was referenced Apr 13, 2020
@kvark
Copy link
Owner

kvark commented Apr 13, 2020

I think we can take that risk, with the gain of disrupting less of existing "safe" users.
Ready to yank it if reports prove that to be a bad idea :)
r? #53 @aloucks @Ralith

@aloucks
Copy link
Contributor Author

aloucks commented Apr 13, 2020

Is there any downside to bumping the minor version?

EDIT: Ah, I saw this before I saw it was released. Nevermind about the version!

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 this pull request may close these issues.

Quaternion memory layout inconsistent with into (and similar libraries/specs)
3 participants