From 0a4b5d93c5f5a1634323cd263d0693f4de80b78c Mon Sep 17 00:00:00 2001 From: Christoph Lendenfeld Date: Thu, 15 Aug 2024 13:35:28 +0200 Subject: [PATCH] Fix #126125: Pose Slider not working with layered actions The issues was that the `get_item_transform_flags` iterated over the `curves` list of the action, ignoring the new structure. Fixed by using the `action_foreach_fcurve` and modifying that to work on legacy actions as well. Pull Request: https://projects.blender.org/blender/blender/pulls/126357 --- .../blender/animrig/ANIM_action_iterators.hh | 3 +- .../animrig/intern/action_iterators.cc | 30 ++++++---- source/blender/editors/armature/pose_utils.cc | 57 ++++++++++--------- 3 files changed, 49 insertions(+), 41 deletions(-) diff --git a/source/blender/animrig/ANIM_action_iterators.hh b/source/blender/animrig/ANIM_action_iterators.hh index 2a3424f4af3..8d0ea59db2c 100644 --- a/source/blender/animrig/ANIM_action_iterators.hh +++ b/source/blender/animrig/ANIM_action_iterators.hh @@ -29,7 +29,8 @@ using slot_handle_t = decltype(::ActionSlot::handle); /** * Iterates over all FCurves of the given slot handle in the Action and executes the callback on - * it. Only works on layered Actions. + * it. Works on layered and legacy actions. When the action is legacy, the slot handle will be + * ignored. * * \note Use lambdas to have access to specific data in the callback. */ diff --git a/source/blender/animrig/intern/action_iterators.cc b/source/blender/animrig/intern/action_iterators.cc index f78004bc152..98e2c2f39c6 100644 --- a/source/blender/animrig/intern/action_iterators.cc +++ b/source/blender/animrig/intern/action_iterators.cc @@ -16,20 +16,26 @@ void action_foreach_fcurve(Action &action, slot_handle_t handle, FunctionRef callback) { - BLI_assert(action.is_action_layered()); - for (Layer *layer : action.layers()) { - for (Strip *strip : layer->strips()) { - if (!strip->is()) { - continue; - } - KeyframeStrip &key_strip = strip->as(); - for (ChannelBag *bag : key_strip.channelbags()) { - if (bag->slot_handle != handle) { + if (action.is_action_legacy()) { + LISTBASE_FOREACH (FCurve *, fcurve, &action.curves) { + callback(*fcurve); + } + } + else if (action.is_action_layered()) { + for (Layer *layer : action.layers()) { + for (Strip *strip : layer->strips()) { + if (!strip->is()) { continue; } - for (FCurve *fcu : bag->fcurves()) { - BLI_assert(fcu != nullptr); - callback(*fcu); + KeyframeStrip &key_strip = strip->as(); + for (ChannelBag *bag : key_strip.channelbags()) { + if (bag->slot_handle != handle) { + continue; + } + for (FCurve *fcu : bag->fcurves()) { + BLI_assert(fcu != nullptr); + callback(*fcu); + } } } } diff --git a/source/blender/editors/armature/pose_utils.cc b/source/blender/editors/armature/pose_utils.cc index 75f0cba2ec1..d9fc53cc591 100644 --- a/source/blender/editors/armature/pose_utils.cc +++ b/source/blender/editors/armature/pose_utils.cc @@ -36,6 +36,8 @@ #include "ED_armature.hh" #include "ED_keyframing.hh" +#include "ANIM_action.hh" +#include "ANIM_action_iterators.hh" #include "ANIM_keyframing.hh" #include "ANIM_keyingsets.hh" @@ -69,11 +71,15 @@ typedef enum eAction_TransformFlags { ACT_TRANS_ALL = (ACT_TRANS_ONLY | ACT_TRANS_PROP), } eAction_TransformFlags; -static eAction_TransformFlags get_item_transform_flags(bAction &act, - Object &ob, +static eAction_TransformFlags get_item_transform_flags(Object &ob, bPoseChannel &pchan, ListBase &curves) { + if (!ob.adt || !ob.adt->action) { + return eAction_TransformFlags(0); + } + blender::animrig::Action &action = ob.adt->action->wrap(); + short flags = 0; /* Build PointerRNA from provided data to obtain the paths to use. */ @@ -88,24 +94,24 @@ static eAction_TransformFlags get_item_transform_flags(bAction &act, /* Search F-Curves for the given properties * - we cannot use the groups, since they may not be grouped in that way... */ - LISTBASE_FOREACH (FCurve *, fcu, &act.curves) { + blender::animrig::action_foreach_fcurve(action, ob.adt->slot_handle, [&](FCurve &fcurve) { const char *bPtr = nullptr, *pPtr = nullptr; /* If enough flags have been found, * we can stop checking unless we're also getting the curves. */ if ((flags == ACT_TRANS_ALL)) { - break; + return; } - if (fcu->rna_path == nullptr) { - continue; + if (fcurve.rna_path == nullptr) { + return; } /* Step 1: check for matching base path */ - bPtr = strstr(fcu->rna_path, basePath->c_str()); + bPtr = strstr(fcurve.rna_path, basePath->c_str()); if (!bPtr) { - continue; + return; } /* We must add len(basePath) bytes to the match so that we are at the end of the @@ -125,8 +131,8 @@ static eAction_TransformFlags get_item_transform_flags(bAction &act, if (pPtr) { flags |= ACT_TRANS_LOC; - BLI_addtail(&curves, BLI_genericNodeN(fcu)); - continue; + BLI_addtail(&curves, BLI_genericNodeN(&fcurve)); + return; } } @@ -135,8 +141,8 @@ static eAction_TransformFlags get_item_transform_flags(bAction &act, if (pPtr) { flags |= ACT_TRANS_SCALE; - BLI_addtail(&curves, BLI_genericNodeN(fcu)); - continue; + BLI_addtail(&curves, BLI_genericNodeN(&fcurve)); + return; } } @@ -145,8 +151,8 @@ static eAction_TransformFlags get_item_transform_flags(bAction &act, if (pPtr) { flags |= ACT_TRANS_ROT; - BLI_addtail(&curves, BLI_genericNodeN(fcu)); - continue; + BLI_addtail(&curves, BLI_genericNodeN(&fcurve)); + return; } } @@ -155,8 +161,8 @@ static eAction_TransformFlags get_item_transform_flags(bAction &act, if (pPtr) { flags |= ACT_TRANS_BBONE; - BLI_addtail(&curves, BLI_genericNodeN(fcu)); - continue; + BLI_addtail(&curves, BLI_genericNodeN(&fcurve)); + return; } } @@ -166,24 +172,21 @@ static eAction_TransformFlags get_item_transform_flags(bAction &act, if (pPtr) { flags |= ACT_TRANS_PROP; - BLI_addtail(&curves, BLI_genericNodeN(fcu)); - continue; + BLI_addtail(&curves, BLI_genericNodeN(&fcurve)); + return; } } - } + }); /* return flags found */ return eAction_TransformFlags(flags); } /* helper for poseAnim_mapping_get() -> get the relevant F-Curves per PoseChannel */ -static void fcurves_to_pchan_links_get(ListBase &pfLinks, - Object &ob, - bAction &act, - bPoseChannel &pchan) +static void fcurves_to_pchan_links_get(ListBase &pfLinks, Object &ob, bPoseChannel &pchan) { ListBase curves = {nullptr, nullptr}; - const eAction_TransformFlags transFlags = get_item_transform_flags(act, ob, pchan, curves); + const eAction_TransformFlags transFlags = get_item_transform_flags(ob, pchan, curves); pchan.flag &= ~(POSE_LOC | POSE_ROT | POSE_SIZE | POSE_BBONE_SHAPE); @@ -278,8 +281,7 @@ void poseAnim_mapping_get(bContext *C, ListBase *pfLinks) continue; } - fcurves_to_pchan_links_get( - *pfLinks, *ob_pose_armature, *ob_pose_armature->adt->action, *pchan); + fcurves_to_pchan_links_get(*pfLinks, *ob_pose_armature, *pchan); } CTX_DATA_END; @@ -304,8 +306,7 @@ void poseAnim_mapping_get(bContext *C, ListBase *pfLinks) continue; } - fcurves_to_pchan_links_get( - *pfLinks, *ob_pose_armature, *ob_pose_armature->adt->action, *pchan); + fcurves_to_pchan_links_get(*pfLinks, *ob_pose_armature, *pchan); } CTX_DATA_END; }