From ae4c5a493c6cb48c44eeb60c90805298f70896a0 Mon Sep 17 00:00:00 2001
From: Hans Goudey <h.goudey@me.com>
Date: Tue, 7 Feb 2023 13:14:19 -0500
Subject: [PATCH 1/4] Fix #104219: Node links are sometimes created from the
 wrong socket

For reasons described in more depth in the new code comment,
reordering nodes can sometimes invalidate the socket locations
in a way that makes random connections after a node is selected.

A better change would probably be to stop reordering nodes and
store their "UI order" separately instead, but that's more involved.
On its own, storing socket locations in a SoA format probably isn't
worth this complexity, but I think it's useful to have something like
this working so more data can be stored this way in the future.

I think the reproduce-ability of this bug depends on Blender's
performance, since I had a much easier time recreating it in a
debug build with a bunch of extra nodes added to the file.
---
 .../blender/editors/space_node/node_draw.cc   | 28 ++++++++++++++++++-
 .../blender/editors/space_node/node_intern.hh |  2 +-
 .../editors/space_node/node_relationships.cc  |  8 +++---
 .../blender/editors/space_node/node_select.cc | 14 +++++-----
 4 files changed, 39 insertions(+), 13 deletions(-)

diff --git a/source/blender/editors/space_node/node_draw.cc b/source/blender/editors/space_node/node_draw.cc
index 53a86e23ce1..24d4b619600 100644
--- a/source/blender/editors/space_node/node_draw.cc
+++ b/source/blender/editors/space_node/node_draw.cc
@@ -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()) {
+      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)
diff --git a/source/blender/editors/space_node/node_intern.hh b/source/blender/editors/space_node/node_intern.hh
index b8cb4a4e9ae..ff46819f3e2 100644
--- a/source/blender/editors/space_node/node_intern.hh
+++ b/source/blender/editors/space_node/node_intern.hh
@@ -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);
 
 void node_set_cursor(wmWindow &win, SpaceNode &snode, const float2 &cursor);
 /* DPI scaled coords */
diff --git a/source/blender/editors/space_node/node_relationships.cc b/source/blender/editors/space_node/node_relationships.cc
index 30a4b950e60..3171febecbd 100644
--- a/source/blender/editors/space_node/node_relationships.cc
+++ b/source/blender/editors/space_node/node_relationships.cc
@@ -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;
diff --git a/source/blender/editors/space_node/node_select.cc b/source/blender/editors/space_node/node_select.cc
index fbd625932dd..e28e53cf98b 100644
--- a/source/blender/editors/space_node/node_select.cc
+++ b/source/blender/editors/space_node/node_select.cc
@@ -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;
-- 
2.30.2


From d629dd98eccb3f1458fbe81a42b55f7870ced654 Mon Sep 17 00:00:00 2001
From: Hans Goudey <h.goudey@me.com>
Date: Mon, 27 Feb 2023 18:09:18 -0500
Subject: [PATCH 2/4] Revert "Fix #104219: Node links are sometimes created
 from the wrong socket"

This reverts commit ae4c5a493c6cb48c44eeb60c90805298f70896a0.
---
 .../blender/editors/space_node/node_draw.cc   | 28 +------------------
 .../blender/editors/space_node/node_intern.hh |  2 +-
 .../editors/space_node/node_relationships.cc  |  8 +++---
 .../blender/editors/space_node/node_select.cc | 14 +++++-----
 4 files changed, 13 insertions(+), 39 deletions(-)

diff --git a/source/blender/editors/space_node/node_draw.cc b/source/blender/editors/space_node/node_draw.cc
index 24d4b619600..53a86e23ce1 100644
--- a/source/blender/editors/space_node/node_draw.cc
+++ b/source/blender/editors/space_node/node_draw.cc
@@ -247,24 +247,8 @@ static bool compare_node_depth(const bNode *a, const bNode *b)
   return false;
 }
 
-void node_sort(SpaceNode &snode, bNodeTree &ntree)
+void node_sort(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()) {
-      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);
 
@@ -283,16 +267,6 @@ void node_sort(SpaceNode &snode, 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)
diff --git a/source/blender/editors/space_node/node_intern.hh b/source/blender/editors/space_node/node_intern.hh
index ff46819f3e2..b8cb4a4e9ae 100644
--- a/source/blender/editors/space_node/node_intern.hh
+++ b/source/blender/editors/space_node/node_intern.hh
@@ -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(SpaceNode &snode, bNodeTree &ntree);
+void node_sort(bNodeTree &ntree);
 
 void node_set_cursor(wmWindow &win, SpaceNode &snode, const float2 &cursor);
 /* DPI scaled coords */
diff --git a/source/blender/editors/space_node/node_relationships.cc b/source/blender/editors/space_node/node_relationships.cc
index 3171febecbd..30a4b950e60 100644
--- a/source/blender/editors/space_node/node_relationships.cc
+++ b/source/blender/editors/space_node/node_relationships.cc
@@ -1725,7 +1725,7 @@ static int node_parent_set_exec(bContext *C, wmOperator * /*op*/)
     }
   }
 
