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

KeyframeTrack: Fix typo in optimize(). #21071

Merged
merged 1 commit into from
Jan 13, 2021

Conversation

donmccurdy
Copy link
Collaborator

Fixes what appears to be a typo in track.optimize(). I haven't run into a reproducible issue but accessing time[0] where time is a number does not seem correct.

Aside: The optimize() function currently only eliminates duplicate keyframes. Have we considered changing the implementation to remove keyframes that can be linearly interpolated from the prev/next keyframes? I think that could remove a lot more from animations that were 'baked' in software like Blender.

@donmccurdy donmccurdy changed the title Fix typo in KeyframeTrack.optimize() KeyframeTrack: Fix typo optimize() Jan 12, 2021
@Mugen87 Mugen87 changed the title KeyframeTrack: Fix typo optimize() KeyframeTrack: Fix typo in optimize(). Jan 12, 2021
@mrdoob mrdoob added this to the r125 milestone Jan 13, 2021
@mrdoob mrdoob merged commit cf04fca into master Jan 13, 2021
@mrdoob
Copy link
Owner

mrdoob commented Jan 13, 2021

Thanks!

@mrdoob
Copy link
Owner

mrdoob commented Jan 13, 2021

Aside: The optimize() function currently only eliminates duplicate keyframes. Have we considered changing the implementation to remove keyframes that can be linearly interpolated from the prev/next keyframes? I think that could remove a lot more from animations that were 'baked' in software like Blender.

Not sure anyone is looking at that code much...

@mrdoob mrdoob deleted the donmccurdy-keyframetrack-optimize-typo branch January 13, 2021 12:47
@mrdoob
Copy link
Owner

mrdoob commented Jan 13, 2021

Ops, this targeted master. Will merge to dev.

@donmccurdy
Copy link
Collaborator Author

Ah sorry about that! Used the github UI and forgot to change the branch dropdown.

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