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 getRotationTo returning nan quaternion for zero vectors, return I… #1040

Closed
wants to merge 0 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@Altren
Copy link
Contributor

Altren commented Jan 10, 2019

…DENTITY instead

@paroj

This comment has been minimized.

Copy link
Member

paroj commented Jan 10, 2019

I would prefer the faster check of b != 2*a

@Altren

This comment has been minimized.

Copy link
Contributor

Altren commented Jan 10, 2019

When both a == 0 and b == 0 (at least one zero vector) your check would fail

@Altren

This comment has been minimized.

Copy link
Contributor

Altren commented Jan 10, 2019

Both branches could produce zero axis there

@paroj

This comment has been minimized.

Copy link
Member

paroj commented Jan 10, 2019

I dont like computing the axis just to throw it away afterwards when we could detect the degenerate case ahead.
We are squeezing for 5% elsewhere #1036, and this is potentially hot code.

@paroj

This comment has been minimized.

Copy link
Member

paroj commented Jan 11, 2019

if (Math::RealEqual(b, 2 * a) || a == 0)
    return Quaternion::IDENTITY;

before the axis computation should do.

Also please add a test here to validate the new bahavior.

@Altren

This comment has been minimized.

Copy link
Contributor

Altren commented Jan 11, 2019

Git it, will check this version and add unit tests for the edge cases

@Altren Altren closed this Jan 12, 2019

@Altren Altren force-pushed the Altren:master branch from 069a864 to 536680e Jan 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment