blender
/
blender
136
406
Fork 589
Code Issues 6.3k Pull Requests 767 Projects 19 Wiki Activity

Cleanup: Use utility function to find groups in node tree #104465

Merged
Hans Goudey merged 23 commits from mod_moder/blender:is_contains into main 2023-02-10 17:30:57 +01:00
Conversation 3 Commits 23 Files Changed 7 +15 -14
2 changed files with 15 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit c04ea480b3 - Show all commits

5
source/blender/blenkernel/BKE_node.h
View File

@ -506,9 +506,10 @@ void ntreeFreeLocalTree(struct bNodeTree *ntree);
struct bNode *ntreeFindType(struct bNodeTree *ntree, int type);
/**
* Check recursively whether a sub node tree contain in host tree.
* Check recursively if a node tree contains another.
mod_moder marked this conversation as resolved
Jacques Lucke commented 2023-02-10 13:03:40 +01:00
Review
Copy Link

Check recursively if a node tree contains another.

`Check recursively if a node tree contains another.`
*/
bool ntreeContainsTree(const struct bNodeTree *parent_tree, const struct bNodeTree *sub_tree);
bool ntreeContainsTree(const struct bNodeTree *tree_to_search_in,
Jacques Lucke commented 2023-02-10 13:04:37 +01:00
Review
Copy Link

I think it would be better to use names like tree_to_search_in and tree_to_search_for. I find that the most clear.

I think it would be better to use names like `tree_to_search_in` and `tree_to_search_for`. I find that the most clear.
👍 1
Iliya Katushenock commented 2023-02-10 14:58:34 +01:00
Review
Copy Link

This only for backend part, or in python is too?
parm = RNA_def_pointer(func, "sub_tree", "NodeTree", "Node Tree", "Node tree for recursive check");

This only for backend part, or in python is too? `parm = RNA_def_pointer(func, "sub_tree", "NodeTree", "Node Tree", "Node tree for recursive check");`
const struct bNodeTree *tree_to_search_for);
void ntreeUpdateAllNew(struct Main *main);
void ntreeUpdateAllUsers(struct Main *main, struct ID *id);

24
source/blender/blenkernel/intern/node.cc
View File

@ -3457,24 +3457,24 @@ bNode *ntreeFindType(bNodeTree *ntree, int type)
return nullptr;
}
mod_moder marked this conversation as resolved
Jacques Lucke commented 2023-02-08 17:33:14 +01:00
Review
Copy Link

This should internally avoid to enter the same node group multiple times. For very large node trees, with lots of nesting that could be quite heavy.

This should internally avoid to enter the same node group multiple times. For very large node trees, with lots of nesting that could be quite heavy.
Iliya Katushenock commented 2023-02-08 17:42:21 +01:00
Review
Copy Link

When i was upload this as diff (https://archive.blender.org/developer/D16870) i pointed, that map (set) for avoiding already checked groups and output of groups stack can be added in future as speed improvement. You want to i do that in this request?

When i was upload this as diff (https://archive.blender.org/developer/D16870) i pointed, that map (set) for avoiding already checked groups and output of groups stack can be added in future as speed improvement. You want to i do that in this request?
Jacques Lucke commented 2023-02-08 18:29:06 +01:00
Review
Copy Link

Ah I see, well either add this to the pull request description or just do the code change now already. Don't see much of a reason not to do it now already.

Ah I see, well either add this to the pull request description or just do the code change now already. Don't see much of a reason not to do it now already.
Iliya Katushenock commented 2023-02-08 20:40:09 +01:00
Review
Copy Link
Timer 'Cache speed': (Average: 16473 ns, Min: 300 ns, Last: 44000 ns)
Timer 'Cache speed': (Average: 1100 ns, Min: 1100 ns, Last: 1100 ns)
Timer 'Cache speed': (Average: 0.2 ms, Min: 0.2 ms, Last: 0.2 ms)

Timer 'Normal speed': (Average: 2.1 ms, Min: 200 ns, Last: 136.1 ms) -- Open searcher
Timer 'Normal speed': (Average: 1200 ns, Min: 1200 ns, Last: 1200 ns) -- Insert other one group in this.
Timer 'Normal speed': (Average: 135.6 ms, Min: 135.6 ms, Last: 135.6 ms) -- Insert this group in other one
``` Timer 'Cache speed': (Average: 16473 ns, Min: 300 ns, Last: 44000 ns) Timer 'Cache speed': (Average: 1100 ns, Min: 1100 ns, Last: 1100 ns) Timer 'Cache speed': (Average: 0.2 ms, Min: 0.2 ms, Last: 0.2 ms) ``` ``` Timer 'Normal speed': (Average: 2.1 ms, Min: 200 ns, Last: 136.1 ms) -- Open searcher Timer 'Normal speed': (Average: 1200 ns, Min: 1200 ns, Last: 1200 ns) -- Insert other one group in this. Timer 'Normal speed': (Average: 135.6 ms, Min: 135.6 ms, Last: 135.6 ms) -- Insert this group in other one ```
static bool ntree_contains_tree_exec(const bNodeTree *parent_tree,
const bNodeTree *sub_tree,
static bool ntree_contains_tree_exec(const bNodeTree *tree_to_search_in,
const bNodeTree *tree_to_search_for,
Set<const bNodeTree *> &already_passed)
{
if (parent_tree == sub_tree) {
if (tree_to_search_in == tree_to_search_for) {
mod_moder marked this conversation as resolved
Hans Goudey commented 2023-02-10 14:36:43 +01:00
Review
Copy Link

reinterpret_cast<bNodeTree *> -> reinterpret_cast<const bNodeTree *>

`reinterpret_cast<bNodeTree *>` -> `reinterpret_cast<const bNodeTree *>`
👀 1
return true;
}
parent_tree->ensure_topology_cache();
for (const bNode *node_group : parent_tree->group_nodes()) {
const bNodeTree *tree = reinterpret_cast<bNodeTree *>(node_group->id);
if (!tree) {
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<const bNodeTree *>(node_group->id);
if (!sub_tree_search_in) {
continue;
}
if (!already_passed.add(tree)) {
if (!already_passed.add(sub_tree_search_in)) {
continue;
}
if (ntree_contains_tree_exec(tree, sub_tree, already_passed)) {
if (ntree_contains_tree_exec(sub_tree_search_in, tree_to_search_for, already_passed)) {
return true;
}
}
@ -3482,14 +3482,14 @@ static bool ntree_contains_tree_exec(const bNodeTree *parent_tree,
return false;
}
bool ntreeContainsTree(const bNodeTree *parent_tree, const bNodeTree *sub_tree)
bool ntreeContainsTree(const bNodeTree *tree_to_search_in, const bNodeTree *tree_to_search_for)
{
if (parent_tree == sub_tree) {
if (tree_to_search_in == tree_to_search_for) {
return true;
}
Set<const bNodeTree *> already_passed;
return ntree_contains_tree_exec(parent_tree, sub_tree, 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)