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

Disable animation track optimizer by default #63211

Conversation

TokageItLab
Copy link
Member

@TokageItLab TokageItLab commented Jul 19, 2022

The current animation track optimizer works well for very simple animations, but does a bad work for most humanoid 3D model animations. In fact, a lot of the same issues have been posted.

It is not good to have the default settings that make the animation corrupt. Compression should be set according to the user's needs, and They should probably be disabled by default.

Fixed #53864
Fixed #58968
Fixed #59150

@Calinou
Copy link
Member

Calinou commented Jul 19, 2022

Fixed #49974

The issue was reported in 3.x, not 4.0. Therefore, that issue is only resolved if this is cherry-picked for 3.6.

Copy link
Member

@fire fire left a comment

Choose a reason for hiding this comment

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

We have multiple options to optimize space usage of animation.

  1. Uncompressed
  2. animation track optimizer corrupts the animation
  3. bitpacking compression cannot be modified
  4. reducing the frame rate corrupts the animation

Choosing uncompressed animations means out of the box animation keyframe takes storage.

Perhaps we can develop other options like force bitpack on export similar to tscn export scn.

I do prefer correct animations that doesn't require quality checks. So either uncompressed or bitpacked at export can work.

@reduz any advice on this?

@TokageItLab
Copy link
Member Author

TokageItLab commented Jul 20, 2022

I changed some of the importer process in #63138 to adapt retargeting to bitpacking. So it's fine to make bitpacking the default if #63138 is approved.

@reduz
Copy link
Member

reduz commented Jul 26, 2022

Some concerns.

  • I think what the optimizer does is fine, at much it needs to be tweaked to be less aggressive or the algorithm improved.
  • Bitpacking is more expensive CPU wise, should not be used by default.
  • If you use cubic interpolation, the optimizer will not work properly, so it must be disabled by default. Cubic is only really good if all keys are imported.

So to me the path forward here would be to simply improve the compression code. The last time I took a look at it, I did not find the algorithm to be fantastic, so it can definitely be improved.

@reduz
Copy link
Member

reduz commented Jul 27, 2022

To explain a bit better, I understand this can be over optimize complex animations leading to a reduction of quality. I think this is definitely unwanted.

But on the other hand, removing repeating unchanged or mostly unchanged keys is also very important. Animations are full of tracks with keys where nothing changes, so the optimizer has to run by default.

I suggest we discuss how to improve it or at least change the defaults so it does not break perfectly good animations.

@TokageItLab
Copy link
Member Author

I agree, as explained in the contributors chat, the current optimizer seems to lack the "key velocity" factor.

@TokageItLab
Copy link
Member Author

I will send a proposal to improve the optimizer later.

@TokageItLab TokageItLab deleted the default-false-anim-optimizer branch September 16, 2022 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants