Fix #107779: Support simulation zone remapping in node group operators #107807

Lukas Tönne · 2023-05-10T12:41:31+02:00

Lukas Tönne commented

2023-05-10 12:41:31 +02:00

Adds a remap_pairing function for node group operators that ensures the simulation input nodes' output_node_id matches the new node are creating a group, ungrouping a node group, or separating from a group.

Also fixes a crash in the "Group Separate" operator when group input/output nodes are included in the selection.

Adds a `remap_pairing` function for node group operators that ensures the simulation input nodes' `output_node_id` matches the new node are creating a group, ungrouping a node group, or separating from a group. Also fixes a crash in the "Group Separate" operator when group input/output nodes are included in the selection.

Lukas Tönne added 3 commits 2023-05-10 12:41:38 +02:00

daea0872f3 Support remapping of simulation zone pairing in the Make Group operator.

5da5daaf93 Fix crashes and simulation zone remapping in the group separate operator.

c92f18eecf Generalized the simulation pairing remap function, support ungroup op.

Lukas Tönne added this to the Nodes & Physics project 2023-05-10 12:42:01 +02:00

Jacques Lucke approved these changes 2023-05-10 14:03:50 +02:00

source/blender/editors/space_node/node_group.cc Outdated

						
				@ -360,7 +389,8 @@ static bool node_group_ungroup(Main *bmain, bNodeTree *ntree, bNode *gnode)

				        /* find internal links to this output */

				        for (bNodeLink *tlink = glinks_first->next; tlink != glinks_last->next;

				             tlink = tlink->next) {

Jacques Lucke commented

2023-05-10 13:55:31 +02:00

Clang format does not change this for me.

Lukas Tönne commented

2023-05-10 17:24:31 +02:00

Not sure why. Both the CLI clang-format and the qtcreator Beautifier plugin produce this output. My clang-format version is 15.0.7

source/blender/editors/space_node/node_group.cc

						
				@ -140,0 +140,4 @@

				/* Maps old to new identifiers for simulation input node pairing. */

				template<typename NodeIterator>

				static void remap_pairing(bNodeTree &dst_tree,

				                          NodeIterator nodes,

Jacques Lucke commented

2023-05-10 13:58:34 +02:00

Think it should be ok to just use a span instead of a template here. Performance doesn't matter at this point. You could construct the vector directly when calling remap_pairing or just create the vector already when inserting elements in node_map.

Think it should be ok to just use a span instead of a template here. Performance doesn't matter at this point. You could construct the vector directly when calling `remap_pairing` or just create the vector already when inserting elements in `node_map`.

👍 2

LukasTonne marked this conversation as resolved

Lukas Tönne changed title from ~~Support simulation zone remapping in node group operators~~ to Fix #107779: Support simulation zone remapping in node group operators

2023-05-10 17:06:13 +02:00

Lukas Tönne commented

2023-05-10 17:09:51 +02:00

This also addresses the flaw in the Group Separate operator mentioned in #106852: That operator now uses a node_map for recreating links instead of relying on unmodified node identifiers.

This also addresses the flaw in the Group Separate operator mentioned in #106852: That operator now uses a `node_map` for recreating links instead of relying on unmodified node identifiers.

Lukas Tönne added 1 commit 2023-05-10 17:26:06 +02:00

07ac423aae Use Span<bNode *> instead of generic NodeIterator template parameter.

Lukas Tönne referenced this pull request

2023-05-10 17:29:07 +02:00

Unify the various copy procedures used in node editor #106852

Lukas Tönne merged commit 3913d22757 into main

2023-05-10 17:56:29 +02:00

Lukas Tönne deleted branch group_op_simzone_remap

2023-05-10 17:56:33 +02:00

Lukas Tönne referenced this issue from a commit

2023-05-10 17:56:39 +02:00

Fix #107779: Support simulation zone remapping in node group operators