-  node_sort(snode, ntree);
+  node_sort(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(snode, ntree);
+  node_sort(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(snode, ntree);
+  node_sort(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(snode, ntree);
+  node_sort(ntree);
   WM_event_add_notifier(C, NC_NODE | ND_DISPLAY, nullptr);
 
   return OPERATOR_FINISHED;
diff --git a/source/blender/editors/space_node/node_select.cc b/source/blender/editors/space_node/node_select.cc
index e28e53cf98b..fbd625932dd 100644
--- a/source/blender/editors/space_node/node_select.cc
+++ b/source/blender/editors/space_node/node_select.cc
@@ -439,7 +439,7 @@ static int node_select_grouped_exec(bContext *C, wmOperator *op)
   }
 
   if (changed) {
-    node_sort(snode, node_tree);
+    node_sort(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(snode, node_tree);
+  node_sort(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(snode, node_tree);
+  node_sort(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(snode, node_tree);
+  node_sort(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(snode, node_tree);
+  node_sort(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(snode, node_tree);
+  node_sort(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(snode, node_tree);
+  node_sort(node_tree);
 
   WM_event_add_notifier(C, NC_NODE | NA_SELECTED, nullptr);
   return OPERATOR_FINISHED;
-- 
2.30.2


From 4bcda9cc2d8e3cfde547eb67f7915b6e2a2351b1 Mon Sep 17 00:00:00 2001
From: Hans Goudey <h.goudey@me.com>
Date: Mon, 27 Feb 2023 18:14:10 -0500
Subject: [PATCH 3/4] Fix #104219: Node links created from wrong socket after
 selection

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
reordere, 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.
---
 source/blender/blenkernel/BKE_node_runtime.hh |  7 ++++
 source/blender/editors/space_node/drawnode.cc | 33 ++++++++++---------
 source/blender/editors/space_node/node_add.cc |  2 +-
 .../blender/editors/space_node/node_draw.cc   |  9 +++--
 .../blender/editors/space_node/node_edit.cc   |  9 ++---
 .../blender/editors/space_node/node_intern.hh |  6 ----
 .../editors/space_node/node_relationships.cc  | 27 ++++++++++-----
 7 files changed, 56 insertions(+), 37 deletions(-)

diff --git a/source/blender/blenkernel/BKE_node_runtime.hh b/source/blender/blenkernel/BKE_node_runtime.hh
index c5322b7c6ba..d85732e50a7 100644
--- a/source/blender/blenkernel/BKE_node_runtime.hh
+++ b/source/blender/blenkernel/BKE_node_runtime.hh
@@ -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,12 @@ 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().
+   */
+  Vector<float2> all_socket_locations;
 };
 
 /**
diff --git a/source/blender/editors/space_node/drawnode.cc b/source/blender/editors/space_node/drawnode.cc
index 34b5ba3ab22..e04e4d50f1c 100644
--- a/source/blender/editors/space_node/drawnode.cc
+++ b/source/blender/editors/space_node/drawnode.cc
@@ -2032,9 +2032,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;
@@ -2056,24 +2058,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 {
@@ -2160,8 +2159,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;
   }
@@ -2220,15 +2220,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;
diff --git a/source/blender/editors/space_node/node_add.cc b/source/blender/editors/space_node/node_add.cc
index 0e3e6f0ece9..c5f259a5db7 100644
--- a/source/blender/editors/space_node/node_add.cc
+++ b/source/blender/editors/space_node/node_add.cc
@@ -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") {
diff --git a/source/blender/editors/space_node/node_draw.cc b/source/blender/editors/space_node/node_draw.cc
index 53a86e23ce1..15e16ff0ed6 100644
--- a/source/blender/editors/space_node/node_draw.cc
+++ b/source/blender/editors/space_node/node_draw.cc
@@ -267,6 +267,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)
@@ -3177,16 +3180,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);
diff --git a/source/blender/editors/space_node/node_edit.cc b/source/blender/editors/space_node/node_edit.cc
index fc039746639..04d9c5b762f 100644
--- a/source/blender/editors/space_node/node_edit.cc
+++ b/source/blender/editors/space_node/node_edit.cc
@@ -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;
   }
diff --git a/source/blender/editors/space_node/node_intern.hh b/source/blender/editors/space_node/node_intern.hh
index b8cb4a4e9ae..11e64bb2f4c 100644
--- a/source/blender/editors/space_node/node_intern.hh
+++ b/source/blender/editors/space_node/node_intern.hh
@@ -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. */
diff --git a/source/blender/editors/space_node/node_relationships.cc b/source/blender/editors/space_node/node_relationships.cc
index 30a4b950e60..47019865279 100644
--- a/source/blender/editors/space_node/node_relationships.cc
+++ b/source/blender/editors/space_node/node_relationships.cc
@@ -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;
@@ -638,7 +642,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);
@@ -1067,7 +1071,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)) {
@@ -1098,8 +1107,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);
         }
       }
     }
@@ -1472,7 +1480,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) {
@@ -1558,7 +1566,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") {
@@ -2041,7 +2049,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);
 
-- 
2.30.2


From 132aaff965c6f33e2b4f9a4ab6fe2b210ba60733 Mon Sep 17 00:00:00 2001
From: Hans Goudey <h.goudey@me.com>
Date: Tue, 28 Feb 2023 11:14:22 -0500
Subject: [PATCH 4/4] Add comment about coordinate space

---
 source/blender/blenkernel/BKE_node_runtime.hh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/source/blender/blenkernel/BKE_node_runtime.hh b/source/blender/blenkernel/BKE_node_runtime.hh
index d85732e50a7..e620b1b980a 100644
--- a/source/blender/blenkernel/BKE_node_runtime.hh
+++ b/source/blender/blenkernel/BKE_node_runtime.hh
@@ -154,7 +154,8 @@ class bNodeTreeRuntime : NonCopyable, NonMovable {
 
   /**
    * The location of all sockets in the tree, calculated while drawing the nodes.
-   * Indexed with #bNodeSocket::index_in_tree().
+   * Indexed with #bNodeSocket::index_in_tree(). In the node tree's "world space"
+   * (the same as #bNode::runtime::totr).
    */
   Vector<float2> all_socket_locations;
 };
-- 
2.30.2