Animation: blend offset slider #106518

Closed
AresDeveaux wants to merge 12 commits from AresDeveaux/blender:blend_offset_slider into main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
6 changed files with 154 additions and 0 deletions

View File

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

View File

@ -479,6 +479,30 @@ void ease_fcurve_segment(FCurve *fcu, FCurveSegment *segment, const float factor
/* ---------------- */
void blend_offset_fcurve_segment(FCurve *fcu, FCurveSegment *segment, const float factor)
{
const BezTriple *left_key = fcurve_segment_start_get(fcu, segment->start_index);
const BezTriple *right_key = fcurve_segment_end_get(fcu, segment->start_index + segment->length);
const BezTriple segment_first_key = fcu->bezt[segment->start_index];
const BezTriple segment_last_key = fcu->bezt[segment->start_index + segment->length - 1];

left_key and segment_first_key are only used when factor <= 0, and right_key and segment_last_key otherwise. Move these into the corresponding if/else bodies, so that their scope is clear.

`left_key` and `segment_first_key` are only used when `factor <= 0`, and `right_key` and `segment_last_key` otherwise. Move these into the corresponding `if`/`else` bodies, so that their scope is clear.
float y_delta;
if (factor > 0) {
y_delta = right_key->vec[1][1] - segment_last_key.vec[1][1];
}
else {
y_delta = left_key->vec[1][1] - segment_first_key.vec[1][1];
}
for (int i = segment->start_index; i < segment->start_index + segment->length; i++) {
const float key_y_value = fcu->bezt[i].vec[1][1] + y_delta * fabs(factor);

this can be simplified to fcu->bezt[segment->start_index]
same for getting the segment end

this can be simplified to `fcu->bezt[segment->start_index]` same for getting the segment end

Since ``y_delta * fabs(factor)` doesnt' change over the course of this loop, take it out of the loop:

const float weighted_delta = y_delta * fabs(factor);
Since ``y_delta * fabs(factor)` doesnt' change over the course of this loop, take it out of the loop: ```c const float weighted_delta = y_delta * fabs(factor); ```
BKE_fcurve_keyframe_move_value_with_handles(&fcu->bezt[i], key_y_value);
}
}
/* ---------------- */
void breakdown_fcurve_segment(FCurve *fcu, FCurveSegment *segment, const float factor)
{

watch out for typos poritive -> positive

watch out for typos poritive -> positive
const BezTriple *left_bezt = fcurve_segment_start_get(fcu, segment->start_index);

View File

@ -439,6 +439,7 @@ 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_offset_fcurve_segment(struct FCurve *fcu, struct FCurveSegment *segment, float factor);

The more functions we add here, the more important it becomes that they are documented. Could you write a doxygen-style comment for it, that explains what it does? Something like this:

/**
 * Shift the FCurve segment up/down so that it aligns with the key before/after
 * the segment.
 *
 * \param factor blend factor from -1.0 to 1.0. The sign determines whether the
 * segment is aligned with the key before or after the segment.
 */
The more functions we add here, the more important it becomes that they are documented. Could you write a doxygen-style comment for it, that explains what it does? Something like this: ```c /** * Shift the FCurve segment up/down so that it aligns with the key before/after * the segment. * * \param factor blend factor from -1.0 to 1.0. The sign determines whether the * segment is aligned with the key before or after the segment. */ ```
bool decimate_fcurve(struct bAnimListElem *ale, float remove_ratio, float error_sq_max);
/**

View File

@ -114,6 +114,7 @@ void GRAPH_OT_clean(struct wmOperatorType *ot);
void GRAPH_OT_blend_to_neighbor(struct wmOperatorType *ot);
void GRAPH_OT_breakdown(struct wmOperatorType *ot);
void GRAPH_OT_ease(struct wmOperatorType *ot);
void GRAPH_OT_blend_offset(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);

View File

@ -463,6 +463,7 @@ void graphedit_operatortypes(void)
WM_operatortype_append(GRAPH_OT_blend_to_neighbor);
WM_operatortype_append(GRAPH_OT_breakdown);
WM_operatortype_append(GRAPH_OT_ease);
WM_operatortype_append(GRAPH_OT_blend_offset);
WM_operatortype_append(GRAPH_OT_blend_to_default);
WM_operatortype_append(GRAPH_OT_gaussian_smooth);
WM_operatortype_append(GRAPH_OT_euler_filter);

View File

@ -967,6 +967,132 @@ void GRAPH_OT_ease(wmOperatorType *ot)
1.0f);
}
/* -------------------------------------------------------------------- */
/** \name Blend Offset Operator
* \{ */
static void blend_offset_graph_keys(bAnimContext *ac, const float factor)

I suspect that more operators will have a structure that could be described as "call a function with a factor for each selected segment".

Instead of duplicating this code and changing which function it calls, I think it would be better to abstract that away a bit. Something like this:


typedef void (*SegmentOperationFunc)(FCurve *fcu, FCurveSegment *segment, float factor);

static void fcurve_for_each_segment(bAnimContext *ac,
                                    SegmentOperationFunc operation,
                                    const float factor)
{
  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) {
      operation(fcu, segment, factor);
    }

    ale->update |= ANIM_UPDATE_DEFAULT;
    BLI_freelistN(&segments);
  }

  ANIM_animdata_update(ac, &anim_data);
  ANIM_animdata_freelist(&anim_data);
}

