[Bf-blender-cvs] [64583f3e8d0] master: Animation: set Action `idroot` at assignment instead of just at evaluation

Sybren A. Stüvel noreply at git.blender.org
Fri Sep 25 14:21:39 CEST 2020


Commit: 64583f3e8d0f337a977c9a90b874bee733a37402
Author: Sybren A. Stüvel
Date:   Fri Sep 25 11:07:32 2020 +0200
Branches: master
https://developer.blender.org/rB64583f3e8d0f337a977c9a90b874bee733a37402

Animation: set Action `idroot` at assignment instead of just at evaluation

Actions are either locked to a specific ID type, or "floating". Actions
in the floating state are now locked when they are assigned to an ID block.
Previously (rB94b99b5d4a7c20cf2) this was done at evaluation time, which
caused various problems:
- The ID type was set on the evaluated copy, and inconsistently flushed
  back to the original.
- A newly created Action could not be assigned to an Action constraint,
  unless a depsgraph evaluation was be forced first.

This is now resolved by calling `BKE_animdata_set_action()` to set the
action (instead of direct assignment) where possible, and calling
`BKE_animdata_action_ensure_idroot()` otherwise.

Manifest Task: https://developer.blender.org/T80986

===================================================================

M	source/blender/blenkernel/BKE_anim_data.h
M	source/blender/blenkernel/intern/action.c
M	source/blender/blenkernel/intern/anim_data.c
M	source/blender/editors/animation/keyframing.c
M	source/blender/makesdna/DNA_anim_types.h
M	source/blender/makesrna/intern/rna_space.c

===================================================================

