griefith/test2
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Anim, remove Action::binding_for_id() method

Browse Source 
Remove the `Action::binding_for_id()` method, as it is a bit dangerous,
only useful in a very specific situation, and can be removed altogether
with just a little bit of refactoring. This in turn made some other
functions a bit simpler too.

Almost no functional changes. The only change is that when through some
magic (aka data corruption or other bug) the Binding is no longer valid
for the animated ID's type, the IDs properties may still show as
animated in the interface.

Pull Request: https://projects.blender.org/blender/blender/pulls/121748
This commit is contained in:
Sybren A. Stüvel
2024-05-13 17:30:37 +02:00
parent 10b5a401dc
commit 0caf6397df
8 changed files with 58 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
source/blender/animrig/ANIM_action.hh
View File

@@ -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.
*

9
source/blender/animrig/ANIM_animdata.hh
View File

@@ -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);

22
source/blender/animrig/intern/animation.cc
View File

@@ -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<const Action *>(this)->binding_for_id(animated_id);
return const_cast<Binding *>(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<ActionBinding>(__func__)->wrap();

21
source/blender/animrig/intern/animdata.cc
View File

@@ -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()) {

3
source/blender/blenkernel/BKE_fcurve.hh
View File

@@ -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,

62
source/blender/blenkernel/intern/fcurve.cc
View File

@@ -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<std::pair<FCurve *, bAction *>> 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<FCurve *>(fcu);
fcu = const_cast<FCurve *>(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<std::pair<FCurve *, bAction *>> 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;

4
source/blender/blenkernel/intern/lib_override.cc
View File

@@ -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;

4
source/blender/editors/animation/anim_channels_edit.cc
View File

@@ -4585,7 +4585,7 @@ static blender::Vector<FCurve *> 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<FCurve *> 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);
}