From ca388fe313c52304ed12b9301f76054c653e1360 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Tue, 18 Feb 2025 16:03:29 +0100 Subject: [PATCH 1/5] Anim: update Action RNA descriptions to clarify legacy status Update the Action RNA descriptions, to clarify which parts of the API are considered legacy. These will only act on the action's first slot, in an attempt to be backward-compatible with the pre-4.4 (non-slotted) actions. No actual functional changes, just a change in the descriptions. Pull Request: https://projects.blender.org/blender/blender/pulls/134683 --- source/blender/makesrna/intern/rna_action.cc | 42 ++++++++++++++------ 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/source/blender/makesrna/intern/rna_action.cc b/source/blender/makesrna/intern/rna_action.cc index 1fcb9875f01..5e178a6ea05 100644 --- a/source/blender/makesrna/intern/rna_action.cc +++ b/source/blender/makesrna/intern/rna_action.cc @@ -2690,7 +2690,11 @@ static void rna_def_action_fcurves(BlenderRNA *brna, PropertyRNA *cprop) RNA_def_property_srna(cprop, "ActionFCurves"); srna = RNA_def_struct(brna, "ActionFCurves", nullptr); RNA_def_struct_sdna(srna, "bAction"); - RNA_def_struct_ui_text(srna, "Action F-Curves", "Collection of action F-Curves"); + RNA_def_struct_ui_text( + srna, + "Action F-Curves", + "Collection of action F-Curves. Note that this is a legacy API that is unaware of action " + "slots, and will only consider the F-Curves for this action's first slot"); RNA_def_property_collection_funcs(cprop, "rna_iterator_Action_fcurves_begin", "rna_iterator_Action_fcurves_next", @@ -2703,7 +2707,9 @@ static void rna_def_action_fcurves(BlenderRNA *brna, PropertyRNA *cprop) /* Action.fcurves.new(...) */ func = RNA_def_function(srna, "new", "rna_Action_fcurve_new"); - RNA_def_function_ui_description(func, "Add an F-Curve to the action"); + RNA_def_function_ui_description(func, + "Add an F-Curve for the first slot of this action, creating the " + "necessary layer, strip, and slot if necessary"); RNA_def_function_flag(func, FUNC_USE_REPORTS | FUNC_USE_MAIN); parm = RNA_def_string(func, "data_path", nullptr, 0, "Data Path", "F-Curve data path to use"); RNA_def_parameter_flags(parm, PropertyFlag(0), PARM_REQUIRED); @@ -2719,7 +2725,7 @@ static void rna_def_action_fcurves(BlenderRNA *brna, PropertyRNA *cprop) RNA_def_function_ui_description( func, "Find an F-Curve. Note that this function performs a linear scan " - "of all F-Curves in the action."); + "of all F-Curves for the action's first slot."); RNA_def_function_flag(func, FUNC_USE_REPORTS); parm = RNA_def_string(func, "data_path", nullptr, 0, "Data Path", "F-Curve data path"); RNA_def_parameter_flags(parm, PropertyFlag(0), PARM_REQUIRED); @@ -2730,7 +2736,7 @@ static void rna_def_action_fcurves(BlenderRNA *brna, PropertyRNA *cprop) /* Action.fcurves.remove(...) */ func = RNA_def_function(srna, "remove", "rna_Action_fcurve_remove"); - RNA_def_function_ui_description(func, "Remove F-Curve"); + RNA_def_function_ui_description(func, "Remove the F-Curve from the action's first slot"); RNA_def_function_flag(func, FUNC_USE_REPORTS); parm = RNA_def_pointer(func, "fcurve", "FCurve", "", "F-Curve to remove"); RNA_def_parameter_flags(parm, PROP_NEVER_NULL, PARM_REQUIRED | PARM_RNAPTR); @@ -2738,7 +2744,7 @@ static void rna_def_action_fcurves(BlenderRNA *brna, PropertyRNA *cprop) /* Action.fcurves.clear() */ func = RNA_def_function(srna, "clear", "rna_Action_fcurve_clear"); - RNA_def_function_ui_description(func, "Remove all F-Curves"); + RNA_def_function_ui_description(func, "Remove all F-Curves from the action's first slot"); } static void rna_def_action_pose_markers(BlenderRNA *brna, PropertyRNA *cprop) @@ -2799,13 +2805,21 @@ static void rna_def_action_legacy(BlenderRNA *brna, StructRNA *srna) prop = RNA_def_property(srna, "fcurves", PROP_COLLECTION, PROP_NONE); RNA_def_property_collection_sdna(prop, nullptr, "curves", nullptr); RNA_def_property_struct_type(prop, "FCurve"); - RNA_def_property_ui_text(prop, "F-Curves", "The individual F-Curves that make up the action"); + RNA_def_property_ui_text( + prop, + "F-Curves", + "Legacy API, for backward compatibility with code that does not handle slotted actions yet. " + "This collection contains the F-Curves for the action's first slot"); rna_def_action_fcurves(brna, prop); prop = RNA_def_property(srna, "groups", PROP_COLLECTION, PROP_NONE); RNA_def_property_collection_sdna(prop, nullptr, "groups", nullptr); RNA_def_property_struct_type(prop, "ActionGroup"); - RNA_def_property_ui_text(prop, "Groups", "Convenient groupings of F-Curves"); + RNA_def_property_ui_text( + prop, + "Groups", + "Legacy API, for backward compatibility with code that does not handle slotted actions yet. " + "This collection contains the F-Curve groups for the action's first slot"); rna_def_action_groups(brna, prop); /* special "type" limiter - should not really be edited in general, @@ -2819,10 +2833,12 @@ static void rna_def_action_legacy(BlenderRNA *brna, StructRNA *srna) "rna_ActionSlot_target_id_type_itemf"); RNA_def_property_update(prop, NC_ANIMATION | ND_ANIMCHAN, "rna_Action_id_root_update"); RNA_def_property_flag(prop, PROP_ENUM_NO_CONTEXT); - RNA_def_property_ui_text(prop, - "ID Root Type", - "Type of ID block that action can be used on - " - "DO NOT CHANGE UNLESS YOU KNOW WHAT YOU ARE DOING"); + RNA_def_property_ui_text( + prop, + "ID Root Type", + "Legacy API, for backward compatibility with code that does not handle slotted actions yet. " + "Type of data-block that the action's first slot can be used on. Do not change unless you " + "know what you are doing"); RNA_def_property_translation_context(prop, BLT_I18NCONTEXT_ID_ID); } @@ -2849,7 +2865,9 @@ static void rna_def_action(BlenderRNA *brna) prop, "Is Legacy Action", "Return whether this is a legacy Action. Legacy Actions have no layers or slots. An " - "empty Action considered as both a 'legacy' and a 'layered' Action."); + "empty Action considered as both a 'legacy' and a 'layered' Action. Since Blender 4.4 " + "actions are automatically updated to layered actions, and thus this will only return True " + "when the action is empty"); RNA_def_property_boolean_funcs(prop, "rna_Action_is_action_legacy_get", nullptr); prop = RNA_def_property(srna, "is_action_layered", PROP_BOOLEAN, PROP_NONE); From 2393d498cb28b2a680f084024d8002b4aded7f53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Tue, 18 Feb 2025 16:04:00 +0100 Subject: [PATCH 2/5] Anim: add RNA function `Action.fcurve_ensure_for_datablock(...)` Expose the convenience function `blender::animrig::action_fcurve_ensure()` to RNA as `Action.fcurve_ensure_for_datablock(...)`. The function requires that the Action is already assigned to the data-block. It then takes care of slot assignment / creation, as well as the creation of a layer and a keyframe strip. This function call: ```python fcurve = action.fcurve_ensure_for_datablock(ob_cube, "location", index=2) ``` effectively performs this logic: ```python # Ensure the slot exists and is assigned: slot = ob_cube.animation_data.slot if not slot: slot = find_slot_for_keying(action) if not slot: slot = action.slots.new(ob_cube.name) ob_cube.animation_data.slot = slot # Ensure a layer exists: if action.layers: layer = action.layers[0] else: layer = action.layers.new("Layer") # Ensure a keyframe strip exists: if layer.strips: strip = layer.strips[0] else: strip = layer.strips.new('KEYFRAME') # Ensure the channelbag exists: channelbag = strip.channelbag(slot, ensure=True) # Ensure the F-Curve exists: fcurve = channelbag.fcurves.find("location", index=1) if not fcurve: fcurve = channelbag.fcurves.new("location", index=1) ``` Here `find_slot_for_keying()` represents the logic that's also used when creating keys via the user interface or the `bpy_struct.keyframe_insert()` function. Pull Request: https://projects.blender.org/blender/blender/pulls/134686 --- source/blender/animrig/ANIM_action.hh | 36 ++++++++-- source/blender/animrig/intern/action.cc | 28 +++++--- source/blender/animrig/intern/action_test.cc | 8 +-- source/blender/animrig/intern/keyframing.cc | 2 +- .../editors/interface/interface_ops_test.cc | 2 +- .../editors/object/object_constraint.cc | 4 +- .../editors/object/object_relations.cc | 2 +- source/blender/makesrna/intern/rna_action.cc | 68 ++++++++++++++++++- tests/python/bl_animation_action.py | 42 ++++++++++++ 9 files changed, 168 insertions(+), 24 deletions(-) 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 From a228f150b398a14f13c631ba87d8690ad3522c48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Tue, 18 Feb 2025 16:05:07 +0100 Subject: [PATCH 3/5] Fix: compiler warning on Windows about an always-true check on enum value Remove an overly-careful check on an enum value being greater than zero. The check was added back in the day when this value was declared as `int` and thus could easily be assigned anything. The check for the upper limit was kept, and augmented with a `BLI_assert()` so that failures here will actually go noticed and can get fixed. No actually functional changes. The data being checked is purely runtime, and is assumed to be generated correctly already. Pull Request: https://projects.blender.org/blender/blender/pulls/134736 --- source/blender/editors/animation/anim_channels_defines.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/source/blender/editors/animation/anim_channels_defines.cc b/source/blender/editors/animation/anim_channels_defines.cc index 4cb98cab621..bc9d5e063fe 100644 --- a/source/blender/editors/animation/anim_channels_defines.cc +++ b/source/blender/editors/animation/anim_channels_defines.cc @@ -4690,12 +4690,12 @@ const bAnimChannelType *ANIM_channel_get_typeinfo(const bAnimListElem *ale) /* init the typeinfo if not available yet... */ ANIM_init_channel_typeinfo_data(); - /* check if type is in bounds... */ - if ((ale->type >= 0) && (ale->type < ANIMTYPE_NUM_TYPES)) { - return animchannelTypeInfo[ale->type]; + BLI_assert(ale->type < ANIMTYPE_NUM_TYPES); + if (ale->type >= ANIMTYPE_NUM_TYPES) { + return nullptr; } - return nullptr; + return animchannelTypeInfo[ale->type]; } /* --------------------------- */ From c0f0e2ca6f3fc0aefab93542211e01bc61be8f34 Mon Sep 17 00:00:00 2001 From: Brecht Van Lommel Date: Tue, 18 Feb 2025 16:20:59 +0100 Subject: [PATCH 4/5] Fix #129596: Cycles oneAPI crash with interactive BVH updates There is a bug in Embree that makes BVH updates crash. Disabling multithreaded BVH updates after the initial BVH build appears to work around it, at the cost of some performance. This will not affect performance of the initial BVH build, transforming objects or editing a single mesh. It will only affect performance when multiple smaller meshes are edited together, as those can no longer have their BVH updated in parallel or benefit from parallellization over many primitives. Pull Request: https://projects.blender.org/blender/blender/pulls/134747 --- intern/cycles/device/device.cpp | 13 +++++++++++++ intern/cycles/device/device.h | 2 ++ intern/cycles/scene/geometry.cpp | 14 ++++++++++++-- intern/cycles/scene/geometry.h | 1 + 4 files changed, 28 insertions(+), 2 deletions(-) diff --git a/intern/cycles/device/device.cpp b/intern/cycles/device/device.cpp index cf77f76bec6..44fa7988f2a 100644 --- a/intern/cycles/device/device.cpp +++ b/intern/cycles/device/device.cpp @@ -816,4 +816,17 @@ bool GPUDevice::is_shared(const void *shared_pointer, /* DeviceInfo */ +bool DeviceInfo::contains_device_type(const DeviceType type) const +{ + if (this->type == type) { + return true; + } + for (const DeviceInfo &info : multi_devices) { + if (info.contains_device_type(type)) { + return true; + } + } + return false; +} + CCL_NAMESPACE_END diff --git a/intern/cycles/device/device.h b/intern/cycles/device/device.h index 8912000f788..f55fa3f7f18 100644 --- a/intern/cycles/device/device.h +++ b/intern/cycles/device/device.h @@ -130,6 +130,8 @@ class DeviceInfo { { return !(*this == info); } + + bool contains_device_type(const DeviceType type) const; }; /* Device */ diff --git a/intern/cycles/scene/geometry.cpp b/intern/cycles/scene/geometry.cpp index d51e7c6270c..7443dbbd8f2 100644 --- a/intern/cycles/scene/geometry.cpp +++ b/intern/cycles/scene/geometry.cpp @@ -943,13 +943,23 @@ void GeometryManager::device_update(Device *device, }); TaskPool pool; + /* Work around Embree/oneAPI bug #129596 with BVH updates. */ + const bool use_multithreaded_build = first_bvh_build || + !device->info.contains_device_type(DEVICE_ONEAPI); + first_bvh_build = false; + size_t i = 0; for (Geometry *geom : scene->geometry) { if (geom->is_modified() || geom->need_update_bvh_for_offset) { need_update_scene_bvh = true; - pool.push([geom, device, dscene, scene, &progress, i, num_bvh] { + if (use_multithreaded_build) { + pool.push([geom, device, dscene, scene, &progress, i, num_bvh] { + geom->compute_bvh(device, dscene, &scene->params, &progress, i, num_bvh); + }); + } + else { geom->compute_bvh(device, dscene, &scene->params, &progress, i, num_bvh); - }); + } if (geom->need_build_bvh(bvh_layout)) { i++; } diff --git a/intern/cycles/scene/geometry.h b/intern/cycles/scene/geometry.h index 4b57d7fa692..6144bf19c83 100644 --- a/intern/cycles/scene/geometry.h +++ b/intern/cycles/scene/geometry.h @@ -225,6 +225,7 @@ class GeometryManager { /* Update Flags */ bool need_flags_update; + bool first_bvh_build = true; /* Constructor/Destructor */ GeometryManager(); From d0a89a0950e2a133f7b184f14b88dd0a14412700 Mon Sep 17 00:00:00 2001 From: Hans Goudey Date: Tue, 18 Feb 2025 10:54:41 -0500 Subject: [PATCH 5/5] Fix #134707: Dynamic paint brush object movement ignored Caused by bcfe4c34dac7c541547bb0e3dc967b9ede825834. The code was missing a dirty tag for the positions. --- source/blender/blenkernel/intern/dynamicpaint.cc | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/source/blender/blenkernel/intern/dynamicpaint.cc b/source/blender/blenkernel/intern/dynamicpaint.cc index 577f5d1931f..9a6fbc6e889 100644 --- a/source/blender/blenkernel/intern/dynamicpaint.cc +++ b/source/blender/blenkernel/intern/dynamicpaint.cc @@ -2198,7 +2198,16 @@ Mesh *dynamicPaint_Modifier_do( /* Create a surface for uv image sequence format. */ #define JITTER_SAMPLES \ { \ - 0.0f, 0.0f, -0.2f, -0.4f, 0.2f, 0.4f, 0.4f, -0.2f, -0.4f, 0.3f, \ + 0.0f, \ + 0.0f, \ + -0.2f, \ + -0.4f, \ + 0.2f, \ + 0.4f, \ + 0.4f, \ + -0.2f, \ + -0.4f, \ + 0.3f, \ } struct DynamicPaintCreateUVSurfaceData { @@ -4339,6 +4348,8 @@ static bool dynamicPaint_paintMesh(Depsgraph *depsgraph, } } + mesh->tag_positions_changed(); + if (brush->flags & MOD_DPAINT_PROX_PROJECT && brush->collision != MOD_DPAINT_COL_VOLUME) { mul_v3_fl(avg_brushNor, 1.0f / float(numOfVerts)); /* instead of null vector use positive z */