blender
/
blender
135
403
Fork 586
Code Issues 6.2k Pull Requests 756 Projects 19 Wiki Activity

Animation: Gaussian Smooth operator for Graph Editor #105635

Merged
Christoph Lendenfeld merged 15 commits from ChrisLend/blender:graph_gauss_smooth into main 2023-03-24 12:11:33 +01:00
Conversation 8 Commits 15 Files Changed 6 +2 -2
2 changed files with 2 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 58756c8936 - Show all commits

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

@ -431,8 +431,8 @@ void smooth_fcurve_segment(FCurve *fcu,
const int segment_start_x = fcu->bezt[segment->start_index].vec[1][0];
for (int i = segment->start_index; i < segment_end_index; i++) {
const int sample_index = (int)(fcu->bezt[i].vec[1][0] - segment_start_x) + kernel_size;
double filter_result = samples[sample_index] * kernel[0];
/* Apply the kernel. */
double filter_result = samples[sample_index] * kernel[0];
Sybren A. Stüvel commented 2023-03-24 10:36:18 +01:00
Outdated
Review
Copy Link

This comment should swap with the line above it, as double filter_result = samples[sample_index] * kernel[0]; is already part of applying the kernel.

This comment should swap with the line above it, as `double filter_result = samples[sample_index] * kernel[0];` is already part of applying the kernel.
for (int j = 1; j <= kernel_size; j++) {
Sybren A. Stüvel commented 2023-03-21 17:15:43 +01:00
Outdated
Review
Copy Link

What do you think would be faster? The current approach? Or halving the loop and avoiding the call to abs(j)?

    double filter_result = samples[sample_index] * kernel[0];
    for (int j = 1; j <= kernel_size; j++) {
      const double kernel_value = kernel[j];
      filter_result += samples[sample_index + j] * kernel_value;
      filter_result += samples[sample_index - j] * kernel_value;
    }
What do you think would be faster? The current approach? Or halving the loop and avoiding the call to `abs(j)`? ```c double filter_result = samples[sample_index] * kernel[0]; for (int j = 1; j <= kernel_size; j++) { const double kernel_value = kernel[j]; filter_result += samples[sample_index + j] * kernel_value; filter_result += samples[sample_index - j] * kernel_value; } ```
const double kernel_value = kernel[j];
filter_result += samples[sample_index + j] * kernel_value;

2
source/blender/editors/space_graph/graph_slider_ops.c
View File

@ -195,7 +195,7 @@ static void graph_slider_exit(bContext *C, wmOperator *op)
return;
}
if (gso->operator_data) {
if (gso->free_operator_data != NULL) {
gso->free_operator_data(gso->operator_data);
Sybren A. Stüvel commented 2023-03-24 10:39:51 +01:00
Review
Copy Link

I think it's better to do a NULL check on gso->free_operator_data itself. Calling gso->free_operator_data(...) is guaranteed to crash if that pointer is NULL, whereas gso->free_operator_data(NULL) might be fine, depending on the implementation of that function.

It also simplifies the API, in that, regardless of any other field, the function is simply called when it's not-NULL.

I think it's better to do a `NULL` check on `gso->free_operator_data` itself. Calling `gso->free_operator_data(...)` is guaranteed to crash if that pointer is `NULL`, whereas `gso->free_operator_data(NULL)` might be fine, depending on the implementation of that function. It also simplifies the API, in that, regardless of any other field, the function is simply called when it's not-`NULL`.
}