diff options
author | Jacques Lucke <jacques@blender.org> | 2020-08-06 13:50:08 +0300 |
---|---|---|
committer | Jacques Lucke <jacques@blender.org> | 2020-08-06 13:50:08 +0300 |
commit | 769ec7ffe6f9510d4289381fbaea57529338b18e (patch) | |
tree | a79ca2637feeb5642396032894011a9b5411457a | |
parent | 82150f564136427b4e0436af41e6f1abd4fe2574 (diff) |
Fix T79408: ungroup operation update animation data incorrectly
Reviewers: sybren, sergey
Differential Revision: https://developer.blender.org/D8464
-rw-r--r-- | source/blender/blenkernel/BKE_animsys.h | 13 | ||||
-rw-r--r-- | source/blender/blenkernel/intern/anim_data.c | 86 | ||||
-rw-r--r-- | source/blender/editors/space_node/node_group.c | 73 |
3 files changed, 102 insertions, 70 deletions
diff --git a/source/blender/blenkernel/BKE_animsys.h b/source/blender/blenkernel/BKE_animsys.h index ddfe6b61cfb..583cac79d8e 100644 --- a/source/blender/blenkernel/BKE_animsys.h +++ b/source/blender/blenkernel/BKE_animsys.h @@ -148,17 +148,18 @@ bool BKE_animdata_fix_paths_remove(struct ID *id, const char *path); /* -------------------------------------- */ +typedef struct AnimationBasePathChange { + struct AnimationBasePathChange *next, *prev; + const char *src_basepath; + const char *dst_basepath; +} AnimationBasePathChange; + /* Move animation data from src to destination if it's paths are based on basepaths */ -void BKE_animdata_separate_by_basepath(struct Main *bmain, +void BKE_animdata_transfer_by_basepath(struct Main *bmain, struct ID *srcID, struct ID *dstID, struct ListBase *basepaths); -/* Move F-Curves from src to destination if it's path is based on basepath */ -void action_move_fcurves_by_basepath(struct bAction *srcAct, - struct bAction *dstAct, - const char basepath[]); - char *BKE_animdata_driver_path_hack(struct bContext *C, struct PointerRNA *ptr, struct PropertyRNA *prop, diff --git a/source/blender/blenkernel/intern/anim_data.c b/source/blender/blenkernel/intern/anim_data.c index 61181278c60..038a0d14ddb 100644 --- a/source/blender/blenkernel/intern/anim_data.c +++ b/source/blender/blenkernel/intern/anim_data.c @@ -505,24 +505,43 @@ static bool animpath_matches_basepath(const char path[], const char basepath[]) return (path && basepath) && STRPREFIX(path, basepath); } +static void animpath_update_basepath(FCurve *fcu, + const char *old_basepath, + const char *new_basepath) +{ + BLI_assert(animpath_matches_basepath(fcu->rna_path, old_basepath)); + if (STREQ(old_basepath, new_basepath)) { + return; + } + + char *new_path = BLI_sprintfN("%s%s", new_basepath, fcu->rna_path + strlen(old_basepath)); + MEM_freeN(fcu->rna_path); + fcu->rna_path = new_path; +} + /* Move F-Curves in src action to dst action, setting up all the necessary groups * for this to happen, but only if the F-Curves being moved have the appropriate * "base path". * - This is used when data moves from one data-block to another, causing the * F-Curves to need to be moved over too */ -void action_move_fcurves_by_basepath(bAction *srcAct, bAction *dstAct, const char basepath[]) +static void action_move_fcurves_by_basepath(bAction *srcAct, + bAction *dstAct, + const char *src_basepath, + const char *dst_basepath) { FCurve *fcu, *fcn = NULL; /* sanity checks */ - if (ELEM(NULL, srcAct, dstAct, basepath)) { + if (ELEM(NULL, srcAct, dstAct, src_basepath, dst_basepath)) { if (G.debug & G_DEBUG) { CLOG_ERROR(&LOG, - "srcAct: %p, dstAct: %p, basepath: %p has insufficient info to work with", + "srcAct: %p, dstAct: %p, src_basepath: %p, dst_basepath: %p has insufficient " + "info to work with", (void *)srcAct, (void *)dstAct, - (void *)basepath); + (void *)src_basepath, + (void *)dst_basepath); } return; } @@ -540,7 +559,7 @@ void action_move_fcurves_by_basepath(bAction *srcAct, bAction *dstAct, const cha /* should F-Curve be moved over? * - we only need the start of the path to match basepath */ - if (animpath_matches_basepath(fcu->rna_path, basepath)) { + if (animpath_matches_basepath(fcu->rna_path, src_basepath)) { bActionGroup *agrp = NULL; /* if grouped... */ @@ -562,6 +581,8 @@ void action_move_fcurves_by_basepath(bAction *srcAct, bAction *dstAct, const cha /* perform the migration now */ action_groups_remove_channel(srcAct, fcu); + animpath_update_basepath(fcu, src_basepath, dst_basepath); + if (agrp) { action_groups_add_channel(dstAct, agrp, fcu); } @@ -594,14 +615,31 @@ void action_move_fcurves_by_basepath(bAction *srcAct, bAction *dstAct, const cha } } +static void animdata_move_drivers_by_basepath(AnimData *srcAdt, + AnimData *dstAdt, + const char *src_basepath, + const char *dst_basepath) +{ + LISTBASE_FOREACH_MUTABLE (FCurve *, fcu, &srcAdt->drivers) { + if (animpath_matches_basepath(fcu->rna_path, src_basepath)) { + animpath_update_basepath(fcu, src_basepath, dst_basepath); + BLI_remlink(&srcAdt->drivers, fcu); + BLI_addtail(&dstAdt->drivers, fcu); + + /* TODO: add depsgraph flushing calls? */ + } + } +} + /* Transfer the animation data from srcID to dstID where the srcID * animation data is based off "basepath", creating new AnimData and - * associated data as necessary + * associated data as necessary. + * + * basepaths is a list of AnimationBasePathChange. */ -void BKE_animdata_separate_by_basepath(Main *bmain, ID *srcID, ID *dstID, ListBase *basepaths) +void BKE_animdata_transfer_by_basepath(Main *bmain, ID *srcID, ID *dstID, ListBase *basepaths) { AnimData *srcAdt = NULL, *dstAdt = NULL; - LinkData *ld; /* sanity checks */ if (ELEM(NULL, srcID, dstID)) { @@ -643,35 +681,19 @@ void BKE_animdata_separate_by_basepath(Main *bmain, ID *srcID, ID *dstID, ListBa } /* loop over base paths, trying to fix for each one... */ - for (ld = basepaths->first; ld; ld = ld->next) { - const char *basepath = (const char *)ld->data; - action_move_fcurves_by_basepath(srcAdt->action, dstAdt->action, basepath); + LISTBASE_FOREACH (const AnimationBasePathChange *, basepath_change, basepaths) { + action_move_fcurves_by_basepath(srcAdt->action, + dstAdt->action, + basepath_change->src_basepath, + basepath_change->dst_basepath); } } /* drivers */ if (srcAdt->drivers.first) { - FCurve *fcu, *fcn = NULL; - - /* check each driver against all the base paths to see if any should go */ - for (fcu = srcAdt->drivers.first; fcu; fcu = fcn) { - fcn = fcu->next; - - /* try each basepath in turn, but stop on the first one which works */ - for (ld = basepaths->first; ld; ld = ld->next) { - const char *basepath = (const char *)ld->data; - - if (animpath_matches_basepath(fcu->rna_path, basepath)) { - /* just need to change lists */ - BLI_remlink(&srcAdt->drivers, fcu); - BLI_addtail(&dstAdt->drivers, fcu); - - /* TODO: add depsgraph flushing calls? */ - - /* can stop now, as moved already */ - break; - } - } + LISTBASE_FOREACH (const AnimationBasePathChange *, basepath_change, basepaths) { + animdata_move_drivers_by_basepath( + srcAdt, dstAdt, basepath_change->src_basepath, basepath_change->dst_basepath); } } } diff --git a/source/blender/editors/space_node/node_group.c b/source/blender/editors/space_node/node_group.c index 8dc67ad48f5..7894fade517 100644 --- a/source/blender/editors/space_node/node_group.c +++ b/source/blender/editors/space_node/node_group.c @@ -180,6 +180,26 @@ void NODE_OT_group_edit(wmOperatorType *ot) /* ******************** Ungroup operator ********************** */ +/* The given paths will be owned by the returned instance. Both pointers are allowed to point to + * the same string. */ +static AnimationBasePathChange *animation_basepath_change_new(const char *src_basepath, + const char *dst_basepath) +{ + AnimationBasePathChange *basepath_change = MEM_callocN(sizeof(*basepath_change), AT); + basepath_change->src_basepath = src_basepath; + basepath_change->dst_basepath = dst_basepath; + return basepath_change; +} + +static void animation_basepath_change_free(AnimationBasePathChange *basepath_change) +{ + if (basepath_change->src_basepath != basepath_change->dst_basepath) { + MEM_freeN((void *)basepath_change->src_basepath); + } + MEM_freeN((void *)basepath_change->dst_basepath); + MEM_freeN(basepath_change); +} + /* returns 1 if its OK */ static int node_group_ungroup(Main *bmain, bNodeTree *ntree, bNode *gnode) { @@ -218,16 +238,11 @@ static int node_group_ungroup(Main *bmain, bNodeTree *ntree, bNode *gnode) /* keep track of this node's RNA "base" path (the part of the path identifying the node) * if the old nodetree has animation data which potentially covers this node */ + const char *old_animation_basepath = NULL; if (wgroup->adt) { PointerRNA ptr; - char *path; - RNA_pointer_create(&wgroup->id, &RNA_Node, node, &ptr); - path = RNA_path_from_ID_to_struct(&ptr); - - if (path) { - BLI_addtail(&anim_basepaths, BLI_genericNodeN(path)); - } + old_animation_basepath = RNA_path_from_ID_to_struct(&ptr); } /* migrate node */ @@ -237,6 +252,14 @@ static int node_group_ungroup(Main *bmain, bNodeTree *ntree, bNode *gnode) /* ensure unique node name in the node tree */ nodeUniqueName(ntree, node); + if (wgroup->adt) { + PointerRNA ptr; + RNA_pointer_create(&ntree->id, &RNA_Node, node, &ptr); + const char *new_animation_basepath = RNA_path_from_ID_to_struct(&ptr); + BLI_addtail(&anim_basepaths, + animation_basepath_change_new(old_animation_basepath, new_animation_basepath)); + } + if (!node->parent) { node->locx += gnode->locx; node->locy += gnode->locy; @@ -259,7 +282,6 @@ static int node_group_ungroup(Main *bmain, bNodeTree *ntree, bNode *gnode) /* and copy across the animation, * note that the animation data's action can be NULL here */ if (wgroup->adt) { - LinkData *ld, *ldn = NULL; bAction *waction; /* firstly, wgroup needs to temporary dummy action @@ -267,14 +289,11 @@ static int node_group_ungroup(Main *bmain, bNodeTree *ntree, bNode *gnode) waction = wgroup->adt->action = BKE_action_copy(bmain, wgroup->adt->action); /* now perform the moving */ - BKE_animdata_separate_by_basepath(bmain, &wgroup->id, &ntree->id, &anim_basepaths); + BKE_animdata_transfer_by_basepath(bmain, &wgroup->id, &ntree->id, &anim_basepaths); /* paths + their wrappers need to be freed */ - for (ld = anim_basepaths.first; ld; ld = ldn) { - ldn = ld->next; - - MEM_freeN(ld->data); - BLI_freelinkN(&anim_basepaths, ld); + LISTBASE_FOREACH_MUTABLE (AnimationBasePathChange *, basepath_change, &anim_basepaths) { + animation_basepath_change_free(basepath_change); } /* free temp action too */ @@ -459,7 +478,7 @@ static int node_group_separate_selected( path = RNA_path_from_ID_to_struct(&ptr); if (path) { - BLI_addtail(&anim_basepaths, BLI_genericNodeN(path)); + BLI_addtail(&anim_basepaths, animation_basepath_change_new(path, path)); } } @@ -512,17 +531,12 @@ static int node_group_separate_selected( /* and copy across the animation, * note that the animation data's action can be NULL here */ if (ngroup->adt) { - LinkData *ld, *ldn = NULL; - /* now perform the moving */ - BKE_animdata_separate_by_basepath(bmain, &ngroup->id, &ntree->id, &anim_basepaths); + BKE_animdata_transfer_by_basepath(bmain, &ngroup->id, &ntree->id, &anim_basepaths); /* paths + their wrappers need to be freed */ - for (ld = anim_basepaths.first; ld; ld = ldn) { - ldn = ld->next; - - MEM_freeN(ld->data); - BLI_freelinkN(&anim_basepaths, ld); + LISTBASE_FOREACH_MUTABLE (AnimationBasePathChange *, basepath_change, &anim_basepaths) { + animation_basepath_change_free(basepath_change); } } @@ -763,7 +777,7 @@ static void node_group_make_insert_selected(const bContext *C, bNodeTree *ntree, path = RNA_path_from_ID_to_struct(&ptr); if (path) { - BLI_addtail(&anim_basepaths, BLI_genericNodeN(path)); + BLI_addtail(&anim_basepaths, animation_basepath_change_new(path, path)); } } @@ -783,16 +797,11 @@ static void node_group_make_insert_selected(const bContext *C, bNodeTree *ntree, /* move animation data over */ if (ntree->adt) { - LinkData *ld, *ldn = NULL; - - BKE_animdata_separate_by_basepath(bmain, &ntree->id, &ngroup->id, &anim_basepaths); + BKE_animdata_transfer_by_basepath(bmain, &ntree->id, &ngroup->id, &anim_basepaths); /* paths + their wrappers need to be freed */ - for (ld = anim_basepaths.first; ld; ld = ldn) { - ldn = ld->next; - - MEM_freeN(ld->data); - BLI_freelinkN(&anim_basepaths, ld); + LISTBASE_FOREACH_MUTABLE (AnimationBasePathChange *, basepath_change, &anim_basepaths) { + animation_basepath_change_free(basepath_change); } } |