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

Generalized slerp() and powf() for Rotation<T,N> #1100

Open
wants to merge 14 commits into
base: dev
Choose a base branch
from

Conversation

jsmith628
Copy link
Contributor

Implements Rotation::slerp() and Rotation::powf() for all static dimensions.

Solves #908. Potentially interacts with #657, #1093, and #1094

The algorithm I used for powf() works by first computing the Real Schur form of the backing matrix to get it in a block-diagonal form, and then interpolating each 2x2 block as if it were a 2D rotation. The slerp() implementation then interpolates using (R2/R1)^t * R1. If the dimension is 2 or 3 though, it falls back on the old implementation using complex numbers and Quaternions.

My first consideration was to instead compute powf() by first taking the logarithm, multiplying by the exponent, and then taking the exponential. The issue with that was that in order to compute the log, we'd most likely have to either take a sequence of square roots using newton's method (which can get quite unstable when starting near a 180degree rotation) or do a Schur based algorithm. So since we'd be computing the form anyway, I opted to just do both the exponential and logarithm steps in one go with a Schur decomposition.

In order to do all of this, I obviously had to extend the API a bit, but for the most part, it should just be drop in place for most users. The implementation does add a fair number of extra trait bounds for the dimension so it could use the Schur decomposition, but in the 2D and 3D cases, those should just resolve down and not affect any code using the old versions. However, I did have to force T to be a RealField, which is an added restriction from the old 2D case, which previously only required SimdRealField. Unless it's decided the generalized pow and slerp should be separate from the old implementations, I don't really know how else to unify them without this problem happening.

Lastly, from my experience working with Geometric algebra, I do know of an additional algorithm that works in the special case of 4D and 5D. I haven't implemented it here yet, but if there's enough desire for it, I could implement it and run some benchmarks to see if it should be added as a specialization for sake of optimization.

/// assert_eq!(q.euler_angles(), (std::f32::consts::FRAC_PI_2, 0.0, 0.0));
/// ```
///
//FIXME: merging slerp for Rotation2 and Rotation3 into this raises the trait bounds
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes this a breaking change, right? Would it be acceptable to instead leave the existing slerp functions in place and call this something like generalized_slerp instead? That would mean it wouldn't be an issue for this to have different bounds.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is correct. This was basically me leaving the final API choice to later; it's not great to change that without input from everyone else.

Obviously, it'd be nice to just have one slerp() function that gets specialized for all of them, but if changing the bounds is going to send some people into generic hell, we can also just accept it and change the names.

I don't know how often people try to use Complex with a SIMD scalar though? I can't imagine it's really that often, but I suppose the real issue is that this will propagate to all of the dependent trait bounds.

src/geometry/rotation_interpolation.rs Outdated Show resolved Hide resolved
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

Successfully merging this pull request may close these issues.

None yet

3 participants