From 615cb4641286e08754f63208320fa8d8b091c4dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Fri, 20 Sep 2024 08:07:15 +0200 Subject: [PATCH] Anim: add Action Slot selector to Action Constraint Add slotted Actions support to Action constraints. The user interface can be improved once #127751 lands. Ref: #120406 Pull Request: https://projects.blender.org/blender/blender/pulls/127749 --- scripts/startup/bl_operators/anim.py | 52 +++++--- .../startup/bl_ui/properties_constraint.py | 12 +- .../animrig/intern/action_iterators.cc | 44 +++++++ source/blender/blenkernel/intern/anim_sys.cc | 2 +- .../blender/blenkernel/intern/constraint.cc | 30 +++-- .../editors/object/object_constraint.cc | 21 +++- .../blender/makesdna/DNA_constraint_types.h | 3 + .../blender/makesrna/intern/rna_constraint.cc | 113 ++++++++++++++++++ tests/python/bl_rigging_symmetrize.py | 15 ++- 9 files changed, 263 insertions(+), 29 deletions(-) diff --git a/scripts/startup/bl_operators/anim.py b/scripts/startup/bl_operators/anim.py index 38af51e7332..e9d742e82f9 100644 --- a/scripts/startup/bl_operators/anim.py +++ b/scripts/startup/bl_operators/anim.py @@ -731,7 +731,28 @@ class ANIM_OT_slot_unassign_from_id(Operator): return {'FINISHED'} -class ANIM_OT_slot_unassign_from_nla_strip(Operator): +class generic_slot_unassign_mixin(): + context_property_name = "" + """Which context attribute to use to get the to-be-manipulated data-block.""" + + @classmethod + def poll(cls, context): + slot_user = getattr(context, cls.context_property_name, None) + if not slot_user: + return False + + if not slot_user.action_slot: + cls.poll_message_set("No Action slot is assigned, so there is nothing to un-assign") + return False + return True + + def execute(self, context): + slot_user = getattr(context, self.context_property_name, None) + slot_user.action_slot = None + return {'FINISHED'} + + +class ANIM_OT_slot_unassign_from_nla_strip(generic_slot_unassign_mixin, Operator): """Un-assign the assigned Action Slot from an NLA strip. Note that _which_ NLA strip should get this slot unassigned must be set in @@ -744,21 +765,23 @@ class ANIM_OT_slot_unassign_from_nla_strip(Operator): bl_description = "Un-assign the action slot from this NLA strip, effectively making it non-animated" bl_options = {'REGISTER', 'UNDO'} - @classmethod - def poll(cls, context): - nla_strip = getattr(context, "nla_strip", None) - if not nla_strip: - return False + context_property_name = "nla_strip" - if not nla_strip.action or not nla_strip.action_slot: - cls.poll_message_set("This NLA strip has no Action slot assigned") - return False - return True - def execute(self, context): - nla_strip = getattr(context, "nla_strip", None) - nla_strip.action_slot = None - return {'FINISHED'} +class ANIM_OT_slot_unassign_from_constraint(generic_slot_unassign_mixin, Operator): + """Un-assign the assigned Action Slot from an Action constraint. + + Note that _which_ constraint should get this slot unassigned must be set in + the "constraint" context pointer, using: + + >>> layout.context_pointer_set("constraint", constraint) + """ + bl_idname = "anim.slot_unassign_from_constraint" + bl_label = "Unassign Slot" + bl_description = "Un-assign the action slot from this constraint" + bl_options = {'REGISTER', 'UNDO'} + + context_property_name = "constraint" classes = ( @@ -773,4 +796,5 @@ classes = ( ANIM_OT_slot_new_for_id, ANIM_OT_slot_unassign_from_id, ANIM_OT_slot_unassign_from_nla_strip, + ANIM_OT_slot_unassign_from_constraint, ) diff --git a/scripts/startup/bl_ui/properties_constraint.py b/scripts/startup/bl_ui/properties_constraint.py index 1a01714b525..582a4d1043b 100644 --- a/scripts/startup/bl_ui/properties_constraint.py +++ b/scripts/startup/bl_ui/properties_constraint.py @@ -1126,7 +1126,17 @@ class ConstraintButtonsSubPanel: layout.use_property_split = True layout.use_property_decorate = True - layout.prop(con, "action") + col = layout.column(align=True) + col.prop(con, "action") + if context.preferences.experimental.use_animation_baklava and con.action and con.action.is_action_layered: + col.context_pointer_set("animated_id", con.id_data) + col.template_search( + con, "action_slot", + con, "action_slots", + new="", # No use in making a new slot here. + unlink="anim.slot_unassign_from_constraint", + ) + layout.prop(con, "use_bone_object_action") col = layout.column(align=True) diff --git a/source/blender/animrig/intern/action_iterators.cc b/source/blender/animrig/intern/action_iterators.cc index 9bfd86246a2..3e5ed97caa2 100644 --- a/source/blender/animrig/intern/action_iterators.cc +++ b/source/blender/animrig/intern/action_iterators.cc @@ -14,6 +14,8 @@ #include "BKE_anim_data.hh" #include "BKE_nla.hh" +#include "DNA_constraint_types.h" + namespace blender::animrig { void action_foreach_fcurve(Action &action, @@ -73,6 +75,48 @@ bool foreach_action_slot_use( } } + /* The rest of the code deals with constraints, so only relevant when this is an Object. */ + if (GS(animated_id.name) != ID_OB) { + return true; + } + + const Object &object = reinterpret_cast(animated_id); + + /** + * Visit a constraint, and call the callback if it's an Action constraint. + * + * \returns whether to continue looping over possible uses of Actions, i.e. + * the return value of the callback. + */ + auto visit_constraint = [&](const bConstraint &constraint) -> bool { + if (constraint.type != CONSTRAINT_TYPE_ACTION) { + return true; + } + bActionConstraint *constraint_data = static_cast(constraint.data); + if (!constraint_data->act) { + return true; + } + return callback(constraint_data->act->wrap(), constraint_data->action_slot_handle); + }; + + /* Visit Object constraints. */ + LISTBASE_FOREACH (bConstraint *, con, &object.constraints) { + if (!visit_constraint(*con)) { + return false; + } + } + + /* Visit Pose Bone constraints. */ + if (object.type == OB_ARMATURE) { + LISTBASE_FOREACH (bPoseChannel *, pchan, &object.pose->chanbase) { + LISTBASE_FOREACH (bConstraint *, con, &pchan->constraints) { + if (!visit_constraint(*con)) { + return false; + } + } + } + } + return true; } diff --git a/source/blender/blenkernel/intern/anim_sys.cc b/source/blender/blenkernel/intern/anim_sys.cc index 3af03202261..551284f442e 100644 --- a/source/blender/blenkernel/intern/anim_sys.cc +++ b/source/blender/blenkernel/intern/anim_sys.cc @@ -854,7 +854,7 @@ void animsys_evaluate_action_group(PointerRNA *ptr, const auto visit_fcurve = [&](FCurve *fcu) { /* check if this curve should be skipped */ - if ((fcu->flag & (FCURVE_MUTED | FCURVE_DISABLED)) == 0 && !BKE_fcurve_is_empty(fcu)) { + if ((fcu->flag & FCURVE_MUTED) == 0 && !BKE_fcurve_is_empty(fcu)) { PathResolvedRNA anim_rna; if (BKE_animsys_rna_path_resolve(ptr, fcu->rna_path, fcu->array_index, &anim_rna)) { const float curval = calculate_fcurve(&anim_rna, fcu, anim_eval_context); diff --git a/source/blender/blenkernel/intern/constraint.cc b/source/blender/blenkernel/intern/constraint.cc index 3b13f966327..3b80d8aaf5c 100644 --- a/source/blender/blenkernel/intern/constraint.cc +++ b/source/blender/blenkernel/intern/constraint.cc @@ -2982,18 +2982,19 @@ static void actcon_get_tarmat(Depsgraph *depsgraph, (cob->pchan) ? cob->pchan->name : nullptr); } - /* TODO: add an action slot selector to the constraint settings. */ - const blender::animrig::slot_handle_t slot_handle = blender::animrig::first_slot_handle( - *data->act); - /* Get the appropriate information from the action */ if (cob->type == CONSTRAINT_OBTYPE_OBJECT || (data->flag & ACTCON_BONE_USE_OBJECT_ACTION)) { Object workob; /* evaluate using workob */ /* FIXME: we don't have any consistent standards on limiting effects on object... */ - what_does_obaction( - cob->ob, &workob, nullptr, data->act, slot_handle, nullptr, &anim_eval_context); + what_does_obaction(cob->ob, + &workob, + nullptr, + data->act, + data->action_slot_handle, + nullptr, + &anim_eval_context); BKE_object_to_mat4(&workob, ct->matrix); } else if (cob->type == CONSTRAINT_OBTYPE_BONE) { @@ -3010,8 +3011,13 @@ static void actcon_get_tarmat(Depsgraph *depsgraph, tchan->rotmode = pchan->rotmode; /* evaluate action using workob (it will only set the PoseChannel in question) */ - what_does_obaction( - cob->ob, &workob, &pose, data->act, slot_handle, pchan->name, &anim_eval_context); + what_does_obaction(cob->ob, + &workob, + &pose, + data->act, + data->action_slot_handle, + pchan->name, + &anim_eval_context); /* convert animation to matrices for use here */ BKE_pchan_calc_mat(tchan); @@ -6673,3 +6679,11 @@ void BKE_constraint_blend_read_data(BlendDataReader *reader, ID *id_owner, ListB } } } + +/* Some static asserts to ensure that the bActionConstraint data is using the expected types for + * some of the fields. This check is done here instead of in DNA_constraint_types.h to avoid the + * inclusion of an DNA_anim_types.h in DNA_constraint_types.h just for this assert. */ +static_assert( + std::is_same_v); +static_assert( + std::is_same_v); diff --git a/source/blender/editors/object/object_constraint.cc b/source/blender/editors/object/object_constraint.cc index d6545bacdda..8031b3ca216 100644 --- a/source/blender/editors/object/object_constraint.cc +++ b/source/blender/editors/object/object_constraint.cc @@ -357,10 +357,23 @@ static void test_constraint( /* must have action */ con->flag |= CONSTRAINT_DISABLE; } - else if (data->act->idroot != ID_OB) { - /* only object-rooted actions can be used */ - data->act = nullptr; - con->flag |= CONSTRAINT_DISABLE; + else { + animrig::Action &action = data->act->wrap(); + if (action.is_action_legacy()) { + if (data->act->idroot != ID_OB) { + /* Only object-rooted actions can be used. */ + data->act = nullptr; + con->flag |= CONSTRAINT_DISABLE; + } + } + else { + /* The slot was assigned, so assume that it is suitable to animate the + * owner (only suitable slots appear in the drop-down). */ + animrig::Slot *slot = action.slot_for_handle(data->action_slot_handle); + if (!slot) { + con->flag |= CONSTRAINT_DISABLE; + } + } } /* Skip target checking if we're not using it */ diff --git a/source/blender/makesdna/DNA_constraint_types.h b/source/blender/makesdna/DNA_constraint_types.h index 58fa8507012..2a3edd156b3 100644 --- a/source/blender/makesdna/DNA_constraint_types.h +++ b/source/blender/makesdna/DNA_constraint_types.h @@ -335,6 +335,9 @@ typedef struct bActionConstraint { char _pad[3]; float eval_time; /* Only used when flag ACTCON_USE_EVAL_TIME is set. */ struct bAction *act; + int32_t action_slot_handle; + char action_slot_name[66]; /* MAX_ID_NAME */ + char _pad1[2]; /** MAX_ID_NAME-2. */ char subtarget[64]; } bActionConstraint; diff --git a/source/blender/makesrna/intern/rna_constraint.cc b/source/blender/makesrna/intern/rna_constraint.cc index 0e85f0a2dca..c4038ff17f4 100644 --- a/source/blender/makesrna/intern/rna_constraint.cc +++ b/source/blender/makesrna/intern/rna_constraint.cc @@ -29,6 +29,11 @@ #include "ED_object.hh" +#ifdef WITH_ANIM_BAKLAVA +# include "ANIM_action.hh" +# include "rna_action_tools.hh" +#endif + /* Please keep the names in sync with `constraint.cc`. */ const EnumPropertyItem rna_enum_constraint_type_items[] = { RNA_ENUM_ITEM_HEADING(N_("Motion Tracking"), nullptr), @@ -707,6 +712,49 @@ static void rna_ActionConstraint_minmax_range( } } +# ifdef WITH_ANIM_BAKLAVA +static void rna_ActionConstraint_action_slot_handle_set( + PointerRNA *ptr, const blender::animrig::slot_handle_t new_slot_handle) +{ + bConstraint *con = (bConstraint *)ptr->data; + bActionConstraint *acon = (bActionConstraint *)con->data; + + rna_generic_action_slot_handle_set(new_slot_handle, + *ptr->owner_id, + acon->act, + acon->action_slot_handle, + acon->action_slot_name); +} + +static PointerRNA rna_ActionConstraint_action_slot_get(PointerRNA *ptr) +{ + bConstraint *con = (bConstraint *)ptr->data; + bActionConstraint *acon = (bActionConstraint *)con->data; + + return rna_generic_action_slot_get(acon->act, acon->action_slot_handle); +} + +static void rna_ActionConstraint_action_slot_set(PointerRNA *ptr, + PointerRNA value, + ReportList *reports) +{ + bConstraint *con = (bConstraint *)ptr->data; + bActionConstraint *acon = (bActionConstraint *)con->data; + + rna_generic_action_slot_set( + value, *ptr->owner_id, acon->act, acon->action_slot_handle, acon->action_slot_name, reports); +} + +static void rna_iterator_ActionConstraint_action_slots_begin(CollectionPropertyIterator *iter, + PointerRNA *ptr) +{ + bConstraint *con = (bConstraint *)ptr->data; + bActionConstraint *acon = (bActionConstraint *)con->data; + + rna_iterator_generic_action_slots_begin(iter, acon->act); +} +# endif /* WITH_ANIM_BAKLAVA */ + static int rna_SplineIKConstraint_joint_bindings_get_length(const PointerRNA *ptr, int length[RNA_MAX_ARRAY_DIMENSION]) { @@ -1859,6 +1907,71 @@ static void rna_def_constraint_action(BlenderRNA *brna) RNA_def_property_flag(prop, PROP_EDITABLE | PROP_ID_REFCOUNT); RNA_def_property_update(prop, NC_OBJECT | ND_CONSTRAINT, "rna_Constraint_update"); +# ifdef WITH_ANIM_BAKLAVA + /* This property is not necessary for the Python API (that is better off using + * slot references/pointers directly), but it is needed for library overrides + * to work. */ + prop = RNA_def_property(srna, "action_slot_handle", PROP_INT, PROP_NONE); + RNA_def_property_int_sdna(prop, nullptr, "action_slot_handle"); + RNA_def_property_int_funcs( + prop, nullptr, "rna_ActionConstraint_action_slot_handle_set", nullptr); + RNA_def_property_ui_text(prop, + "Action Slot Handle", + "A number that identifies which sub-set of the Action is considered " + "to be for this Action Constraint"); + RNA_def_property_override_flag(prop, PROPOVERRIDE_OVERRIDABLE_LIBRARY); + RNA_def_property_update(prop, NC_ANIMATION | ND_NLA_ACTCHANGE, "rna_Constraint_update"); + + prop = RNA_def_property(srna, "action_slot_name", PROP_STRING, PROP_NONE); + RNA_def_property_string_sdna(prop, nullptr, "action_slot_name"); + RNA_def_property_ui_text( + prop, + "Action Slot Name", + "The name of the action slot. The slot identifies which sub-set of the Action " + "is considered to be for this constraint, and its name is used to find the right slot " + "when assigning an Action."); + + prop = RNA_def_property(srna, "action_slot", PROP_POINTER, PROP_NONE); + RNA_def_property_struct_type(prop, "ActionSlot"); + RNA_def_property_flag(prop, PROP_EDITABLE); + RNA_def_property_clear_flag(prop, PROP_ANIMATABLE); + RNA_def_property_ui_text( + prop, + "Action Slot", + "The slot identifies which sub-set of the Action is considered to be for this " + "strip, and its name is used to find the right slot when assigning another Action"); + RNA_def_property_pointer_funcs(prop, + "rna_ActionConstraint_action_slot_get", + "rna_ActionConstraint_action_slot_set", + nullptr, + nullptr); + RNA_def_property_update(prop, NC_ANIMATION | ND_NLA_ACTCHANGE, "rna_Constraint_update"); + /* `strip.action_slot` is exposed to RNA as a pointer for things like the action slot selector in + * the GUI. The ground truth of the assigned slot, however, is `action_slot_handle` declared + * above. That property is used for library override operations, and this pointer property should + * just be ignored. + * + * This needs PROPOVERRIDE_IGNORE; PROPOVERRIDE_NO_COMPARISON is not suitable here. This property + * should act as if it is an overridable property (as from the user's perspective, it is), but an + * override operation should not be created for it. It will be created for `action_slot_handle`, + * and that's enough. */ + RNA_def_property_override_flag(prop, PROPOVERRIDE_IGNORE); + + prop = RNA_def_property(srna, "action_slots", PROP_COLLECTION, PROP_NONE); + RNA_def_property_struct_type(prop, "ActionSlot"); + RNA_def_property_collection_funcs(prop, + "rna_iterator_ActionConstraint_action_slots_begin", + "rna_iterator_array_next", + "rna_iterator_array_end", + "rna_iterator_array_dereference_get", + nullptr, + nullptr, + nullptr, + nullptr); + RNA_def_property_ui_text( + prop, "Action Slots", "The list of action slots suitable for this NLA strip"); +# endif /* WITH_ANIM_BAKLAVA */ + prop = RNA_def_property(srna, "use_bone_object_action", PROP_BOOLEAN, PROP_NONE); RNA_def_property_boolean_sdna(prop, nullptr, "flag", ACTCON_BONE_USE_OBJECT_ACTION); RNA_def_property_ui_text(prop, diff --git a/tests/python/bl_rigging_symmetrize.py b/tests/python/bl_rigging_symmetrize.py index 3a9d0d8dde6..898dcf684da 100644 --- a/tests/python/bl_rigging_symmetrize.py +++ b/tests/python/bl_rigging_symmetrize.py @@ -116,6 +116,10 @@ def check_constraints(self, input_arm, expected_arm, bone, exp_bone): "Mismatching constraint value types in pose.bones[%s].constraints[%s].%s" % ( bone.name, const_name, var)) + if isinstance(value, bpy.types.bpy_prop_collection): + # Don't compare collection properties. + continue + if isinstance(value, str): self.assertEqual(value, exp_value, "Mismatching constraint value in pose.bones[%s].constraints[%s].%s" % ( @@ -133,10 +137,19 @@ def check_constraints(self, input_arm, expected_arm, bone, exp_bone): self.assertEqual(value, exp_value, "Mismatching constraint boolean in pose.bones[%s].constraints[%s].%s" % ( bone.name, const_name, var)) - else: + elif isinstance(value, float): msg = "Mismatching constraint value in pose.bones[%s].constraints[%s].%s" % ( bone.name, const_name, var) self.assertAlmostEqual(value, exp_value, places=6, msg=msg) + elif isinstance(value, int): + msg = "Mismatching constraint value in pose.bones[%s].constraints[%s].%s" % ( + bone.name, const_name, var) + self.assertEqual(value, exp_value, msg=msg) + elif value is None: + # Since above the types were compared already, if value is none, so is exp_value. + pass + else: + self.fail(f"unexpected value type: {value!r} is of type {type(value)}") class AbstractAnimationTest: