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

Quaternion order DOES matter, can we please have a macro for the order? #983

Closed
Cazadorro opened this issue Dec 16, 2019 · 3 comments
Closed

Comments

@Cazadorro
Copy link

Cazadorro commented Dec 16, 2019

Problem

Okay, I know this was discussed back in September and back in July before that. but there are legitimate needs beyond API consistency reasons why the internal quaternion order should be w,x,y,z, and that isn't arbitrary. There are also needs for it to be x,y,z,w. I need some way to compile the library with one order or the other.

Previously on this discussion there were several issues brought up for why w, x, y, z should be the internal order, all relating to the inconsistency of the API. With those changes now reverted GLM now has an even bigger issue. People do not always update which GLM version they have, and any one who happened to pick that version of the API now will potentially have different answers to those Stack Overflow questions creating a big mess. For some people, that w,x,y,z api will be the only option they have.

So now we have two issues:

  • The perceived api inconsistency
  • The api changed a detail that now matters for how code functions, and people who needed this change have to stick with a very short lived version of GLM to get what they need to work.

Now AFAIK the arguments for why the change should have be reverted were:

  • Breaking change + backward compatibility
  • other libraries do this.

Breaking changes are definitely reason enough on their own that this shouldn't have been a change that effected everyone who updated the library. Heck this even effected me on other projects! But I don't think the second argument is a good one, because now I'm running in to the opposite issue of other libraries that don't do x,y,z,w.

One of the most popular conventions in learning quaternions is w,x,y,z, and is the reason why we see virtually all libraries take constructors with w,x,y,z, and not x,y,z,w, and most print the same way. This convention has implications in math in general, but the real place this screws up is when I need to use glm in combination with other libraries, where the layout of the data matters.

With glm being x,y,z,w order, custom advanced jacobian matrix operations, which use larger than 4x4 and thus aren't done through glm math, change depending on the order of the quaternions in non trivial ways. Math needs to be re-written to account for the difference in order of the quatnerions, otherwise we take a hit in performance due to the conversion.

One place this comes up a lot is when interopting with numpy or replacing parts of numpy using pyglm
where glm is a much faster substitute for the small matrix operations. Quaternion notation in pyquaternion ended up being the opposite in glm, and because of that, internal changes when replacing pyquaternion with glm.dquat caused weeks of effort for our codebase. By allowing the switching of the order, we could have used pyglm's facilities to just straight up convert between numpy and quaternions and insert into our matrices. This now requires custom functions that switch back and forth between what numpy doesn't have and what glm does, and vice versa.

Solution

I'm in a unique position where both changing the default to w,x,y,z gave me breaking changes on some projects, and also solved issues on others. Instead of changing default behavior, or ignoring the need for different data ordering, can we please just have a macro that changes the order that we can configure at build? Something like

#ifdef GLM_QUAT_DATA_ORDER_WXYZ
    T w, x, y, z;
#else
    T x, y, z, w;
#endif

This would help people like me who would really appreciate the ordering of the quaternions to be w,x,y,z for interop reasons, but would not introduce any breaking changes to currently existing code, nor set the standard that all code from this point on should be w,x,y,z.

Technically the API inconsistency is still there, and one way you would fix that is to reorder the constructor arguments. But really the way the data elements are ordered should technically be an implementation issue so I would argue that the order of the function elements shouldn't be changed, at least not on the grounds that because internally they are ordered one way the constructor function needs to match that.

@Xottab-DUTY
Copy link

Agree with the solution

@Groovounet
Copy link
Member

Sounds good to me !

@Groovounet
Copy link
Member

This issue should be resolved with 638eb14 which landed with GLM 0.9.9.7 release.

Thanks for contributing,
Christophe

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

No branches or pull requests

3 participants