Fix for #35291, Deleting 'Group Output' node in compositor causes Segfault. A group without an output node in compositor would leave the original Node instance in the graph with outgoing connections. This causes trouble because the Node is expected to be a NodeOperation. Now group nodes always get disconnected and if no output node is present will use the default group output values (which is slightly less confusing than using input values from connected nodes).

2 files changed, 93 insertions, 0 deletions

diff --git a/source/blender/compositor/nodes/COM_GroupNode.cpp b/source/blender/compositor/nodes/COM_GroupNode.cpp
index 09824a7b439..85554d7a7aa 100644
--- a/source/blender/compositor/nodes/COM_GroupNode.cpp
+++ b/source/blender/compositor/nodes/COM_GroupNode.cpp
@@ -26,6 +26,13 @@
 #include "COM_SocketProxyNode.h"
 #include "COM_SetColorOperation.h"
 #include "COM_ExecutionSystemHelper.h"
+#include "COM_SetValueOperation.h"
+#include "COM_SetVectorOperation.h"
+#include "COM_SetColorOperation.h"
+
+extern "C" {
+#include "RNA_access.h"
+}
 
 GroupNode::GroupNode(bNode *editorNode) : Node(editorNode)
 {
@@ -83,6 +90,7 @@ void GroupNode::ungroup(ExecutionSystem &system)
 
 	const bool groupnodeBuffering = system.getContext().isGroupnodeBufferEnabled();
 
+	bool has_output = false;
 	/* create proxy nodes for group input/output nodes */
 	for (bNode *bionode = (bNode *)subtree->nodes.first; bionode; bionode = bionode->next) {
 		if (bionode->type == NODE_GROUP_INPUT) {
@@ -100,6 +108,7 @@ void GroupNode::ungroup(ExecutionSystem &system)
 		}
 		
 		if (bionode->type == NODE_GROUP_OUTPUT && (bionode->flag & NODE_DO_OUTPUT)) {
+			has_output = true;
 			for (bNodeSocket *bsock = (bNodeSocket *)bionode->inputs.first; bsock; bsock = bsock->next) {
 				OutputSocket *gsock;
 				find_group_output(this, bsock->identifier, &gsock);
@@ -114,11 +123,91 @@ void GroupNode::ungroup(ExecutionSystem &system)
 		}
 	}
 	
+	/* in case no output node exists, add input value operations using defaults */
+	if (!has_output) {
+		for (int index = 0; index < getNumberOfOutputSockets(); ++index) {
+			OutputSocket *output = getOutputSocket(index);
+			addDefaultOutputOperation(system, output);
+		}
+	}
+	
 	/* unlink the group node itself, input links have been duplicated */
 	for (int index = 0; index < this->getNumberOfInputSockets(); ++index) {
 		InputSocket *sock = this->getInputSocket(index);
 		sock->unlinkConnections(&system);
 	}
+	for (int index = 0; index < this->getNumberOfOutputSockets(); ++index) {
+		OutputSocket *sock = this->getOutputSocket(index);
+		sock->clearConnections();
+	}
 	
 	ExecutionSystemHelper::addbNodeTree(system, nodes_start, subtree, this->getInstanceKey());
 }
+
+bNodeSocket *GroupNode::findInterfaceInput(InputSocket *socket)
+{
+	bNode *bnode = this->getbNode();
+	bNodeTree *subtree = (bNodeTree *)bnode->id;
+	if (!subtree)
+		return NULL;
+	
+	const char *identifier = socket->getbNodeSocket()->identifier;
+	for (bNodeSocket *iosock = (bNodeSocket *)subtree->inputs.first; iosock; iosock = iosock->next)
+		if (STREQ(iosock->identifier, identifier))
+			return iosock;
+	return NULL;
+}
+
+bNodeSocket *GroupNode::findInterfaceOutput(OutputSocket *socket)
+{
+	bNode *bnode = this->getbNode();
+	bNodeTree *subtree = (bNodeTree *)bnode->id;
+	if (!subtree)
+		return NULL;
+	
+	const char *identifier = socket->getbNodeSocket()->identifier;
+	for (bNodeSocket *iosock = (bNodeSocket *)subtree->outputs.first; iosock; iosock = iosock->next)
+		if (STREQ(iosock->identifier, identifier))
+			return iosock;
+	return NULL;
+}
+
+void GroupNode::addDefaultOutputOperation(ExecutionSystem &system, OutputSocket *outputsocket)
+{
+	bNodeSocket *iosock = findInterfaceOutput(outputsocket);
+	if (!iosock)
+		return;
+	
+	PointerRNA ptr;
+	RNA_pointer_create(&getbNodeTree()->id, &RNA_NodeSocket, iosock, &ptr);
+	
+	NodeOperation *operation = NULL;
+	switch (iosock->typeinfo->type) {
+		case SOCK_FLOAT: {
+			float value = RNA_float_get(&ptr, "default_value");
+			SetValueOperation *value_op = new SetValueOperation();
+			value_op->setValue(value);
+			operation = value_op;
+			break;
+		}
+		case SOCK_VECTOR: {
+			float vector[3];
+			RNA_float_get_array(&ptr, "default_value", vector);
+			SetVectorOperation *vector_op = new SetVectorOperation();
+			vector_op->setVector(vector);
+			operation = vector_op;
+			break;
+		}
+		case SOCK_RGBA: {
+			float color[4];
+			RNA_float_get_array(&ptr, "default_value", color);
+			SetColorOperation *color_op = new SetColorOperation();
+			color_op->setChannels(color);
+			operation = color_op;
+			break;
+		}
+	}
+	
+	outputsocket->relinkConnections(operation->getOutputSocket());
+	system.addOperation(operation);
+}
diff --git a/source/blender/compositor/nodes/COM_GroupNode.h b/source/blender/compositor/nodes/COM_GroupNode.h
index ba270eaf16c..02b63fe27e3 100644
--- a/source/blender/compositor/nodes/COM_GroupNode.h
+++ b/source/blender/compositor/nodes/COM_GroupNode.h
@@ -52,6 +52,10 @@ public:
 	 * @param system the ExecutionSystem where to add the subtree
 	 */
 	void ungroup(ExecutionSystem &system);
+
+	bNodeSocket *findInterfaceInput(InputSocket *socket);
+	bNodeSocket *findInterfaceOutput(OutputSocket *socket);
+	void addDefaultOutputOperation(ExecutionSystem &system, OutputSocket *outputsocket);
 };
 
 #endif