ilusha/blender
1
0
Fork 0
mirror of https://github.com/blender/blender synced 2026-09-29 04:37:17 +03:00
Code Issues Projects Releases Packages Wiki Activity Actions

Fix #152602: Avoid type conversion when making node groups

Browse source 
Making node groups uses the first internal socket type for an input,
which is shared with all the internal group sockets connected to the
same external socket. This can force a type conversion where there
wasn't any before making the group, leading to changes in behavior.

Exposing output sockets also adds multiple redundant group outputs,
which all use the same socket type and are connected to only one
external socket.

This patch fixes both of these issues:
- The type of a group node input is based on the external socket type.
  If this is connected to multiple internal inputs they still share the
  same   group input connection, but no new type conversions are
  created.
- Exposing an output socket with multiple links creates just one unique
  group socket now.

To make this work the `add_interface_socket_from_node` function has been
extended with an `in_out` argument, so that the socket type can be based
on the external (output) socket but create a group input socket. Some
variants of this function have been removed because they are
unnecessary. The function had a `socket_type` argument that wasn't
actually used, and wouldn't have worked anyway because
`interface_from_socket` expects matching socket/io_socket types.

Pull Request: https://projects.blender.org/blender/blender/pulls/153992
This commit is contained in:
Lukas Tönne 2026-02-24 09:38:14 +01:00
parent 6d9d9b0e06
commit 63fccca32d
7 changed files with 53 additions and 70 deletions
Download patch file Download diff file Expand all files Collapse all files

36
source/blender/blenkernel/BKE_node_tree_interface.hh
View file

