From bb00621965c567aaafb6bc6cbd63e0d311af2dde Mon Sep 17 00:00:00 2001 From: Bastien Montagne Date: Wed, 6 Mar 2024 10:02:53 +0100 Subject: [PATCH] Fix recent regression preventing animation on any linked data. Regression in 427eed292d. Root of the issue was that animation system was using a single same check to decide if an F-Curve/driver was valid to use to animate some data, and whether user can create/edit animation for that data. Both cases are actually different, since e.g. linked data is not user-editable, but it can still be animated (either by related linked Actions, drivers defined in the linked data, or some more hackish changes like py API/RNA scripting). This commit now defines two checks: * `RNA_property_animateable`: whether a RNA pointer & propoerty is animateable, based on their types and definition. * `RNA_property_anim_editable`: whether a specific data referenced by the RNA pointer and property is effectively user-editable. The new `driveable` check added by 427eed292d is also renamed to `RNA_property_driver_editable` (since the basic type-based `RNA_property_animateable` is also valid for drivers currently). Pull Request: https://projects.blender.org/blender/blender/pulls/119089 --- source/blender/editors/animation/drivers.cc | 12 +++--- .../blender/editors/animation/keyframing.cc | 4 +- .../blender/editors/animation/keyingsets.cc | 2 +- .../eyedroppers/eyedropper_driver.cc | 2 +- .../interface/interface_context_menu.cc | 2 +- .../editors/space_outliner/outliner_edit.cc | 4 +- source/blender/makesrna/RNA_access.hh | 33 ++++++++++++++-- source/blender/makesrna/intern/rna_access.cc | 38 +++++++++++-------- 8 files changed, 65 insertions(+), 32 deletions(-) diff --git a/source/blender/editors/animation/drivers.cc b/source/blender/editors/animation/drivers.cc index 86418d5e17f..5bb9e4fe0f8 100644 --- a/source/blender/editors/animation/drivers.cc +++ b/source/blender/editors/animation/drivers.cc @@ -891,7 +891,7 @@ static const EnumPropertyItem *driver_mapping_type_itemf(bContext *C, UI_context_active_but_prop_get(C, &ptr, &prop, &index); - if (ptr.owner_id && ptr.data && prop && RNA_property_drivable(&ptr, prop)) { + if (ptr.owner_id && ptr.data && prop && RNA_property_driver_editable(&ptr, prop)) { const bool is_array = RNA_property_array_check(prop); while (input->identifier) { @@ -927,7 +927,7 @@ static bool add_driver_button_poll(bContext *C) if (!(ptr.owner_id && ptr.data && prop)) { return false; } - if (!RNA_property_drivable(&ptr, prop)) { + if (!RNA_property_driver_editable(&ptr, prop)) { return false; } @@ -952,7 +952,7 @@ static int add_driver_button_none(bContext *C, wmOperator *op, short mapping_typ index = -1; } - if (ptr.owner_id && ptr.data && prop && RNA_property_drivable(&ptr, prop)) { + if (ptr.owner_id && ptr.data && prop && RNA_property_driver_editable(&ptr, prop)) { short flags = CREATEDRIVER_WITH_DEFAULT_DVAR; if (const std::optional path = RNA_path_from_ID_to_property(&ptr, prop)) { @@ -1043,7 +1043,7 @@ static int add_driver_button_invoke(bContext *C, wmOperator *op, const wmEvent * UI_context_active_but_prop_get(C, &ptr, &prop, &index); - if (ptr.owner_id && ptr.data && prop && RNA_property_drivable(&ptr, prop)) { + if (ptr.owner_id && ptr.data && prop && RNA_property_driver_editable(&ptr, prop)) { /* 1) Create a new "empty" driver for this property */ short flags = CREATEDRIVER_WITH_DEFAULT_DVAR; bool changed = false; @@ -1181,7 +1181,7 @@ static int copy_driver_button_exec(bContext *C, wmOperator *op) UI_context_active_but_prop_get(C, &ptr, &prop, &index); - if (ptr.owner_id && ptr.data && prop && RNA_property_drivable(&ptr, prop)) { + if (ptr.owner_id && ptr.data && prop && RNA_property_driver_editable(&ptr, prop)) { if (const std::optional path = RNA_path_from_ID_to_property(&ptr, prop)) { /* only copy the driver for the button that this was involved for */ changed = ANIM_copy_driver(op->reports, ptr.owner_id, path->c_str(), index, 0); @@ -1220,7 +1220,7 @@ static int paste_driver_button_exec(bContext *C, wmOperator *op) UI_context_active_but_prop_get(C, &ptr, &prop, &index); - if (ptr.owner_id && ptr.data && prop && RNA_property_drivable(&ptr, prop)) { + if (ptr.owner_id && ptr.data && prop && RNA_property_driver_editable(&ptr, prop)) { if (const std::optional path = RNA_path_from_ID_to_property(&ptr, prop)) { /* only copy the driver for the button that this was involved for */ changed = ANIM_paste_driver(op->reports, ptr.owner_id, path->c_str(), index, 0); diff --git a/source/blender/editors/animation/keyframing.cc b/source/blender/editors/animation/keyframing.cc index 273018eb274..809df7a8b01 100644 --- a/source/blender/editors/animation/keyframing.cc +++ b/source/blender/editors/animation/keyframing.cc @@ -902,7 +902,7 @@ static int insert_key_button_exec(bContext *C, wmOperator *op) return (OPERATOR_CANCELLED | OPERATOR_PASS_THROUGH); } - if ((ptr.owner_id && ptr.data && prop) && RNA_property_animateable(&ptr, prop)) { + if ((ptr.owner_id && ptr.data && prop) && RNA_property_anim_editable(&ptr, prop)) { if (ptr.type == &RNA_NlaStrip) { /* Handle special properties for NLA Strips, whose F-Curves are stored on the * strips themselves. These are stored separately or else the properties will @@ -1004,7 +1004,7 @@ static int insert_key_button_exec(bContext *C, wmOperator *op) } } else { - if (prop && !RNA_property_animateable(&ptr, prop)) { + if (prop && !RNA_property_anim_editable(&ptr, prop)) { BKE_reportf(op->reports, RPT_WARNING, "\"%s\" property cannot be animated", diff --git a/source/blender/editors/animation/keyingsets.cc b/source/blender/editors/animation/keyingsets.cc index eb5b9c8b960..eff6523b15d 100644 --- a/source/blender/editors/animation/keyingsets.cc +++ b/source/blender/editors/animation/keyingsets.cc @@ -305,7 +305,7 @@ static int add_keyingset_button_exec(bContext *C, wmOperator *op) /* Check if property is able to be added. */ const bool all = RNA_boolean_get(op->ptr, "all"); bool changed = false; - if (ptr.owner_id && ptr.data && prop && RNA_property_animateable(&ptr, prop)) { + if (ptr.owner_id && ptr.data && prop && RNA_property_anim_editable(&ptr, prop)) { if (const std::optional path = RNA_path_from_ID_to_property(&ptr, prop)) { if (all) { pflag |= KSP_FLAG_WHOLE_ARRAY; diff --git a/source/blender/editors/interface/eyedroppers/eyedropper_driver.cc b/source/blender/editors/interface/eyedroppers/eyedropper_driver.cc index 3ee4995da45..87d98cba734 100644 --- a/source/blender/editors/interface/eyedroppers/eyedropper_driver.cc +++ b/source/blender/editors/interface/eyedroppers/eyedropper_driver.cc @@ -54,7 +54,7 @@ static bool driverdropper_init(bContext *C, wmOperator *op) uiBut *but = UI_context_active_but_prop_get(C, &ddr->ptr, &ddr->prop, &ddr->index); if ((ddr->ptr.data == nullptr) || (ddr->prop == nullptr) || - (RNA_property_drivable(&ddr->ptr, ddr->prop) == false) || (but->flag & UI_BUT_DRIVEN)) + (RNA_property_driver_editable(&ddr->ptr, ddr->prop) == false) || (but->flag & UI_BUT_DRIVEN)) { MEM_freeN(ddr); return false; diff --git a/source/blender/editors/interface/interface_context_menu.cc b/source/blender/editors/interface/interface_context_menu.cc index 80acd1798e2..eac0fb63619 100644 --- a/source/blender/editors/interface/interface_context_menu.cc +++ b/source/blender/editors/interface/interface_context_menu.cc @@ -553,7 +553,7 @@ bool ui_popup_context_menu_for_button(bContext *C, uiBut *but, const wmEvent *ev PropertyRNA *prop = but->rnaprop; const PropertyType type = RNA_property_type(prop); const PropertySubType subtype = RNA_property_subtype(prop); - bool is_anim = RNA_property_animateable(ptr, prop); + bool is_anim = RNA_property_anim_editable(ptr, prop); const bool is_idprop = RNA_property_is_idprop(prop); /* second slower test, diff --git a/source/blender/editors/space_outliner/outliner_edit.cc b/source/blender/editors/space_outliner/outliner_edit.cc index a9d3d0a1bb5..aaa86123dec 100644 --- a/source/blender/editors/space_outliner/outliner_edit.cc +++ b/source/blender/editors/space_outliner/outliner_edit.cc @@ -1793,7 +1793,7 @@ static void do_outliner_drivers_editop(SpaceOutliner *space_outliner, PropertyRNA *prop = te_rna ? te_rna->get_property_rna() : nullptr; /* check if RNA-property described by this selected element is an animatable prop */ - if (prop && RNA_property_animateable(&ptr, prop)) { + if (prop && RNA_property_anim_editable(&ptr, prop)) { /* get id + path + index info from the selected element */ tree_element_to_path(te, tselem, &id, &path, &array_index, &flag, &groupmode); } @@ -1984,7 +1984,7 @@ static void do_outliner_keyingset_editop(SpaceOutliner *space_outliner, const TreeElementRNACommon *te_rna = tree_element_cast(te); PointerRNA ptr = te_rna->get_pointer_rna(); if (te_rna && te_rna->get_property_rna() && - RNA_property_animateable(&ptr, te_rna->get_property_rna())) + RNA_property_anim_editable(&ptr, te_rna->get_property_rna())) { /* get id + path + index info from the selected element */ tree_element_to_path(te, tselem, &id, &path, &array_index, &flag, &groupmode); diff --git a/source/blender/makesrna/RNA_access.hh b/source/blender/makesrna/RNA_access.hh index e7c0e55d3db..802cffb763d 100644 --- a/source/blender/makesrna/RNA_access.hh +++ b/source/blender/makesrna/RNA_access.hh @@ -303,14 +303,39 @@ bool RNA_property_editable_index(const PointerRNA *ptr, PropertyRNA *prop, const */ bool RNA_property_editable_flag(const PointerRNA *ptr, PropertyRNA *prop); +/** + * A property is animateable if its ID and the RNA property itself are defined as editable. + * It does not imply that user can _edit_ such animation though, see #RNA_property_anim_editable + * for this. + * + * This check is only based on information stored in the data _types_ (IDTypeInfo and RNA property + * definition), not on the actual data itself. + */ bool RNA_property_animateable(const PointerRNA *ptr, PropertyRNA *prop); +/** + * A property is anim-editable if it is animateable, and the related data is editable. + * + * Unlike #RNA_property_animateable, this check the actual data referenced by the RNA pointer and + * property, and not only their type info. + * + * Typically (with a few exceptions like the #PROP_LIB_EXCEPTION PropertyRNA flag), editable data + * belongs to local ID. + */ +bool RNA_property_anim_editable(const PointerRNA *ptr, PropertyRNA *prop); bool RNA_property_animated(PointerRNA *ptr, PropertyRNA *prop); /** - * With LibOverrides, a property may be animatable, but not drivable (in case the reference data - * already has an animation data, its Action can ba an editable local ID, but the drivers are - * directly stored in the animdata, overriding these is not supported currently). + * With LibOverrides, a property may be animatable and anim-editable, but not driver-editable (in + * case the reference data already has an animation data, its Action can ba an editable local ID, + * but the drivers are directly stored in the animdata, overriding these is not supported + * currently). + * + * Like #RNA_property_anim_editable, this also checks the actual data referenced by the RNA pointer + * and property. + * + * Currently, it is assumed that if an IDType and RNAProperty are animatable, they are also + * driveable, so #RNA_property_animateable can be used for drivers as well. */ -bool RNA_property_drivable(const PointerRNA *ptr, PropertyRNA *prop); +bool RNA_property_driver_editable(const PointerRNA *ptr, PropertyRNA *prop); /** * \note Does not take into account editable status, this has to be checked separately * (using #RNA_property_editable_flag() usually). diff --git a/source/blender/makesrna/intern/rna_access.cc b/source/blender/makesrna/intern/rna_access.cc index 00435c3ed34..74ba9ce3a92 100644 --- a/source/blender/makesrna/intern/rna_access.cc +++ b/source/blender/makesrna/intern/rna_access.cc @@ -10,7 +10,6 @@ #include #include #include -#include #include #include @@ -2100,13 +2099,12 @@ int RNA_property_ui_icon(const PropertyRNA *prop) static bool rna_property_editable_do(const PointerRNA *ptr, PropertyRNA *prop_orig, - std::optional prop_ensured, const int index, const char **r_info) { ID *id = ptr->owner_id; - PropertyRNA *prop = prop_ensured ? *prop_ensured : rna_ensure_property(prop_orig); + PropertyRNA *prop = rna_ensure_property(prop_orig); const char *info = ""; const int flag = (prop->itemeditable != nullptr && index >= 0) ? @@ -2162,12 +2160,12 @@ static bool rna_property_editable_do(const PointerRNA *ptr, bool RNA_property_editable(const PointerRNA *ptr, PropertyRNA *prop) { - return rna_property_editable_do(ptr, prop, std::nullopt, -1, nullptr); + return rna_property_editable_do(ptr, prop, -1, nullptr); } bool RNA_property_editable_info(const PointerRNA *ptr, PropertyRNA *prop, const char **r_info) { - return rna_property_editable_do(ptr, prop, std::nullopt, -1, r_info); + return rna_property_editable_do(ptr, prop, -1, r_info); } bool RNA_property_editable_flag(const PointerRNA *ptr, PropertyRNA *prop) @@ -2184,7 +2182,7 @@ bool RNA_property_editable_index(const PointerRNA *ptr, PropertyRNA *prop, const { BLI_assert(index >= 0); - return rna_property_editable_do(ptr, prop, std::nullopt, index, nullptr); + return rna_property_editable_do(ptr, prop, index, nullptr); } bool RNA_property_animateable(const PointerRNA *ptr, PropertyRNA *prop_orig) @@ -2194,6 +2192,22 @@ bool RNA_property_animateable(const PointerRNA *ptr, PropertyRNA *prop_orig) return false; } + PropertyRNA *prop_ensured = rna_ensure_property(prop_orig); + + if (!(prop_ensured->flag & PROP_ANIMATABLE)) { + return false; + } + + return true; +} + +bool RNA_property_anim_editable(const PointerRNA *ptr, PropertyRNA *prop_orig) +{ + /* check that base ID-block can support animation data */ + if (!RNA_property_animateable(ptr, prop_orig)) { + return false; + } + /* Linked or LibOverride Action IDs are not editable at the FCurve level. */ if (ptr->owner_id) { AnimData *anim_data = BKE_animdata_from_id(ptr->owner_id); @@ -2204,18 +2218,12 @@ bool RNA_property_animateable(const PointerRNA *ptr, PropertyRNA *prop_orig) } } - PropertyRNA *prop_ensured = rna_ensure_property(prop_orig); - - if (!(prop_ensured->flag & PROP_ANIMATABLE)) { - return false; - } - - return rna_property_editable_do(ptr, prop_orig, prop_ensured, -1, nullptr); + return rna_property_editable_do(ptr, prop_orig, -1, nullptr); } -bool RNA_property_drivable(const PointerRNA *ptr, PropertyRNA *prop) +bool RNA_property_driver_editable(const PointerRNA *ptr, PropertyRNA *prop) { - if (!RNA_property_animateable(ptr, prop)) { + if (!RNA_property_anim_editable(ptr, prop)) { return false; }