Nodes: revert the inline (pass-through) socket feature

Inlined sockets in the same vertical space are no longer supported.
This removes `input_output` socket declarations, the inlining feature in
node drawing, and the `Both` option for node group interface sockets.

Versioning code splits existing node group sockets into individual
sockets again. Unfortunately some links may get lost in versioning files
using the feature, because of an unnoticed bug: Socket identifiers have
to be unique in the node group items list but inlined input/output
sockets have the same identifier. This still works for most situations
because uniqueness is only required within input/output lists. Creating
proper unique identifiers will discard any link from the previous output
socket. This cannot easily be fixed without `after_linking` versioning
code, which should be avoided.

Pull Request: https://projects.blender.org/blender/blender/pulls/112560

source/blender/blenloader/intern/versioning_400.cc

@@ -791,6 +791,115 @@ static void version_principled_bsdf_coat(bNodeTree *ntree)
       ntree, SH_NODE_BSDF_PRINCIPLED, "Clearcoat Normal", "Coat Normal");
 }
 
+static void version_copy_socket(bNodeTreeInterfaceSocket &dst,
+                                const bNodeTreeInterfaceSocket &src,
+                                char *identifier)
+{
+  /* Node socket copy function based on bNodeTreeInterface::item_copy to avoid using blenkernel. */
+  dst.name = BLI_strdup(src.name);
+  dst.description = BLI_strdup_null(src.description);
+  dst.socket_type = BLI_strdup(src.socket_type);
+  dst.default_attribute_name = BLI_strdup_null(src.default_attribute_name);
+  dst.identifier = identifier;
+  if (src.properties) {
+    dst.properties = IDP_CopyProperty_ex(src.properties, 0);
+  }
+  if (src.socket_data != nullptr) {
+    dst.socket_data = MEM_dupallocN(src.socket_data);
+    /* No user count increment needed, gets reset after versioning. */
+  }
+}
+
+static int version_nodes_find_valid_insert_position_for_item(const bNodeTreeInterfacePanel &panel,
+                                                             const bNodeTreeInterfaceItem &item,
+                                                             const int initial_pos)
+{
+  const bool sockets_above_panels = !(panel.flag &
+                                      NODE_INTERFACE_PANEL_ALLOW_SOCKETS_AFTER_PANELS);
+  const blender::Span<const bNodeTreeInterfaceItem *> items = {panel.items_array, panel.items_num};
+
+  int pos = initial_pos;
+
+  if (sockets_above_panels) {
+    if (item.item_type == NODE_INTERFACE_PANEL) {
+      /* Find the closest valid position from the end, only panels at or after #position. */
+      for (int test_pos = items.size() - 1; test_pos >= initial_pos; test_pos--) {
+        if (test_pos < 0) {
+          /* Initial position is out of range but valid. */
+          break;
+        }
+        if (items[test_pos]->item_type != NODE_INTERFACE_PANEL) {
+          /* Found valid position, insert after the last socket item. */
+          pos = test_pos + 1;
+          break;
+        }
+      }
+    }
+    else {
+      /* Find the closest valid position from the start, no panels at or after #position. */
+      for (int test_pos = 0; test_pos <= initial_pos; test_pos++) {
+        if (test_pos >= items.size()) {
+          /* Initial position is out of range but valid. */
+          break;
+        }
+        if (items[test_pos]->item_type == NODE_INTERFACE_PANEL) {
+          /* Found valid position, inserting moves the first panel. */
+          pos = test_pos;
+          break;
+        }
+      }
+    }
+  }
+
+  return pos;
+}
+
+static void version_nodes_insert_item(bNodeTreeInterfacePanel &parent,
+                                      bNodeTreeInterfaceSocket &socket,
+                                      int position)
+{
+  /* Apply any constraints on the item positions. */
+  position = version_nodes_find_valid_insert_position_for_item(parent, socket.item, position);
+  position = std::min(std::max(position, 0), parent.items_num);
+
+  blender::MutableSpan<bNodeTreeInterfaceItem *> old_items = {parent.items_array,
+                                                              parent.items_num};
+  parent.items_num++;
+  parent.items_array = MEM_cnew_array<bNodeTreeInterfaceItem *>(parent.items_num, __func__);
+  parent.items().take_front(position).copy_from(old_items.take_front(position));
+  parent.items().drop_front(position + 1).copy_from(old_items.drop_front(position));
+  parent.items()[position] = &socket.item;
+
+  if (old_items.data()) {
+    MEM_freeN(old_items.data());
+  }
+}
+
+/* Node group interface copy function based on bNodeTreeInterface::insert_item_copy. */
+static void version_node_group_split_socket(bNodeTreeInterface &tree_interface,
+                                            bNodeTreeInterfaceSocket &socket,
+                                            bNodeTreeInterfacePanel *parent,
+                                            int position)
+{
+  if (parent == nullptr) {
+    parent = &tree_interface.root_panel;
+  }
+
+  bNodeTreeInterfaceSocket *csocket = static_cast<bNodeTreeInterfaceSocket *>(
+      MEM_dupallocN(&socket));
+  /* Generate a new unique identifier.
+   * This might break existing links, but the identifiers were duplicate anyway. */
+  char *dst_identifier = BLI_sprintfN("Socket_%d", tree_interface.next_uid++);
+  version_copy_socket(*csocket, socket, dst_identifier);
+
+  version_nodes_insert_item(*parent, *csocket, position);
+
+  /* Original socket becomes output. */
+  socket.flag &= ~NODE_INTERFACE_SOCKET_INPUT;
+  /* Copied socket becomes input. */
+  csocket->flag &= ~NODE_INTERFACE_SOCKET_OUTPUT;
+}
+
 void blo_do_versions_400(FileData *fd, Library * /*lib*/, Main *bmain)
 {
   if (!MAIN_VERSION_FILE_ATLEAST(bmain, 400, 1)) {
@@ -1220,6 +1329,28 @@ void blo_do_versions_400(FileData *fd, Library * /*lib*/, Main *bmain)
         }
       }
     }
