diff --git a/source/blender/animrig/ANIM_action.hh b/source/blender/animrig/ANIM_action.hh index 872d912a3d7..15a4cb090d1 100644 --- a/source/blender/animrig/ANIM_action.hh +++ b/source/blender/animrig/ANIM_action.hh @@ -163,9 +163,6 @@ class Action : public ::bAction { Binding *binding_find_by_name(StringRefNull binding_name); - Binding *binding_for_id(const ID &animated_id); - const Binding *binding_for_id(const ID &animated_id) const; - /** * Create a new, unused Binding. * diff --git a/source/blender/animrig/ANIM_animdata.hh b/source/blender/animrig/ANIM_animdata.hh index 691b45db32b..2f74d3c157c 100644 --- a/source/blender/animrig/ANIM_animdata.hh +++ b/source/blender/animrig/ANIM_animdata.hh @@ -59,15 +59,16 @@ bool animdata_remove_empty_action(AnimData *adt); * \note The returned FCurve should NOT be used for keyframe manipulation. Its * existence is an indicator for "this property is animated". * + * \note This function assumes that `adt->action` actually points to a layered + * Action. It is a bug to call this with a legacy Action, or without one. + * * This function should probably be limited to the active layer (for the given * property, once pinning to layers is there), so that the "this is keyed" color * is more accurate. * - * Again, this is just to hook up the new Animation data-block to the old - * Blender UI code. + * Again, this is just to hook up the layered Action to the old Blender UI code. */ -const FCurve *fcurve_find_by_rna_path(const Action &anim, - const ID &animated_id, +const FCurve *fcurve_find_by_rna_path(const AnimData &adt, StringRefNull rna_path, int array_index); diff --git a/source/blender/animrig/intern/animation.cc b/source/blender/animrig/intern/animation.cc index c076f9beb6b..eac3dfed2db 100644 --- a/source/blender/animrig/intern/animation.cc +++ b/source/blender/animrig/intern/animation.cc @@ -316,28 +316,6 @@ Binding *Action::binding_find_by_name(const StringRefNull binding_name) return nullptr; } -Binding *Action::binding_for_id(const ID &animated_id) -{ - const Binding *binding = const_cast(this)->binding_for_id(animated_id); - return const_cast(binding); -} - -const Binding *Action::binding_for_id(const ID &animated_id) const -{ - const AnimData *adt = BKE_animdata_from_id(&animated_id); - - /* Note that there is no check that `adt->action` is actually `this`. */ - - const Binding *binding = this->binding_for_handle(adt->binding_handle); - if (!binding) { - return nullptr; - } - if (!binding->is_suitable_for(animated_id)) { - return nullptr; - } - return binding; -} - Binding &Action::binding_allocate() { Binding &binding = MEM_new(__func__)->wrap(); diff --git a/source/blender/animrig/intern/animdata.cc b/source/blender/animrig/intern/animdata.cc index e8329b2ab96..1df10a0d0c4 100644 --- a/source/blender/animrig/intern/animdata.cc +++ b/source/blender/animrig/intern/animdata.cc @@ -184,21 +184,32 @@ void reevaluate_fcurve_errors(bAnimContext *ac) } } -const FCurve *fcurve_find_by_rna_path(const Action &anim, - const ID &animated_id, +const FCurve *fcurve_find_by_rna_path(const AnimData &adt, const StringRefNull rna_path, const int array_index) { - const Binding *binding = anim.binding_for_id(animated_id); + BLI_assert(adt.action); + if (!adt.action) { + return nullptr; + } + + const Action &action = adt.action->wrap(); + BLI_assert(action.is_action_layered()); + + const Binding *binding = action.binding_for_handle(adt.binding_handle); if (!binding) { /* No need to inspect anything if this ID does not have an animation Binding. */ return nullptr; } + /* No check for the binding's ID type. Not only do we not have the actual ID + * to do this check, but also, since the Action and the binding have been + * assigned, just trust that it's valid. */ + /* Iterate the layers top-down, as higher-up animation overrides (or at least can override) * lower-down animation. */ - for (int layer_idx = anim.layer_array_num - 1; layer_idx >= 0; layer_idx--) { - const Layer *layer = anim.layer(layer_idx); + for (int layer_idx = action.layer_array_num - 1; layer_idx >= 0; layer_idx--) { + const Layer *layer = action.layer(layer_idx); /* TODO: refactor this into something nicer once we have different strip types. */ for (const Strip *strip : layer->strips()) { diff --git a/source/blender/blenkernel/BKE_fcurve.hh b/source/blender/blenkernel/BKE_fcurve.hh index 657acfb3b6e..a840a87b474 100644 --- a/source/blender/blenkernel/BKE_fcurve.hh +++ b/source/blender/blenkernel/BKE_fcurve.hh @@ -314,8 +314,7 @@ int BKE_fcurves_filter(ListBase *dst, ListBase *src, const char *dataPrefix, con * property, what is returned is a best-effort guess. The topmost layer has priority, and it is * assumed that when it has a strip, it's infinite. */ -FCurve *BKE_animadata_fcurve_find_by_rna_path(const ID *id, - AnimData *animdata, +FCurve *BKE_animadata_fcurve_find_by_rna_path(AnimData *animdata, const char *rna_path, const int rna_index, bAction **r_action, diff --git a/source/blender/blenkernel/intern/fcurve.cc b/source/blender/blenkernel/intern/fcurve.cc index d667c787196..82218857c03 100644 --- a/source/blender/blenkernel/intern/fcurve.cc +++ b/source/blender/blenkernel/intern/fcurve.cc @@ -17,6 +17,7 @@ #include "ANIM_action.hh" #include "ANIM_animdata.hh" +#include "DNA_action_types.h" #include "DNA_anim_types.h" #include "DNA_object_types.h" #include "DNA_text_types.h" @@ -258,7 +259,7 @@ FCurve *id_data_find_fcurve( * needs to be re-checked I think?. */ bool is_driven = false; FCurve *fcu = BKE_animadata_fcurve_find_by_rna_path( - id, adt, path->c_str(), index, nullptr, &is_driven); + adt, path->c_str(), index, nullptr, &is_driven); if (is_driven) { if (r_driven != nullptr) { *r_driven = is_driven; @@ -355,34 +356,38 @@ int BKE_fcurves_filter(ListBase *dst, ListBase *src, const char *dataPrefix, con return matches; } -FCurve *action_fcurve_find_by_rna_path(const ID *id, - blender::animrig::Action &action, - const char *rna_path, - const int rna_index) +static std::optional> animdata_fcurve_find_by_rna_path( + AnimData &adt, const char *rna_path, const int rna_index) { + if (!adt.action) { + return std::nullopt; + } + + blender::animrig::Action &action = adt.action->wrap(); if (action.is_empty()) { - return nullptr; + return std::nullopt; } + FCurve *fcu = nullptr; if (action.is_action_layered()) { - BLI_assert(id); - const FCurve *fcu = blender::animrig::fcurve_find_by_rna_path( - action, *id, rna_path, rna_index); - - /* The new Animation data-block is stricter with const-ness than older code, hence the + const FCurve *const_fcurve = blender::animrig::fcurve_find_by_rna_path( + adt, rna_path, rna_index); + /* The new layered Action code is stricter with const-ness than older code, hence the * const_cast. */ - return const_cast(fcu); + fcu = const_cast(const_fcurve); + } + else { + fcu = BKE_fcurve_find(&action.curves, rna_path, rna_index); } - return BKE_fcurve_find(&action.curves, rna_path, rna_index); + if (!fcu) { + return std::nullopt; + } + return std::make_pair(fcu, &action); } -FCurve *BKE_animadata_fcurve_find_by_rna_path(const ID *id, - AnimData *animdata, - const char *rna_path, - int rna_index, - bAction **r_action, - bool *r_driven) +FCurve *BKE_animadata_fcurve_find_by_rna_path( + AnimData *animdata, const char *rna_path, int rna_index, bAction **r_action, bool *r_driven) { if (r_driven != nullptr) { *r_driven = false; @@ -391,17 +396,14 @@ FCurve *BKE_animadata_fcurve_find_by_rna_path(const ID *id, *r_action = nullptr; } - /* Action takes priority over drivers. */ - if (animdata->action) { - blender::animrig::Action &action = animdata->action->wrap(); - /* TODO: pass the binding handle instead of the id. */ - FCurve *fcurve = action_fcurve_find_by_rna_path(id, action, rna_path, rna_index); - if (fcurve) { - if (r_action) { - *r_action = &action; - } - return fcurve; + std::optional> found = animdata_fcurve_find_by_rna_path( + *animdata, rna_path, rna_index); + if (found) { + /* Action takes priority over drivers. */ + if (r_action) { + *r_action = found->second; } + return found->first; } /* If not animated, check if driven. */ @@ -496,7 +498,7 @@ FCurve *BKE_fcurve_find_by_rna_context_ui(bContext * /*C*/, /* Standard F-Curve from animdata - Animation (Action) or Drivers. */ FCurve *fcu = BKE_animadata_fcurve_find_by_rna_path( - ptr->owner_id, adt, rna_path->c_str(), rnaindex, r_action, r_driven); + adt, rna_path->c_str(), rnaindex, r_action, r_driven); if (fcu != nullptr && r_animdata != nullptr) { *r_animdata = adt; diff --git a/source/blender/blenkernel/intern/lib_override.cc b/source/blender/blenkernel/intern/lib_override.cc index 43dba1f8f38..3e286dccf10 100644 --- a/source/blender/blenkernel/intern/lib_override.cc +++ b/source/blender/blenkernel/intern/lib_override.cc @@ -361,12 +361,12 @@ bool BKE_lib_override_library_property_is_animated( const char index_token_start_backup = *index_token_start; *index_token_start = '\0'; fcurve = BKE_animadata_fcurve_find_by_rna_path( - id, anim_data, liboverride_prop->rna_path, rnaprop_index, nullptr, nullptr); + anim_data, liboverride_prop->rna_path, rnaprop_index, nullptr, nullptr); *index_token_start = index_token_start_backup; } else { fcurve = BKE_animadata_fcurve_find_by_rna_path( - id, anim_data, liboverride_prop->rna_path, 0, nullptr, nullptr); + anim_data, liboverride_prop->rna_path, 0, nullptr, nullptr); } if (fcurve != nullptr) { return true; diff --git a/source/blender/editors/animation/anim_channels_edit.cc b/source/blender/editors/animation/anim_channels_edit.cc index 101ff5de56d..0cee0dddbd5 100644 --- a/source/blender/editors/animation/anim_channels_edit.cc +++ b/source/blender/editors/animation/anim_channels_edit.cc @@ -4585,7 +4585,7 @@ static blender::Vector get_fcurves_of_property( const int length = RNA_property_array_length(ptr, prop); for (int i = 0; i < length; i++) { FCurve *fcurve = BKE_animadata_fcurve_find_by_rna_path( - id, anim_data, path->c_str(), i, nullptr, nullptr); + anim_data, path->c_str(), i, nullptr, nullptr); if (fcurve != nullptr) { fcurves.append(fcurve); } @@ -4593,7 +4593,7 @@ static blender::Vector get_fcurves_of_property( } else { FCurve *fcurve = BKE_animadata_fcurve_find_by_rna_path( - id, anim_data, path->c_str(), index, nullptr, nullptr); + anim_data, path->c_str(), index, nullptr, nullptr); if (fcurve != nullptr) { fcurves.append(fcurve); }