Fix ID user counting issues with NodeCustomGroup.
authorMiguel Porces <cmporces>
Sat, 16 Mar 2019 17:54:00 +0000 (18:54 +0100)
committerBrecht Van Lommel <brechtvanlommel@gmail.com>
Sat, 16 Mar 2019 19:21:16 +0000 (20:21 +0100)
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.

source/blender/blenkernel/BKE_node.h
source/blender/blenkernel/intern/node.c
source/blender/editors/space_node/node_edit.c
source/blender/editors/space_node/node_group.c
source/blender/editors/space_node/node_templates.c
source/blender/editors/transform/transform_conversions.c
source/blender/makesrna/intern/rna_nodetree.c
source/blender/nodes/shader/node_shader_tree.c
source/blender/nodes/texture/node_texture_tree.c
source/blender/windowmanager/intern/wm_init_exit.c

index 6e6ec31..951962d 100644 (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);
index 09b44db..57dcbcb 100644 (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 */
 
index b1cada9..1a18e88 100644 (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);
                }
        }
 
index 8c5bf60..808d937 100644 (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;
 
index aef36b4..b3edf98 100644 (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);
                }
        }
 }
index 43e038e..4383a32 100644 (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);
                        }
                }
        }
index 01da2be..74f47f7 100644 (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;
        }
index 9c00786..50bec04 100644 (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);
                }
        }
 
index f876ceb..6aeaf8d 100644 (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);
                }
        }
 }
index 946e7c4..00ab4fd 100644 (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();