Cleanup: Clarify multi-socket input sorting

The multi-socket input sorting was used for two purposes: moving links
to the proper positions when dragging a new link, and resetting the
multi-input indices on the links when removing a link. They are now
separated into two functions, and the sorting when making a group
node that didn't accomplish anything is removed (in that case a
proper implementation would copy the indices from the original
exterior sockets).

3 files changed, 36 insertions, 27 deletions

diff --git a/source/blender/editors/space_node/node_group.cc b/source/blender/editors/space_node/node_group.cc
index 482b2864d58..21def1bd9d7 100644
--- a/source/blender/editors/space_node/node_group.cc
+++ b/source/blender/editors/space_node/node_group.cc
@@ -25,6 +25,7 @@
 #include "BKE_context.h"
 #include "BKE_lib_id.h"
 #include "BKE_main.h"
+#include "BKE_node_runtime.hh"
 #include "BKE_node_tree_update.h"
 #include "BKE_report.h"
 
@@ -836,8 +837,8 @@ static void node_group_make_insert_selected(const bContext &C, bNodeTree &ntree,
 
   /* relink external sockets */
   LISTBASE_FOREACH_MUTABLE (bNodeLink *, link, &ntree.links) {
-    int fromselect = node_group_make_use_node(*link->fromnode, gnode);
-    int toselect = node_group_make_use_node(*link->tonode, gnode);
+    const bool fromselect = node_group_make_use_node(*link->fromnode, gnode);
+    const bool toselect = node_group_make_use_node(*link->tonode, gnode);
 
     if ((fromselect && link->tonode == gnode) || (toselect && link->fromnode == gnode)) {
       /* remove all links to/from the gnode.
@@ -917,8 +918,8 @@ static void node_group_make_insert_selected(const bContext &C, bNodeTree &ntree,
 
   /* move internal links */
   LISTBASE_FOREACH_MUTABLE (bNodeLink *, link, &ntree.links) {
-    int fromselect = node_group_make_use_node(*link->fromnode, gnode);
-    int toselect = node_group_make_use_node(*link->tonode, gnode);
+    const bool fromselect = node_group_make_use_node(*link->fromnode, gnode);
+    const bool toselect = node_group_make_use_node(*link->tonode, gnode);
 
     if (fromselect && toselect) {
       BLI_remlink(&ntree.links, link);
@@ -1041,11 +1042,6 @@ static int node_group_make_exec(bContext *C, wmOperator *op)
     nodeSetActive(&ntree, gnode);
     if (ngroup) {
       ED_node_tree_push(&snode, ngroup, gnode);
-
-      ngroup->ensure_topology_cache();
-      LISTBASE_FOREACH (bNode *, node, &ngroup->nodes) {
-        sort_multi_input_socket_links(*node, nullptr, nullptr);
-      }
     }
   }
 
diff --git a/source/blender/editors/space_node/node_intern.hh b/source/blender/editors/space_node/node_intern.hh
index 011b9f6b631..6754673cffc 100644
--- a/source/blender/editors/space_node/node_intern.hh
+++ b/source/blender/editors/space_node/node_intern.hh
@@ -267,8 +267,6 @@ void NODE_OT_group_edit(wmOperatorType *ot);
 
 /* node_relationships.cc */
 
-void sort_multi_input_socket_links(bNode &node, bNodeLink *drag_link, const float2 *cursor);
-
 void NODE_OT_link(wmOperatorType *ot);
 void NODE_OT_link_make(wmOperatorType *ot);
 void NODE_OT_links_cut(wmOperatorType *ot);
diff --git a/source/blender/editors/space_node/node_relationships.cc b/source/blender/editors/space_node/node_relationships.cc
index e95de7b8e85..067c01dcc58 100644
--- a/source/blender/editors/space_node/node_relationships.cc
+++ b/source/blender/editors/space_node/node_relationships.cc
@@ -73,6 +73,8 @@ static void clear_picking_highlight(ListBase *links)
 
 namespace blender::ed::space_node {
 
+void update_multi_input_indices_for_removed_links(bNode &node);
+
 /* -------------------------------------------------------------------- */
 /** \name Add Node
  * \{ */
@@ -103,11 +105,9 @@ static void pick_link(
 
   nldrag.links.append(link);
   nodeRemLink(snode.edittree, &link_to_pick);
-
-  BLI_assert(nldrag.last_node_hovered_while_dragging_a_link != nullptr);
-
   snode.edittree->ensure_topology_cache();
-  sort_multi_input_socket_links(*nldrag.last_node_hovered_while_dragging_a_link, nullptr, nullptr);
+  BLI_assert(nldrag.last_node_hovered_while_dragging_a_link != nullptr);
+  update_multi_input_indices_for_removed_links(*nldrag.last_node_hovered_while_dragging_a_link);
 
   /* Send changed event to original link->tonode. */
   if (node) {
@@ -292,7 +292,9 @@ struct LinkAndPosition {
   float2 multi_socket_position;
 };
 
-void sort_multi_input_socket_links(bNode &node, bNodeLink *drag_link, const float2 *cursor)
+static void sort_multi_input_socket_links_with_drag(bNode &node,
+                                                    bNodeLink &drag_link,
+                                                    const float2 &cursor)
 {
   for (bNodeSocket *socket : node.input_sockets()) {
     if (!socket->is_multi_input()) {
@@ -307,10 +309,7 @@ void sort_multi_input_socket_links(bNode &node, bNodeLink *drag_link, const floa
       links.append({link, location});
     };
 
-    if (drag_link) {
-      BLI_assert(cursor != nullptr);
-      links.append({drag_link, *cursor});
-    }
+    links.append({&drag_link, cursor});
 
     std::sort(links.begin(), links.end(), [](const LinkAndPosition a, const LinkAndPosition b) {
       return a.multi_socket_position.y < b.multi_socket_position.y;
@@ -322,6 +321,23 @@ void sort_multi_input_socket_links(bNode &node, bNodeLink *drag_link, const floa
   }
 }
 
+void update_multi_input_indices_for_removed_links(bNode &node)
+{
+  for (bNodeSocket *socket : node.input_sockets()) {
+    if (!socket->is_multi_input()) {
+      continue;
+    }
+    Vector<bNodeLink *, 8> links = socket->directly_linked_links();
+    std::sort(links.begin(), links.end(), [](const bNodeLink *a, const bNodeLink *b) {
+      return a->multi_input_socket_index < b->multi_input_socket_index;
+    });
+
+    for (const int i : links.index_range()) {
+      links[i]->multi_input_socket_index = i;
+    }
+  }
+}
+
 static void snode_autoconnect(SpaceNode &snode, const bool allow_multiple, const bool replace)
 {
   bNodeTree *ntree = snode.edittree;
@@ -944,19 +960,19 @@ static void node_link_find_socket(bContext &C, wmOperator &op, const float2 &cur
           continue;
         }
         if (link->tosock && link->tosock->flag & SOCK_MULTI_INPUT) {
-          sort_multi_input_socket_links(*tnode, link, &cursor);
+          sort_multi_input_socket_links_with_drag(*tnode, *link, cursor);
         }
       }
     }
     else {
       for (bNodeLink *link : nldrag->links) {
-        if (nldrag->last_node_hovered_while_dragging_a_link) {
-          sort_multi_input_socket_links(
-              *nldrag->last_node_hovered_while_dragging_a_link, nullptr, nullptr);
-        }
         link->tonode = nullptr;
         link->tosock = nullptr;
       }
+      if (nldrag->last_node_hovered_while_dragging_a_link) {
+        update_multi_input_indices_for_removed_links(
+            *nldrag->last_node_hovered_while_dragging_a_link);
+      }
     }
   }
   else {
@@ -1347,9 +1363,8 @@ static int cut_links_exec(bContext *C, wmOperator *op)
   }
 
   node_tree.ensure_topology_cache();
-
   for (bNode *node : affected_nodes) {
-    sort_multi_input_socket_links(*node, nullptr, nullptr);
+    update_multi_input_indices_for_removed_links(*node);
   }
 
   ED_node_tree_propagate_change(C, CTX_data_main(C), snode.edittree);