+
+    /* Convert sockets with both input and output flag into two separate sockets. */
+    FOREACH_NODETREE_BEGIN (bmain, ntree, id) {
+      blender::Vector<bNodeTreeInterfaceSocket *> sockets_to_split;
+      ntree->tree_interface.foreach_item([&](bNodeTreeInterfaceItem &item) {
+        if (item.item_type == NODE_INTERFACE_SOCKET) {
+          bNodeTreeInterfaceSocket &socket = reinterpret_cast<bNodeTreeInterfaceSocket &>(item);
+          if ((socket.flag & NODE_INTERFACE_SOCKET_INPUT) &&
+              (socket.flag & NODE_INTERFACE_SOCKET_OUTPUT)) {
+            sockets_to_split.append(&socket);
+          }
+        }
+        return true;
+      });
+
+      for (bNodeTreeInterfaceSocket *socket : sockets_to_split) {
+        const int position = ntree->tree_interface.find_item_position(socket->item);
+        bNodeTreeInterfacePanel *parent = ntree->tree_interface.find_item_parent(socket->item);
+        version_node_group_split_socket(ntree->tree_interface, *socket, parent, position + 1);
+      }
+    }
+    FOREACH_NODETREE_END;
   }
 
   /**

source/blender/editors/space_node/node_draw.cc

@@ -588,44 +588,19 @@ static Vector<NodeInterfaceItemData> node_build_item_data(bNode &node)
     if (const nodes::SocketDeclaration *socket_decl =
             dynamic_cast<const nodes::SocketDeclaration *>(item_decl->get()))
     {
-      bNodeSocket *used_input = nullptr;
-      bNodeSocket *used_output = nullptr;
       switch (socket_decl->in_out) {
         case SOCK_IN:
           BLI_assert(input != input_end);
-          used_input = *input;
+          result.append({socket_decl, *input, nullptr});
           ++input;
           break;
         case SOCK_OUT:
           BLI_assert(output != output_end);
-          used_output = *output;
+          result.append({socket_decl, nullptr, *output});
           ++output;
           break;
       }
       ++item_decl;
-
-      if (socket_decl->inline_with_next && item_decl != item_decl_end) {
-        /* Consume the next item as well when inlining sockets. */
-        const nodes::SocketDeclaration *next_socket_decl =
-            dynamic_cast<const nodes::SocketDeclaration *>(item_decl->get());
-        if (next_socket_decl && next_socket_decl->in_out != socket_decl->in_out) {
-          switch (next_socket_decl->in_out) {
-            case SOCK_IN:
-              BLI_assert(input != input_end);
-              used_input = *input;
-              ++input;
-              break;
-            case SOCK_OUT:
-              BLI_assert(output != output_end);
-              used_output = *output;
-              ++output;
-              break;
-          }
-          ++item_decl;
-        }
-      }
-
-      result.append({socket_decl, used_input, used_output});
     }
     else if (const nodes::PanelDeclaration *panel_decl =
                  dynamic_cast<const nodes::PanelDeclaration *>(item_decl->get()))
