blender
/
blender
136
404
Fork 586
Code Issues 6.2k Pull Requests 758 Projects 19 Wiki Activity

Animation: blend to infinity slider #106517

Closed
AresDeveaux wants to merge 15 commits from AresDeveaux/blender:blend_to_infinity_slider into main
pull from: AresDeveaux/blender:blend_to_infinity_slider
merge into: blender:main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
Conversation 12 Commits 15 Files Changed 6 +13 -34
1 changed files with 13 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit c3f8c3a35d - Show all commits

47
source/blender/editors/animation/keyframes_general.c
View File

@ -494,34 +494,13 @@ void ease_fcurve_segment(FCurve *fcu, FCurveSegment *segment, const float factor
void blend_to_infinity_fcurve_segment(FCurve *fcu, FCurveSegment *segment, const float factor)
{
const BezTriple *left_key = fcurve_segment_start_get(fcu, segment->start_index);
const float left_x = left_key->vec[1][0];
const float left_y = left_key->vec[1][1];
const BezTriple *right_key = fcurve_segment_end_get(fcu, segment->start_index + segment->length);
const float right_x = right_key->vec[1][0];
const float right_y = right_key->vec[1][1];
/* One key on the outside left side of the neighboring keys is needed to use as reference. */
/* One key on the outside of the neighboring keys is needed to use as reference. */
const BezTriple *beyond_left_key = fcurve_segment_start_get(fcu, segment->start_index - 1);
const float beyond_left_x = beyond_left_key->vec[1][0];
const float beyond_left_y = beyond_left_key->vec[1][1];
/* One key on the outside right side of the neighboring keys is needed to use as reference. */
const BezTriple *beyond_right_key = fcurve_segment_end_get(
Christoph Lendenfeld commented 2023-04-09 20:00:38 +02:00
Outdated
Review
Copy Link

I think instead of splitting the vector into its two components it would be better to keep it as a vector and use the functions in math_vector_inline.c

This will potentially simplify your code quite a lot.

I think instead of splitting the vector into its two components it would be better to keep it as a vector and use the functions in `math_vector_inline.c` This will potentially simplify your code quite a lot.
fcu, segment->start_index + segment->length + 1);
const float beyond_right_x = beyond_right_key->vec[1][0];
const float beyond_right_y = beyond_right_key->vec[1][1];
const float key_x_range = right_x - left_x;
/* Happens if there is only 1 key on the FCurve. Needs to be skipped because it
* would be a divide by 0. */
if (IS_EQF(key_x_range, 0.0f)) {
return;
}
/* The calculation needs diferent values for each side of the slider. */
const bool slider_right_side = factor >= 0.5;
const bool slider_left_side = factor < 0.5;
@ -540,16 +519,16 @@ void blend_to_infinity_fcurve_segment(FCurve *fcu, FCurveSegment *segment, const
if (segment->start_index + segment->length == fcu->totvert) {
return;
}
y_delta = beyond_right_y - right_y;
x_delta = beyond_right_x - right_x;
y_delta = beyond_right_key->vec[1][1] - right_key->vec[1][1];
x_delta = beyond_right_key->vec[1][0] - right_key->vec[1][0];
Christoph Lendenfeld commented 2023-04-15 19:18:45 +02:00
Outdated
Review
Copy Link

This would be clearer if you checked explicitly to 0
segment->start_index == 0

the start index will never be smaller than 0

This would be clearer if you checked explicitly to 0 `segment->start_index == 0` the start index will never be smaller than 0
Christoph Lendenfeld commented 2023-04-20 11:07:51 +02:00
Outdated
Review
Copy Link

I am glad you didn't listen to me here, because I just realized what I suggested is wrong 🤦

I am glad you didn't listen to me here, because I just realized what I suggested is wrong 🤦
}
else if (slider_left_side) {
/* Stop the fucntion if there is no key beyond the left neighboring one. */
Christoph Lendenfeld commented 2023-04-21 10:28:34 +02:00
Outdated
Review
Copy Link

shouldn't this return false?
also now that I think about it, I am not sure x_delta can ever be 0. It would only be 0 if the beyond key and the reference key are the same, which they cannot be since you are getting different indices.
Hm ok writing this now, you can have the curve in a state where two keys are at the same point in time. Shouldn't happen, but could...

Anyway, the function should return true if it ran and false if it didn't. Since you are exiting early here, I think it should return false.

shouldn't this return false? also now that I think about it, I am not sure x_delta can ever be 0. It would only be 0 if the beyond key and the reference key are the same, which they cannot be since you are getting different indices. Hm ok writing this now, you can have the curve in a state where two keys are at the same point in time. Shouldn't happen, but could... Anyway, the function should return true if it ran and false if it didn't. Since you are exiting early here, I think it should return false.
if (segment->start_index == 0) {
return;
}
y_delta = beyond_left_y - left_y;
x_delta = beyond_left_x - left_x;
y_delta = beyond_left_key->vec[1][1] - left_key->vec[1][1];
x_delta = beyond_left_key->vec[1][0] - left_key->vec[1][0];
Christoph Lendenfeld commented 2023-04-15 19:16:05 +02:00
Outdated
Review
Copy Link

since this is now outside of the if else, you can to
const float y_delta = ...
and then remove the lines 503 and 504

since this is now outside of the if else, you can to `const float y_delta = ...` and then remove the lines 503 and 504
}
else {
y_delta = 1;
@ -560,26 +539,26 @@ void blend_to_infinity_fcurve_segment(FCurve *fcu, FCurveSegment *segment, const
Christoph Lendenfeld commented 2023-04-15 19:25:23 +02:00
Outdated
Review
Copy Link

this can move into the for loop and you can then call it
const float new_x_delta = ...

this can move into the for loop and you can then call it `const float new_x_delta = ...`
float new_x_delta = 0;
float new_y_delta = 0;
float refe = 0;
float reference_y = 0;
Christoph Lendenfeld commented 2023-04-15 19:24:53 +02:00
Outdated
Review
Copy Link

this code can be moved to the first if else

this code can be moved to the first `if else`
/* This new deltas are used to determin the relationship between the current key and the
/* These new deltas are used to determine the relationship between the current key and the
* bookend ones. */
if (slider_right_side) {
new_x_delta = fcu->bezt[i].vec[1][0] - right_x;
refe = right_y;
new_x_delta = fcu->bezt[i].vec[1][0] - right_key->vec[1][0];
reference_y = right_key->vec[1][1];
}
else {
new_x_delta = fcu->bezt[i].vec[1][0] - left_x;
refe = left_key->vec[1][1];
new_x_delta = fcu->bezt[i].vec[1][0] - left_key->vec[1][0];
reference_y = left_key->vec[1][1];
}
/* we use compound rule of 3 to find the "Y" delta we are missing using the other deltas we
/* We use compound rule of 3 to find the "Y" delta we are missing using the other deltas we
* know. */
if (x_delta != 0) {
new_y_delta = new_x_delta * y_delta / x_delta;
}
const float delta = refe + new_y_delta - fcu->bezt[i].vec[1][1];
const float delta = reference_y + new_y_delta - fcu->bezt[i].vec[1][1];
const float key_y_value = fcu->bezt[i].vec[1][1] + delta * ping_pong_factor;
Christoph Lendenfeld commented 2023-04-09 20:06:37 +02:00
Outdated
Review
Copy Link

generally it is better to not shorten variables. I assume this means reference
I think it would be even better to be more concrete and call it reference_y

Also I think this, including the if(slider_right_side), can be moved outside of the for loop.
It seems to not change within the for loop

generally it is better to not shorten variables. I assume this means `reference` I think it would be even better to be more concrete and call it `reference_y` Also I think this, including the `if(slider_right_side)`, can be moved outside of the for loop. It seems to not change within the for loop
move_key(&fcu->bezt[i], key_y_value);