Refactor process of animation to retrive keys more exactly #69336
+394
−275
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
396f945 to
cafb211
Compare
TokageItLab
commented
Nov 29, 2022
| @@ -75,9 +75,10 @@ | |||
| <param index="3" name="seeked" type="bool" /> | |||
| <param index="4" name="is_external_seeking" type="bool" /> | |||
| <param index="5" name="blend" type="float" /> | |||
| <param index="6" name="pingponged" type="int" default="0" /> | |||
| <param index="6" name="looped_flag" type="int" enum="Animation.LoopedFlag" default="0" /> | |||
There was a problem hiding this comment.
I renamed it because this is available in linear loop as well as pingpong. In addition, the enum is now properly defined.
Comment on lines
+2771
to
+2778
| if (!p_is_backward) { | ||
| for (int i = from; i <= to; i++) { | ||
| p_indices->push_back(i); | ||
| } | ||
| } else { | ||
| for (int i = to; i >= to; i--) { | ||
| p_indices->push_back(i); | ||
| } |
There was a problem hiding this comment.
I reworked _track_get_key_indices_in_range(). Also, for reverse playback, the order in which keys are stored should be reversed.
| @@ -2879,207 +2964,209 @@ void Animation::track_get_key_indices_in_range(int p_track, double p_time, doubl | |||
| to_time = Math::pingpong(to_time, length); | |||
| } | |||
|
|
|||
| if ((int)Math::floor(abs(p_delta) / length) % 2 == 0) { | |||
There was a problem hiding this comment.
As noted in the overview, this check for extreme corner case should not be a concern.
Comment on lines
+2836
to
+2842
| if (!is_backward) { | ||
| _track_get_key_indices_in_range(tt->positions, from_time, anim_end, p_indices, is_backward); | ||
| _track_get_key_indices_in_range(tt->positions, anim_start, to_time, p_indices, is_backward); | ||
| } else { | ||
| _track_get_key_indices_in_range(tt->positions, anim_start, to_time, p_indices, is_backward); | ||
| _track_get_key_indices_in_range(tt->positions, from_time, anim_end, p_indices, is_backward); | ||
| } |
There was a problem hiding this comment.
Just like what it do in _track_get_key_indices_in_range(), we need to care about the order in which the keys are stored outside of it.
cafb211 to
e8a6221
Compare
ca54c34 to
cd957d7
Compare
cd957d7 to
7f1f9f2
Compare
7f1f9f2 to
1fc3833
Compare
reduz
approved these changes
Dec 2, 2022
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Follow up #68992.
Fixes #61853 (completely!)
Fixes #69415
Fundamentally change the process of retrieving keys in animations.
Here is the current key retrieving process:
Due to reason 2, there was a problem as #64644. This has been fixed by #68992 for now.
However, there is a problem with getting duplicate keys by 1. So it conflicts with the elimination of the seek and playback delays that was the problem in #52518.
This fix changes it so that it does not always get the value of the frame where it is currently located. Whenever an animation is played from a stopped state, there is always some method. So it will be no problem by getting the key of the first frame there.
This also eliminates the need to hack to get the key of the end frame. Compared to before, it should now be possible to retrieve keys in a much more stable.
Includes removal of some unnecessary code and clarify of arguments. For example, check
(int)Math::floor(abs(p_delta) / length) % 2 == 0is to correctly calculate the pingpong if the delta length is longer than the animation length. However, this is an extreme corner case and affects performance, so it has been removed.