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

Using acos in some methods leads to NaNs if the two vecs are near identical #47

Open
Imberflur opened this issue Mar 27, 2020 · 2 comments

Comments

@Imberflur
Copy link
Contributor

The two affected methods:

pub fn slerp_unclamped(from: Self, to: Self, factor: T) -> Self
    where T: Sum + Real + Clamp + Lerp<T,Output=T>
{
    // From GLM, gtx/rotate_vector.inl
    let (mag_from, mag_to) = (from.magnitude(), to.magnitude());
    let (from, to) = (from/mag_from, to/mag_to);
    let cos_alpha = from.dot(to);
    let alpha = cos_alpha.acos();
    let sin_alpha = alpha.sin();
    let t1 = ((T::one() - factor) * alpha).sin() / sin_alpha;
    let t2 = (factor * alpha).sin() / sin_alpha;
    (from * t1 + to * t2) * Lerp::lerp_unclamped(mag_from, mag_to, factor)
}
pub fn angle_between(self, v: Self) -> T where T: Sum + Real {
    self.normalized().dot(v.normalized()).acos()
}

Due to floating point error the magnitude of normalized vecs can be slightly over 1.0 so when the two input vecs are near identical it is possible for the dot product to be over 1 (or under -1 if they are opposite).
Unfortunately, acos returns NaN outside the range [-1, 1]

Is it intended for the user to handle this case or should it be handled in these methods?

Another concern with slerp_unclamped is that it doesn't handle the case where the two vectors are not linearly independent.

@yoanlcq
Copy link
Owner

yoanlcq commented Mar 29, 2020

Hi, thanks for reporting this.

I think we want these methods to clamp the dot product to the [-1; 1] range. We probably don't mind the extra overhead if it prevents broken results.

Another concern with slerp_unclamped is that it doesn't handle the case where the two vectors are not linearly independent.

IIRC I just copied GLM's implementation which is widely used. I do not think they handle what you described properly.

Robustness remains desirable, however, but I do not know what would be the best way to handle the "not linearly independant" case.

yoanlcq added a commit that referenced this issue Mar 29, 2020
@Imberflur
Copy link
Contributor Author

For slerping I mentioned the issue with two vectors pointing in the same or opposite directions because I realized while writing that this coincides with the case where the dot product needs to be clamped. If they are pointing in almost the same direction I think returning to would be the best solution. However, if they are in the opposite direction then we have to choose from an infinite selection of planes to rotate through.

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

2 participants