Modification of node groups by adding/removing nodes is not possible yet. This patch extends the 'Make Group' operator and adds a new 'Separate' operator to add such functionality.

1) For inserting into existing groups:
  The 'Make Group from selected' (CTRL+g) operator shows a selection popup (like the object parenting operator), with options depending on the type of the active node (last selected):
  * "New" -> regular operator, creates new group type with all selected nodes inside.
  * "Insert" (only if active node is a group) -> adds all other selected nodes into the group.

  Currently still prohibits groups inside groups in general, though would be technically possible as long as no actual recursion occurs (group containing itself).

2) For extracting from an existing group:
  New 'Separate from group' operator (p), works similar to separating vertices/edges/faces from mesh. Two modes:
  * "Copy" makes a copy of the nodes in the parent tree, but keeps the original group intact.
  * "Move" removes selected nodes from the node group and adds them to the parent tree

2 files changed, 0 insertions, 298 deletions

diff --git a/source/blender/nodes/intern/node_common.c b/source/blender/nodes/intern/node_common.c
index 24d1bd70e5a..150bede4b7c 100644
--- a/source/blender/nodes/intern/node_common.c
+++ b/source/blender/nodes/intern/node_common.c
@@ -32,8 +32,6 @@
 
 #include <string.h>
 
-#include "DNA_action_types.h"
-#include "DNA_anim_types.h"
 #include "DNA_node_types.h"
 
 #include "BLI_listbase.h"
@@ -42,8 +40,6 @@
 
 #include "BLF_translation.h"
 
-#include "BKE_action.h"
-#include "BKE_animsys.h"
 #include "BKE_global.h"
 #include "BKE_library.h"
 #include "BKE_main.h"
@@ -112,298 +108,6 @@ bNodeSocket *node_group_add_extern_socket(bNodeTree *UNUSED(ntree), ListBase *lb
 	return sock;
 }
 
