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

6 files changed, 621 insertions, 310 deletions

diff --git a/source/blender/blenkernel/BKE_node.h b/source/blender/blenkernel/BKE_node.h
index e4d96c2219c..ef664f8fc4f 100644
--- a/source/blender/blenkernel/BKE_node.h
+++ b/source/blender/blenkernel/BKE_node.h
@@ -448,9 +448,7 @@ struct bNodeSocket *node_group_add_socket(struct bNodeTree *ngroup, const char *
 struct bNodeSocket *node_group_expose_socket(struct bNodeTree *ngroup, struct bNodeSocket *sock, int in_out);
 void node_group_expose_all_sockets(struct bNodeTree *ngroup);
 void node_group_remove_socket(struct bNodeTree *ngroup, struct bNodeSocket *gsock, int in_out);
-
-struct bNode	*node_group_make_from_selected(struct bNodeTree *ntree);
-int				node_group_ungroup(struct bNodeTree *ntree, struct bNode *gnode);
+struct bNodeSocket *node_group_add_extern_socket(struct bNodeTree *ntree, ListBase *lb, int in_out, struct bNodeSocket *gsock);
 
 /* in node_common.c */
 void register_node_type_frame(struct bNodeTreeType *ttype);
diff --git a/source/blender/editors/space_node/node_edit.c b/source/blender/editors/space_node/node_edit.c
index b4e07546fa9..81e375f26bc 100644
--- a/source/blender/editors/space_node/node_edit.c
+++ b/source/blender/editors/space_node/node_edit.c
@@ -46,11 +46,15 @@
 #include "DNA_particle_types.h"
 #include "DNA_scene_types.h"
 #include "DNA_world_types.h"
+#include "DNA_action_types.h"
+#include "DNA_anim_types.h"
 
 #include "BLI_math.h"
 #include "BLI_blenlib.h"
 #include "BLI_utildefines.h"
 
+#include "BKE_action.h"
+#include "BKE_animsys.h"
 #include "BKE_context.h"
 #include "BKE_depsgraph.h"
 #include "BKE_global.h"
@@ -94,6 +98,7 @@
 #include "GPU_material.h"
 
 #include "node_intern.h"
+#include "NOD_socket.h"
 
 static EnumPropertyItem socket_in_out_items[] = {
 	{ SOCK_IN, "SOCK_IN", 0, "Input", "" },
@@ -1109,6 +1114,155 @@ void NODE_OT_group_socket_move_down(wmOperatorType *ot)
 
 /* ******************** Ungroup operator ********************** */
 
+/* returns 1 if its OK */
+static 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 path 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;
+	
+	return 1;
+}
+
 static int node_group_ungroup_exec(bContext *C, wmOperator *op)
 {
 	SpaceNode *snode = CTX_wm_space_node(C);
@@ -1129,7 +1283,10 @@ static int node_group_ungroup_exec(bContext *C, wmOperator *op)
 		BKE_report(op->reports, RPT_WARNING, "Not a group");
 		return OPERATOR_CANCELLED;
 	}
-	else if (!node_group_ungroup(snode->edittree, gnode)) {
+	else if (node_group_ungroup(snode->nodetree, gnode)) {
+		ntreeUpdateTree(snode->nodetree);
+	}
+	else {
 		BKE_report(op->reports, RPT_WARNING, "Can't ungroup");
 		return OPERATOR_CANCELLED;
 	}
@@ -1155,6 +1312,200 @@ void NODE_OT_group_ungroup(wmOperatorType *ot)
 	ot->flag = OPTYPE_REGISTER|OPTYPE_UNDO;
 }
 
+/* ******************** Separate operator ********************** */
+
+/* returns 1 if its OK */
+static int node_group_separate_selected(bNodeTree *ntree, bNode *gnode, int make_copy)
+{
+	bNodeLink *link, *link_next;
+	bNode *node, *node_next, *newnode;
+	bNodeTree *ngroup;
+	ListBase anim_basepaths = {NULL, NULL};
+	
+	ngroup= (bNodeTree *)gnode->id;
+	if (ngroup==NULL) return 0;
+	
+	/* deselect all nodes in the target tree */
+	for (node=ntree->nodes.first; node; node=node->next)
+		node_deselect(node);
+	
+	/* clear new pointers, set in nodeCopyNode */
+	for (node= ngroup->nodes.first; node; node= node->next)
+		node->new_node= NULL;
+	
+	/* add selected nodes into the ntree */
+	for (node= ngroup->nodes.first; node; node= node_next) {
+		node_next = node->next;
+		if (!(node->flag & NODE_SELECT))
+			continue;
+		
+		if (make_copy) {
+			/* make a copy */
+			newnode = nodeCopyNode(ngroup, node);
+		}
+		else {
+			/* use the existing node */
+			newnode = node;
+		}
+		
+		/* keep track of this node's RNA "base" path (the part of the path identifying the node) 
+		 * if the old nodetree has animation data which potentially covers this node
+		 */
+		if (ngroup->adt) {
+			PointerRNA ptr;
+			char *path;
+			
+			RNA_pointer_create(&ngroup->id, &RNA_Node, newnode, &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 inside the group */
+		if (newnode->parent && !(newnode->parent->flag & NODE_SELECT))
+			nodeDetachNode(newnode);
+		
+		/* migrate node */
+		BLI_remlink(&ngroup->nodes, newnode);
+		BLI_addtail(&ntree->nodes, newnode);
+		
+		newnode->locx += gnode->locx;
+		newnode->locy += gnode->locy;
+	}
+	
+	/* add internal links to the ntree */
+	for (link= ngroup->links.first; link; link= link_next) {
+		int fromselect = (link->fromnode && (link->fromnode->flag & NODE_SELECT));
+		int toselect = (link->tonode && (link->tonode->flag & NODE_SELECT));
+		link_next = link->next;
+		
+		if (make_copy) {
+			/* make a copy of internal links */
+			if (fromselect && toselect)
+				nodeAddLink(ntree, link->fromnode->new_node, link->fromsock->new_sock, link->tonode->new_node, link->tosock->new_sock);
+		}
+		else {
+			/* move valid links over, delete broken links */
+			if (fromselect && toselect) {
+				BLI_remlink(&ngroup->links, link);
+				BLI_addtail(&ntree->links, link);
+			}
+			else if (fromselect || toselect) {
+				nodeRemLink(ngroup, link);
+			}
+		}
+	}
+	
+	/* and copy across the animation,
+	 * note that the animation data's action can be NULL here */
+	if (ngroup->adt) {
+		LinkData *ld, *ldn=NULL;
+		
+		/* now perform the moving */
+		BKE_animdata_separate_by_basepath(&ngroup->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);
+		}
+	}
+	
+	ntree->update |= NTREE_UPDATE_NODES | NTREE_UPDATE_LINKS;
+	if (!make_copy)
+		ngroup->update |= NTREE_UPDATE_NODES | NTREE_UPDATE_LINKS;
+	
+	return 1;
+}
+
+typedef enum eNodeGroupSeparateType {
+	NODE_GS_COPY,
+	NODE_GS_MOVE
+} eNodeGroupSeparateType;
+
+/* Operator Property */
+EnumPropertyItem node_group_separate_types[] = {
+    {NODE_GS_COPY, "COPY", 0, "Copy", "Copy to parent node tree, keep group intact"},
+	{NODE_GS_MOVE, "MOVE", 0, "Move", "Move to parent node tree, remove from group"},
+	{0, NULL, 0, NULL, NULL}
+};
+
+static int node_group_separate_exec(bContext *C, wmOperator *op)
+{
+	SpaceNode *snode = CTX_wm_space_node(C);
+	bNode *gnode;
+	int type = RNA_enum_get(op->ptr, "type");
+
+	ED_preview_kill_jobs(C);
+
+	/* are we inside of a group? */
+	gnode= node_tree_get_editgroup(snode->nodetree);
+	if (!gnode) {
+		BKE_report(op->reports, RPT_WARNING, "Not inside node group");
+		return OPERATOR_CANCELLED;
+	}
+	
+	switch (type) {
+	case NODE_GS_COPY:
+		if (!node_group_separate_selected(snode->nodetree, gnode, 1)) {
+			BKE_report(op->reports, RPT_WARNING, "Can't separate nodes");
+			return OPERATOR_CANCELLED;
+		}
+		break;
+	case NODE_GS_MOVE:
+		if (!node_group_separate_selected(snode->nodetree, gnode, 0)) {
+			BKE_report(op->reports, RPT_WARNING, "Can't separate nodes");
+			return OPERATOR_CANCELLED;
+		}
+		break;
+	}
+	
+	/* switch to parent tree */
+	snode_make_group_editable(snode, NULL);
+	
+	ntreeUpdateTree(snode->nodetree);
+	
+	snode_notify(C, snode);
+	snode_dag_update(C, snode);
+
+	return OPERATOR_FINISHED;
+}
+
+static int node_group_separate_invoke(bContext *C, wmOperator *UNUSED(op), wmEvent *UNUSED(event))
+{
+	uiPopupMenu *pup = uiPupMenuBegin(C, "Separate", ICON_NONE);
+	uiLayout *layout = uiPupMenuLayout(pup);
+	
+	uiLayoutSetOperatorContext(layout, WM_OP_EXEC_DEFAULT);
+	uiItemEnumO(layout, "NODE_OT_group_separate", NULL, 0, "type", NODE_GS_COPY);
+	uiItemEnumO(layout, "NODE_OT_group_separate", NULL, 0, "type", NODE_GS_MOVE);
+	
+	uiPupMenuEnd(C, pup);
+	
+	return OPERATOR_CANCELLED;
+}
+
+void NODE_OT_group_separate(wmOperatorType *ot)
+{
+	/* identifiers */
+	ot->name = "Separate";
+	ot->description = "Separate selected nodes from the node group";
+	ot->idname = "NODE_OT_group_separate";
+	
+	/* api callbacks */
+	ot->invoke = node_group_separate_invoke;
+	ot->exec = node_group_separate_exec;
+	ot->poll = ED_operator_node_active;
+	
+	/* flags */
+	ot->flag = OPTYPE_REGISTER|OPTYPE_UNDO;
+	
+	RNA_def_enum(ot->srna, "type", node_group_separate_types, NODE_GS_COPY, "Type", "");
+}
+
 /* ************************** Node generic ************** */
 
 /* is rct in visible part of node? */
@@ -3249,10 +3600,219 @@ void NODE_OT_render_changed(wmOperatorType *ot)
 
 /* ****************** Make Group operator ******************* */
 
+static int node_group_make_test(bNodeTree *ntree, bNode *gnode)
+{
+	bNode *node;
+	bNodeLink *link;
+	int totnode = 0;
+	
+	/* is there something to group? also do some clearing */
+	for (node= ntree->nodes.first; node; node= node->next) {
+		if (node == gnode)
+			continue;
+		
+		if (node->flag & NODE_SELECT) {
+			/* no groups in groups */
+			if (node->type==NODE_GROUP)
+				return 0;
+			totnode++;
+		}
+		
+		node->done = 0;
+	}
+	if (totnode==0) return 0;
+	
+	/* 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->fromnode != gnode)
+			link->tonode->done |= 1;
+		if (link->fromnode && link->tonode && link->tonode->flag & NODE_SELECT && link->tonode != gnode)
+			link->fromnode->done |= 2;
+	}
+	
+	for (node= ntree->nodes.first; node; node= node->next) {
+		if (node == gnode)
+			continue;
+		if ((node->flag & NODE_SELECT)==0)
+			if (node->done==3)
+				break;
+	}
+	if (node) 
+		return 0;
+	
+	return 1;
+}
+
+static void node_get_selected_minmax(bNodeTree *ntree, bNode *gnode, float *min, float *max)
+{
+	bNode *node;
+	INIT_MINMAX2(min, max);
+	for (node= ntree->nodes.first; node; node= node->next) {
+		if (node == gnode)
+			continue;
+		if (node->flag & NODE_SELECT) {
+			DO_MINMAX2((&node->locx), min, max);
+		}
+	}
+}
+
+static int node_group_make_insert_selected(bNodeTree *ntree, bNode *gnode)
+{
+	bNodeTree *ngroup = (bNodeTree *)gnode->id;
+	bNodeLink *link, *linkn;
+	bNode *node, *nextn;
+	bNodeSocket *gsock;
+	ListBase anim_basepaths = {NULL, NULL};
+	float min[2], max[2];
+	
+	/* deselect all nodes in the target tree */
+	for (node = ngroup->nodes.first; node; node=node->next)
+		node_deselect(node);
+	
+	node_get_selected_minmax(ntree, gnode, min, max);
+	
+	/* move nodes over */
+	for (node= ntree->nodes.first; node; node= nextn) {
+		nextn= node->next;
+		if (node == gnode)
+			continue;
+		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);
+	
+	/* relink external sockets */
+	for (link= ntree->links.first; link; link= linkn) {
+		int fromselect = (link->fromnode && (link->fromnode->flag & NODE_SELECT) && link->fromnode != gnode);
+		int toselect = (link->tonode && (link->tonode->flag & NODE_SELECT) && link->tonode != gnode);
+		linkn= link->next;
+		
+		if (gnode && ((fromselect && link->tonode == gnode) || (toselect && link->fromnode == gnode))) {
+			/* remove all links to/from the gnode.
+			 * this can remove link information, but there's no general way to preserve it.
+			 */
+			nodeRemLink(ntree, link);
+		}
+		else if (fromselect && toselect) {
+			BLI_remlink(&ntree->links, link);
+			BLI_addtail(&ngroup->links, link);
+		}
+		else if (toselect) {
+			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 (fromselect) {
+			/* 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;
+	/* update of the tree containing the group instance node */
+	ntree->update |= NTREE_UPDATE_NODES | NTREE_UPDATE_LINKS;
+
+	return 1;
+}
+
+static bNode *node_group_make_from_selected(bNodeTree *ntree)
+{
+	bNode *gnode;
+	bNodeTree *ngroup;
+	float min[2], max[2];
+	bNodeTemplate ntemp;
+	
+	node_get_selected_minmax(ntree, NULL, min, max);
+	
+	/* new nodetree */
+	ngroup= ntreeAddTree("NodeGroup", ntree->type, NODE_GROUP);
+	
+	/* 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]);
+	
+	node_group_make_insert_selected(ntree, gnode);
+
+	/* update of the tree containing the group instance node */
+	ntree->update |= NTREE_UPDATE_NODES;
+
+	return gnode;
+}
+
+typedef enum eNodeGroupMakeType {
+	NODE_GM_NEW,
+	NODE_GM_INSERT
+} eNodeGroupMakeType;
+
+/* Operator Property */
+EnumPropertyItem node_group_make_types[] = {
+	{NODE_GM_NEW, "NEW", 0, "New", "Create a new node group from selected nodes"},
+	{NODE_GM_INSERT, "INSERT", 0, "Insert", "Insert into active node group"},
+	{0, NULL, 0, NULL, NULL}
+};
+
 static int node_group_make_exec(bContext *C, wmOperator *op)
 {
 	SpaceNode *snode = CTX_wm_space_node(C);
 	bNode *gnode;
+	int type = RNA_enum_get(op->ptr, "type");
 	
 	if (snode->edittree!=snode->nodetree) {
 		BKE_report(op->reports, RPT_WARNING, "Can not add a new Group in a Group");
@@ -3272,25 +3832,70 @@ static int node_group_make_exec(bContext *C, wmOperator *op)
 			return OPERATOR_CANCELLED;
 		}
 	}
-
-	ED_preview_kill_jobs(C);
 	
-	gnode= node_group_make_from_selected(snode->nodetree);
-	if (gnode==NULL) {
-		BKE_report(op->reports, RPT_WARNING, "Can not make Group");
-		return OPERATOR_CANCELLED;
+	ED_preview_kill_jobs(C);
+
+	switch (type) {
+	case NODE_GM_NEW:
+		if (node_group_make_test(snode->nodetree, NULL)) {
+			gnode = node_group_make_from_selected(snode->nodetree);
+		}
+		else {
+			BKE_report(op->reports, RPT_WARNING, "Can not make Group");
+			return OPERATOR_CANCELLED;
+		}
+		break;
+	case NODE_GM_INSERT:
+		gnode = nodeGetActive(snode->nodetree);
+		if (!gnode || gnode->type != NODE_GROUP) {
+			BKE_report(op->reports, RPT_WARNING, "No active Group node");
+			return OPERATOR_CANCELLED;
+		}
+		if (node_group_make_test(snode->nodetree, gnode)) {
+			node_group_make_insert_selected(snode->nodetree, gnode);
+		}
+		else {
+			BKE_report(op->reports, RPT_WARNING, "Can not insert into Group");
+			return OPERATOR_CANCELLED;
+		}
+		break;
 	}
-	else {
+
+	if (gnode) {
 		nodeSetActive(snode->nodetree, gnode);
-		ntreeUpdateTree(snode->nodetree);
+		snode_make_group_editable(snode, gnode);
 	}
 	
+	if (gnode)
+		ntreeUpdateTree((bNodeTree *)gnode->id);
+	ntreeUpdateTree(snode->nodetree);
+
 	snode_notify(C, snode);
 	snode_dag_update(C, snode);
 	
 	return OPERATOR_FINISHED;
 }
 
+static int node_group_make_invoke(bContext *C, wmOperator *UNUSED(op), wmEvent *UNUSED(event))
+{
+	SpaceNode *snode = CTX_wm_space_node(C);
+	bNode *act = nodeGetActive(snode->edittree);
+	uiPopupMenu *pup = uiPupMenuBegin(C, "Make Group", ICON_NONE);
+	uiLayout *layout = uiPupMenuLayout(pup);
+	
+	uiLayoutSetOperatorContext(layout, WM_OP_EXEC_DEFAULT);
+	uiItemEnumO(layout, "NODE_OT_group_make", NULL, 0, "type", NODE_GM_NEW);
+	
+	/* if active node is a group, add insert option */
+	if (act && act->type == NODE_GROUP) {
+		uiItemEnumO(layout, "NODE_OT_group_make", NULL, 0, "type", NODE_GM_INSERT);
+	}
+	
+	uiPupMenuEnd(C, pup);
+	
+	return OPERATOR_CANCELLED;
+}
+
 void NODE_OT_group_make(wmOperatorType *ot)
 {
 	/* identifiers */
@@ -3299,11 +3904,14 @@ void NODE_OT_group_make(wmOperatorType *ot)
 	ot->idname = "NODE_OT_group_make";
 	
 	/* api callbacks */
+	ot->invoke = node_group_make_invoke;
 	ot->exec = node_group_make_exec;
 	ot->poll = ED_operator_node_active;
 	
 	/* flags */
 	ot->flag = OPTYPE_REGISTER|OPTYPE_UNDO;
+	
+	RNA_def_enum(ot->srna, "type", node_group_make_types, NODE_GM_NEW, "Type", "");
 }
 
 /* ****************** Hide operator *********************** */
diff --git a/source/blender/editors/space_node/node_intern.h b/source/blender/editors/space_node/node_intern.h
index 9e873799f1c..d9dbd646fa5 100644
--- a/source/blender/editors/space_node/node_intern.h
+++ b/source/blender/editors/space_node/node_intern.h
@@ -149,6 +149,7 @@ void NODE_OT_add_reroute(struct wmOperatorType *ot);
 
 void NODE_OT_group_make(struct wmOperatorType *ot);
 void NODE_OT_group_ungroup(struct wmOperatorType *ot);
+void NODE_OT_group_separate(struct wmOperatorType *ot);
 void NODE_OT_group_edit(struct wmOperatorType *ot);
 void NODE_OT_group_socket_add(struct wmOperatorType *ot);
 void NODE_OT_group_socket_remove(struct wmOperatorType *ot);
diff --git a/source/blender/editors/space_node/node_ops.c b/source/blender/editors/space_node/node_ops.c
index 8a977d2f112..ff1661f0327 100644
--- a/source/blender/editors/space_node/node_ops.c
+++ b/source/blender/editors/space_node/node_ops.c
@@ -83,6 +83,7 @@ void node_operatortypes(void)
 
 	WM_operatortype_append(NODE_OT_group_make);
 	WM_operatortype_append(NODE_OT_group_ungroup);
+	WM_operatortype_append(NODE_OT_group_separate);
 	WM_operatortype_append(NODE_OT_group_edit);
 	WM_operatortype_append(NODE_OT_group_socket_add);
 	WM_operatortype_append(NODE_OT_group_socket_remove);
@@ -269,6 +270,7 @@ void node_keymap(struct wmKeyConfig *keyconf)
 
 	WM_keymap_add_item(keymap, "NODE_OT_group_make", GKEY, KM_PRESS, KM_CTRL, 0);
 	WM_keymap_add_item(keymap, "NODE_OT_group_ungroup", GKEY, KM_PRESS, KM_ALT, 0);
+	WM_keymap_add_item(keymap, "NODE_OT_group_separate", PKEY, KM_PRESS, 0, 0);
 	WM_keymap_add_item(keymap, "NODE_OT_group_edit", TABKEY, KM_PRESS, 0, 0);
 	
 	WM_keymap_add_item(keymap, "NODE_OT_read_renderlayers", RKEY, KM_PRESS, KM_CTRL, 0);
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);