Fix ID user counting issues with NodeCustomGroup.

User counting now happens before init() and after free() methods, so that
the ID users are in a valid state when Python might modify them. ID user
counting was moved into node.c and simplified.

Patch by Miguel with further refactoring by Brecht. Ref D4370.
This commit is contained in:
Miguel Porces 2019-03-16 18:54:00 +01:00 committed by Brecht Van Lommel
parent b9af4efe41
commit 5797a5fc65
10 changed files with 73 additions and 71 deletions

View File

@ -337,6 +337,7 @@ void ntreeUserDecrefID(struct bNodeTree *ntree);
struct bNodeTree *ntreeFromID(const struct ID *id);
void ntreeMakeLocal(struct Main *bmain, struct bNodeTree *ntree, bool id_in_mainlist, const bool lib_local);
void ntreeFreeLocalNode(struct bNodeTree *ntree, struct bNode *node);
void ntreeFreeLocalTree(struct bNodeTree *ntree);
struct bNode *ntreeFindType(const struct bNodeTree *ntree, int type);
bool ntreeHasType(const struct bNodeTree *ntree, int type);
@ -445,10 +446,8 @@ struct bNode *nodeAddStaticNode(const struct bContext *C, struct bNodeTree *ntre
void nodeUnlinkNode(struct bNodeTree *ntree, struct bNode *node);
void nodeUniqueName(struct bNodeTree *ntree, struct bNode *node);
/* Frees the node itself, without affect to anything else. */
void nodeFreeNode(struct bNodeTree *ntree, struct bNode *node);
/* Will additionally cleanup things like f-curves which uses this node. */
void nodeDeleteNode(struct Main *bmain, struct bNodeTree *ntree, struct bNode *node);
/* Delete node, associated animation data and ID user count. */
void nodeRemoveNode(struct Main *bmain, struct bNodeTree *ntree, struct bNode *node, bool do_id_user);
struct bNode *BKE_node_copy_ex(struct bNodeTree *ntree, struct bNode *node_src, const int flag);
@ -502,6 +501,7 @@ void ntreeTagUsedSockets(struct bNodeTree *ntree);
/* Node Clipboard */
void BKE_node_clipboard_init(struct bNodeTree *ntree);
void BKE_node_clipboard_clear(void);
void BKE_node_clipboard_free(void);
bool BKE_node_clipboard_validate(void);
void BKE_node_clipboard_add_node(struct bNode *node);
void BKE_node_clipboard_add_link(struct bNodeLink *link);

View File

@ -139,6 +139,9 @@ static void node_init(const struct bContext *C, bNodeTree *ntree, bNode *node)
if (ntree->typeinfo->node_add_init != NULL)
ntree->typeinfo->node_add_init(ntree, node);
if (node->id)
id_us_plus(node->id);
/* extra init callback */
if (ntype->initfunc_api) {
PointerRNA ptr;
@ -151,9 +154,6 @@ static void node_init(const struct bContext *C, bNodeTree *ntree, bNode *node)
ntype->initfunc_api(C, &ptr);
}
if (node->id)
id_us_plus(node->id);
node->flag |= NODE_INIT;
}
@ -1007,7 +1007,8 @@ bNode *BKE_node_copy_ex(bNodeTree *ntree, bNode *node_src, const int flag)
node_src->new_node = node_dst;
node_dst->new_node = NULL;
if (node_dst->typeinfo->copyfunc_api) {
bool do_copy_api = !((flag & LIB_ID_CREATE_NO_MAIN) || (flag & LIB_ID_COPY_LOCALIZE));
if (node_dst->typeinfo->copyfunc_api && do_copy_api) {
PointerRNA ptr;
RNA_pointer_create((ID *)ntree, &RNA_Node, node_dst, &ptr);
@ -1677,26 +1678,12 @@ static void node_unlink_attached(bNodeTree *ntree, bNode *parent)
}
}
/** \note caller needs to manage node->id user */
static void node_free_node_ex(
Main *bmain, bNodeTree *ntree, bNode *node,
bool remove_animdata, bool use_api_free_cb)
/* Free the node itself. ID user refcounting is up the caller,
* that does not happen here. */
static void node_free_node(bNodeTree *ntree, bNode *node)
{
bNodeSocket *sock, *nextsock;
/* don't remove node animdata if the tree is localized,
* Action is shared with the original tree (T38221)
*/
remove_animdata &= ntree && !(ntree->id.tag & LIB_TAG_LOCALIZED);
/* extra free callback */
if (use_api_free_cb && node->typeinfo->freefunc_api) {
PointerRNA ptr;
RNA_pointer_create((ID *)ntree, &RNA_Node, node, &ptr);
node->typeinfo->freefunc_api(&ptr);
}
/* since it is called while free database, node->id is undefined */
/* can be called for nodes outside a node tree (e.g. clipboard) */
@ -1707,20 +1694,6 @@ static void node_free_node_ex(
BLI_remlink(&ntree->nodes, node);
if (remove_animdata) {
char propname_esc[MAX_IDPROP_NAME * 2];
char prefix[MAX_IDPROP_NAME * 2];
BLI_strescape(propname_esc, node->name, sizeof(propname_esc));
BLI_snprintf(prefix, sizeof(prefix), "nodes[\"%s\"]", propname_esc);
if (BKE_animdata_fix_paths_remove((ID *)ntree, prefix)) {
if (bmain != NULL) {
DEG_relations_tag_update(bmain);
}
}
}
if (ntree->typeinfo->free_node_cache)
ntree->typeinfo->free_node_cache(ntree, node);
@ -1762,14 +1735,49 @@ static void node_free_node_ex(
ntree->update |= NTREE_UPDATE_NODES;
}
void nodeFreeNode(bNodeTree *ntree, bNode *node)
void ntreeFreeLocalNode(bNodeTree *ntree, bNode *node)
{
node_free_node_ex(NULL, ntree, node, false, true);
/* For removing nodes while editing localized node trees. */
BLI_assert((ntree->id.tag & LIB_TAG_LOCALIZED) != 0);
node_free_node(ntree, node);
}
void nodeDeleteNode(Main *bmain, bNodeTree *ntree, bNode *node)
void nodeRemoveNode(Main *bmain, bNodeTree *ntree, bNode *node, bool do_id_user)
{
node_free_node_ex(bmain, ntree, node, true, true);
/* This function is not for localized node trees, we do not want
* do to ID user refcounting and removal of animdation data then. */
BLI_assert((ntree->id.tag & LIB_TAG_LOCALIZED) == 0);
if (do_id_user) {
/* Free callback for NodeCustomGroup. */
if (node->typeinfo->freefunc_api) {
PointerRNA ptr;
RNA_pointer_create((ID *)ntree, &RNA_Node, node, &ptr);
node->typeinfo->freefunc_api(&ptr);
}
/* Do user counting. */
if (node->id) {
id_us_min(node->id);
}
}
/* Remove animation data. */
char propname_esc[MAX_IDPROP_NAME * 2];
char prefix[MAX_IDPROP_NAME * 2];
BLI_strescape(propname_esc, node->name, sizeof(propname_esc));
BLI_snprintf(prefix, sizeof(prefix), "nodes[\"%s\"]", propname_esc);
if (BKE_animdata_fix_paths_remove((ID *)ntree, prefix)) {
if (bmain != NULL) {
DEG_relations_tag_update(bmain);
}
}
/* Free node itself. */
node_free_node(ntree, node);
}
static void node_socket_interface_free(bNodeTree *UNUSED(ntree), bNodeSocket *sock)
@ -1804,7 +1812,8 @@ static void free_localized_node_groups(bNodeTree *ntree)
}
}
/** Free (or release) any data used by this nodetree (does not free the nodetree itself). */
/* Free (or release) any data used by this nodetree. Does not free the
* nodetree itself and does no ID user counting. */
void ntreeFreeTree(bNodeTree *ntree)
{
bNode *node, *next;
@ -1839,7 +1848,7 @@ void ntreeFreeTree(bNodeTree *ntree)
for (node = ntree->nodes.first; node; node = next) {
next = node->next;
node_free_node_ex(NULL, ntree, node, false, false);
node_free_node(ntree, node);
}
/* free interface sockets */
@ -2594,7 +2603,7 @@ void BKE_node_clipboard_clear(void)
for (node = node_clipboard.nodes.first; node; node = node_next) {
node_next = node->next;
node_free_node_ex(NULL, NULL, node, false, false);
node_free_node(NULL, node);
}
BLI_listbase_clear(&node_clipboard.nodes);
@ -2697,6 +2706,11 @@ int BKE_node_clipboard_get_type(void)
return node_clipboard.type;
}
void BKE_node_clipboard_free(void)
{
BKE_node_clipboard_validate();
BKE_node_clipboard_clear();
}
/* Node Instance Hash */