static void blend_offset_graph_keys(bAnimContext *ac, const float factor)
{
  fcurve_for_each_segment(ac, blend_offset_fcurve_segment, factor);
}

// other operators can then have a function like:
static void other_function_name(bAnimContext *ac, const float factor)
{
  fcurve_for_each_segment(ac, other_function_name_segment, factor);
}
I suspect that more operators will have a structure that could be described as "call a function with a factor for each selected segment". Instead of duplicating this code and changing which function it calls, I think it would be better to abstract that away a bit. Something like this: ```c typedef void (*SegmentOperationFunc)(FCurve *fcu, FCurveSegment *segment, float factor); static void fcurve_for_each_segment(bAnimContext *ac, SegmentOperationFunc operation, const float factor) { 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) { operation(fcu, segment, factor); } ale->update |= ANIM_UPDATE_DEFAULT; BLI_freelistN(&segments); } ANIM_animdata_update(ac, &anim_data); ANIM_animdata_freelist(&anim_data); } static void blend_offset_graph_keys(bAnimContext *ac, const float factor) { fcurve_for_each_segment(ac, blend_offset_fcurve_segment, factor); } // other operators can then have a function like: static void other_function_name(bAnimContext *ac, const float factor) { fcurve_for_each_segment(ac, other_function_name_segment, factor); } ```

this exists from a recent refactor: apply_fcu_segment_function

this exists from a recent refactor: `apply_fcu_segment_function`
{
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) {
blend_offset_fcurve_segment(fcu, segment, factor);
}
ale->update |= ANIM_UPDATE_DEFAULT;
BLI_freelistN(&segments);
}
ANIM_animdata_update(ac, &anim_data);
ANIM_animdata_freelist(&anim_data);
}
static void blend_offset_draw_status_header(bContext *C, tGraphSliderOp *gso)

I think this can be rewritten to call common_draw_status_header().

I think this can be rewritten to call `common_draw_status_header()`.
{
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_("Blend Offset Keys"));
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 blend_offset_modal_update(bContext *C, wmOperator *op)
{
tGraphSliderOp *gso = op->customdata;
blend_offset_draw_status_header(C, gso);
/* Reset keyframes to the state at invoke. */
reset_bezts(gso);
const float factor = slider_factor_get_and_remember(op);
blend_offset_graph_keys(&gso->ac, factor);
WM_event_add_notifier(C, NC_ANIMATION | ND_KEYFRAME | NA_EDITED, NULL);
}
static int blend_offset_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 = blend_offset_modal_update;
gso->factor_prop = RNA_struct_find_property(op->ptr, "factor");
blend_offset_draw_status_header(C, gso);
ED_slider_factor_bounds_set(gso->slider, -1, 1);
ED_slider_factor_set(gso->slider, 0.0f);
return invoke_result;
}
static int blend_offset_exec(bContext *C, wmOperator *op)
{
bAnimContext ac;
/* Get editor data. */
if (ANIM_animdata_get_context(C, &ac) == 0) {
return OPERATOR_CANCELLED;
}
const float factor = RNA_float_get(op->ptr, "factor");
blend_offset_graph_keys(&ac, factor);
/* Set notifier that keyframes have changed. */
WM_event_add_notifier(C, NC_ANIMATION | ND_KEYFRAME | NA_EDITED, NULL);
return OPERATOR_FINISHED;
}
void GRAPH_OT_blend_offset(wmOperatorType *ot)
{
/* Identifiers. */
ot->name = "Blend Offset Keyframes";
ot->idname = "GRAPH_OT_blend_offset";
ot->description = "Shift selected keys to the value of the neighboring keys as a block";
/* API callbacks. */
ot->invoke = blend_offset_invoke;
ot->modal = graph_slider_modal;
ot->exec = blend_offset_exec;
ot->poll = graphop_editable_keyframes_poll;
/* Flags. */
ot->flag = OPTYPE_REGISTER | OPTYPE_UNDO;
RNA_def_float_factor(ot->srna,
"factor",
0.0f,
-FLT_MAX,
FLT_MAX,
"Curve Bend",
"Control the bend of the curve",
-1.0f,
1.0f);
}
/** \} */
/* -------------------------------------------------------------------- */
/** \name Gauss Smooth Operator