@ -353,28 +353,20 @@ template<typename T> const T &get_socket_data_as(const bNodeTreeInterfaceSocket
return *static_cast<const T *>(item.socket_data);
}
bNodeTreeInterfaceSocket *add_interface_socket_from_node(bNodeTree &ntree,
const bNode &from_node,
const bNodeSocket &from_sock,
StringRef socket_type,
StringRef name);
inline bNodeTreeInterfaceSocket *add_interface_socket_from_node(bNodeTree &ntree,
const bNode &from_node,
const bNodeSocket &from_sock,
const StringRef socket_type)
{
return add_interface_socket_from_node(
ntree, from_node, from_sock, socket_type, node_socket_label(from_sock));
}
inline bNodeTreeInterfaceSocket *add_interface_socket_from_node(bNodeTree &ntree,
const bNode &from_node,
const bNodeSocket &from_sock)
{
return add_interface_socket_from_node(
ntree, from_node, from_sock, from_sock.typeinfo->idname, node_socket_label(from_sock));
}
/**
* Add a tree interface socket based on the properties of an existing node socket.
* \param ntree Node tree where the interface socket is added.
* \param from_node Node that owns the template socket. Does not have to be part of \a ntree.
* \param from_sock Template socket on which the interface properties are based.
* \param name Optional custom socket name. By default the visible socket label or name is used.
* \param in_out Optional input/output direction. By default same as the template socket.
*/
bNodeTreeInterfaceSocket *add_interface_socket_from_node(
bNodeTree &ntree,
const bNode &from_node,
const bNodeSocket &from_sock,
std::optional<StringRef> name = std::nullopt,
std::optional<eNodeSocketInOut> in_out = std::nullopt);
/**
* Reference to a node tree's interface item.

29
source/blender/blenkernel/intern/node_tree_interface.cc
View file

@ -1345,13 +1345,19 @@ static bNodeTreeInterfaceSocket *make_socket(const int uid,
return new_socket;
}
bNodeTreeInterfaceSocket *add_interface_socket_from_node(bNodeTree &ntree,
const bNode &from_node,
const bNodeSocket &from_sock,
const StringRef socket_type,
const StringRef name)
bNodeTreeInterfaceSocket *add_interface_socket_from_node(
bNodeTree &ntree,
const bNode &from_node,
const bNodeSocket &from_sock,
const std::optional<StringRef> name,
const std::optional<eNodeSocketInOut> in_out)
{
ntree.ensure_topology_cache();
BLI_assert(from_sock.typeinfo);
const StringRef socket_type = from_sock.typeinfo->idname;
const bool is_input = in_out ? bool(*in_out & SOCK_IN) : from_sock.is_input();
bNodeTreeInterfaceSocket *iosock = nullptr;
if (from_node.is_group()) {
if (const bNodeTree *group = reinterpret_cast<const bNodeTree *>(from_node.id)) {
@ -1359,16 +1365,16 @@ bNodeTreeInterfaceSocket *add_interface_socket_from_node(bNodeTree &ntree,
*/
group->ensure_interface_cache();
const bNodeTreeInterfaceSocket &src_io_socket =
from_sock.is_input() ? *group->interface_inputs()[from_sock.index()] :
*group->interface_outputs()[from_sock.index()];
is_input ? *group->interface_inputs()[from_sock.index()] :
*group->interface_outputs()[from_sock.index()];
iosock = reinterpret_cast<bNodeTreeInterfaceSocket *>(
ntree.tree_interface.add_item_copy(src_io_socket.item, nullptr));
}
}
if (!iosock) {
NodeTreeInterfaceSocketFlag flag = NodeTreeInterfaceSocketFlag(0);
SET_FLAG_FROM_TEST(flag, from_sock.in_out & SOCK_IN, NODE_INTERFACE_SOCKET_INPUT);
SET_FLAG_FROM_TEST(flag, from_sock.in_out & SOCK_OUT, NODE_INTERFACE_SOCKET_OUTPUT);
SET_FLAG_FROM_TEST(flag, is_input, NODE_INTERFACE_SOCKET_INPUT);
SET_FLAG_FROM_TEST(flag, !is_input, NODE_INTERFACE_SOCKET_OUTPUT);
const nodes::SocketDeclaration *decl = from_sock.runtime->declaration;
StringRef description = from_sock.description;
@ -1377,14 +1383,15 @@ bNodeTreeInterfaceSocket *add_interface_socket_from_node(bNodeTree &ntree,
description = decl->description;
}
SET_FLAG_FROM_TEST(flag, decl->optional_label, NODE_INTERFACE_SOCKET_OPTIONAL_LABEL);
if (socket_type == "NodeSocketMenu" && from_sock.type == SOCK_MENU) {
if (from_sock.type == SOCK_MENU) {
if (const auto *menu_decl = dynamic_cast<const nodes::decl::Menu *>(decl)) {
SET_FLAG_FROM_TEST(flag, menu_decl->is_expanded, NODE_INTERFACE_SOCKET_MENU_EXPANDED);
}
}
}
iosock = ntree.tree_interface.add_socket(name, description, socket_type, flag, nullptr);
const StringRef io_name = name ? *name : node_socket_label(from_sock);
iosock = ntree.tree_interface.add_socket(io_name, description, socket_type, flag, nullptr);
if (iosock) {
if (decl) {

6
source/blender/editors/space_node/link_drag_search.cc
View file

@ -97,11 +97,7 @@ static void add_group_input_node_fn(nodes::LinkSearchOpParams &params)
{
/* Add a group input based on the connected socket, and add a new group input node. */
bNodeTreeInterfaceSocket *socket_iface = bke::node_interface::add_interface_socket_from_node(
params.node_tree,
params.node,
params.socket,
params.socket.typeinfo->idname,
params.socket.name);
params.node_tree, params.node, params.socket, params.socket.name);
params.node_tree.tree_interface.active_item_set(&socket_iface->item);
bNode &group_input = params.add_node("NodeGroupInput");

36
source/blender/editors/space_node/node_copy_util.cc
View file

@ -238,6 +238,7 @@ static const bNodeSocket &find_socket_to_use_for_interface(const bNodeTree &node
static bNodeTreeInterfaceSocket *add_interface_from_socket(const bNodeTree &original_tree,
const bNodeSocket &socket,
const eNodeSocketInOut in_out,
bNodeTree &tree_for_interface,
bNodeTreeInterfacePanel *parent)
{
@ -252,7 +253,7 @@ static bNodeTreeInterfaceSocket *add_interface_from_socket(const bNodeTree &orig
const bNode &node_for_io = socket_for_io.owner_node();
const bNodeSocket &socket_for_name = prefer_node_for_interface_name ? socket : socket_for_io;
bNodeTreeInterfaceSocket *io_socket = bke::node_interface::add_interface_socket_from_node(
tree_for_interface, node_for_io, socket_for_io, socket_for_io.idname, socket_for_name.name);
tree_for_interface, node_for_io, socket_for_io, socket_for_name.name, in_out);
if (io_socket) {
tree_for_interface.tree_interface.move_item_to_parent(io_socket->item, parent, INT32_MAX);
}
@ -336,22 +337,23 @@ void NodeSetInterfaceBuilder::expose_socket(const bNodeSocket &src_socket,
if (params_.skip_hidden && !src_socket.is_visible()) {
return;
}
const eNodeSocketInOut in_out = eNodeSocketInOut(src_socket.in_out);
auto try_add_socket_data = [&](const bNodeSocket &key,
const bNodeSocket &template_socket) -> InterfaceSocketData * {
auto try_add_socket_data = [&](const bNodeSocket &key_socket) -> InterfaceSocketData * {
InterfaceSocketData *data = io_mapping_.socket_data.lookup_ptr(
data_by_socket_.lookup_default(&key, nullptr));
data_by_socket_.lookup_default(&key_socket, nullptr));
if (data) {
return data;
}
bNodeTreeInterfaceSocket *io_socket = add_interface_from_socket(
src_tree, template_socket, dst_tree_, parent);
src_tree, key_socket, in_out, dst_tree_, parent);
if (io_socket) {
data = &io_mapping_.socket_data.lookup_or_add(io_socket, {});
data_by_socket_.add_new(&key, io_socket);
data_by_socket_.add_new(&key_socket, io_socket);
data->hidden = template_socket.flag & SOCK_HIDDEN;
data->collapsed = template_socket.flag & SOCK_COLLAPSED;
data->hidden = key_socket.flag & SOCK_HIDDEN;
data->collapsed = key_socket.flag & SOCK_COLLAPSED;
return data;
}
return nullptr;
@ -366,7 +368,7 @@ void NodeSetInterfaceBuilder::expose_socket(const bNodeSocket &src_socket,
if (external_links.is_empty()) {
if (!params_.skip_unconnected) {
if (InterfaceSocketData *data = try_add_socket_data(src_socket, src_socket)) {
if (InterfaceSocketData *data = try_add_socket_data(src_socket)) {
data->internal_sockets.add({src_socket.owner_node(), src_socket});
}
}
@ -378,20 +380,8 @@ void NodeSetInterfaceBuilder::expose_socket(const bNodeSocket &src_socket,
params_.use_unique_output;
if (use_external_socket_key) {
/* Create a unique interface socket for each external link. */
/* TODO This creates some problems:
* - Input sockets with the same external link still use the internal socket as the interface
* template. The first input defines the interface type, which can lead to incorrect type
* conversion for the remaining sockets. Interface state is also based on the first internal
* socket.
* The external link should define be the interface template here.
* - Output sockets with multiple external links are redundant because the internal socket is
* used as the interface template.
* Outputs should not create unique interface sockets for each link.
*/
for (const MutableNodeAndSocket &external_socket : external_links) {
if (InterfaceSocketData *data = try_add_socket_data(external_socket.find_socket(),
src_socket))
{
if (InterfaceSocketData *data = try_add_socket_data(external_socket.find_socket())) {
data->internal_sockets.add({src_socket.owner_node(), src_socket});
data->external_sockets.add(external_socket);
}
@ -399,7 +389,7 @@ void NodeSetInterfaceBuilder::expose_socket(const bNodeSocket &src_socket,
}
else {
/* Create interface based on the internal socket. */
if (InterfaceSocketData *data = try_add_socket_data(src_socket, src_socket)) {
if (InterfaceSocketData *data = try_add_socket_data(src_socket)) {
data->internal_sockets.add({src_socket.owner_node(), src_socket});
data->external_sockets.add_multiple(external_links);
}

8
source/blender/editors/space_node/node_group.cc
View file

@ -580,13 +580,9 @@ static void node_group_make_insert_selected(const bContext &C,
params.skip_hidden = true;
/* Expose only connected sockets if there is more than one node. */
params.skip_unconnected = (nodes.size() > 1);
/* TODO Shared external connection will only create a single interface socket, but its type is
* based on the first internal socket. This creates potential conversion conflicts.
* (see also NodeSetInterfaceBuilder::expose_socket). */
/* Share external connections if a socket has multiple links. */
params.use_unique_input = false;
/* TODO Unique output interface sockets are redundant and all use the same internal socket
* template. (see also NodeSetInterfaceBuilder::expose_socket). */
params.use_unique_output = true;
params.use_unique_output = false;
const NodeTreeInterfaceMapping io_mapping = build_node_set_interface(
params, ntree, nodes, group);

4
tests/files/node_group/node_copy_operators.blend
View file

@ -1,3 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:a7abc44d5d50ec98c2b022c94cc492a19249cf1e36f492c5f52e3cceab9fb570
size 268335
oid sha256:5cf70e286eabcca49bd7cdd8a6a838cd54a3b143fea9a4dd7ce8af5087c4ee33
size 259371

4
tests/python/bl_node_copy_operators.py
View file

@ -267,6 +267,8 @@ def execute_group_insert(test_case, test_tree, expected_tree=None):
# Should have one node group labeled "external".
assert len(test_nodes_external) == 1
group_node = test_nodes_external[0]
# Ensure single-user node group, so that inserting nodes does not modify a shared tree.
group_node.node_tree = group_node.node_tree.copy()
with node_editor_context_override(bpy.context, test_tree, selected_nodes=test_nodes_internal + [group_node], active_node=group_node):
bpy.ops.node.group_insert()
@ -400,7 +402,7 @@ class AbstractNodeCopyOperatorTest(unittest.TestCase):
self.assertTrue(args.testdir.exists(),
'Test dir {0} should exist'.format(args.testdir))
open_test_file()
self.assertEqual(bpy.data.version, (5, 1, 27))
self.assertEqual(bpy.data.version, (5, 2, 5))
def tearDown(self):
self._tempdir.cleanup()