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
6 changed files with 340 additions and 0 deletions

View File

@ -330,6 +330,7 @@ class GRAPH_MT_slider(Menu):
layout.operator("graph.blend_to_neighbor", text="Blend to Neighbor")
layout.operator("graph.blend_to_default", text="Blend to Default Value")
layout.operator("graph.ease", text="Ease")
layout.operator("graph.gaussian_smooth", text="Smooth")
class GRAPH_MT_view_pie(Menu):

View File

@ -392,7 +392,56 @@ void blend_to_default_fcurve(PointerRNA *id_ptr, FCurve *fcu, const float factor
move_key(&fcu->bezt[i], key_y_value);
}
}
/* ---------------- */
void ED_ANIM_get_1d_gauss_kernel(const float sigma, const int kernel_size, double *r_kernel)

For non-static functions, I think it might be better to move the ED_anim_... prefixes.

For non-static functions, I think it might be better to move the `ED_anim_...` prefixes.
{
dr.sybren marked this conversation as resolved

You could add

BLI_assert(sigma > 0.0f);
BLI_assert(kernel_size > 0);

to document the assumptions of the code. If sigma == 0 it'll cause some nice division issues.

You could add ```c BLI_assert(sigma > 0.0f); BLI_assert(kernel_size > 0); ``` to document the assumptions of the code. If `sigma == 0` it'll cause some nice division issues.
BLI_assert(sigma > 0.0f);

I think you can remove norm from the calculations completely. Since you normalise the end result anyway, this constant factor doesn't have an impact on the final result.

I think you can remove `norm` from the calculations completely. Since you normalise the end result anyway, this constant factor doesn't have an impact on the final result.
BLI_assert(kernel_size > 0);

const

`const`
const double sigma_sq = 2.0 * sigma * sigma;
double sum = 0.0;
for (int i = 0; i < kernel_size; i++) {
const double normalized_index = (double)i / (kernel_size - 1);
r_kernel[i] = exp(-normalized_index * normalized_index / sigma_sq);
if (i == 0) {
sum += r_kernel[i];
}
else {
/* We only calculate half the kernel,
* the normalization needs to take that into account. */
sum += r_kernel[i] * 2;
}
}
/* Normalize kernel values. */
for (int i = 0; i < kernel_size; i++) {
dr.sybren marked this conversation as resolved
r_kernel[i] /= sum;
```c r_kernel[i] /= sum; ```
r_kernel[i] /= sum;
}
}
void smooth_fcurve_segment(FCurve *fcu,
FCurveSegment *segment,
float *samples,
const float factor,
const int kernel_size,
double *kernel)
{
dr.sybren marked this conversation as resolved

segment->start_index + segment->length and fcu->bezt[segment->start_index].vec[1][0] don't change during the loop, so you can store them in a constant outside of the loop and use that instead.

`segment->start_index + segment->length` and `fcu->bezt[segment->start_index].vec[1][0]` don't change during the loop, so you can store them in a constant outside of the loop and use that instead.
const int segment_end_index = segment->start_index + segment->length;
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;
/* Apply the kernel. */
double filter_result = samples[sample_index] * kernel[0];

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++) {

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;
filter_result += samples[sample_index - j] * kernel_value;
}
const float key_y_value = interpf((float)filter_result, samples[sample_index], factor);
move_key(&fcu->bezt[i], key_y_value);
}
}
/* ---------------- */
void ease_fcurve_segment(FCurve *fcu, FCurveSegment *segment, const float factor)
@ -680,6 +729,16 @@ typedef struct TempFrameValCache {
float frame, val;
} TempFrameValCache;
void sample_fcurve_segment(FCurve *fcu,
const float start_frame,
float *samples,
const int sample_count)
{
for (int i = 0; i < sample_count; i++) {
samples[i] = evaluate_fcurve(fcu, start_frame + i);
}
}
void sample_fcurve(FCurve *fcu)
{
BezTriple *bezt, *start = NULL, *end = NULL;

View File

@ -425,6 +425,17 @@ void blend_to_neighbor_fcurve_segment(struct FCurve *fcu,
struct FCurveSegment *segment,
float factor);
void breakdown_fcurve_segment(struct FCurve *fcu, struct FCurveSegment *segment, float factor);
/** Get a 1D gauss kernel. Since the kernel is symmetrical, only calculates the positive side.
* \param sigma The shape of the gauss distribution.
* \param kernel_size How long the kernel array is.
*/
void ED_ANIM_get_1d_gauss_kernel(const float sigma, int kernel_size, double *r_kernel);
void smooth_fcurve_segment(struct FCurve *fcu,
struct FCurveSegment *segment,
float *samples,
float factor,
int kernel_size,
double *kernel);
void ease_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);
@ -433,6 +444,10 @@ void blend_to_default_fcurve(struct PointerRNA *id_ptr, struct FCurve *fcu, floa
*/
void smooth_fcurve(struct FCurve *fcu);
void sample_fcurve(struct FCurve *fcu);
void sample_fcurve_segment(struct FCurve *fcu,
float start_frame,
float *r_samples,
int sample_count);
/* ----------- */

View File

@ -116,6 +116,7 @@ void GRAPH_OT_breakdown(struct wmOperatorType *ot);
void GRAPH_OT_ease(struct wmOperatorType *ot);
void GRAPH_OT_decimate(struct wmOperatorType *ot);
void GRAPH_OT_blend_to_default(struct wmOperatorType *ot);
void GRAPH_OT_gaussian_smooth(struct wmOperatorType *ot);
void GRAPH_OT_sample(struct wmOperatorType *ot);
void GRAPH_OT_bake(struct wmOperatorType *ot);
void GRAPH_OT_unbake(struct wmOperatorType *ot);

View File

@ -464,6 +464,7 @@ void graphedit_operatortypes(void)
WM_operatortype_append(GRAPH_OT_breakdown);
WM_operatortype_append(GRAPH_OT_ease);
WM_operatortype_append(GRAPH_OT_blend_to_default);
WM_operatortype_append(GRAPH_OT_gaussian_smooth);
WM_operatortype_append(GRAPH_OT_euler_filter);
WM_operatortype_append(GRAPH_OT_delete);
WM_operatortype_append(GRAPH_OT_duplicate);

View File

@ -69,6 +69,10 @@ typedef struct tGraphSliderOp {
/* Each operator has a specific update function. */
void (*modal_update)(struct bContext *, struct wmOperator *);
/* If an operator stores custom data, it also needs to provide the function to clean it up. */
void *operator_data;
void (*free_operator_data)(void *operator_data);
NumInput num;
} tGraphSliderOp;
@ -191,6 +195,10 @@ static void graph_slider_exit(bContext *C, wmOperator *op)
return;
}
if (gso->free_operator_data != NULL) {
gso->free_operator_data(gso->operator_data);

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`.
}
ScrArea *area = gso->area;
LinkData *link;
@ -1053,3 +1061,258 @@ void GRAPH_OT_ease(wmOperatorType *ot)
}
/** \} */
/* -------------------------------------------------------------------- */
/** \name Gauss Smooth Operator
* \{ */
/* It is necessary to store data for smoothing when running in modal, because the sampling of
* FCurves shouldn't be done on every update. */
typedef struct tGaussOperatorData {
double *kernel;
ListBase segment_links; /* tFCurveSegmentLink */
ListBase anim_data; /* bAnimListElem */
} tGaussOperatorData;
/* Store data to smooth an FCurve segment. */

Document what this struct contains / what it's for.

Document what this struct contains / what it's for.
typedef struct tFCurveSegmentLink {
struct tFCurveSegmentLink *prev, *next;
FCurve *fcu;
FCurveSegment *segment;
float *samples; /* Array of y-values of the FCurve segment. */
} tFCurveSegmentLink;
static void gaussian_smooth_allocate_operator_data(tGraphSliderOp *gso,
const int filter_width,
const float sigma)
{
tGaussOperatorData *operator_data = MEM_callocN(sizeof(tGaussOperatorData),
"tGaussOperatorData");
const int kernel_size = filter_width + 1;
double *kernel = MEM_callocN(sizeof(double) * kernel_size, "Gauss Kernel");
ED_ANIM_get_1d_gauss_kernel(sigma, kernel_size, kernel);
operator_data->kernel = kernel;
ListBase anim_data = {NULL, NULL};
ANIM_animdata_filter(&gso->ac, &anim_data, OPERATOR_DATA_FILTER, gso->ac.data, gso->ac.datatype);
ListBase segment_links = {NULL, NULL};
LISTBASE_FOREACH (bAnimListElem *, ale, &anim_data) {
FCurve *fcu = (FCurve *)ale->key_data;
ListBase fcu_segments = find_fcurve_segments(fcu);
LISTBASE_FOREACH (FCurveSegment *, segment, &fcu_segments) {
tFCurveSegmentLink *segment_link = MEM_callocN(sizeof(tFCurveSegmentLink),
"FCurve Segment Link");
segment_link->fcu = fcu;
segment_link->segment = segment;
BezTriple left_bezt = fcu->bezt[segment->start_index];
BezTriple right_bezt = fcu->bezt[segment->start_index + segment->length - 1];
const int sample_count = (int)(right_bezt.vec[1][0] - left_bezt.vec[1][0]) +
(filter_width * 2 + 1);
float *samples = MEM_callocN(sizeof(float) * sample_count, "Smooth FCurve Op Samples");
sample_fcurve_segment(fcu, left_bezt.vec[1][0] - filter_width, samples, sample_count);
segment_link->samples = samples;
BLI_addtail(&segment_links, segment_link);
}
}
operator_data->anim_data = anim_data;
operator_data->segment_links = segment_links;
gso->operator_data = operator_data;
}
static void gaussian_smooth_free_operator_data(void *operator_data)
{
tGaussOperatorData *gauss_data = (tGaussOperatorData *)operator_data;
LISTBASE_FOREACH (tFCurveSegmentLink *, segment_link, &gauss_data->segment_links) {
MEM_freeN(segment_link->samples);
MEM_freeN(segment_link->segment);
}
MEM_freeN(gauss_data->kernel);
BLI_freelistN(&gauss_data->segment_links);
ANIM_animdata_freelist(&gauss_data->anim_data);
MEM_freeN(gauss_data);
}
static void gaussian_smooth_draw_status_header(bContext *C, tGraphSliderOp *gso)
{
char status_str[UI_MAX_DRAW_STR];
char mode_str[32];
char slider_string[UI_MAX_DRAW_STR];
ED_slider_status_string_get(gso->slider, slider_string, UI_MAX_DRAW_STR);
strcpy(mode_str, TIP_("Gauss Smooth"));

Change this to:

const char *mode_str = TIP_("Gaussian Smooth");

It's using the old name now. Copying to a small fixed buffer like that can very easily overflow, but you don't need to copy at all.

Change this to: ``` const char *mode_str = TIP_("Gaussian Smooth"); ``` It's using the old name now. Copying to a small fixed buffer like that can very easily overflow, but you don't need to copy at all.
if (hasNumInput(&gso->num)) {
char str_ofs[NUM_STR_REP_LEN];
outputNumInput(&gso->num, str_ofs, &gso->scene->unit);
BLI_snprintf(status_str, sizeof(status_str), "%s: %s", mode_str, str_ofs);
}
else {
BLI_snprintf(status_str, sizeof(status_str), "%s: %s", mode_str, slider_string);
}
ED_workspace_status_text(C, status_str);
}
static void gaussian_smooth_modal_update(bContext *C, wmOperator *op)
{
tGraphSliderOp *gso = op->customdata;
bAnimContext ac;
if (ANIM_animdata_get_context(C, &ac) == 0) {
return;
}
gaussian_smooth_draw_status_header(C, gso);
const float factor = slider_factor_get_and_remember(op);
tGaussOperatorData *operator_data = (tGaussOperatorData *)gso->operator_data;
const int filter_width = RNA_int_get(op->ptr, "filter_width");
LISTBASE_FOREACH (tFCurveSegmentLink *, segment, &operator_data->segment_links) {
smooth_fcurve_segment(segment->fcu,
segment->segment,
segment->samples,
factor,
filter_width,
operator_data->kernel);
}
LISTBASE_FOREACH (bAnimListElem *, ale, &operator_data->anim_data) {
ale->update |= ANIM_UPDATE_DEFAULT;
}
ANIM_animdata_update(&ac, &operator_data->anim_data);
WM_event_add_notifier(C, NC_ANIMATION | ND_KEYFRAME | NA_EDITED, NULL);
}
static int gaussian_smooth_invoke(bContext *C, wmOperator *op, const wmEvent *event)
{
const int invoke_result = graph_slider_invoke(C, op, event);
if (invoke_result == OPERATOR_CANCELLED) {
return invoke_result;
}
tGraphSliderOp *gso = op->customdata;
gso->modal_update = gaussian_smooth_modal_update;
gso->factor_prop = RNA_struct_find_property(op->ptr, "factor");
const float sigma = RNA_float_get(op->ptr, "sigma");
const int filter_width = RNA_int_get(op->ptr, "filter_width");
gaussian_smooth_allocate_operator_data(gso, filter_width, sigma);
gso->free_operator_data = gaussian_smooth_free_operator_data;
ED_slider_allow_overshoot_set(gso->slider, false);
ED_slider_factor_set(gso->slider, 0.0f);
gaussian_smooth_draw_status_header(C, gso);
return invoke_result;
}
static void gaussian_smooth_graph_keys(bAnimContext *ac,
const float factor,
double *kernel,
const int filter_width)
{
ListBase anim_data = {NULL, NULL};
ANIM_animdata_filter(ac, &anim_data, OPERATOR_DATA_FILTER, ac->data, ac->datatype);
LISTBASE_FOREACH (bAnimListElem *, ale, &anim_data) {
FCurve *fcu = (FCurve *)ale->key_data;
ListBase segments = find_fcurve_segments(fcu);
LISTBASE_FOREACH (FCurveSegment *, segment, &segments) {
BezTriple left_bezt = fcu->bezt[segment->start_index];
BezTriple right_bezt = fcu->bezt[segment->start_index + segment->length - 1];
const int sample_count = (int)(right_bezt.vec[1][0] - left_bezt.vec[1][0]) +
(filter_width * 2 + 1);
float *samples = MEM_callocN(sizeof(float) * sample_count, "Smooth FCurve Op Samples");
sample_fcurve_segment(fcu, left_bezt.vec[1][0] - filter_width, samples, sample_count);
smooth_fcurve_segment(fcu, segment, samples, factor, filter_width, kernel);
MEM_freeN(samples);
}
BLI_freelistN(&segments);
ale->update |= ANIM_UPDATE_DEFAULT;
}
ANIM_animdata_update(ac, &anim_data);
ANIM_animdata_freelist(&anim_data);
}
static int gaussian_smooth_exec(bContext *C, wmOperator *op)
{
bAnimContext ac;
if (ANIM_animdata_get_context(C, &ac) == 0) {
return OPERATOR_CANCELLED;
}
const float factor = RNA_float_get(op->ptr, "factor");
const int filter_width = RNA_int_get(op->ptr, "filter_width");
const int kernel_size = filter_width + 1;
double *kernel = MEM_callocN(sizeof(double) * kernel_size, "Gauss Kernel");
ED_ANIM_get_1d_gauss_kernel(RNA_float_get(op->ptr, "sigma"), kernel_size, kernel);
gaussian_smooth_graph_keys(&ac, factor, kernel, filter_width);
MEM_freeN(kernel);
/* Set notifier that keyframes have changed. */
WM_event_add_notifier(C, NC_ANIMATION | ND_KEYFRAME | NA_EDITED, NULL);
return OPERATOR_FINISHED;
}
void GRAPH_OT_gaussian_smooth(wmOperatorType *ot)
{
/* Identifiers. */
ot->name = "Gaussian Smooth";
ot->idname = "GRAPH_OT_gaussian_smooth";
ot->description = "Smooth the curve using a Gauss filter";
/* API callbacks. */
ot->invoke = gaussian_smooth_invoke;
ot->modal = graph_slider_modal;
ot->exec = gaussian_smooth_exec;
ot->poll = graphop_editable_keyframes_poll;
/* Flags. */
ot->flag = OPTYPE_REGISTER | OPTYPE_UNDO;
RNA_def_float_factor(ot->srna,
"factor",
1.0f,
0.0f,
FLT_MAX,
"Factor",
"How much to blend to the default value",
0.0f,
1.0f);
RNA_def_float(ot->srna,
"sigma",
0.33f,
0.001f,
FLT_MAX,
"Sigma",
"The shape of the gauss distribution, lower values make it sharper",
0.001f,
100.0f);
RNA_def_int(ot->srna,
"filter_width",
6,
1,
64,
"Filter Width",
"How far to each side the operator will average the key values",
1,
32);
}
/** \} */