From d85e7f457707fe888ae90a645b2dbc6314364703 Mon Sep 17 00:00:00 2001 From: Christoph Lendenfeld Date: Fri, 6 Oct 2023 13:51:04 +0200 Subject: [PATCH] Fix #87160: Clean Keyframes only works if channels are selected The issue was that the code filtered for selected channels, while the expectation was that it would only filter for selected keys. This PR changes the behavior of the operator in the following way: * when "Clean Channels" is **disabled**, it will clean only selected keyframes, regardless of the channel selection * when "Clean Channels" is **enabled**, it will clean selected channels regardless of keyframe selection The same logic was applied to the Graph Editor code. It only makes a difference in the case when "Clean Channels" is enabled. That is because channels were automatically selected when a key was selected. In addition to that I moved the menu entry for "Clean Channels" to the channel menu to reduce confusion. Another solution would have been to make the Dope Sheet select channels when keys are selected. This might still be done in the future, but I think the only correct fix is to change the actual operator behavior. Pull Request: https://projects.blender.org/blender/blender/pulls/113335 --- scripts/startup/bl_ui/space_dopesheet.py | 2 +- source/blender/editors/animation/keyframes_general.cc | 8 ++++++-- source/blender/editors/include/ED_keyframes_edit.hh | 6 +++++- source/blender/editors/space_action/action_edit.cc | 10 ++++++++-- source/blender/editors/space_graph/graph_edit.cc | 9 +++++++-- 5 files changed, 27 insertions(+), 8 deletions(-) diff --git a/scripts/startup/bl_ui/space_dopesheet.py b/scripts/startup/bl_ui/space_dopesheet.py index c0d1463aaec..c2192b204a9 100644 --- a/scripts/startup/bl_ui/space_dopesheet.py +++ b/scripts/startup/bl_ui/space_dopesheet.py @@ -495,6 +495,7 @@ class DOPESHEET_MT_channel(Menu): layout.operator_context = 'INVOKE_REGION_CHANNELS' layout.operator("anim.channels_delete") + layout.operator("action.clean", text="Clean Channels").channels = True layout.separator() layout.operator("anim.channels_group") @@ -555,7 +556,6 @@ class DOPESHEET_MT_key(Menu): layout.separator() layout.operator("action.clean").channels = False - layout.operator("action.clean", text="Clean Channels").channels = True layout.operator("action.bake_keys") layout.separator() diff --git a/source/blender/editors/animation/keyframes_general.cc b/source/blender/editors/animation/keyframes_general.cc index fcf1dcce709..1722b9cd7be 100644 --- a/source/blender/editors/animation/keyframes_general.cc +++ b/source/blender/editors/animation/keyframes_general.cc @@ -91,7 +91,11 @@ bool duplicate_fcurve_keys(FCurve *fcu) /** \name Various Tools * \{ */ -void clean_fcurve(bAnimContext *ac, bAnimListElem *ale, float thresh, bool cleardefault) +void clean_fcurve(bAnimContext *ac, + bAnimListElem *ale, + float thresh, + bool cleardefault, + const bool only_selected_keys) { FCurve *fcu = (FCurve *)ale->key_data; BezTriple *old_bezts, *bezt, *beztn; @@ -144,7 +148,7 @@ void clean_fcurve(bAnimContext *ac, bAnimListElem *ale, float thresh, bool clear cur[0] = bezt->vec[1][0]; cur[1] = bezt->vec[1][1]; - if (!(bezt->f2 & SELECT)) { + if (only_selected_keys && !(bezt->f2 & SELECT)) { insert_bezt_fcurve(fcu, bezt, eInsertKeyFlags(0)); lastb = (fcu->bezt + (fcu->totvert - 1)); lastb->f1 = lastb->f2 = lastb->f3 = 0; diff --git a/source/blender/editors/include/ED_keyframes_edit.hh b/source/blender/editors/include/ED_keyframes_edit.hh index 090bccdda45..1affbbe1a06 100644 --- a/source/blender/editors/include/ED_keyframes_edit.hh +++ b/source/blender/editors/include/ED_keyframes_edit.hh @@ -427,7 +427,11 @@ struct FCurveSegment { * The caller is responsible for freeing the memory. */ ListBase find_fcurve_segments(FCurve *fcu); -void clean_fcurve(bAnimContext *ac, bAnimListElem *ale, float thresh, bool cleardefault); +void clean_fcurve(bAnimContext *ac, + bAnimListElem *ale, + float thresh, + bool cleardefault, + bool only_selected_keys); void blend_to_neighbor_fcurve_segment(FCurve *fcu, FCurveSegment *segment, float factor); void breakdown_fcurve_segment(FCurve *fcu, FCurveSegment *segment, float factor); void scale_average_fcurve_segment(struct FCurve *fcu, struct FCurveSegment *segment, float factor); diff --git a/source/blender/editors/space_action/action_edit.cc b/source/blender/editors/space_action/action_edit.cc index 281b86cf5bd..538b511c36f 100644 --- a/source/blender/editors/space_action/action_edit.cc +++ b/source/blender/editors/space_action/action_edit.cc @@ -1182,12 +1182,18 @@ static void clean_action_keys(bAnimContext *ac, float thresh, bool clean_chan) /* filter data */ filter = (ANIMFILTER_DATA_VISIBLE | ANIMFILTER_LIST_VISIBLE | ANIMFILTER_FOREDIT | - ANIMFILTER_SEL | ANIMFILTER_FCURVESONLY | ANIMFILTER_NODUPLIS); + ANIMFILTER_FCURVESONLY | ANIMFILTER_NODUPLIS); + + if (clean_chan) { + filter |= ANIMFILTER_SEL; + } + ANIM_animdata_filter(ac, &anim_data, filter, ac->data, eAnimCont_Types(ac->datatype)); + const bool only_selected_keys = !clean_chan; /* loop through filtered data and clean curves */ LISTBASE_FOREACH (bAnimListElem *, ale, &anim_data) { - clean_fcurve(ac, ale, thresh, clean_chan); + clean_fcurve(ac, ale, thresh, clean_chan, only_selected_keys); ale->update |= ANIM_UPDATE_DEFAULT; } diff --git a/source/blender/editors/space_graph/graph_edit.cc b/source/blender/editors/space_graph/graph_edit.cc index 3c909056334..fe0b4eef053 100644 --- a/source/blender/editors/space_graph/graph_edit.cc +++ b/source/blender/editors/space_graph/graph_edit.cc @@ -830,13 +830,18 @@ static void clean_graph_keys(bAnimContext *ac, float thresh, bool clean_chan) /* Filter data. */ filter = (ANIMFILTER_DATA_VISIBLE | ANIMFILTER_CURVE_VISIBLE | ANIMFILTER_FCURVESONLY | - ANIMFILTER_FOREDIT | ANIMFILTER_SEL | ANIMFILTER_NODUPLIS); + ANIMFILTER_FOREDIT | ANIMFILTER_NODUPLIS); + if (clean_chan) { + filter |= ANIMFILTER_SEL; + } ANIM_animdata_filter( ac, &anim_data, eAnimFilter_Flags(filter), ac->data, eAnimCont_Types(ac->datatype)); + const bool only_selected_keys = !clean_chan; /* Loop through filtered data and clean curves. */ LISTBASE_FOREACH (bAnimListElem *, ale, &anim_data) { - clean_fcurve(ac, ale, thresh, clean_chan); + + clean_fcurve(ac, ale, thresh, clean_chan, only_selected_keys); ale->update |= ANIM_UPDATE_DEFAULT; }