archive/blender-archive
Archived
1
1
Fork 0
Code Pull Requests Packages Activity

Fix #104219: Node links dragged from wrong socket after selection

Browse Source 
Nodes are sorted based on the selection. In some cases (even depending
on processor speed, nodes can be selected and reordered, and another
operation can run before the next redraw). That gives a window where
operators mapped to the same input as selection can run with invalid
socket locations (which aren't updated after the nodes are reordered,
since they are stored in a separate array).

To fix this, move the socket locations from the node editor runtime
data to the node tree, tag them as invalid when the nodes are
reordered, and check for that status in a few more places.

A better longer term solution is not reordering nodes based on
UI status and instead storing the UI drawing order separately.

Pull Request #104420
This commit is contained in:
Hans Goudey
2023-02-28 11:35:32 -05:00
parent 30a81f1b55
commit 076a33ccd1
7 changed files with 57 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
source/blender/blenkernel/BKE_node_runtime.hh
View File

@@ -6,6 +6,7 @@
#include <mutex>
#include "BLI_cache_mutex.hh"
#include "BLI_math_vector_types.hh"
#include "BLI_multi_value_map.hh"
#include "BLI_resource_scope.hh"
#include "BLI_utility_mixins.hh"
@@ -150,6 +151,13 @@ class bNodeTreeRuntime : NonCopyable, NonMovable {
Vector<bNode *> root_frames;
Vector<bNodeSocket *> interface_inputs;
Vector<bNodeSocket *> interface_outputs;
/**
* The location of all sockets in the tree, calculated while drawing the nodes.
* Indexed with #bNodeSocket::index_in_tree(). In the node tree's "world space"
* (the same as #bNode::runtime::totr).
*/
Vector<float2> all_socket_locations;
};
/**

33
source/blender/editors/space_node/drawnode.cc
View File

@@ -2039,9 +2039,11 @@ static NodeLinkDrawConfig nodelink_get_draw_config(const bContext &C,
draw_config.th_col2 = th_col2;
draw_config.th_col3 = th_col3;
const bNodeTree &node_tree = *snode.edittree;
draw_config.dim_factor = selected ? 1.0f :
node_link_dim_factor(
snode.runtime->all_socket_locations, v2d, link);
node_tree.runtime->all_socket_locations, v2d, link);
bTheme *btheme = UI_GetTheme();
draw_config.dash_alpha = btheme->space_node.dash_alpha;
@@ -2063,24 +2065,21 @@ static NodeLinkDrawConfig nodelink_get_draw_config(const bContext &C,
if (snode.overlay.flag & SN_OVERLAY_SHOW_OVERLAYS &&
snode.overlay.flag & SN_OVERLAY_SHOW_WIRE_COLORS) {
PointerRNA from_node_ptr, to_node_ptr;
RNA_pointer_create((ID *)snode.edittree, &RNA_Node, link.fromnode, &from_node_ptr);
RNA_pointer_create((ID *)snode.edittree, &RNA_Node, link.tonode, &to_node_ptr);
RNA_pointer_create((ID *)&node_tree, &RNA_Node, link.fromnode, &from_node_ptr);
RNA_pointer_create((ID *)&node_tree, &RNA_Node, link.tonode, &to_node_ptr);
if (link.fromsock) {
node_socket_color_get(
C, *snode.edittree, from_node_ptr, *link.fromsock, draw_config.start_color);
node_socket_color_get(C, node_tree, from_node_ptr, *link.fromsock, draw_config.start_color);
}
else {
node_socket_color_get(
C, *snode.edittree, to_node_ptr, *link.tosock, draw_config.start_color);
node_socket_color_get(C, node_tree, to_node_ptr, *link.tosock, draw_config.start_color);
}
if (link.tosock) {
node_socket_color_get(C, *snode.edittree, to_node_ptr, *link.tosock, draw_config.end_color);
node_socket_color_get(C, node_tree, to_node_ptr, *link.tosock, draw_config.end_color);
}
else {
node_socket_color_get(
C, *snode.edittree, from_node_ptr, *link.fromsock, draw_config.end_color);
node_socket_color_get(C, node_tree, from_node_ptr, *link.fromsock, draw_config.end_color);
}
}
else {
@@ -2167,8 +2166,9 @@ void node_draw_link_bezier(const bContext &C,
const int th_col3,
const bool selected)
{
const std::array<float2, 4> points = node_link_bezier_points(snode.runtime->all_socket_locations,
link);
const bNodeTree &node_tree = *snode.edittree;
const std::array<float2, 4> points = node_link_bezier_points(
node_tree.runtime->all_socket_locations, link);
if (!node_link_draw_is_visible(v2d, points)) {
return;
}
@@ -2227,15 +2227,18 @@ void node_draw_link(const bContext &C,
static std::array<float2, 4> node_link_bezier_points_dragged(const SpaceNode &snode,
const bNodeLink &link)
{
const bNodeTree &node_tree = *snode.edittree;
const float2 cursor = snode.runtime->cursor * UI_DPI_FAC;
std::array<float2, 4> points;
points[0] = link.fromsock ?
socket_link_connection_location(
snode.runtime->all_socket_locations, *link.fromnode, *link.fromsock, link) :
socket_link_connection_location(node_tree.runtime->all_socket_locations,
*link.fromnode,
*link.fromsock,
link) :
cursor;
points[3] = link.tosock ?
socket_link_connection_location(
snode.runtime->all_socket_locations, *link.tonode, *link.tosock, link) :
node_tree.runtime->all_socket_locations, *link.tonode, *link.tosock, link) :
cursor;
calculate_inner_link_bezier_points(points);
return points;

2
source/blender/editors/space_node/node_add.cc
View File

@@ -138,7 +138,7 @@ static int add_reroute_exec(bContext *C, wmOperator *op)
const ARegion &region = *CTX_wm_region(C);
SpaceNode &snode = *CTX_wm_space_node(C);
bNodeTree &ntree = *snode.edittree;
const Span<float2> socket_locations = snode.runtime->all_socket_locations;
const Span<float2> socket_locations = ntree.runtime->all_socket_locations;
Vector<float2> path;
RNA_BEGIN (op->ptr, itemptr, "path") {

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

@@ -268,6 +268,9 @@ void node_sort(bNodeTree &ntree)
ntree.runtime->nodes_by_id.add_new(sort_nodes[i]);
sort_nodes[i]->runtime->index_in_tree = i;
}
/* Nodes have been reordered; the socket locations are invalid until the node tree is redrawn. */
ntree.runtime->all_socket_locations.clear();
}
static Array<uiBlock *> node_uiblocks_init(const bContext &C, const Span<bNode *> nodes)
@@ -3178,16 +3181,16 @@ static void draw_nodetree(const bContext &C,
else if (ntree.type == NTREE_COMPOSIT) {
tree_draw_ctx.used_by_realtime_compositor = realtime_compositor_is_in_use(C);
}
snode->runtime->all_socket_locations.reinitialize(ntree.all_sockets().size());
ntree.runtime->all_socket_locations.reinitialize(ntree.all_sockets().size());
node_update_nodetree(
C, tree_draw_ctx, ntree, nodes, blocks, snode->runtime->all_socket_locations);
C, tree_draw_ctx, ntree, nodes, blocks, ntree.runtime->all_socket_locations);
node_draw_nodetree(C,
tree_draw_ctx,
region,
*snode,
ntree,
snode->runtime->all_socket_locations,
ntree.runtime->all_socket_locations,
nodes,
blocks,
parent_key);

