From d411be8a99fe47dbedf9110823d321b44727591f Mon Sep 17 00:00:00 2001 From: Iliya Katueshenock Date: Fri, 10 Feb 2023 17:30:55 +0100 Subject: [PATCH] Cleanup: Use utility function to find groups in node tree Add `contains_group` method in python api for `NodeTree` type, cleanup `ntreeHasTree` function, reuse `ntreeHasTree` in more place in code. The algorithm has been changed to not recheck trees by using set. Performance gains from avoiding already checked node trees: Based on tests, can say that for large files with a huge number of trees, the response speed of opening the search menu in the node editor increased by ~200 times (for really large projects with 16 individual groups in 6 levels of nesting). Group insert operations are also accelerated, but this is different in some cases. Pull Request #104465 --- .../scripts/startup/bl_ui/node_add_menu.py | 11 +----- release/scripts/startup/nodeitems_builtins.py | 12 +------ source/blender/blenkernel/BKE_node.h | 8 ++++- source/blender/blenkernel/intern/node.cc | 34 +++++++++++++++---- .../blender/editors/space_node/node_edit.cc | 4 +-- .../blender/editors/space_node/node_group.cc | 20 +---------- source/blender/makesrna/intern/rna_nodetree.c | 15 ++++++++ 7 files changed, 54 insertions(+), 50 deletions(-) diff --git a/release/scripts/startup/bl_ui/node_add_menu.py b/release/scripts/startup/bl_ui/node_add_menu.py index 873dbd533a5..a0dad139243 100644 --- a/release/scripts/startup/bl_ui/node_add_menu.py +++ b/release/scripts/startup/bl_ui/node_add_menu.py @@ -35,19 +35,10 @@ def draw_node_group_add_menu(context, layout): if node_tree: from nodeitems_builtins import node_tree_group_type - def contains_group(nodetree, group): - if nodetree == group: - return True - for node in nodetree.nodes: - if node.bl_idname in node_tree_group_type.values() and node.node_tree is not None: - if contains_group(node.node_tree, group): - return True - return False - groups = [ group for group in context.blend_data.node_groups if (group.bl_idname == node_tree.bl_idname and - not contains_group(group, node_tree) and + not group.contains_tree(node_tree) and not group.name.startswith('.')) ] if groups: diff --git a/release/scripts/startup/nodeitems_builtins.py b/release/scripts/startup/nodeitems_builtins.py index 2856e33931b..3deec135888 100644 --- a/release/scripts/startup/nodeitems_builtins.py +++ b/release/scripts/startup/nodeitems_builtins.py @@ -76,21 +76,11 @@ def node_group_items(context): yield NodeItemCustom(draw=lambda self, layout, context: layout.separator()) - def contains_group(nodetree, group): - if nodetree == group: - return True - else: - for node in nodetree.nodes: - if node.bl_idname in node_tree_group_type.values() and node.node_tree is not None: - if contains_group(node.node_tree, group): - return True - return False - for group in context.blend_data.node_groups: if group.bl_idname != ntree.bl_idname: continue # filter out recursive groups - if contains_group(group, ntree): + if group.contains_tree(ntree): continue # filter out hidden nodetrees if group.name.startswith('.'): diff --git a/source/blender/blenkernel/BKE_node.h b/source/blender/blenkernel/BKE_node.h index c0690c1c3a8..f387122079b 100644 --- a/source/blender/blenkernel/BKE_node.h +++ b/source/blender/blenkernel/BKE_node.h @@ -504,7 +504,13 @@ struct bNodeTree *ntreeFromID(struct ID *id); void ntreeFreeLocalNode(struct bNodeTree *ntree, struct bNode *node); void ntreeFreeLocalTree(struct bNodeTree *ntree); struct bNode *ntreeFindType(struct bNodeTree *ntree, int type); -bool ntreeHasTree(const struct bNodeTree *ntree, const struct bNodeTree *lookup); + +/** + * Check recursively if a node tree contains another. + */ +bool ntreeContainsTree(const struct bNodeTree *tree_to_search_in, + const struct bNodeTree *tree_to_search_for); + void ntreeUpdateAllNew(struct Main *main); void ntreeUpdateAllUsers(struct Main *main, struct ID *id); diff --git a/source/blender/blenkernel/intern/node.cc b/source/blender/blenkernel/intern/node.cc index cb64ed07e1d..0d18709a6d6 100644 --- a/source/blender/blenkernel/intern/node.cc +++ b/source/blender/blenkernel/intern/node.cc @@ -3457,21 +3457,41 @@ bNode *ntreeFindType(bNodeTree *ntree, int type) return nullptr; } -bool ntreeHasTree(const bNodeTree *ntree, const bNodeTree *lookup) +static bool ntree_contains_tree_exec(const bNodeTree *tree_to_search_in, + const bNodeTree *tree_to_search_for, + Set &already_passed) { - if (ntree == lookup) { + if (tree_to_search_in == tree_to_search_for) { return true; } - for (const bNode *node : ntree->all_nodes()) { - if (ELEM(node->type, NODE_GROUP, NODE_CUSTOM_GROUP) && node->id) { - if (ntreeHasTree((bNodeTree *)node->id, lookup)) { - return true; - } + + tree_to_search_in->ensure_topology_cache(); + for (const bNode *node_group : tree_to_search_in->group_nodes()) { + const bNodeTree *sub_tree_search_in = reinterpret_cast(node_group->id); + if (!sub_tree_search_in) { + continue; + } + if (!already_passed.add(sub_tree_search_in)) { + continue; + } + if (ntree_contains_tree_exec(sub_tree_search_in, tree_to_search_for, already_passed)) { + return true; } } + return false; } +bool ntreeContainsTree(const bNodeTree *tree_to_search_in, const bNodeTree *tree_to_search_for) +{ + if (tree_to_search_in == tree_to_search_for) { + return true; + } + + Set already_passed; + return ntree_contains_tree_exec(tree_to_search_in, tree_to_search_for, already_passed); +} + bNodeLink *nodeFindLink(bNodeTree *ntree, const bNodeSocket *from, const bNodeSocket *to) { LISTBASE_FOREACH (bNodeLink *, link, &ntree->links) { diff --git a/source/blender/editors/space_node/node_edit.cc b/source/blender/editors/space_node/node_edit.cc index fc039746639..3ce4a9bf22f 100644 --- a/source/blender/editors/space_node/node_edit.cc +++ b/source/blender/editors/space_node/node_edit.cc @@ -715,7 +715,7 @@ void ED_node_set_active( if ((node->flag & NODE_ACTIVE_TEXTURE) && !was_active_texture) { /* If active texture changed, free glsl materials. */ LISTBASE_FOREACH (Material *, ma, &bmain->materials) { - if (ma->nodetree && ma->use_nodes && ntreeHasTree(ma->nodetree, ntree)) { + if (ma->nodetree && ma->use_nodes && ntreeContainsTree(ma->nodetree, ntree)) { GPU_material_free(&ma->gpumaterial); /* Sync to active texpaint slot, otherwise we can end up painting on a different slot @@ -734,7 +734,7 @@ void ED_node_set_active( } LISTBASE_FOREACH (World *, wo, &bmain->worlds) { - if (wo->nodetree && wo->use_nodes && ntreeHasTree(wo->nodetree, ntree)) { + if (wo->nodetree && wo->use_nodes && ntreeContainsTree(wo->nodetree, ntree)) { GPU_material_free(&wo->gpumaterial); } } diff --git a/source/blender/editors/space_node/node_group.cc b/source/blender/editors/space_node/node_group.cc index ca0361e82dc..7a025f05b10 100644 --- a/source/blender/editors/space_node/node_group.cc +++ b/source/blender/editors/space_node/node_group.cc @@ -1094,24 +1094,6 @@ void NODE_OT_group_make(wmOperatorType *ot) /** \name Group Insert Operator * \{ */ -static bool node_tree_contains_tree_recursive(const bNodeTree &ntree_to_search_in, - const bNodeTree &ntree_to_search_for) -{ - if (&ntree_to_search_in == &ntree_to_search_for) { - return true; - } - ntree_to_search_in.ensure_topology_cache(); - for (const bNode *node : ntree_to_search_in.group_nodes()) { - if (node->id) { - if (node_tree_contains_tree_recursive(*reinterpret_cast(node->id), - ntree_to_search_for)) { - return true; - } - } - } - return false; -} - static int node_group_insert_exec(bContext *C, wmOperator *op) { SpaceNode *snode = CTX_wm_space_node(C); @@ -1133,7 +1115,7 @@ static int node_group_insert_exec(bContext *C, wmOperator *op) if (!group->is_group() || group->id == nullptr) { continue; } - if (node_tree_contains_tree_recursive(*reinterpret_cast(group->id), *ngroup)) { + if (ntreeContainsTree(reinterpret_cast(group->id), ngroup)) { BKE_reportf( op->reports, RPT_WARNING, "Can not insert group '%s' in '%s'", group->name, gnode->name); return OPERATOR_CANCELLED; diff --git a/source/blender/makesrna/intern/rna_nodetree.c b/source/blender/makesrna/intern/rna_nodetree.c index 7ba3971b4ea..ec81cf2ddfc 100644 --- a/source/blender/makesrna/intern/rna_nodetree.c +++ b/source/blender/makesrna/intern/rna_nodetree.c @@ -1435,6 +1435,11 @@ static void rna_NodeTree_active_output_set(PointerRNA *ptr, int value) } } +static bool rna_NodeTree_contains_tree(bNodeTree *tree, bNodeTree *sub_tree) +{ + return ntreeContainsTree(tree, sub_tree); +} + static bNodeSocket *rna_NodeTree_inputs_new( bNodeTree *ntree, Main *bmain, ReportList *reports, const char *type, const char *name) { @@ -12719,6 +12724,16 @@ static void rna_def_nodetree(BlenderRNA *brna) parm = RNA_def_pointer(func, "context", "Context", "", ""); RNA_def_parameter_flags(parm, PROP_NEVER_NULL, PARM_REQUIRED); + func = RNA_def_function(srna, "contains_tree", "rna_NodeTree_contains_tree"); + RNA_def_function_ui_description( + func, + "Check if the node tree contains another. Used to avoid creating recursive node groups"); + parm = RNA_def_pointer( + func, "sub_tree", "NodeTree", "Node Tree", "Node tree for recursive check"); + RNA_def_parameter_flags(parm, PROP_NEVER_NULL, PARM_REQUIRED); + parm = RNA_def_property(func, "contained", PROP_BOOLEAN, PROP_NONE); + RNA_def_function_return(func, parm); + /* registration */ prop = RNA_def_property(srna, "bl_idname", PROP_STRING, PROP_NONE); RNA_def_property_string_sdna(prop, NULL, "typeinfo->idname");