blender
/
blender
136
407
Fork 589
Code Issues 6.3k Pull Requests 782 Projects 19 Wiki Activity

Fix #104219: Node links are sometimes created from the wrong socket #104420

Closed
Hans Goudey wants to merge 5 commits from HooglyBoogly:fix-node-editor-sort-socket-locations into blender-v3.5-release
pull from: HooglyBoogly:fix-node-editor-sort-socket-locations
merge into: blender:blender-v3.5-release

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
Conversation 0 Commits 5 Files Changed 7 +39 -13
4 changed files with 39 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit ae4c5a493c - Show all commits

28
source/blender/editors/space_node/node_draw.cc
View File

@ -247,8 +247,24 @@ static bool compare_node_depth(const bNode *a, const bNode *b)
return false;
}
void node_sort(bNodeTree &ntree)
void node_sort(SpaceNode &snode, bNodeTree &ntree)
{
/* A second operation can depend on the socket locations after nodes are reordered. Usually that
* is fine, but occasionally there is no redraw in between to recalculate the socket positions
* with the new node order. For example, node selection picking and the link drag operator can
* happen without a redraw in between. For these cases, we have to reorder the socket positions
* as well. */
const Span<float2> old_socket_locations = snode.runtime->all_socket_locations;
const bool copy_socket_locations = snode.edittree == &ntree && !old_socket_locations.is_empty();
Map<const bNodeSocket *, int> old_socket_indices;
if (copy_socket_locations) {
ntree.ensure_topology_cache();
old_socket_indices.reserve(ntree.all_sockets().size());
for (const bNodeSocket *socket : ntree.all_sockets()) {
HooglyBoogly marked this conversation as resolved
Jacques Lucke commented 2023-02-10 20:15:49 +01:00
Review
Copy Link

I think this could be implemented more efficiently (e.g. by storing something on bNodeSocketRuntime, or by only keeping track of the old node index which might work because the sockets of a node are consecutive in all_sockets()).
It's unlikely that this significantly impacts performance in practice though.

I think this could be implemented more efficiently (e.g. by storing something on `bNodeSocketRuntime`, or by only keeping track of the old node index which might work because the sockets of a node are consecutive in `all_sockets()`). It's unlikely that this significantly impacts performance in practice though.
old_socket_indices.add_new(socket, socket->index_in_tree());
}
}
Array<bNode *> sort_nodes = ntree.all_nodes();
std::stable_sort(sort_nodes.begin(), sort_nodes.end(), compare_node_depth);
@ -267,6 +283,16 @@ void node_sort(bNodeTree &ntree)
ntree.runtime->nodes_by_id.add_new(sort_nodes[i]);
sort_nodes[i]->runtime->index_in_tree = i;
}
if (copy_socket_locations) {
ntree.ensure_topology_cache();
Vector<float2> new_socket_locations(ntree.all_sockets().size());
for (const bNodeSocket *socket : ntree.all_sockets()) {
const float2 old_location = old_socket_locations[old_socket_indices.lookup(socket)];
new_socket_locations[socket->index_in_tree()] = old_location;
}
snode.runtime->all_socket_locations = std::move(new_socket_locations);
}
}
static Array<uiBlock *> node_uiblocks_init(const bContext &C, const Span<bNode *> nodes)

2
source/blender/editors/space_node/node_intern.hh
View File

