diff --git a/source/blender/animrig/ANIM_action.hh b/source/blender/animrig/ANIM_action.hh index ced0cb0fafe..6a6dbd43081 100644 --- a/source/blender/animrig/ANIM_action.hh +++ b/source/blender/animrig/ANIM_action.hh @@ -1625,6 +1625,29 @@ animrig::Channelbag *channelbag_for_action_slot(Action &action, slot_handle_t sl Span fcurves_for_action_slot(Action &action, slot_handle_t slot_handle); Span fcurves_for_action_slot(const Action &action, slot_handle_t slot_handle); +/** + * Find or create an F-Curve on the given action that matches the given fcurve + * descriptor. + * + * \note This function also ensures that dependency graph relationships are + * rebuilt. This is necessary when adding a new F-Curve, as a + * previously-unanimated depsgraph component may become animated now. + * + * \param action: MUST already be assigned to the animated ID. + * + * \param animated_id: The ID that is animated by this Action. It is used to + * create and assign an appropriate slot if needed when creating the fcurve, and + * set the fcurve color properly + * + * \param fcurve_descriptor: description of the fcurve to lookup/create. Note + * that this is *not* relative to `ptr` (e.g. if `ptr` is not an ID). It should + * contain the exact data path of the fcurve to be looked up/created. + */ +FCurve *action_fcurve_ensure(Main *bmain, + bAction &action, + ID &animated_id, + FCurveDescriptor fcurve_descriptor); + /** * Find or create an F-Curve on the given action that matches the given fcurve * descriptor. @@ -1646,12 +1669,15 @@ Span fcurves_for_action_slot(const Action &action, slot_handle_t * \param fcurve_descriptor: description of the fcurve to lookup/create. Note * that this is *not* relative to `ptr` (e.g. if `ptr` is not an ID). It should * contain the exact data path of the fcurve to be looked up/created. + * + * \see action_fcurve_ensure for a function that is specific to layered actions, + * and is easier to use because it does not depend on an RNA pointer. */ -FCurve *action_fcurve_ensure(Main *bmain, - bAction *act, - const char group[], - PointerRNA *ptr, - FCurveDescriptor fcurve_descriptor); +FCurve *action_fcurve_ensure_ex(Main *bmain, + bAction *act, + const char group[], + PointerRNA *ptr, + FCurveDescriptor fcurve_descriptor); /** * Same as above, but creates a legacy Action. diff --git a/source/blender/animrig/intern/action.cc b/source/blender/animrig/intern/action.cc index 995d3b95a32..2cdef100ee1 100644 --- a/source/blender/animrig/intern/action.cc +++ b/source/blender/animrig/intern/action.cc @@ -2595,11 +2595,11 @@ Vector fcurves_in_listbase_filtered(ListBase /* FCurve * */ fcurves, return found; } -FCurve *action_fcurve_ensure(Main *bmain, - bAction *act, - const char group[], - PointerRNA *ptr, - FCurveDescriptor fcurve_descriptor) +FCurve *action_fcurve_ensure_ex(Main *bmain, + bAction *act, + const char group[], + PointerRNA *ptr, + FCurveDescriptor fcurve_descriptor) { if (act == nullptr) { return nullptr; @@ -2625,13 +2625,21 @@ FCurve *action_fcurve_ensure(Main *bmain, * hold, or if this is even the best place to handle the layered action * cases at all, was leading to discussion of larger changes than made sense * to tackle at that point. */ - Action &action = act->wrap(); - BLI_assert(ptr != nullptr); if (ptr == nullptr || ptr->owner_id == nullptr) { return nullptr; } - ID &animated_id = *ptr->owner_id; + + return action_fcurve_ensure(bmain, *act, *ptr->owner_id, fcurve_descriptor); +} + +FCurve *action_fcurve_ensure(Main *bmain, + bAction &dna_action, + ID &animated_id, + FCurveDescriptor fcurve_descriptor) +{ + Action &action = dna_action.wrap(); + BLI_assert(get_action(animated_id) == &action); if (get_action(animated_id) != &action) { return nullptr; @@ -2640,7 +2648,9 @@ FCurve *action_fcurve_ensure(Main *bmain, /* Ensure the id has an assigned slot. */ Slot *slot = assign_action_ensure_slot_for_keying(action, animated_id); if (!slot) { - /* This means the ID type is not animatable. */ + /* This means the ID type is not animatable. How did an Action get assigned + * in the first place? */ + BLI_assert_unreachable(); return nullptr; } diff --git a/source/blender/animrig/intern/action_test.cc b/source/blender/animrig/intern/action_test.cc index accaa08c1dd..1261933664e 100644 --- a/source/blender/animrig/intern/action_test.cc +++ b/source/blender/animrig/intern/action_test.cc @@ -1208,11 +1208,11 @@ TEST_F(ActionLayersTest, action_move_slot) PointerRNA cube_rna_pointer = RNA_id_pointer_create(&cube->id); PointerRNA suzanne_rna_pointer = RNA_id_pointer_create(&suzanne->id); - action_fcurve_ensure(bmain, action, "Test", &cube_rna_pointer, {"location", 0}); - action_fcurve_ensure(bmain, action, "Test", &cube_rna_pointer, {"rotation_euler", 1}); + action_fcurve_ensure_ex(bmain, action, "Test", &cube_rna_pointer, {"location", 0}); + action_fcurve_ensure_ex(bmain, action, "Test", &cube_rna_pointer, {"rotation_euler", 1}); - action_fcurve_ensure(bmain, action_2, "Test_2", &suzanne_rna_pointer, {"location", 0}); - action_fcurve_ensure(bmain, action_2, "Test_2", &suzanne_rna_pointer, {"rotation_euler", 1}); + action_fcurve_ensure_ex(bmain, action_2, "Test_2", &suzanne_rna_pointer, {"location", 0}); + action_fcurve_ensure_ex(bmain, action_2, "Test_2", &suzanne_rna_pointer, {"rotation_euler", 1}); ASSERT_EQ(action->layer_array_num, 1); ASSERT_EQ(action_2->layer_array_num, 1); diff --git a/source/blender/animrig/intern/keyframing.cc b/source/blender/animrig/intern/keyframing.cc index 8f117b2e6e8..3ce891e7dc4 100644 --- a/source/blender/animrig/intern/keyframing.cc +++ b/source/blender/animrig/intern/keyframing.cc @@ -607,7 +607,7 @@ static SingleKeyingResult insert_keyframe_fcurve_value(Main *bmain, */ FCurve *fcu = key_insertion_may_create_fcurve(flag) ? - action_fcurve_ensure(bmain, act, group, ptr, {rna_path, array_index}) : + action_fcurve_ensure_ex(bmain, act, group, ptr, {rna_path, array_index}) : fcurve_find_in_action(act, {rna_path, array_index}); /* We may not have a F-Curve when we're replacing only. */ diff --git a/source/blender/editors/interface/interface_ops_test.cc b/source/blender/editors/interface/interface_ops_test.cc index fe6201eedb1..5c877992c56 100644 --- a/source/blender/editors/interface/interface_ops_test.cc +++ b/source/blender/editors/interface/interface_ops_test.cc @@ -111,7 +111,7 @@ class CopyDriversToSelected : public testing::Test { /* Add animation to cube's fourth quaternion element. */ PointerRNA cube_ptr = RNA_pointer_create_discrete(&cube->id, &RNA_Object, &cube->id); bAction *act = animrig::id_action_ensure(bmain, &cube->id); - FCurve *fcu = animrig::action_fcurve_ensure( + FCurve *fcu = animrig::action_fcurve_ensure_ex( bmain, act, "Object Transforms", &cube_ptr, {"rotation_quaternion", 3}); animrig::KeyframeSettings keyframe_settings = {BEZT_KEYTYPE_KEYFRAME, HD_AUTO, BEZT_IPO_BEZ}; insert_vert_fcurve(fcu, {1.0, 1.0}, keyframe_settings, INSERTKEY_NOFLAGS); diff --git a/source/blender/editors/object/object_constraint.cc b/source/blender/editors/object/object_constraint.cc index ed5bdcb4a82..6855854d0b3 100644 --- a/source/blender/editors/object/object_constraint.cc +++ b/source/blender/editors/object/object_constraint.cc @@ -1090,7 +1090,7 @@ static int followpath_path_animate_exec(bContext *C, wmOperator *op) /* create F-Curve for path animation */ act = animrig::id_action_ensure(bmain, &cu->id); PointerRNA id_ptr = RNA_id_pointer_create(&cu->id); - fcu = animrig::action_fcurve_ensure(bmain, act, nullptr, &id_ptr, {"eval_time", 0}); + fcu = animrig::action_fcurve_ensure_ex(bmain, act, nullptr, &id_ptr, {"eval_time", 0}); /* standard vertical range - 1:1 = 100 frames */ standardRange = 100.0f; @@ -1115,7 +1115,7 @@ static int followpath_path_animate_exec(bContext *C, wmOperator *op) /* create F-Curve for constraint */ act = animrig::id_action_ensure(bmain, &ob->id); PointerRNA id_ptr = RNA_id_pointer_create(&ob->id); - fcu = animrig::action_fcurve_ensure(bmain, act, nullptr, &id_ptr, {path->c_str(), 0}); + fcu = animrig::action_fcurve_ensure_ex(bmain, act, nullptr, &id_ptr, {path->c_str(), 0}); /* standard vertical range - 0.0 to 1.0 */ standardRange = 1.0f; diff --git a/source/blender/editors/object/object_relations.cc b/source/blender/editors/object/object_relations.cc index bbbd68ebac7..436e8b6a672 100644 --- a/source/blender/editors/object/object_relations.cc +++ b/source/blender/editors/object/object_relations.cc @@ -546,7 +546,7 @@ bool parent_set(ReportList *reports, /* get or create F-Curve */ bAction *act = animrig::id_action_ensure(bmain, &cu->id); PointerRNA id_ptr = RNA_id_pointer_create(&cu->id); - FCurve *fcu = animrig::action_fcurve_ensure( + FCurve *fcu = animrig::action_fcurve_ensure_ex( bmain, act, nullptr, &id_ptr, {"eval_time", 0}); /* setup dummy 'generator' modifier here to get 1-1 correspondence still working */ diff --git a/source/blender/makesrna/intern/rna_action.cc b/source/blender/makesrna/intern/rna_action.cc index 5e178a6ea05..b5878d53923 100644 --- a/source/blender/makesrna/intern/rna_action.cc +++ b/source/blender/makesrna/intern/rna_action.cc @@ -16,6 +16,7 @@ #include "BKE_action.hh" #include "BKE_blender.hh" +#include "BKE_report.hh" #include "RNA_access.hh" #include "RNA_define.hh" @@ -1369,6 +1370,44 @@ static void rna_Action_deselect_keys(bAction *act) WM_main_add_notifier(NC_ANIMATION | ND_KEYFRAME | NA_EDITED, nullptr); } +static FCurve *rna_Action_fcurve_ensure_for_datablock(bAction *_self, + Main *bmain, + ReportList *reports, + ID *datablock, + const char *data_path, + const int array_index) +{ + /* Precondition checks. */ + { + if (blender::animrig::get_action(*datablock) != _self) { + BKE_reportf(reports, + RPT_ERROR_INVALID_INPUT, + "Assign action \"%s\" to \"%s\" before calling this function", + _self->id.name + 2, + datablock->name + 2); + return nullptr; + } + + BLI_assert(data_path != nullptr); + if (data_path[0] == '\0') { + BKE_report(reports, RPT_ERROR_INVALID_INPUT, "F-Curve data path empty, invalid argument"); + return nullptr; + } + } + + FCurve *fcurve = blender::animrig::action_fcurve_ensure( + bmain, *_self, *datablock, {data_path, array_index}); + + if (!fcurve) { + /* This should never happen, given the precondition check above. */ + BLI_assert_unreachable(); + return nullptr; + } + + WM_main_add_notifier(NC_ANIMATION | ND_KEYFRAME | NA_EDITED, nullptr); + return fcurve; +} + /** * Used to check if an action (value pointer) * is suitable to be assigned to the ID-block that is ptr. @@ -2847,6 +2886,9 @@ static void rna_def_action(BlenderRNA *brna) StructRNA *srna; PropertyRNA *prop; + FunctionRNA *func; + PropertyRNA *parm; + srna = RNA_def_struct(brna, "Action", "ID"); RNA_def_struct_sdna(srna, "bAction"); RNA_def_struct_ui_text(srna, "Action", "A collection of F-Curves for animation"); @@ -2987,10 +3029,34 @@ static void rna_def_action(BlenderRNA *brna) RNA_def_property_float_funcs(prop, "rna_Action_curve_frame_range_get", nullptr, nullptr); RNA_def_property_clear_flag(prop, PROP_EDITABLE); - FunctionRNA *func = RNA_def_function(srna, "deselect_keys", "rna_Action_deselect_keys"); + func = RNA_def_function(srna, "deselect_keys", "rna_Action_deselect_keys"); RNA_def_function_ui_description( func, "Deselects all keys of the Action. The selection status of F-Curves is unchanged."); + /* action.fcurve_ensure_for_datablock() */ + func = RNA_def_function( + srna, "fcurve_ensure_for_datablock", "rna_Action_fcurve_ensure_for_datablock"); + RNA_def_function_ui_description( + func, + "Ensure that an F-Curve exists, with the given data path and array index, for the given " + "data-block. This action must already be assigned to the data-block. This function will " + "also create the layer, keyframe strip, and action slot if necessary, and take care of " + "assigning the action slot too"); + RNA_def_function_flag(func, FUNC_USE_MAIN | FUNC_USE_REPORTS); + + parm = RNA_def_pointer(func, + "datablock", + "ID", + "", + "The data-block animated by this action, for which to ensure the F-Curve " + "exists. This action must already be assigned to the data-block"); + RNA_def_parameter_flags(parm, PROP_NEVER_NULL, PARM_REQUIRED); + parm = RNA_def_string(func, "data_path", nullptr, 0, "Data Path", "F-Curve data path"); + RNA_def_parameter_flags(parm, PropertyFlag(0), PARM_REQUIRED); + RNA_def_int(func, "index", 0, 0, INT_MAX, "Index", "Array index", 0, INT_MAX); + parm = RNA_def_pointer(func, "fcurve", "FCurve", "", "The found or created F-Curve"); + RNA_def_function_return(func, parm); + rna_def_action_legacy(brna, srna); /* API calls */ diff --git a/tests/python/bl_animation_action.py b/tests/python/bl_animation_action.py index 19072174418..520eba5a761 100644 --- a/tests/python/bl_animation_action.py +++ b/tests/python/bl_animation_action.py @@ -913,6 +913,48 @@ class SlotHandleLibraryOverridesTest(unittest.TestCase): -1, "Suzanne should be significantly below Z=0 when animated by the library Action") +class ConvenienceFunctionsTest(unittest.TestCase): + + def setUp(self) -> None: + bpy.ops.wm.read_homefile(use_factory_startup=True) + + self.action = bpy.data.actions.new('Action') + + def test_fcurve_ensure_for_datablock(self) -> None: + # The function should be None-safe. + with self.assertRaises(TypeError): + self.action.fcurve_ensure_for_datablock(None, "location") + self.assertEqual(0, len(self.action.layers)) + self.assertEqual(0, len(self.action.slots)) + + # The function should not work unless the Action is assigned to its target. + ob_cube = bpy.data.objects["Cube"] + with self.assertRaises(RuntimeError): + self.action.fcurve_ensure_for_datablock(ob_cube, "location") + self.assertEqual(0, len(self.action.layers)) + self.assertEqual(0, len(self.action.slots)) + + # The function should not work on empty data paths. + adt = ob_cube.animation_data_create() + adt.action = self.action + with self.assertRaises(RuntimeError): + self.action.fcurve_ensure_for_datablock(ob_cube, "") + self.assertEqual(0, len(self.action.layers)) + self.assertEqual(0, len(self.action.slots)) + + # And finally the happy flow. + fcurve = self.action.fcurve_ensure_for_datablock(ob_cube, "location", index=2) + self.assertEqual(1, len(self.action.layers)) + self.assertEqual(1, len(self.action.layers[0].strips)) + self.assertEqual('KEYFRAME', self.action.layers[0].strips[0].type) + self.assertEqual(1, len(self.action.slots)) + self.assertEqual("location", fcurve.data_path) + self.assertEqual(2, fcurve.array_index) + + channelbag = self.action.layers[0].strips[0].channelbags[0] + self.assertEqual(fcurve, channelbag.fcurves[0]) + + def main(): global args import argparse