View File

@ -1590,11 +1590,8 @@ static int node_delete_exec(bContext *C, wmOperator *UNUSED(op))
for (node = snode->edittree->nodes.first; node; node = next) {
next = node->next;
if (node->flag & SELECT) {
/* check id user here, nodeFreeNode is called for free dbase too */
do_tag_update |= (do_tag_update || node_connected_to_output(bmain, snode->edittree, node));
if (node->id)
id_us_min(node->id);
nodeDeleteNode(bmain, snode->edittree, node);
nodeRemoveNode(bmain, snode->edittree, node, true);
}
}
@ -1684,11 +1681,7 @@ static int node_delete_reconnect_exec(bContext *C, wmOperator *UNUSED(op))
next = node->next;
if (node->flag & SELECT) {
nodeInternalRelink(snode->edittree, node);
/* check id user here, nodeFreeNode is called for free dbase too */
if (node->id)
id_us_min(node->id);
nodeDeleteNode(bmain, snode->edittree, node);
nodeRemoveNode(bmain, snode->edittree, node, true);
}
}

View File

@ -337,11 +337,11 @@ static int node_group_ungroup(Main *bmain, bNodeTree *ntree, bNode *gnode)
while (nodes_delayed_free) {
node = BLI_linklist_pop(&nodes_delayed_free);
nodeDeleteNode(bmain, ntree, node);
nodeRemoveNode(bmain, ntree, node, false);
}
/* delete the group instance */
nodeDeleteNode(bmain, ntree, gnode);
nodeRemoveNode(bmain, ntree, gnode, false);
ntree->update |= NTREE_UPDATE_NODES | NTREE_UPDATE_LINKS;

