[Bf-blender-cvs] [ab4926bcffe] master: Fix: Various mishandling of node identifiers and vector
Hans Goudey
noreply at git.blender.org
Fri Dec 2 20:28:41 CET 2022
Commit: ab4926bcffeef20cfb9225d23f68429c1f1d0c87
Author: Hans Goudey
Date: Fri Dec 2 13:20:40 2022 -0600
Branches: master
https://developer.blender.org/rBab4926bcffeef20cfb9225d23f68429c1f1d0c87
Fix: Various mishandling of node identifiers and vector
In a few places, nodes were added without updating the Identifiers and
vector. In other places nodes we removed without removing from and
rebuilding the vector. This is solved in a few ways. First I exposed
a function to rebuild the vector from scratch, and added unique ID
finding to a few places.
The changes to node group building and separating are more involved,
mostly because it was hard to see the correct behavior without some
refactoring. Now `VectorSet` is used to store nodes involved in the
operation. Some things are handled more simply with the topology
cache and by passing a span of nodes.
===================================================================
M source/blender/blenkernel/BKE_node.h
M source/blender/blenkernel/intern/node.cc
M source/blender/blenloader/intern/versioning_250.c
M source/blender/editors/space_node/node_edit.cc
M source/blender/editors/space_node/node_group.cc
M source/blender/editors/space_node/node_intern.hh
M source/blender/editors/space_node/node_relationships.cc
M source/blender/editors/space_node/node_select.cc
M source/blender/nodes/shader/node_shader_tree.cc
===================================================================
diff --git a/source/blender/blenkernel/BKE_node.h b/source/blender/blenkernel/BKE_node.h
index 74d6f8becd1..3680d8ac8cd 100644
--- a/source/blender/blenkernel/BKE_node.h
+++ b/source/blender/blenkernel/BKE_node.h
@@ -670,6 +670,13 @@ void nodeUnlinkNode(struct bNodeTree *ntree, struct bNode *node);
void nodeUniqueName(struct bNodeTree *ntree, struct bNode *node);
void nodeUniqueID(struct bNodeTree *ntree, struct bNode *node);
+/**
+ * Rebuild the `node_by_id` runtime vector set. Call after removing a node if not handled
+ * separately. This is important instead of just using `nodes_by_id.remove()` since it maintains
+ * the node order.
+ */
+void nodeRebuildIDVector(struct bNodeTree *node_tree);
+
/**
* Delete node, associated animation data and ID user count.
*/
diff --git a/source/blender/blenkernel/intern/node.cc b/source/blender/blenkernel/intern/node.cc
index c502381fc62..8bff3a8f997 100644
--- a/source/blender/blenkernel/intern/node.cc
+++ b/source/blender/blenkernel/intern/node.cc
@@ -2933,12 +2933,12 @@ static void node_unlink_attached(bNodeTree *ntree, bNode *parent)
}
}
-static void rebuild_nodes_vector(bNodeTree &node_tree)
+void nodeRebuildIDVector(bNodeTree *node_tree)
{
/* Rebuild nodes #VectorSet which must have the same order as the list. */
- node_tree.runtime->nodes_by_id.clear();
- LISTBASE_FOREACH (bNode *, node, &node_tree.nodes) {
- node_tree.runtime->nodes_by_id.add_new(node);
+ node_tree->runtime->nodes_by_id.clear();
+ LISTBASE_FOREACH (bNode *, node, &node_tree->nodes) {
+ node_tree->runtime->nodes_by_id.add_new(node);
}
}
@@ -2955,7 +2955,7 @@ static void node_free_node(bNodeTree *ntree, bNode *node)
if (ntree) {
BLI_remlink(&ntree->nodes, node);
/* Rebuild nodes #VectorSet which must have the same order as the list. */
- rebuild_nodes_vector(*ntree);
+ nodeRebuildIDVector(ntree);
/* texture node has bad habit of keeping exec data around */
if (ntree->type == NTREE_TEXTURE && ntree->runtime->execdata) {
@@ -3013,7 +3013,7 @@ void ntreeFreeLocalNode(bNodeTree *ntree, bNode *node)
node_unlink_attached(ntree, node);
node_free_node(ntree, node);
- rebuild_nodes_vector(*ntree);
+ nodeRebuildIDVector(ntree);
}
void nodeRemoveNode(Main *bmain, bNodeTree *ntree, bNode *node, bool do_id_user)
@@ -3073,7 +3073,7 @@ void nodeRemoveNode(Main *bmain, bNodeTree *ntree, bNode *node, bool do_id_user)
/* Free node itself. */
node_free_node(ntree, node);
- rebuild_nodes_vector(*ntree);
+ nodeRebuildIDVector(ntree);
}
static void node_socket_interface_free(bNodeTree * /*ntree*/,
diff --git a/source/blender/blenloader/intern/versioning_250.c b/source/blender/blenloader/intern/versioning_250.c
index f87034548df..5dd64051881 100644
--- a/source/blender/blenloader/intern/versioning_250.c
+++ b/source/blender/blenloader/intern/versioning_250.c
@@ -2001,6 +2001,7 @@ void blo_do_versions_250(FileData *fd, Library *lib, Main *bmain)
*/
link = MEM_callocN(sizeof(bNodeLink), "link");
BLI_addtail(&ntree->links, link);
+ nodeUniqueID(ntree, node);
link->fromnode = NULL;
link->fromsock = gsock;
link->tonode = node;
@@ -2024,6 +2025,7 @@ void blo_do_versions_250(FileData *fd, Library *lib, Main *bmain)
*/
link = MEM_callocN(sizeof(bNodeLink), "link");
BLI_addtail(&ntree->links, link);
+ nodeUniqueID(ntree, node);
link->fromnode = node;
link->fromsock = sock;
link->tonode = NULL;
diff --git a/source/blender/editors/space_node/node_edit.cc b/source/blender/editors/space_node/node_edit.cc
index 005e441727a..38af2669bdc 100644
--- a/source/blender/editors/space_node/node_edit.cc
+++ b/source/blender/editors/space_node/node_edit.cc
@@ -2795,7 +2795,7 @@ static bool node_shader_script_update_text_recursive(RenderEngine *engine,
RenderEngineType *type,
bNodeTree *ntree,
Text *text,
- Set<bNodeTree *> &done_trees)
+ VectorSet<bNodeTree *> &done_trees)
{
bool found = false;
@@ -2855,7 +2855,7 @@ static int node_shader_script_update_exec(bContext *C, wmOperator *op)
if (text) {
- Set<bNodeTree *> done_trees;
+ VectorSet<bNodeTree *> done_trees;
FOREACH_NODETREE_BEGIN (bmain, ntree, id) {
if (ntree->type == NTREE_SHADER) {
diff --git a/source/blender/editors/space_node/node_group.cc b/source/blender/editors/space_node/node_group.cc
index 294bb687c4b..93d2774c4b8 100644
--- a/source/blender/editors/space_node/node_group.cc
+++ b/source/blender/editors/space_node/node_group.cc
@@ -269,6 +269,7 @@ static bool node_group_ungroup(Main *bmain, bNodeTree *ntree, bNode *gnode)
node->flag |= NODE_SELECT;
}
+ wgroup->runtime->nodes_by_id.clear();
bNodeLink *glinks_first = (bNodeLink *)ntree->links.last;
@@ -446,30 +447,24 @@ static bool node_group_separate_selected(
ListBase anim_basepaths = {nullptr, nullptr};
- Map<const bNode *, bNode *> node_map;
Map<const bNodeSocket *, bNodeSocket *> socket_map;
- /* add selected nodes into the ntree */
- LISTBASE_FOREACH_MUTABLE (bNode *, node, &ngroup.nodes) {
- if (!(node->flag & NODE_SELECT)) {
- continue;
- }
-
- /* ignore interface nodes */
- if (ELEM(node->type, NODE_GROUP_INPUT, NODE_GROUP_OUTPUT)) {
- nodeSetSelected(node, false);
- continue;
- }
+ /* Add selected nodes into the ntree, ignoring interface nodes. */
+ VectorSet<bNode *> nodes_to_move = get_selected_nodes(ngroup);
+ nodes_to_move.remove_if(
+ [](const bNode *node) { return node->is_group_input() || node->is_group_output(); });
+ for (bNode *node : nodes_to_move) {
bNode *newnode;
if (make_copy) {
- /* make a copy */
- newnode = bke::node_copy_with_mapping(&ngroup, *node, LIB_ID_COPY_DEFAULT, true, socket_map);
- node_map.add_new(node, newnode);
+ newnode = bke::node_copy_with_mapping(&ntree, *node, LIB_ID_COPY_DEFAULT, true, socket_map);
}
else {
- /* use the existing node */
newnode = node;
+ BLI_remlink(&ngroup.nodes, newnode);
+ BLI_addtail(&ntree.nodes, newnode);
+ nodeUniqueID(&ntree, newnode);
+ nodeUniqueName(&ntree, newnode);
}
/* Keep track of this node's RNA "base" path (the part of the path identifying the node)
@@ -491,17 +486,14 @@ static bool node_group_separate_selected(
nodeDetachNode(&ngroup, newnode);
}
- /* migrate node */
- BLI_remlink(&ngroup.nodes, newnode);
- BLI_addtail(&ntree.nodes, newnode);
- nodeUniqueID(&ntree, newnode);
- nodeUniqueName(&ntree, newnode);
-
if (!newnode->parent) {
newnode->locx += offset.x;
newnode->locy += offset.y;
}
}
+ if (!make_copy) {
+ nodeRebuildIDVector(&ngroup);
+ }
/* add internal links to the ntree */
LISTBASE_FOREACH_MUTABLE (bNodeLink *, link, &ngroup.links) {
@@ -512,9 +504,9 @@ static bool node_group_separate_selected(
/* make a copy of internal links */
if (fromselect && toselect) {
nodeAddLink(&ntree,
- node_map.lookup(link->fromnode),
+ ntree.node_by_id(link->fromnode->identifier),
socket_map.lookup(link->fromsock),
- node_map.lookup(link->tonode),
+ ntree.node_by_id(link->tonode->identifier),
socket_map.lookup(link->tosock));
}
}
@@ -642,97 +634,97 @@ void NODE_OT_group_separate(wmOperatorType *ot)
/** \name Make Group Operator
* \{ */
-static bool node_group_make_use_node(const bNode &node, bNode *gnode)
+static VectorSet<bNode *> get_nodes_to_group(bNodeTree &node_tree, bNode *group_node)
{
- return (&node != gnode && !ELEM(node.type, NODE_GROUP_INPUT, NODE_GROUP_OUTPUT) &&
- (node.flag & NODE_SELECT));
+ VectorSet<bNode *> nodes_to_group = get_selected_nodes(node_tree);
+ nodes_to_group.remove_if(
+ [](bNode *node) { return node->is_group_input() || node->is_group_output(); });
+ nodes_to_group.remove(group_node);
+ return nodes_to_group;
}
static bool node_group_make_test_selected(bNodeTree &ntree,
- bNode *gnode,
+ const VectorSet<bNode *> &nodes_to_group,
const char *ntree_idname,
ReportList &reports)
{
- int ok = true;
-
+ if (nodes_to_group.is_empty()) {
+ return false;
+ }
/* make a local pseudo node tree to pass to the node poll functions */
bNodeTree *ngroup = ntreeAddTree(nullptr, "Pseudo Node Group", ntree_idname);
+ BLI_SCOPED_DEFER([&]() {
+ ntreeFreeTree(ngroup);
+ MEM_freeN(ngroup);
+ });
/* check poll functions for selected nodes */
- for (bNode *node : ntree.all_nodes()) {
- if (node_group_make_use_node(*node, gnode)) {
- const char *disabled_hint = nullptr;
- if (node->typeinfo->poll_instance &&
- !node->typeinfo->poll_instance(node, ngroup, &disabled_hint)) {
- if (disabled_hint) {
- BKE_reportf(&reports,
- RPT_WARNING,
- "Can not add node '%s' in a group:\n %s",
- node->name,
- disabled_hint);
- }
- else {
- BKE_reportf(&reports, RPT_WARNING, "Can not add node '%s' in a group", node->name);
- }
- ok = false;
- break;
+ for (bNode *node : nodes_to_group) {
+ const char *disabled_hint = nullptr;
+ if (node->typeinfo->poll_instance &&
+ !node->typeinfo->poll_instance(node, ngroup, &disabled_hint)) {
+ if (disabled_hint) {
+ BKE_reportf(&reports,
+ RPT_WARNING,
+ "Can not add node '%s' in a group:\n %s",
+ node->name,
+ disabled_hint);
}
+
@@ Diff output truncated at 10240 characters. @@
More information about the Bf-blender-cvs
mailing list