diff --git a/source/blender/blenkernel/BKE_anim_data.h b/source/blender/blenkernel/BKE_anim_data.h
index 48ea06ea9d8..5293d8dd9b0 100644
--- a/source/blender/blenkernel/BKE_anim_data.h
+++ b/source/blender/blenkernel/BKE_anim_data.h
@@ -58,6 +58,10 @@ bool BKE_animdata_set_action(struct ReportList *reports, struct ID *id, struct b
 
 bool BKE_animdata_action_editable(const struct AnimData *adt);
 
+/* Ensure that the action's idroot is set correctly given the ID type of the owner.
+ * Return true if it is, false if it was already set to an incompatible type. */
+bool BKE_animdata_action_ensure_idroot(const struct ID *owner, struct bAction *action);
+
 /* Free AnimData */
 void BKE_animdata_free(struct ID *id, const bool do_id_user);
 
diff --git a/source/blender/blenkernel/intern/action.c b/source/blender/blenkernel/intern/action.c
index 64a8ae15fd3..1ce0d07a135 100644
--- a/source/blender/blenkernel/intern/action.c
+++ b/source/blender/blenkernel/intern/action.c
@@ -47,6 +47,7 @@
 #include "BLT_translation.h"
 
 #include "BKE_action.h"
+#include "BKE_anim_data.h"
 #include "BKE_anim_visualization.h"
 #include "BKE_animsys.h"
 #include "BKE_armature.h"
@@ -1817,6 +1818,7 @@ void what_does_obaction(Object *ob,
     workob->adt = &adt;
 
     adt.action = act;
+    BKE_animdata_action_ensure_idroot(&workob->id, act);
 
     /* execute effects of Action on to workob (or it's PoseChannels) */
     BKE_animsys_evaluate_animdata(&workob->id, &adt, anim_eval_context, ADT_RECALC_ANIM, false);
diff --git a/source/blender/blenkernel/intern/anim_data.c b/source/blender/blenkernel/intern/anim_data.c
index 30b75734e77..eb2cdf585c4 100644
--- a/source/blender/blenkernel/intern/anim_data.c
+++ b/source/blender/blenkernel/intern/anim_data.c
@@ -186,6 +186,11 @@ bool BKE_animdata_set_action(ReportList *reports, ID *id, bAction *act)
     return false;
   }
 
+  if (adt->action == act) {
+    /* Don't bother reducing and increasing the user count when there is nothing changing. */
+    return true;
+  }
+
   if (!BKE_animdata_action_editable(adt)) {
     /* Cannot remove, otherwise things turn to custard. */
     BKE_report(reports, RPT_ERROR, "Cannot change action, as it is still being edited in NLA");
@@ -204,7 +209,7 @@ bool BKE_animdata_set_action(ReportList *reports, ID *id, bAction *act)
   }
 
   /* Action must have same type as owner. */
-  if (!ELEM(act->idroot, 0, GS(id->name))) {
+  if (!BKE_animdata_action_ensure_idroot(id, act)) {
     /* Cannot set to this type. */
     BKE_reportf(
         reports,
@@ -230,6 +235,19 @@ bool BKE_animdata_action_editable(const AnimData *adt)
   return !is_tweaking_strip;
 }
 
+bool BKE_animdata_action_ensure_idroot(const ID *owner, bAction *action)
+{
+  const int idcode = GS(owner->name);
+
+  if (action->idroot == 0) {
+    /* First time this Action is assigned, lock it to this ID type. */
+    action->idroot = idcode;
+    return true;
+  }
+
+  return (action->idroot == idcode);
+}
+
 /* Freeing -------------------------------------------- */
 
 /* Free AnimData used by the nominated ID-block, and clear ID-block's AnimData pointer */
@@ -673,6 +691,7 @@ void BKE_animdata_transfer_by_basepath(Main *bmain, ID *srcID, ID *dstID, ListBa
      * and name it in a similar way so that it can be easily found again. */
     if (dstAdt->action == NULL) {
       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) {
       CLOG_WARN(&LOG,
@@ -685,6 +704,7 @@ void BKE_animdata_transfer_by_basepath(Main *bmain, ID *srcID, ID *dstID, ListBa
       /* 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);
     }
 
     /* loop over base paths, trying to fix for each one... */
diff --git a/source/blender/editors/animation/keyframing.c b/source/blender/editors/animation/keyframing.c
index fb4c0ae0758..3701e9dc91c 100644
--- a/source/blender/editors/animation/keyframing.c
+++ b/source/blender/editors/animation/keyframing.c
@@ -162,7 +162,7 @@ bAction *ED_id_action_ensure(Main *bmain, ID *id)
      * so that users can't accidentally break actions by assigning them
      * to the wrong places
      */
-    adt->action->idroot = GS(id->name);
+    BKE_animdata_action_ensure_idroot(id, adt->action);
 
     /* Tag depsgraph to be rebuilt to include time dependency. */
     DEG_relations_tag_update(bmain);
diff --git a/source/blender/makesdna/DNA_anim_types.h b/source/blender/makesdna/DNA_anim_types.h
index 858daaac47c..7cb9978f768 100644
--- a/source/blender/makesdna/DNA_anim_types.h
+++ b/source/blender/makesdna/DNA_anim_types.h
@@ -1047,8 +1047,12 @@ typedef struct AnimOverride {
  * See blenkernel/intern/anim_sys.c for details.
  */
 typedef struct AnimData {
-  /** active action - acts as the 'tweaking track' for the NLA */
+  /**
+   * Active action - acts as the 'tweaking track' for the NLA.
+   * Either use BKE_animdata_set_action() to set this, or call BKE_animdata_action_ensure_idroot()
+   * after setting. */
   bAction *action;
+
   /** temp-storage for the 'real' active action (i.e. the one used before the tweaking-action
    * took over to be edited in the Animation Editors)
    */
diff --git a/source/blender/makesrna/intern/rna_space.c b/source/blender/makesrna/intern/rna_space.c
index 59a974b8b0b..3fa577d4230 100644
--- a/source/blender/makesrna/intern/rna_space.c
+++ b/source/blender/makesrna/intern/rna_space.c
@@ -1929,10 +1929,12 @@ static void rna_SpaceDopeSheetEditor_action_update(bContext *C, PointerRNA *ptr)
   }
 
   AnimData *adt = NULL;
+  ID *id = NULL;
   switch (saction->mode) {
     case SACTCONT_ACTION:
       /* TODO: context selector could help decide this with more control? */
       adt = BKE_animdata_add_id(&obact->id);
+      id = &obact->id;
       break;
     case SACTCONT_SHAPEKEY: {
       Key *key = BKE_key_from_object(obact);
@@ -1940,6 +1942,7 @@ static void rna_SpaceDopeSheetEditor_action_update(bContext *C, PointerRNA *ptr)
         return;
       }
       adt = BKE_animdata_add_id(&key->id);
+      id = &key->id;
       break;
     }
     case SACTCONT_GPENCIL:
@@ -1963,40 +1966,24 @@ static void rna_SpaceDopeSheetEditor_action_update(bContext *C, PointerRNA *ptr)
   /* Exit editmode first - we cannot change actions while in tweakmode. */
   BKE_nla_tweakmode_exit(adt);
 
-  /* NLA Tweak Mode needs special handling... */
-  if (adt->flag & ADT_NLA_EDIT_ON) {
-    /* Assign new action, and adjust the usercounts accordingly */
-    adt->action = saction->action;
-    id_us_plus((ID *)adt->action);
+  /* To prevent data loss (i.e. if users flip between actions using the Browse menu),
+   * stash this action if nothing else uses it.
+   *
+   * EXCEPTION:
+   * This callback runs when unlinking actions. In that case, we don't want to
+   * stash the action, as the user is signaling that they want to detach it.
+   * This can be reviewed again later,
+   * but it could get annoying if we keep these instead.
+   */
+  if (adt->action != NULL && adt->action->id.us <= 0 && saction->action != NULL) {
+    /* XXX: Things here get dodgy if this action is only partially completed,
+     *      and the user then uses the browse menu to get back to this action,
+     *      assigning it as the active action (i.e. the stash strip gets out of sync)
+     */
+    BKE_nla_action_stash(adt);
   }
-  else {
-    /* Handle old action... */
-    if (adt->action) {
-      /* Fix id-count of action we're replacing */
-      id_us_min(&adt->action->id);
-
-      /* To prevent data loss (i.e. if users flip between actions using the Browse menu),
-       * stash this action if nothing else uses it.
-       *
-       * EXCEPTION:
-       * This callback runs when unlinking actions. In that case, we don't want to
-       * stash the action, as the user is signaling that they want to detach it.
-       * This can be reviewed again later,
-       * but it could get annoying if we keep these instead.
-       */
-      if ((adt->action->id.us <= 0) && (saction->action != NULL)) {
-        /* XXX: Things here get dodgy if this action is only partially completed,
-         *      and the user then uses the browse menu to get back to this action,
-         *      assigning it as the active action (i.e. the stash strip gets out of sync)
-         */
-        BKE_nla_action_stash(adt);
-      }
-    }
 
-    /* Assign new action, and adjust the usercounts accordingly */
-    adt->action = saction->action;
-    id_us_plus((ID *)adt->action);
-  }
+  BKE_animdata_set_action(NULL, id, saction->action);
 
   DEG_id_tag_update(&obact->id, ID_RECALC_ANIMATION | ID_RECALC_TRANSFORM | ID_RECALC_GEOMETRY);



More information about the Bf-blender-cvs mailing list