blender
/
blender
136
404
Fork 586
Code Issues 6.2k Pull Requests 762 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 +21 -15
2 changed files with 21 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 0c3f7500be - Show all commits

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

@ -500,7 +500,7 @@ void blend_to_infinity_fcurve_segment(FCurve *fcu, FCurveSegment *segment, const
const BezTriple *right_key = fcurve_segment_end_get(fcu, segment->start_index + segment->length);
const float right_x = right_key->vec[1][0];
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.
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. */
const BezTriple *beyond_left_key = fcurve_segment_start_get(fcu, segment->start_index - 1);
@ -508,7 +508,8 @@ void blend_to_infinity_fcurve_segment(FCurve *fcu, FCurveSegment *segment, const
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(fcu, segment->start_index + segment->length + 1);
const BezTriple *beyond_right_key = fcurve_segment_end_get(
fcu, segment->start_index + segment->length + 1);
Christoph Lendenfeld commented 2023-04-15 19:21:33 +02:00
Outdated
Review
Copy Link

this can also be clearer.
segment->start_index + segment->length > fcu->totvert

this can also be clearer. `segment->start_index + segment->length > fcu->totvert`
AresDeveaux commented 2023-04-18 03:48:32 +02:00
Outdated
Review
Copy Link

I tried this change as well as the next one and it doesn't work as it should. Remember, we need 2 keys before and after the segment, the way you suggest just account for one. If I don't prevent not having 2 keys at each side of the segment the tool goes crazy.

I tried this change as well as the next one and it doesn't work as it should. Remember, we need 2 keys before and after the segment, the way you suggest just account for one. If I don't prevent not having 2 keys at each side of the segment the tool goes crazy.
const float beyond_right_x = beyond_right_key->vec[1][0];
const float beyond_right_y = beyond_right_key->vec[1][1];
@ -525,26 +526,27 @@ void blend_to_infinity_fcurve_segment(FCurve *fcu, FCurveSegment *segment, const
const bool slider_right_side = factor >= 0.5;
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.
const bool slider_left_side = factor < 0.5;
/* The factor goes from 0 to 1, but for this tool it needs to go from 0 to 1 on each side of the slider. */
/* The factor goes from 0 to 1, but for this tool it needs to go from 0 to 1 on each side of the
* slider. */
const float ping_pong_factor = fabs(factor * 2 - 1);
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
float x_delta = 0;
float y_delta = 0;
/* This delta values are used to know the relationship between the bookend keys and the
/* This delta values are used to know the relationship between the bookend keys and the
* reference keys beyong those. */
if(slider_right_side){
if (slider_right_side) {
/* Stop the fucntion if there is no key beyond the the right neighboring one. */
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 = ...`
if(segment->start_index + segment->length == fcu->totvert) {
return;
if (segment->start_index + segment->length == fcu->totvert) {
return;
}
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`
y_delta = beyond_right_y - right_y;
x_delta = beyond_right_x - right_x;
}
else if(slider_left_side){
else if (slider_left_side) {
/* Stop the fucntion if there is no key beyond the left neighboring one. */
if(segment->start_index == 0) {
return;
if (segment->start_index == 0) {
return;
}
y_delta = beyond_left_y - left_y;
x_delta = beyond_left_x - left_x;
@ -555,12 +557,13 @@ void blend_to_infinity_fcurve_segment(FCurve *fcu, FCurveSegment *segment, const
}
for (int i = segment->start_index; i < segment->start_index + segment->length; i++) {
float new_x_delta = 0;
float new_y_delta = 0;
float refe = 0;
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
/* This new deltas are used to determin the relationship between the current key and the bookend ones. */
/* This new deltas are used to determin the relationship between the current key and the
Christoph Lendenfeld commented 2023-04-09 20:04:08 +02:00
Outdated
Review
Copy Link

These new deltas are used to determine...

These new deltas are used to determine...
* bookend ones. */
if (slider_right_side) {
new_x_delta = fcu->bezt[i].vec[1][0] - right_x;
refe = right_y;
@ -570,8 +573,9 @@ void blend_to_infinity_fcurve_segment(FCurve *fcu, FCurveSegment *segment, const
refe = left_key->vec[1][1];
}
/* we use compound rule of 3 to find the "Y" delta we are missing using the other deltas we know. */
if(x_delta != 0){
/* we use compound rule of 3 to find the "Y" delta we are missing using the other deltas we
Christoph Lendenfeld commented 2023-04-09 20:04:37 +02:00
Outdated
Review
Copy Link

We ...
comments start with capital letters.

We ... comments start with capital letters.
* know. */
if (x_delta != 0) {
new_y_delta = new_x_delta * y_delta / x_delta;
}

4
source/blender/editors/include/ED_keyframes_edit.h
View File

@ -437,7 +437,9 @@ void smooth_fcurve_segment(struct FCurve *fcu,
int kernel_size,
double *kernel);
void ease_fcurve_segment(struct FCurve *fcu, struct FCurveSegment *segment, float factor);
void blend_to_infinity_fcurve_segment(struct FCurve *fcu, struct FCurveSegment *segment, float factor);
void blend_to_infinity_fcurve_segment(struct FCurve *fcu,
struct FCurveSegment *segment,
float factor);
bool decimate_fcurve(struct bAnimListElem *ale, float remove_ratio, float error_sq_max);
void blend_to_default_fcurve(struct PointerRNA *id_ptr, struct FCurve *fcu, float factor);
/**