-bNode *node_group_make_from_selected(bNodeTree *ntree)
-{
-	bNodeLink *link, *linkn;
-	bNode *node, *gnode, *nextn;
-	bNodeTree *ngroup;
-	bNodeSocket *gsock;
-	ListBase anim_basepaths = {NULL, NULL};
-	float min[2], max[2];
-	int totnode=0;
-	bNodeTemplate ntemp;
-	
-	INIT_MINMAX2(min, max);
-	
-	/* is there something to group? also do some clearing */
-	for (node= ntree->nodes.first; node; node= node->next) {
-		if (node->flag & NODE_SELECT) {
-			/* no groups in groups */
-			if (node->type==NODE_GROUP)
-				return NULL;
-			DO_MINMAX2((&node->locx), min, max);
-			totnode++;
-		}
-		node->done = FALSE;
-	}
-	if (totnode==0) return NULL;
-	
-	/* check if all connections are OK, no unselected node has both
-	 * inputs and outputs to a selection */
-	for (link= ntree->links.first; link; link= link->next) {
-		if (link->fromnode && link->tonode && link->fromnode->flag & NODE_SELECT)
-			link->tonode->done |= 1;
-		if (link->fromnode && link->tonode && link->tonode->flag & NODE_SELECT)
-			link->fromnode->done |= 2;
-	}	
-	
-	for (node= ntree->nodes.first; node; node= node->next) {
-		if ((node->flag & NODE_SELECT)==0)
-			if (node->done==3)
-				break;
-	}
-	if (node) 
-		return NULL;
-	
-	/* OK! new nodetree */
-	ngroup= ntreeAddTree("NodeGroup", ntree->type, NODE_GROUP);
-	
-	/* move nodes over */
-	for (node= ntree->nodes.first; node; node= nextn) {
-		nextn= node->next;
-		if (node->flag & NODE_SELECT) {
-			/* keep track of this node's RNA "base" path (the part of the pat identifying the node) 
-			 * if the old nodetree has animation data which potentially covers this node
-			 */
-			if (ntree->adt) {
-				PointerRNA ptr;
-				char *path;
-				
-				RNA_pointer_create(&ntree->id, &RNA_Node, node, &ptr);
-				path = RNA_path_from_ID_to_struct(&ptr);
-				
-				if (path)
-					BLI_addtail(&anim_basepaths, BLI_genericNodeN(path));
-			}
-			
-			/* ensure valid parent pointers, detach if parent stays outside the group */
-			if (node->parent && !(node->parent->flag & NODE_SELECT))
-				nodeDetachNode(node);
-			
-			/* change node-collection membership */
-			BLI_remlink(&ntree->nodes, node);
-			BLI_addtail(&ngroup->nodes, node);
-			
-			node->locx-= 0.5f*(min[0]+max[0]);
-			node->locy-= 0.5f*(min[1]+max[1]);
-		}
-	}
-
-	/* move animation data over */
-	if (ntree->adt) {
-		LinkData *ld, *ldn=NULL;
-		
-		BKE_animdata_separate_by_basepath(&ntree->id, &ngroup->id, &anim_basepaths);
-		
-		/* paths + their wrappers need to be freed */
-		for (ld = anim_basepaths.first; ld; ld = ldn) {
-			ldn = ld->next;
-			
-			MEM_freeN(ld->data);
-			BLI_freelinkN(&anim_basepaths, ld);
-		}
-	}
-	
-	/* node groups don't use internal cached data */
-	ntreeFreeCache(ngroup);
-	
-	/* make group node */
-	ntemp.type = NODE_GROUP;
-	ntemp.ngroup = ngroup;
-	gnode= nodeAddNode(ntree, &ntemp);
-	gnode->locx= 0.5f*(min[0]+max[0]);
-	gnode->locy= 0.5f*(min[1]+max[1]);
-	
-	/* relink external sockets */
-	for (link= ntree->links.first; link; link= linkn) {
-		linkn= link->next;
-		
-		if (link->fromnode && link->tonode && (link->fromnode->flag & link->tonode->flag & NODE_SELECT)) {
-			BLI_remlink(&ntree->links, link);
-			BLI_addtail(&ngroup->links, link);
-		}
-		else if (link->tonode && (link->tonode->flag & NODE_SELECT)) {
-			gsock = node_group_expose_socket(ngroup, link->tosock, SOCK_IN);
-			link->tosock->link = nodeAddLink(ngroup, NULL, gsock, link->tonode, link->tosock);
-			link->tosock = node_group_add_extern_socket(ntree, &gnode->inputs, SOCK_IN, gsock);
-			link->tonode = gnode;
-		}
-		else if (link->fromnode && (link->fromnode->flag & NODE_SELECT)) {
-			/* search for existing group node socket */
-			for (gsock=ngroup->outputs.first; gsock; gsock=gsock->next)
-				if (gsock->link && gsock->link->fromsock==link->fromsock)
-					break;
-			if (!gsock) {
-				gsock = node_group_expose_socket(ngroup, link->fromsock, SOCK_OUT);
-				gsock->link = nodeAddLink(ngroup, link->fromnode, link->fromsock, NULL, gsock);
-				link->fromsock = node_group_add_extern_socket(ntree, &gnode->outputs, SOCK_OUT, gsock);
-			}
-			else
-				link->fromsock = node_group_find_output(gnode, gsock);
-			link->fromnode = gnode;
-		}
-	}
-
-	/* update of the group tree */
-	ngroup->update |= NTREE_UPDATE;
-	ntreeUpdateTree(ngroup);
-	/* update of the tree containing the group instance node */
-	ntree->update |= NTREE_UPDATE_NODES | NTREE_UPDATE_LINKS;
-	ntreeUpdateTree(ntree);
-
-	return gnode;
-}
-
-/* returns 1 if its OK */
-int node_group_ungroup(bNodeTree *ntree, bNode *gnode)
-{
-	bNodeLink *link, *linkn;
-	bNode *node, *nextn;
-	bNodeTree *ngroup, *wgroup;
-	ListBase anim_basepaths = {NULL, NULL};
-	
-	ngroup= (bNodeTree *)gnode->id;
-	if (ngroup==NULL) return 0;
-	
-	/* clear new pointers, set in copytree */
-	for (node= ntree->nodes.first; node; node= node->next)
-		node->new_node= NULL;
-	
-	/* wgroup is a temporary copy of the NodeTree we're merging in
-	 *	- all of wgroup's nodes are transferred across to their new home
-	 *	- ngroup (i.e. the source NodeTree) is left unscathed
-	 */
-	wgroup= ntreeCopyTree(ngroup);
-	
-	/* add the nodes into the ntree */
-	for (node= wgroup->nodes.first; node; node= nextn) {
-		nextn= node->next;
-		
-		/* keep track of this node's RNA "base" path (the part of the pat identifying the node) 
-		 * if the old nodetree has animation data which potentially covers this node
-		 */
-		if (wgroup->adt) {
-			PointerRNA ptr;
-			char *path;
-			
-			RNA_pointer_create(&wgroup->id, &RNA_Node, node, &ptr);
-			path = RNA_path_from_ID_to_struct(&ptr);
-			
-			if (path)
-				BLI_addtail(&anim_basepaths, BLI_genericNodeN(path));
-		}
-		
-		/* migrate node */
-		BLI_remlink(&wgroup->nodes, node);
-		BLI_addtail(&ntree->nodes, node);
-		
-		node->locx += gnode->locx;
-		node->locy += gnode->locy;
-		
-		node->flag |= NODE_SELECT;
-	}
-	
-	/* restore external links to and from the gnode */
-	for (link= ntree->links.first; link; link= link->next) {
-		if (link->fromnode==gnode) {
-			if (link->fromsock->groupsock) {
-				bNodeSocket *gsock= link->fromsock->groupsock;
-				if (gsock->link) {
-					if (gsock->link->fromnode) {
-						/* NB: using the new internal copies here! the groupsock pointer still maps to the old tree */
-						link->fromnode = (gsock->link->fromnode ? gsock->link->fromnode->new_node : NULL);
-						link->fromsock = gsock->link->fromsock->new_sock;
-					}
-					else {
-						/* group output directly maps to group input */
-						bNodeSocket *insock= node_group_find_input(gnode, gsock->link->fromsock);
-						if (insock->link) {
-							link->fromnode = insock->link->fromnode;
-							link->fromsock = insock->link->fromsock;
-						}
-					}
-				}
-				else {
-					/* copy the default input value from the group socket default to the external socket */
-					node_socket_convert_default_value(link->tosock->type, link->tosock->default_value, gsock->type, gsock->default_value);
-				}
-			}
-		}
-	}
-	/* remove internal output links, these are not used anymore */
-	for (link=wgroup->links.first; link; link= linkn) {
-		linkn = link->next;
-		if (!link->tonode)
-			nodeRemLink(wgroup, link);
-	}
-	/* restore links from internal nodes */
-	for (link= wgroup->links.first; link; link= link->next) {
-		/* indicates link to group input */
-		if (!link->fromnode) {
-			/* NB: can't use find_group_node_input here,
-			 * because gnode sockets still point to the old tree!
-			 */
-			bNodeSocket *insock;
-			for (insock= gnode->inputs.first; insock; insock= insock->next)
-				if (insock->groupsock->new_sock == link->fromsock)
-					break;
-			if (insock->link) {
-				link->fromnode = insock->link->fromnode;
-				link->fromsock = insock->link->fromsock;
-			}
-			else {
-				/* copy the default input value from the group node socket default to the internal socket */
-				node_socket_convert_default_value(link->tosock->type, link->tosock->default_value, insock->type, insock->default_value);
-				nodeRemLink(wgroup, link);
-			}
-		}
-	}
-	
-	/* add internal links to the ntree */
-	for (link= wgroup->links.first; link; link= linkn) {
-		linkn= link->next;
-		BLI_remlink(&wgroup->links, link);
-		BLI_addtail(&ntree->links, link);
-	}
-	
-	/* and copy across the animation,
-	 * note that the animation data's action can be NULL here */
-	if (wgroup->adt) {
-		LinkData *ld, *ldn=NULL;
-		bAction *waction;
-		
-		/* firstly, wgroup needs to temporary dummy action that can be destroyed, as it shares copies */
-		waction = wgroup->adt->action = BKE_action_copy(wgroup->adt->action);
-		
-		/* now perform the moving */
-		BKE_animdata_separate_by_basepath(&wgroup->id, &ntree->id, &anim_basepaths);
-		
-		/* paths + their wrappers need to be freed */
-		for (ld = anim_basepaths.first; ld; ld = ldn) {
-			ldn = ld->next;
-			
-			MEM_freeN(ld->data);
-			BLI_freelinkN(&anim_basepaths, ld);
-		}
-		
-		/* free temp action too */
-		if (waction) {
-			BKE_libblock_free(&G.main->action, waction);
-		}
-	}
-	
-	/* delete the group instance. this also removes old input links! */
-	nodeFreeNode(ntree, gnode);
-
-	/* free the group tree (takes care of user count) */
-	BKE_libblock_free(&G.main->nodetree, wgroup);
-	
-	ntree->update |= NTREE_UPDATE_NODES | NTREE_UPDATE_LINKS;
-	ntreeUpdateTree(ntree);
-	
-	return 1;
-}
-
 bNodeSocket *node_group_add_socket(bNodeTree *ngroup, const char *name, int type, int in_out)
 {
 	bNodeSocketType *stype = ntreeGetSocketType(type);
diff --git a/source/blender/nodes/intern/node_common.h b/source/blender/nodes/intern/node_common.h
index 616221d6658..f1bb837e483 100644
--- a/source/blender/nodes/intern/node_common.h
+++ b/source/blender/nodes/intern/node_common.h
@@ -37,8 +37,6 @@
 
 struct bNodeTree;
 
-struct bNodeSocket *node_group_add_extern_socket(struct bNodeTree *ntree, ListBase *lb, int in_out, struct bNodeSocket *gsock);
-
 void node_group_init(struct bNodeTree *ntree, struct bNode *node, struct bNodeTemplate *ntemp);
 void node_forloop_init(struct bNodeTree *ntree, struct bNode *node, struct bNodeTemplate *ntemp);
 void node_whileloop_init(struct bNodeTree *ntree, struct bNode *node, struct bNodeTemplate *ntemp);