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: Assert fails when ungrouping multiple node groups

Browse source 
Ungrouping multiple group nodes handles each group node in sequence. After the
first group is ungrouped the topology cache is invalidated which makes asserts
fail when ungrouping the next node.

The fix is to update the topology cache before mapping the node group interface
(`map_group_node_interface` function).

The bl_node_copy_operators test is extended to cover this test case, using two
group nodes that are ungrouped together. Supporting this requires a few
modifications of the test, such that multiple "internal" nodes are allowed in
the "ungroup" and "separate" functions.

Recursively mapping groups to compare operator results is now integrated into
the NodeMapping class.

Additionally this includes a fix for node selection after group operators:
Instead of deselecting nodes each time a set is copied, deselect once at the
beginning. This ensures multiple groups can be ungrouped and all the internal
nodes are selected in the end. Selection is how the test identifies ungrouped
nodes.

Pull Request: https://projects.blender.org/blender/blender/pulls/153755
This commit is contained in:
Lukas Tönne 2026-02-02 13:42:43 +01:00
parent 98b6c45877
commit 88cd2187ae
5 changed files with 60 additions and 52 deletions
Download patch file Download diff file Expand all files Collapse all files

92
tests/python/bl_node_copy_operators.py
View file

@ -61,7 +61,8 @@ all_test_cases = [
('test_anim_data', {}),
('test_extension_sockets', {}),
('test_pass_through', {'NODE_GROUP'}),
('test_insert_group_with_sockets', {'GROUP_INSERT'})
('test_insert_group_with_sockets', {'GROUP_INSERT'}),
('test_ungroup_multiple', {'NODE_GROUP'}),
]
@ -75,7 +76,7 @@ class NodeMapping:
def add_tree(self, test_tree, expected_tree):
self.tree_map[test_tree] = expected_tree
def add_node(self, test_node, expected_node):
def add_node(self, test_node, expected_node, map_group_node_trees):
assert self.node_map.get(test_node, expected_node) == expected_node
self.node_map[test_node] = expected_node
# Add all sockets of mapped nodes to their own dictionary, assuming the socket order is the same.
@ -86,12 +87,18 @@ class NodeMapping:
assert self.socket_map.get(test_socket, expected_socket) == expected_socket
self.socket_map[test_socket] = expected_socket
def add_nodes_by_name(self, test_nodes, expected_nodes):
if map_group_node_trees and isinstance(expected_node, bpy.types.GeometryNodeGroup):
assert isinstance(test_node, bpy.types.GeometryNodeGroup)
self.add_tree(test_node.node_tree, expected_node.node_tree)
self.add_nodes_by_name(test_node.node_tree.nodes, expected_node.node_tree.nodes, map_group_node_trees)
def add_nodes_by_name(self, test_nodes, expected_nodes, map_group_node_trees):
expected_node_by_name = {node.name: node for node in expected_nodes}
for test_node in test_nodes:
# Raises key error if not all test nodes can be mapped.
expected_node = expected_node_by_name.pop(test_node.name)
self.add_node(test_node, expected_node)
self.add_node(test_node, expected_node, map_group_node_trees)
# Should map all expected nodes.
assert not expected_node_by_name
@ -203,23 +210,17 @@ def execute_skip(test_case, test_tree, expected_tree=None):
return NodeMapping()
# NOOP case when the test starts out with a node group.
# NOOP case when the test starts out with node groups.
def execute_node_group_noop(test_case, test_tree, expected_tree=None):
test_nodes_internal, test_nodes_external = find_test_nodes(test_tree, test_case)
assert len(test_nodes_internal) == 1
group_node = test_nodes_internal[0]
if not expected_tree:
return
# Map resulting nodes to expected nodes.
expected_nodes_internal, expected_nodes_external = find_test_nodes(expected_tree, test_case)
assert len(expected_nodes_internal) == 1
expected_node = expected_nodes_internal[0]
mapping = NodeMapping()
mapping.add_tree(group_node.node_tree, expected_node.node_tree)
mapping.add_node(group_node, expected_node)
mapping.add_nodes_by_name(test_nodes_external, expected_nodes_external)
mapping.add_nodes_by_name(group_node.node_tree.nodes, expected_node.node_tree.nodes)
mapping.add_nodes_by_name(test_nodes_internal, expected_nodes_internal, True)
mapping.add_nodes_by_name(test_nodes_external, expected_nodes_external, False)
return mapping
@ -246,10 +247,8 @@ def execute_make_group(test_case, test_tree, expected_tree=None):
assert len(expected_nodes_internal) == 1
expected_node = expected_nodes_internal[0]
mapping = NodeMapping()
mapping.add_tree(group_node.node_tree, expected_node.node_tree)
mapping.add_node(group_node, expected_node)
mapping.add_nodes_by_name(test_nodes_external, expected_nodes_external)
mapping.add_nodes_by_name(group_node.node_tree.nodes, expected_node.node_tree.nodes)
mapping.add_node(group_node, expected_node, True)
mapping.add_nodes_by_name(test_nodes_external, expected_nodes_external, False)
return mapping
@ -279,10 +278,8 @@ def execute_group_insert(test_case, test_tree, expected_tree=None):
assert len(expected_nodes_external) == 1
expected_node = expected_nodes_external[0]
mapping = NodeMapping()
mapping.add_tree(group_node.node_tree, expected_node.node_tree)
mapping.add_node(group_node, expected_node)
mapping.add_nodes_by_name(test_nodes_external, expected_nodes_external)
mapping.add_nodes_by_name(group_node.node_tree.nodes, expected_node.node_tree.nodes)
mapping.add_node(group_node, expected_node, True)
mapping.add_nodes_by_name(test_nodes_external, expected_nodes_external, False)
return mapping
# Make empty node group.
@ -317,10 +314,8 @@ def execute_group_insert(test_case, test_tree, expected_tree=None):
assert len(expected_nodes_internal) == 1
expected_node = expected_nodes_internal[0]
mapping = NodeMapping()
mapping.add_tree(group_node.node_tree, expected_node.node_tree)
mapping.add_node(group_node, expected_node)
mapping.add_nodes_by_name(test_nodes_external, expected_nodes_external)
mapping.add_nodes_by_name(group_node.node_tree.nodes, expected_node.node_tree.nodes)
mapping.add_node(group_node, expected_node, True)
mapping.add_nodes_by_name(test_nodes_external, expected_nodes_external, False)
return mapping
@ -344,8 +339,8 @@ def execute_ungroup(test_case, test_tree, expected_tree=None):
# Map resulting nodes to expected nodes.
expected_nodes_internal, expected_nodes_external = find_test_nodes(expected_tree, test_case)
mapping = NodeMapping()
mapping.add_nodes_by_name(ungrouped_nodes, expected_nodes_internal)
mapping.add_nodes_by_name(test_nodes_external, expected_nodes_external)
mapping.add_nodes_by_name(ungrouped_nodes, expected_nodes_internal, False)
mapping.add_nodes_by_name(test_nodes_external, expected_nodes_external, False)
return mapping
@ -358,28 +353,31 @@ def execute_group_separate(type, test_case, test_tree, expected_tree=None):
test_nodes_internal, test_nodes_external = find_test_nodes(test_tree, test_case)
# Test nodes should be node groups
assert len(test_nodes_internal) == 1
group_node = test_nodes_internal[0]
assert isinstance(group_node, bpy.types.GeometryNodeGroup)
# Ensure single-user node group, so that moving nodes out does not modify a shared tree.
group_node.node_tree = group_node.node_tree.copy()
all_group_trees = dict()
for group_node in test_nodes_internal:
assert isinstance(group_node, bpy.types.GeometryNodeGroup)
group_tree = all_group_trees.get(group_node.node_tree)
if not group_tree:
group_tree = group_node.node_tree.copy()
all_group_trees[group_node.node_tree] = group_tree
group_node.node_tree = group_tree
with node_editor_context_override(bpy.context, test_tree, selected_nodes=[group_node]):
bpy.ops.node.group_edit(exit=False)
# Stay in current context so that the tree path has a valid "parent" tree to copy nodes into.
with node_editor_context_override(bpy.context, test_tree, selected_nodes=[group_node]):
bpy.ops.node.group_edit(exit=False)
# Stay in current context so that the tree path has a valid "parent" tree to copy nodes into.
# Select all nodes for separating.
select_nodes(group_node.node_tree, selected_nodes=group_node.node_tree.nodes)
bpy.ops.node.group_separate(type=type)
# Select all nodes for separating.
select_nodes(group_node.node_tree, selected_nodes=group_node.node_tree.nodes)
bpy.ops.node.group_separate(type=type)
separated_nodes = [node for node in test_tree.nodes if node.select and node.parent is None]
centroid = node_centroid(separated_nodes)
# Re-attach to the parent frame to identify the operator result.
for node in separated_nodes:
offset = node.location - centroid
node.parent = find_test_frame(test_tree, test_case)
node.location = group_node.location + Vector((0, -1000)) + offset
separated_nodes = [node for node in test_tree.nodes if node.select and node.parent is None]
centroid = node_centroid(separated_nodes)
# Re-attach to the parent frame to identify the operator result.
for node in separated_nodes:
offset = node.location - centroid
node.parent = find_test_frame(test_tree, test_case)
node.location = group_node.location + Vector((0, -1000)) + offset
if not expected_tree:
return
@ -387,8 +385,8 @@ def execute_group_separate(type, test_case, test_tree, expected_tree=None):
result_nodes_internal, _ = find_test_nodes(test_tree, test_case)
expected_nodes_internal, expected_nodes_external = find_test_nodes(expected_tree, test_case)
mapping = NodeMapping()
mapping.add_nodes_by_name(result_nodes_internal, expected_nodes_internal)
mapping.add_nodes_by_name(test_nodes_external, expected_nodes_external)
mapping.add_nodes_by_name(result_nodes_internal, expected_nodes_internal, False)
mapping.add_nodes_by_name(test_nodes_external, expected_nodes_external, False)
return mapping