@ -173,7 +173,7 @@ void node_socket_add_tooltip(const bNodeTree &ntree, const bNodeSocket &sock, ui
* Sort nodes by selection: unselected nodes first, then selected,
* then the active node at the very end. Relative order is kept intact.
*/
void node_sort(bNodeTree &ntree);
void node_sort(SpaceNode &snode, bNodeTree &ntree);
HooglyBoogly marked this conversation as resolved
Jacques Lucke commented 2023-02-10 20:16:53 +01:00
Review
Copy Link

This is problematic, because the tree might be visible in multiple node editors, all of which would have to be updated.

This is problematic, because the tree might be visible in multiple node editors, all of which would have to be updated.
void node_set_cursor(wmWindow &win, SpaceNode &snode, const float2 &cursor);
/* DPI scaled coords */

8
source/blender/editors/space_node/node_relationships.cc
View File

@ -1725,7 +1725,7 @@ static int node_parent_set_exec(bContext *C, wmOperator * /*op*/)
}
}
node_sort(ntree);
node_sort(snode, ntree);
WM_event_add_notifier(C, NC_NODE | ND_DISPLAY, nullptr);
return OPERATOR_FINISHED;
@ -1812,7 +1812,7 @@ static int node_join_exec(bContext *C, wmOperator * /*op*/)
}
}
node_sort(ntree);
node_sort(snode, ntree);
ED_node_tree_propagate_change(C, &bmain, snode.edittree);
WM_event_add_notifier(C, NC_NODE | ND_DISPLAY, nullptr);
@ -1903,7 +1903,7 @@ static int node_attach_invoke(bContext *C, wmOperator * /*op*/, const wmEvent *e
}
}
node_sort(ntree);
node_sort(snode, ntree);
WM_event_add_notifier(C, NC_NODE | ND_DISPLAY, nullptr);
return OPERATOR_FINISHED;
@ -1978,7 +1978,7 @@ static int node_detach_exec(bContext *C, wmOperator * /*op*/)
}
}
node_sort(ntree);
node_sort(snode, ntree);
WM_event_add_notifier(C, NC_NODE | ND_DISPLAY, nullptr);
return OPERATOR_FINISHED;

14
source/blender/editors/space_node/node_select.cc
View File

@ -439,7 +439,7 @@ static int node_select_grouped_exec(bContext *C, wmOperator *op)
}
if (changed) {
node_sort(node_tree);
node_sort(snode, node_tree);
WM_event_add_notifier(C, NC_NODE | NA_SELECTED, nullptr);
return OPERATOR_FINISHED;
}
@ -507,7 +507,7 @@ void node_select_single(bContext &C, bNode &node)
ED_node_set_active(bmain, &snode, &node_tree, &node, &active_texture_changed);
ED_node_set_active_viewer_key(&snode);
node_sort(node_tree);
node_sort(snode, node_tree);
if (active_texture_changed && has_workbench_in_texture_color(wm, scene, ob)) {
DEG_id_tag_update(&node_tree.id, ID_RECALC_COPY_ON_WRITE);
}
@ -665,7 +665,7 @@ static bool node_mouse_select(bContext *C,
viewer_path::activate_geometry_node(bmain, snode, *node);
}
ED_node_set_active_viewer_key(&snode);
node_sort(node_tree);
node_sort(snode, node_tree);
if ((active_texture_changed && has_workbench_in_texture_color(wm, scene, ob)) ||
viewer_node_changed) {
DEG_id_tag_update(&snode.edittree->id, ID_RECALC_COPY_ON_WRITE);
@ -794,7 +794,7 @@ static int node_box_select_exec(bContext *C, wmOperator *op)
}
}
node_sort(node_tree);
node_sort(snode, node_tree);
WM_event_add_notifier(C, NC_NODE | NA_SELECTED, nullptr);
@ -1101,7 +1101,7 @@ static int node_select_all_exec(bContext *C, wmOperator *op)
break;
}
node_sort(node_tree);
node_sort(snode, node_tree);
WM_event_add_notifier(C, NC_NODE | NA_SELECTED, nullptr);
return OPERATOR_FINISHED;
@ -1153,7 +1153,7 @@ static int node_select_linked_to_exec(bContext *C, wmOperator * /*op*/)
}
}
node_sort(node_tree);
node_sort(snode, node_tree);
WM_event_add_notifier(C, NC_NODE | NA_SELECTED, nullptr);
return OPERATOR_FINISHED;
@ -1203,7 +1203,7 @@ static int node_select_linked_from_exec(bContext *C, wmOperator * /*op*/)
}
}
node_sort(node_tree);
node_sort(snode, node_tree);
WM_event_add_notifier(C, NC_NODE | NA_SELECTED, nullptr);
return OPERATOR_FINISHED;