-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Exposing Transforms.rotationMatrixFromPositionVelocity #8934
Conversation
… tests for Transforms.rotationMatrixFromPositionVelocity
Thanks for the pull request @easternmotors!
Reviewers, don't forget to make sure that:
|
…ionVelocity to CHANGES.md
Updated CHANGES.md |
Specs/Core/TransformsSpec.js
Outdated
Cartesian3.UNIT_Y | ||
); | ||
var expected = new Matrix3(0, 0, 1, 1, 0, 0, 0, 1, 0); | ||
// using Matrix3.abs to fix the 0/-0 comparisons |
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.
You should be able to replace this with toEqualEpsilon
throughout. For example:
expect(matrix).toEqualEpsilon(expected, CesiumMath.EPSILON14);
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.
Awesome that works. Makes me feel a lot better
Thanks @easternmotors, just the one comment to address the abs usage and hopefully this will be good to go. Thanks. |
Thanks! |
Also added tests for Transforms.rotationMatrixFromPositionVelocity
NOTE: I wasn't able to figure out how to get the tests to pass since
Transforms.rotationMatrixFromPositionVelocity
sometimes return 0 or -0 for it's 0 values. I ended up just using theMatrix3.abs
function for now but if there is a better way to handle this let me know.Handles bullet 3 from #8927