-
Notifications
You must be signed in to change notification settings - Fork 727
Add decomposition methods to mat4 and quat #204
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
Conversation
Implemented getAxisAngle method for quat. Decomposes a quaternion into an axis and angle of rotation about that angle. Also implemented associated unit tests.
Added method to get the vector translation component of a mat4 transformation matrix, along with appropriate unit tests.
Added a method to mat4 for pulling the rotation (in the form of a quaternion) from a matrix, along with appropriate unit tests.
* @param {vec3} axis Vector receiving the axis of rotation | ||
* @return {Number} Angle, in radians, of the rotation | ||
*/ | ||
quat.getAxisAngle = function(q, axis) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that for the sake of consistency this should be: function(out_axis, q)
. I'll admit it reads a little funny, but every other function in the library has the output vector/mat/quat as the first argument so we probably shouldn't break that pattern here.
This is really excellent, high quality work! Thank you! (And especially thank you for providing unit tests up front!) I've left one comment about a minor style change, but once that's fixed I'm happy to merge! |
Adjusted order and names for parameters to quat.getAxisAngle to match style. Also adjusted all unit tests using quat.getAxisAngle.
I added the requested style change - I'm glad everything else lines up well. Thanks for the compliment, and thanks for the project - it has been really useful! |
Add decomposition methods to mat4 and quat
Awesome, thanks! |
Is there a reason you did not add getScale? |
There's no good reason I didn't include a |
OVERVIEW
I added methods for extracting useful information from mat4 and quat objects. This addresses issue #136 (which is also an issue I faced using gl-matrix in my own work).
For mat4, I added a mat4.getTranslation and mat4.getRotation method. They return the vec3 translation and quat rotation stored in a transformation matrix, respectively.
For quat, I added a quat.getAxisAngle method. This returns the unit vector axis and angle (radians) rotation stored by the input quaternion.
MATHEMATICAL CONSIDERATIONS
A single axis/angle rotation may be stored by at least two different quaternions. If an axis of rotation is [a, b, c] and the angle of rotation d, the same rotation may be described by an axis of rotation [-a, -b, -c] and an angle 2*PI - 2. The axis and angle returned by quat.getAxisAngle are not guaranteed to be the same that were provided to a quat.setAxisAngle call, but they are guaranteed to represent the same rotation.
A quaternion storing no rotation will always return the axis [1, 0, 0] and angle 0, arbitrarily - the axis itself is not important in this case.
Decomposing a quaternion into its axis and angle components has a potential to yield floating point error. This also applies to decomposing a transformation matrix into its rotational component. Any who use this method should be aware of this limitation. During testing with random values, I came across several cases which were off a relatively large degree of error for 64 bit floating point values after repeated composing and decomposing.
POTENTIAL ISSUES
I'm not extremely familiar with this project, so these are things I was unsure about. If you prefer a different style, let me know and I can make the appropriate changes.
quat.getAxisAngle cannot mutate a passed in Number parameter by reference. Currently, I have the method returning the angle and mutating the axis array passed in the parameter list. Of several less than ideal alternatives, I thought this the most elegant, but it doesn't fit the style of the rest of the code.
Unit tests for mat4.getRotation use more newlines than many other unit tests in the file. This was done for clarity, though if brevity is preferred I can easily make those changes.
EXTERNAL SOURCES USED
Extract rotation quaternion from 3x3 matrix: http://www.euclideanspace.com/maths/geometry/rotations/conversions/matrixToQuaternion/index.htm