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

Fix optimizer miscalculation and add optimization for Animation::ValueTrack #64951

Merged
merged 1 commit into from
Aug 27, 2022

Conversation

TokageItLab
Copy link
Member

@TokageItLab TokageItLab commented Aug 27, 2022

  • Fixed wrong calculation for ratio (I had overlooked that the positive/negative inversion reverses the if judgment)
  • Use double internally to improve accuracy
  • Allowed some types of value tracks to be optimized
    • FLOAT
    • VECTOR2
    • VECTOR3
    • QUATERNION

There are some concerns:

image

In AnimationEditor, the Edit button can access to optimization from previously, but it is not possible to filter by track type. Also, there is no undo function. This is because the optimization is implemented directly in the Animation class. It might be better to implement an AnimationOptimizer class and use static methods to perform optimizations.

@TokageItLab TokageItLab added this to the 4.0 milestone Aug 27, 2022
@TokageItLab TokageItLab force-pushed the value-track-optimizer branch 2 times, most recently from 9db4fdd to ee54797 Compare August 27, 2022 09:21
if (!signbit(v0 * v1)) {
v0 = abs(v0);
v1 = abs(v1);
double ratio = v0 < v1 ? v0 / v1 : v1 / v0;
Copy link
Member Author

@TokageItLab TokageItLab Aug 27, 2022

Choose a reason for hiding this comment

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

In the before method, it does not get the absolute value here, and the positive and negative values are reversed, causing the ratio to be incorrect.

@TokageItLab TokageItLab force-pushed the value-track-optimizer branch 2 times, most recently from f5f7a67 to cb76daf Compare August 27, 2022 10:23
@TokageItLab
Copy link
Member Author

TokageItLab commented Aug 27, 2022

A little change to the UI around the AnimationTrackEditor optimizer (added a warning comment).

image

Also, after optimize/clean-up, it clear not only Animation but also AnimationTrackEditor's history.

@TokageItLab TokageItLab force-pushed the value-track-optimizer branch from cb76daf to f5c49cd Compare August 27, 2022 10:28
@TokageItLab TokageItLab force-pushed the value-track-optimizer branch from f5c49cd to 787ab45 Compare August 27, 2022 13:59
@akien-mga akien-mga merged commit 6ba932e into godotengine:master Aug 27, 2022
@akien-mga
Copy link
Member

Thanks!

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.

4 participants