9
source/blender/editors/space_node/node_edit.cc
View File

@@ -1129,13 +1129,14 @@ bNodeSocket *node_find_indicated_socket(SpaceNode &snode,
rctf rect;
const float size_sock_padded = NODE_SOCKSIZE + 4;
snode.edittree->ensure_topology_cache();
const Span<float2> socket_locations = snode.runtime->all_socket_locations;
if (socket_locations.size() != snode.edittree->all_sockets().size()) {
bNodeTree &node_tree = *snode.edittree;
node_tree.ensure_topology_cache();
const Span<float2> socket_locations = node_tree.runtime->all_socket_locations;
if (socket_locations.size() != node_tree.all_sockets().size()) {
/* Sockets haven't been drawn yet, e.g. when the file is currently opening. */
return nullptr;
}
const Span<bNode *> nodes = snode.edittree->all_nodes();
const Span<bNode *> nodes = node_tree.all_nodes();
if (nodes.is_empty()) {
return nullptr;
}

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

@@ -79,12 +79,6 @@ struct bNodeLinkDrag {
};
struct SpaceNode_Runtime {
/**
* The location of all sockets in the tree, calculated while drawing the nodes.
* To be indexed with #bNodeSocket::index_in_tree().
*/
Vector<float2> all_socket_locations;
float aspect;
/** Mouse position for drawing socket-less links and adding nodes. */

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

@@ -121,7 +121,11 @@ static void pick_input_link_by_link_intersect(const bContext &C,
const float2 &cursor)
{
SpaceNode *snode = CTX_wm_space_node(&C);
const Span<float2> socket_locations = snode->runtime->all_socket_locations;
bNodeTree &node_tree = *snode->edittree;
const Span<float2> socket_locations = node_tree.runtime->all_socket_locations;
if (socket_locations.is_empty()) {
return;
}
float2 drag_start;
RNA_float_get_array(op.ptr, "drag_start", drag_start);
@@ -132,7 +136,7 @@ static void pick_input_link_by_link_intersect(const bContext &C,
const float cursor_link_touch_distance = 12.5f * UI_DPI_FAC;
bNodeLink *link_to_pick = nullptr;
clear_picking_highlight(&snode->edittree->links);
clear_picking_highlight(&node_tree.links);
for (bNodeLink *link : socket->directly_linked_links()) {
/* Test if the cursor is near a link. */
std::array<float2, NODE_LINK_RESOL + 1> coords;
@@ -643,7 +647,7 @@ static int view_socket(const bContext &C,
}
if (viewer_node == nullptr) {
const float2 socket_location =
snode.runtime->all_socket_locations[bsocket_to_view.index_in_tree()];
btree.runtime->all_socket_locations[bsocket_to_view.index_in_tree()];
const int viewer_type = get_default_viewer_type(&C);
const float2 location{socket_location.x / UI_DPI_FAC + 100, socket_location.y / UI_DPI_FAC};
viewer_node = add_static_node(C, viewer_type, location);
@@ -1072,7 +1076,12 @@ static void node_link_cancel(bContext *C, wmOperator *op)
static void node_link_find_socket(bContext &C, wmOperator &op, const float2 &cursor)
{
SpaceNode &snode = *CTX_wm_space_node(&C);
bNodeTree &node_tree = *snode.edittree;
bNodeLinkDrag &nldrag = *static_cast<bNodeLinkDrag *>(op.customdata);
const Span<float2> socket_locations = node_tree.runtime->all_socket_locations;
if (socket_locations.is_empty()) {
return;
}
if (nldrag.in_out == SOCK_OUT) {
if (bNodeSocket *tsock = node_find_indicated_socket(snode, cursor, SOCK_IN)) {
@@ -1103,8 +1112,7 @@ static void node_link_find_socket(bContext &C, wmOperator &op, const float2 &cur
continue;
}
if (tsock && tsock->is_multi_input()) {
sort_multi_input_socket_links_with_drag(
snode.runtime->all_socket_locations, *tsock, link, cursor);
sort_multi_input_socket_links_with_drag(socket_locations, *tsock, link, cursor);
}
}
}
@@ -1477,7 +1485,7 @@ static int cut_links_exec(bContext *C, wmOperator *op)
bNodeTree &node_tree = *snode.edittree;
node_tree.ensure_topology_cache();
const Span<float2> socket_locations = snode.runtime->all_socket_locations;
const Span<float2> socket_locations = node_tree.runtime->all_socket_locations;
Set<bNodeLink *> links_to_remove;
LISTBASE_FOREACH (bNodeLink *, link, &node_tree.links) {
@@ -1563,7 +1571,7 @@ static int mute_links_exec(bContext *C, wmOperator *op)
SpaceNode &snode = *CTX_wm_space_node(C);
const ARegion &region = *CTX_wm_region(C);
bNodeTree &ntree = *snode.edittree;
const Span<float2> socket_locations = snode.runtime->all_socket_locations;
const Span<float2> socket_locations = ntree.runtime->all_socket_locations;
Vector<float2> path;
RNA_BEGIN (op->ptr, itemptr, "path") {
@@ -2046,7 +2054,10 @@ void node_insert_on_link_flags_set(SpaceNode &snode, const ARegion &region)
{
bNodeTree &node_tree = *snode.edittree;
node_tree.ensure_topology_cache();
const Span<float2> socket_locations = snode.runtime->all_socket_locations;
const Span<float2> socket_locations = node_tree.runtime->all_socket_locations;
if (socket_locations.is_empty()) {
return;
}
node_insert_on_link_flags_clear(node_tree);