View File

@ -143,9 +143,7 @@ static void node_remove_linked(Main *bmain, bNodeTree *ntree, bNode *rem_node)
next = node->next;
if (node->flag & NODE_TEST) {
if (node->id)
id_us_min(node->id);
nodeDeleteNode(bmain, ntree, node);
nodeRemoveNode(bmain, ntree, node, true);
}
}
}

View File

@ -6442,7 +6442,7 @@ static void special_aftertrans_update__node(bContext *C, TransInfo *t)
for (node = ntree->nodes.first; node; node = node_next) {
node_next = node->next;
if (node->flag & NODE_SELECT)
nodeDeleteNode(bmain, ntree, node);
nodeRemoveNode(bmain, ntree, node, true);
}
}
}

View File

@ -741,8 +741,8 @@ static void rna_NodeTree_node_remove(bNodeTree *ntree, Main *bmain, ReportList *
return;
}
id_us_min(node->id);
nodeDeleteNode(bmain, ntree, node);
nodeRemoveNode(bmain, ntree, node, true);
RNA_POINTER_INVALIDATE(node_ptr);
ntreeUpdateTree(bmain, ntree); /* update group node socket links */
@ -759,10 +759,7 @@ static void rna_NodeTree_node_clear(bNodeTree *ntree, Main *bmain, ReportList *r
while (node) {
bNode *next_node = node->next;
if (node->id)
id_us_min(node->id);
nodeDeleteNode(bmain, ntree, node);
nodeRemoveNode(bmain, ntree, node, true);
node = next_node;
}

View File

@ -143,7 +143,7 @@ static void localize(bNodeTree *localtree, bNodeTree *UNUSED(ntree))
if (node->flag & NODE_MUTED || node->type == NODE_REROUTE) {
nodeInternalRelink(localtree, node);
nodeFreeNode(localtree, node);
ntreeFreeLocalNode(localtree, node);
}
}
}
@ -247,7 +247,7 @@ static bNode *ntree_shader_relink_output_from_group(bNodeTree *ntree,
SH_NODE_OUTPUT_WORLD,
SH_NODE_OUTPUT_LIGHT))
{
nodeFreeNode(ntree, node);
ntreeFreeLocalNode(ntree, node);
}
}

View File

@ -118,7 +118,7 @@ static void localize(bNodeTree *localtree, bNodeTree *UNUSED(ntree))
if (node->flag & NODE_MUTED || node->type == NODE_REROUTE) {
nodeInternalRelink(localtree, node);
nodeFreeNode(localtree, node);
ntreeFreeLocalNode(localtree, node);
}
}
}

View File

@ -497,6 +497,7 @@ void WM_exit_ext(bContext *C, const bool do_python)
BKE_tracking_clipboard_free();
BKE_mask_clipboard_free();
BKE_vfont_clipboard_free();
BKE_node_clipboard_free();
#ifdef WITH_COMPOSITOR
COM_deinitialize();
@ -518,7 +519,6 @@ void WM_exit_ext(bContext *C, const bool do_python)
ANIM_fmodifiers_copybuf_free();
ED_gpencil_anim_copybuf_free();
ED_gpencil_strokes_copybuf_free();
BKE_node_clipboard_clear();
/* free gizmo-maps after freeing blender, so no deleted data get accessed during cleaning up of areas */
wm_gizmomaptypes_free();