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

Reduce cubic interpolated animation hitch #54811

Conversation

robfram
Copy link
Contributor

@robfram robfram commented Nov 9, 2021

Reduce cubic interpolated animation hitch

Cubic interpolation didn't wrap around first and last keys, clamping
pre and post keys to first and last before interpolating values.

Fixes #20087

@robfram
Copy link
Contributor Author

robfram commented Nov 9, 2021

This should replace godotengine:3.x PR #54730.

@YeldhamDev YeldhamDev added this to the 4.0 milestone Nov 9, 2021
@YeldhamDev YeldhamDev requested a review from a team November 9, 2021 19:19
@YeldhamDev YeldhamDev added the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Nov 9, 2021
@fire
Copy link
Member

fire commented Nov 9, 2021

Reviewing with @TokageItLab. Need also a look by @reduz

@TokageItLab
Copy link
Member

TokageItLab commented Nov 9, 2021

This fix is may valid in 3.x, but in 4.x it is deprecated to control the translate property with the value track. In 4.x, if the same problem is occurring with any property other than the 3D transform property, I think this should be accepted.

However, if the problem occurs only for a specific type of property, the fundamental problem is probably somewhere else apart from this fix, and we should fix it there, not here.

@robfram
Copy link
Contributor Author

robfram commented Nov 10, 2021

I PR to 4.x following instructions received in my 3.x PR. I assumed that 4.x would behave different due to the heavy reimplementation of some systems, but was pointed to send it as a 4.x instead of my original 3.x PR.

As per your question, the original issue occures both in 2D and 3D, and the test scene submitted show a 2D Sprite and a 3D Cube suffering the hitch on screen. I don't know if maybe more problems somewhere apart this, but I think it is clearly a bug in the code I changed, as the cubic interpolation implementation ignores the wrapping nature of key index when loop wrap is enabled.

@TokageItLab
Copy link
Member

I'll test the non-Vector values later. 4.x needs to be implemented differently from 3.x. Since the ping-pong loop is implemented, we should need that if statement.

@TokageItLab
Copy link
Member

In 4.0, there is a lack of adaptation to ping-pong, and as mentioned in #57952, we are planning to adopt makima-interpolate, so we need to salvage this at that time.

@akien-mga
Copy link
Member

Superseded by #58344.

@akien-mga akien-mga closed this Feb 22, 2022
@akien-mga akien-mga added archived and removed cherrypick:3.x Considered for cherry-picking into a future 3.x release labels Feb 22, 2022
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.

Cubic interpolated animations have a hitch at the start and/or end
5 participants