Math: Fix sclerp of dual quaternions with equal rotation and add test. #122
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Squareys
force-pushed
the
dual-quaternion-sclerp-fix
branch
from
2f5bca8
to
2301ee8
Compare
When rotation is identical, the rotation of the first dual quaternion is returned instead, together with the linearly interpolated translation of both (lerp of the vectors of the dual part). The additional include is needed for `Math::lerp(Vector<3, T>, Vector<3, T>, T)`. Signed-off-by: Squareys <Squareys@googlemail.com>
Squareys
force-pushed
the
dual-quaternion-sclerp-fix
branch
from
2301ee8
to
dfc7fe9
Compare
|
Awesome, thanks for being so thorough. I wanted to avoid having math headers dependent on |
|
@mosra A clean way would be to allow this kind of quaternion tranlation lerp (or whatever you want to call it) as a function in Quaternion.h :) If you do that on master, I can adapt this branch for you. |
|
Did a different (uglier?) alternative. Thanks a lot for the fix! |
mosra
added a commit
that referenced
this pull request
Sep 3, 2018
The fix done with #122 (0e05c72) was not tested properly (see previous commit) and thus this code path never worked. This properly lerps the translation part and recombines it with the rotation instead of interpolating just a part of it. Also I'm no longer having any "dotResult" that's done only on the vector part of the rotation, but instead using the full rotation quaternion dot product. I have no idea why it was done this way. This branch was also never properly tested -- it'll be with the introduction of "shortest path" variants in the next commit.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi @mosra !
For previous discussion, see #117 :) This pull request should finally close that issue.
Greetings,
Squareys