Clamp bezier handle length to half the length of animation #93930
Conversation
Arnklit
force-pushed
the
short-animation-length-bezier-handle-fix
branch
from
ae04ce9 to
bbed82d
Compare
akien-mga
added
the
cherrypick:4.2
|
@mihe pointed out to me that there was another code path that this didn't cover. Setting to draft while I fix that and refactor the code. |
Arnklit
force-pushed
the
short-animation-length-bezier-handle-fix
branch
from
bbed82d to
2bd8154
Compare
editor/animation_track_editor.cpp
Outdated
| array[2] = 0; | ||
| array[3] = 0.25; | ||
| array[4] = 0; | ||
| array = animation->make_default_bezier_key(p_id.value); |
There was a problem hiding this comment.
Skip the intermediate array here and just assign straight to value.
editor/animation_track_editor.cpp
Outdated
| arr[3] = 0.25; | ||
| arr[4] = 0; | ||
|
|
||
| arr = animation->make_default_bezier_key(value); |
There was a problem hiding this comment.
Skip the intermediate arr here and just assign straight to id.value.
scene/resources/animation.cpp
Outdated
| @@ -3189,6 +3189,20 @@ StringName Animation::method_track_get_name(int p_track, int p_key_idx) const { | |||
| return pm->methods[p_key_idx].method; | |||
| } | |||
|
|
|||
| Array Animation::make_default_bezier_key(float p_value) { | |||
| const float max_width = length / 2.0; | |||
There was a problem hiding this comment.
Not that it really matters, but you might want to make this a const double instead, seeing as how length is double, as well as the 0.25 constants below.
Arnklit
force-pushed
the
short-animation-length-bezier-handle-fix
branch
from
2bd8154 to
7c6f32d
Compare
There was a problem hiding this comment.
The best approach is to share the calculation with the handle setting method in bezier_track_set_key_handle_mode(), which calculates a value that depends on the key interval. The default value is not defined for HANDLE_SET_MODE_AUTO when keying a newly created key, but in that case, I think the HANDLE_MODE_BALANCED or HANDLE_MODE_MIRRORED calculation should be applied. HANDLE_MODE_MIRRORED should be a concept closer to the current default in that it adopts a length that is x0.25 the interval-dependent length.
Happy to add that in. I had thought it would make more sense as a editor setting where you could change the default behavior. But if it can work for everything by default that is easier. |
|
Thanks! |
Fixes #93807 by clamping the new handle length to half the length of the animation.