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

Anim: migrate Action assignments to the new API

Browse Source 
Instead of assigning Actions by direct pointer manipulation (and the
corresponding juggling of user counts), call `animrig::assign_action()`
and `animrig::unassign_action()`.

These functions not only correctly handle user counts, but also ensure
that slot assignments & user tracking works. The former always happens,
the latter only when building with experimental features enabled.

Because (un)assigning slotted Actions need the animated ID (instead of
just the `AnimData *`), more functions now require an `OwnedAnimData`.

Note that there is still some user count juggling. This is caused by
`BKE_id_new()`, and by extension `BKE_action_add`, returning an ID with
user count = 1, even though that ID is not yet used. A todo task #128017
has been made to change `BKE_action_add()` so that the Action it returns
can be directly fed into `animrig::assign_action()`.

This PR updates the following areas:

- Creating a node group by grouping animated nodes, as this has to move
  the animation data. This PR just handles the assignment of a new
  Action.
- Temporary Action creation for ungrouping node groups.
- Versioning of pre-2.5 animation data.

No functional changes.

Ref: #123424

Pull Request: https://projects.blender.org/blender/blender/pulls/128026
This commit is contained in:
Sybren A. Stüvel
2024-09-23 15:53:32 +02:00
parent cd1c7039ac
commit 6115132998
7 changed files with 51 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -1131,10 +1131,19 @@ void assign_tmpaction(bAction *action, OwnedAnimData owned_adt);
*
* Same as calling `assign_action(nullptr, animated_id)`.
*
* \see assign_action
* \see blender::animrig::assign_action(ID &animated_id)
*/
void unassign_action(ID &animated_id);
/**
* Un-assign the Action assigned to this ID.
*
* Same as calling `assign_action(nullptr, owned_adt)`.
*
* \see blender::animrig::assign_action(OwnedAnimData owned_adt)
*/
void unassign_action(OwnedAnimData owned_adt);
/**
* Assign the Action, ensuring that a Slot is also assigned.
*

5
source/blender/animrig/ANIM_action_legacy.hh
View File

@@ -34,6 +34,11 @@ ChannelBag *channelbag_get(Action &action);
*/
ChannelBag &channelbag_ensure(Action &action);
/**
* Return whether this Action has any F-Curves.
*/
bool action_has_fcurves(const bAction &action);
/**
* Return all F-Curves in the Action.
*

5
source/blender/animrig/intern/action.cc
View File

@@ -1175,6 +1175,11 @@ void unassign_action(ID &animated_id)
assign_action(nullptr, animated_id);
}
void unassign_action(OwnedAnimData owned_adt)
{
assign_action(nullptr, owned_adt);
}
Slot *assign_action_ensure_slot_for_keying(Action &action, ID &animated_id)
{
Slot *slot;

28
source/blender/blenkernel/intern/anim_data.cc
View File

@@ -57,7 +57,7 @@
static CLG_LogRef LOG = {"bke.anim_sys"};
using blender::FunctionRef;
using namespace blender;
/* ***************************************** */
/* AnimData API */
@@ -753,24 +753,26 @@ void BKE_animdata_transfer_by_basepath(Main *bmain, ID *srcID, ID *dstID, ListBa
/* active action */
if (srcAdt->action) {
/* Set up an action if necessary,
* and name it in a similar way so that it can be easily found again. */
if (dstAdt->action == nullptr) {
dstAdt->action = BKE_action_add(bmain, srcAdt->action->id.name + 2);
BKE_animdata_action_ensure_idroot(dstID, dstAdt->action);
}
else if (dstAdt->action == srcAdt->action) {
const OwnedAnimData dst_owned_adt = {*dstID, *dstAdt};
if (dstAdt->action == srcAdt->action) {
CLOG_WARN(&LOG,
"Argh! Source and Destination share animation! "
"Source and Destination share animation! "
"('%s' and '%s' both use '%s') Making new empty action",
srcID->name,
dstID->name,
srcAdt->action->id.name);
/* TODO: review this... */
id_us_min(&dstAdt->action->id);
dstAdt->action = BKE_action_add(bmain, dstAdt->action->id.name + 2);
BKE_animdata_action_ensure_idroot(dstID, dstAdt->action);
/* This sets dstAdt->action to nullptr. */
animrig::unassign_action(dst_owned_adt);
}
/* Set up an action if necessary, and name it in a similar way so that it
* can be easily found again. */
if (!dstAdt->action) {
bAction *new_action = BKE_action_add(bmain, srcAdt->action->id.name + 2);
/* Reduce user count to 0 as the Action is unused before it's assigned. */
id_us_min(&new_action->id);
animrig::assign_action(new_action, dst_owned_adt);
}
/* loop over base paths, trying to fix for each one... */

20
source/blender/blenkernel/intern/ipo.cc
View File

@@ -56,6 +56,8 @@
#include "BKE_main.hh"
#include "BKE_nla.hh"
#include "ANIM_action.hh"
#include "CLG_log.h"
#include "MEM_guardedalloc.h"
@@ -70,6 +72,8 @@
static CLG_LogRef LOG = {"bke.ipo"};
using namespace blender;
static void ipo_free_data(ID *id)
{
Ipo *ipo = (Ipo *)id;
@@ -1874,7 +1878,9 @@ static void ipo_to_animdata(
SNPRINTF(nameBuf, "CDA:%s", ipo->id.name + 2);
adt->action = BKE_action_add(bmain, nameBuf);
bAction *action = BKE_action_add(bmain, nameBuf);
id_us_min(&action->id);
animrig::assign_action(action, {*id, *adt});
if (G.debug & G_DEBUG) {
printf("\t\tadded new action - '%s'\n", nameBuf);
}
@@ -1912,7 +1918,7 @@ static void action_to_animdata(ID *id, bAction *act)
if (G.debug & G_DEBUG) {
printf("act_to_adt - set adt action to act\n");
}
adt->action = act;
animrig::assign_action(act, {*id, *adt});
}
/* convert Action data */
@@ -2127,18 +2133,12 @@ void do_versions_ipos_to_animato(Main *bmain)
nlastrips_to_animdata(id, &ob->nlastrips);
}
else if ((ob->ipo) || (ob->action)) {
/* Add AnimData block */
AnimData *adt = BKE_animdata_ensure_id(id);
/* Action first - so that Action name get conserved */
if (ob->action) {
action_to_animdata(id, ob->action);
/* Only decrease user-count if this Action isn't now being used by AnimData. */
if (ob->action != adt->action) {
id_us_min(&ob->action->id);
ob->action = nullptr;
}
id_us_min(&ob->action->id);
ob->action = nullptr;
}
/* IPO second... */

1
source/blender/editors/space_node/CMakeLists.txt
View File

@@ -52,6 +52,7 @@ set(SRC
)
set(LIB
PRIVATE bf::animrig
PRIVATE bf::blenfont
bf_blenkernel
PRIVATE bf::blenlib

9
source/blender/editors/space_node/node_group.cc
View File

@@ -33,6 +33,8 @@
#include "BKE_node_tree_update.hh"
#include "BKE_report.hh"
#include "ANIM_action.hh"
#include "DEG_depsgraph_build.hh"
#include "ED_node.hh" /* own include */
@@ -341,11 +343,10 @@ static void node_group_ungroup(Main *bmain, bNodeTree *ntree, bNode *gnode)
/* and copy across the animation,
* note that the animation data's action can be nullptr here */
if (wgroup->adt) {
bAction *waction;
/* firstly, wgroup needs to temporary dummy action
* that can be destroyed, as it shares copies */
waction = wgroup->adt->action = (bAction *)BKE_id_copy(bmain, &wgroup->adt->action->id);
bAction *waction = reinterpret_cast<bAction *>(BKE_id_copy(bmain, &wgroup->adt->action->id));
animrig::assign_action(waction, {wgroup->id, *wgroup->adt});
/* now perform the moving */
BKE_animdata_transfer_by_basepath(bmain, &wgroup->id, &ntree->id, &anim_basepaths);
@@ -357,8 +358,8 @@ static void node_group_ungroup(Main *bmain, bNodeTree *ntree, bNode *gnode)
/* free temp action too */
if (waction) {
animrig::unassign_action({wgroup->id, *wgroup->adt});
BKE_id_free(bmain, waction);
wgroup->adt->action = nullptr;
}
}