toji / gl-matrix

Javascript Matrix and Vector library for High Performance WebGL apps
glmatrix.net
MIT License
5.4k stars 725 forks source link

Documentation: Clarify quat.fromEuler rotation order #403

Closed gkjohnson closed 4 years ago

gkjohnson commented 4 years ago

The documentation for quat.fromEuler does not specify what order the euler angles are applied in. Referencing three.js' implementation and looking at the quat code it seems that they are applied in ZYX order using intrinsic rotations. It would be great to add that information to the docs -- I'd assumed XYZ intrinsic.

Semi related but it would be nice to be explicit about the order that the quaternion is laid out in, as well, with elements 0, 1, 2 storing the complex xyz components while w is the real component in element 3. I suppose that's common but I've come across some other conventions that store the real component in element 0 so you never know!

And thanks for the awesome library!

stefnotch commented 4 years ago

That's true, the quat.fromEuler is a bit confusing and should probably be changed to be in line with other libraries.

(Relevant issue about the inverse function: https://github.com/toji/gl-matrix/issues/329 )

And yes, the fact that the quaternion order wasn't documented is a minor oversight. Funnily enough, it is documented for dual quaternions. (Edit: fixed it)

DCtheTall commented 4 years ago

@stefnotch I'd be willing to send a PR for adding documentation and/or changing the function to be more like other libraries if you want that. I am aware it's a breaking change so if you just want to document it that works for me 😄

gkjohnson commented 4 years ago

A bit outside the scope of this issue but it would be nice to be able to specify the rotation order when converting euler to quaternion:

quat.fromEuler( quat, x, y, z, 'xyz' );

It could default to zyx for the sake of backwards compatibility too.

stefnotch commented 4 years ago

After thinking about this for a while, I suppose encoding that information right into the function signature would be a decent idea. For example, DirectXMath has a function named XMQuaternionRotationRollPitchYawFromVector. I'd propose a much shorter name like fromEulerXYZ.

Also, for some reason, the ZXY order is also somewhat popular (e.g. DirectXMath, Unity, SharpDX, ...). Not sure why though.

gkjohnson commented 4 years ago

Also, for some reason, the XZY order is also somewhat popular (e.g. DirectXMath, Unity, SharpDX, ...). Not sure why though.

I believe this has to do with whatever convention is being used for "up", "right", and "forward" and you want to maintain the "roll-pitch-yaw" rotation notation. If "Y" is up it will correspond with Yaw. Other conventions specify "Z" as up meaning it will correspond with Yaw in which case you want a different euler order.

DCtheTall commented 4 years ago

@stefnotch I can add a PR with some documentation and perhaps adding a function which uses XYZ order with the name you suggested?

stefnotch commented 4 years ago

@DCtheTall Yes please, do so. It would be greatly appreciated!