@@ -723,8 +698,8 @@ static void node_update_basis_from_declaration(
     else if (item.is_valid_socket()) {
       if (item.input) {
         SET_FLAG_FROM_TEST(item.input->flag, is_parent_collapsed, SOCK_PANEL_COLLAPSED);
-        /* Draw buttons before the first input, unless it's inline with an output. */
-        if (!item.socket_decl->inline_with_next && !buttons_drawn) {
+        /* Draw buttons before the first input. */
+        if (!buttons_drawn) {
           buttons_drawn = true;
           need_spacer_after_item = node_update_basis_buttons(
               C, ntree, node, node.typeinfo->draw_buttons, block, locy);

source/blender/makesrna/intern/rna_node_tree_interface.cc

@@ -24,11 +24,6 @@ const EnumPropertyItem rna_enum_node_tree_interface_item_type_items[] = {
 static const EnumPropertyItem node_tree_interface_socket_in_out_items[] = {
     {NODE_INTERFACE_SOCKET_INPUT, "INPUT", 0, "Input", "Generate a input node socket"},
     {NODE_INTERFACE_SOCKET_OUTPUT, "OUTPUT", 0, "Output", "Generate a output node socket"},
-    {NODE_INTERFACE_SOCKET_INPUT | NODE_INTERFACE_SOCKET_OUTPUT,
-     "BOTH",
-     0,
-     "Both",
-     "Generate both input and output node socket"},
     {0, nullptr, 0, nullptr, nullptr}};
 
 #ifdef RNA_RUNTIME

source/blender/nodes/NOD_node_declaration.hh

@@ -185,7 +185,6 @@ class SocketDeclaration : public ItemDeclaration {
   bool is_unavailable = false;
   bool is_attribute_name = false;
   bool is_default_link_socket = false;
-  bool inline_with_next = false;
 
   InputSocketFieldType input_field_type = InputSocketFieldType::None;
   OutputFieldDependency output_field_dependency;
@@ -635,10 +634,6 @@ class PanelDeclarationBuilder {
   typename DeclType::Builder &add_input(StringRef name, StringRef identifier = "");
   template<typename DeclType>
   typename DeclType::Builder &add_output(StringRef name, StringRef identifier = "");
-  template<typename DeclType>
-  typename DeclType::Builder &add_input_output(StringRef name,
-                                               StringRef identifier_in = "",
-                                               StringRef identifier_out = "");
 };
 
 using PanelDeclarationPtr = std::unique_ptr<PanelDeclaration>;
@@ -708,10 +703,6 @@ class NodeDeclarationBuilder {
   typename DeclType::Builder &add_input(StringRef name, StringRef identifier = "");
   template<typename DeclType>
   typename DeclType::Builder &add_output(StringRef name, StringRef identifier = "");
-  template<typename DeclType>
-  typename DeclType::Builder &add_input_output(StringRef name,
-                                               StringRef identifier_in = "",
-                                               StringRef identifier_out = "");
   PanelDeclarationBuilder &add_panel(StringRef name, int identifier = -1);
 
   aal::RelationsInNode &get_anonymous_attribute_relations()
@@ -923,21 +914,6 @@ typename DeclType::Builder &PanelDeclarationBuilder::add_output(StringRef name,
   return node_decl_builder_->add_socket<DeclType>(name, "", identifier, SOCK_OUT);
 }
 
-template<typename DeclType>
-typename DeclType::Builder &PanelDeclarationBuilder::add_input_output(StringRef name,
-                                                                      StringRef identifier_in,
-                                                                      StringRef identifier_out)
-{
-  if (is_complete_) {
-    static typename DeclType::Builder dummy_builder = {};
-    BLI_assert_unreachable();
-    return dummy_builder;
-  }
-  ++this->decl_->num_child_decls;
-  return node_decl_builder_->add_socket<DeclType>(
-      name, identifier_in, identifier_out, SOCK_IN | SOCK_OUT);
-}
-
 /** \} */
 
 /* -------------------------------------------------------------------- */
@@ -970,14 +946,6 @@ inline typename DeclType::Builder &NodeDeclarationBuilder::add_output(StringRef
   return this->add_socket<DeclType>(name, "", identifier, SOCK_OUT);
 }
 
-template<typename DeclType>
-inline typename DeclType::Builder &NodeDeclarationBuilder::add_input_output(
-    StringRef name, StringRef identifier_in, StringRef identifier_out)
-{
-  set_active_panel_builder(nullptr);
-  return this->add_socket<DeclType>(name, identifier_in, identifier_out, SOCK_IN | SOCK_OUT);
-}
-
 template<typename DeclType>
 inline typename DeclType::Builder &NodeDeclarationBuilder::add_socket(StringRef name,
                                                                       StringRef identifier_in,

source/blender/nodes/intern/node_common.cc

@@ -387,18 +387,14 @@ void node_group_declare_dynamic(const bNodeTree & /*node_tree*/,
                                                declaration_for_interface_socket(
                                                    *group, socket, SOCK_OUT) :
                                                nullptr;
-        /* Inline with the output socket if using input+output mode. */
-        if (input_decl && output_decl) {
-          input_decl->inline_with_next = true;
+        if (output_decl) {
+          r_declaration.outputs.append(output_decl.get());
+          r_declaration.items.append(std::move(output_decl));
         }
         if (input_decl) {
           r_declaration.inputs.append(input_decl.get());
           r_declaration.items.append(std::move(input_decl));
         }
-        if (output_decl) {
-          r_declaration.outputs.append(output_decl.get());
-          r_declaration.items.append(std::move(output_decl));
-        }
         break;
       }
       case NODE_INTERFACE_PANEL: {

source/blender/nodes/intern/node_declaration.cc

@@ -34,13 +34,6 @@ void build_node_declaration_dynamic(const bNodeTree &node_tree,
 
 void NodeDeclarationBuilder::finalize()
 {
-  /* Declarations generating both input and output should align these sockets. */
-  for (std::unique_ptr<BaseSocketDeclarationBuilder> &socket_builder : socket_builders_) {
-    if (socket_builder->input_declaration() && socket_builder->output_declaration()) {
-      socket_builder->input_declaration()->inline_with_next = true;
-    }
-  }
-
   if (is_function_node_) {
     for (std::unique_ptr<BaseSocketDeclarationBuilder> &socket_builder : socket_builders_) {
       if (SocketDeclaration *socket_decl = socket_builder->input_declaration()) {
@@ -207,17 +200,14 @@ bool NodeDeclaration::is_valid() const
         return false;
       }
 
-      /* Check for consistent outputs.., inputs.. blocks.
-       * Ignored for sockets that are inline pairs. */
-      if (!socket_decl->inline_with_next) {
-        if (state.socket_in_out == SOCK_OUT && socket_decl->in_out == SOCK_IN) {
-          /* Start of input sockets. */
-          state.socket_in_out = SOCK_IN;
-        }
-        if (socket_decl->in_out != state.socket_in_out) {
-          std::cout << "Output socket added after input socket" << std::endl;
-          return false;
-        }
+      /* Check for consistent outputs.., inputs.. blocks. */
+      if (state.socket_in_out == SOCK_OUT && socket_decl->in_out == SOCK_IN) {
+        /* Start of input sockets. */
+        state.socket_in_out = SOCK_IN;
+      }
+      if (socket_decl->in_out != state.socket_in_out) {
+        std::cout << "Output socket added after input socket" << std::endl;
+        return false;
       }
 
       /* Item counting for the panels, but ignore for root items. */

tests/python/bl_node_group_interface.py

@@ -132,18 +132,13 @@ class NodeGroupInterfaceTests:
         out1 = tree.interface.new_socket("Output 1", socket_type=socket_type, in_out='OUTPUT')
         self.assertIsNotNone(out1, f"Could not create socket of type {socket_type}")
 
-        inout0 = tree.interface.new_socket("Input/Output 0", socket_type=socket_type, in_out='BOTH')
-        self.assertIsNotNone(inout0, f"Could not create socket of type {socket_type}")
-
         self.assertSequenceEqual([(s.name, s.bl_idname) for s in group_node.inputs], [
             ("Input 0", socket_type),
             ("Input 1", socket_type),
-            ("Input/Output 0", socket_type),
         ])
         self.assertSequenceEqual([(s.name, s.bl_idname) for s in group_node.outputs], [
             ("Output 0", socket_type),
             ("Output 1", socket_type),
-            ("Input/Output 0", socket_type),
         ])
 
     def do_test_user_count(self, value, expected_users):