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

Discontinue exp map in blending #60308

Merged

Conversation

TokageItLab
Copy link
Member

@TokageItLab TokageItLab commented Apr 16, 2022

Fixed #60306. Discontinue the use of exp map in blending.

The exp map can easily perform multiple quaternion blends. However, it really comes into its own when you need to compute two or more quaternions at the same time, such as when diverting the cubic interpolate formula in an issue like #57951, which is what we are trying to solve now.

In the blending process, quaternions are always complemented one by one by the iterator, so there is not much point in using exp map. The calculation cost is much lower, but the error is larger when there are large rotations, as in #60306. See #58043 and https://swkagami.hatenablog.com/entry/lie_03algebra for a description of the error of the exp map.

Actually, in fact I replaced slerp and confirmed before that it does more than 3 blends correctly.

Here is the video after the fix:

2022-04-17.4.13.41-1.mov

In the opening of the video above you can see that changing the connection order for blending more than 3 Nodes is not a problem, prior to 4.0.alpha4 these results were different and the calculations were not done correctly in the first place.

Earlier, reduz said there was a problem with slerp from an empty quaternion as in #34134 (comment), it was that a 180 degree rotation from the init quaternion from always could not be done. In fact, we can use any base quaternion by adding init_rot to the calculation, so we don't have to worry about that problem.

@fire
Copy link
Member

fire commented Apr 16, 2022

The test cases look ok, but I'm not able to review as much in depth.

@akien-mga akien-mga merged commit f7ca732 into godotengine:master Apr 16, 2022
@akien-mga
Copy link
Member

Thanks!

@TokageItLab TokageItLab deleted the remove-exp-map-in-blending branch May 23, 2022 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AnimationTree unexpected blend node results - drift on x axis
4 participants