Big node groups improvement patch. Node group trees now have their own lists of input/output sockets. Those can be linked to internal nodes just like links between regular nodes. In addition group sockets can be renamed and have a defined order, which can be modified, and they can be removed again.

More details can be found in the patch tracker description (#24883) and on the code.blender.org development blog.

6 files changed, 562 insertions, 148 deletions

diff --git a/source/blender/editors/space_node/drawnode.c b/source/blender/editors/space_node/drawnode.c
index 203e6a618a5..ac1c819c9e4 100644
--- a/source/blender/editors/space_node/drawnode.c
+++ b/source/blender/editors/space_node/drawnode.c
@@ -1604,11 +1604,11 @@ void node_draw_link(View2D *v2d, SpaceNode *snode, bNodeLink *link)
 	int do_shaded= 0, th_col1= TH_HEADER, th_col2= TH_HEADER;
 	int do_triple= 0, th_col3= TH_WIRE;
 	
-	if(link->fromnode==NULL && link->tonode==NULL)
+	if(link->fromsock==NULL && link->tosock==NULL)
 		return;
 	
 	/* new connection */
-	if(link->fromnode==NULL || link->tonode==NULL) {
+	if(!link->fromsock || !link->tosock) {
 		th_col1 = TH_ACTIVE;
 		do_triple = 1;
 	}
@@ -1620,8 +1620,9 @@ void node_draw_link(View2D *v2d, SpaceNode *snode, bNodeLink *link)
 			return;
 		
 		/* a bit ugly... but thats how we detect the internal group links */
-		if(link->fromnode==link->tonode) {
-			th_col1 = TH_GRID;
+		if(!link->fromnode || !link->tonode) {
+			UI_ThemeColorBlend(TH_BACK, TH_WIRE, 0.5f);
+			do_shaded= 0;
 		}
 		else {
 			/* check cyclic */
diff --git a/source/blender/editors/space_node/node_draw.c b/source/blender/editors/space_node/node_draw.c
index 7b9a33c521f..9e93773c271 100644
--- a/source/blender/editors/space_node/node_draw.c
+++ b/source/blender/editors/space_node/node_draw.c
@@ -30,6 +30,8 @@
 #include <stdio.h>
 #include <string.h>
 
+#include "MEM_guardedalloc.h"
+
 #include "DNA_node_types.h"
 #include "DNA_material_types.h"
 #include "DNA_object_types.h"
@@ -68,6 +70,9 @@
 
 #include "node_intern.h"
 
+/* width of socket columns in group display */
+#define NODE_GROUP_FRAME		120
+
 // XXX interface.h
 extern void ui_dropshadow(rctf *rct, float radius, float aspect, int select);
 
@@ -380,9 +385,10 @@ static void node_update_group(const bContext *C, bNodeTree *ntree, bNode *gnode)
 {
 	bNodeTree *ngroup= (bNodeTree *)gnode->id;
 	bNode *node;
-	bNodeSocket *nsock;
+	bNodeSocket *sock, *gsock;
 	rctf *rect= &gnode->totr;
 	int counter;
+	int dy;
 	
 	/* center them, is a bit of abuse of locx and locy though */
 	for(node= ngroup->nodes.first; node; node= node->next) {
@@ -405,6 +411,11 @@ static void node_update_group(const bContext *C, bNodeTree *ntree, bNode *gnode)
 		else
 			BLI_union_rctf(rect, &node->totr);
 	}
+	
+	/* add some room for links to group sockets */
+	rect->xmin -= 3*NODE_DY;
+	rect->xmax += 3*NODE_DY;
+	
 	if(counter==1) return;	/* should be prevented? */
 	
 	rect->xmin-= NODE_DY;
@@ -412,16 +423,22 @@ static void node_update_group(const bContext *C, bNodeTree *ntree, bNode *gnode)
 	rect->xmax+= NODE_DY;
 	rect->ymax+= NODE_DY;
 	
-	/* output sockets */
-	for(nsock= gnode->outputs.first; nsock; nsock= nsock->next) {
-		nsock->locx= rect->xmax;
-		nsock->locy= nsock->tosock->locy;
+	/* input sockets */
+	dy = 0.5f*(rect->ymin+rect->ymax) + NODE_DY*(BLI_countlist(&gnode->inputs)-1);
+	for(gsock=ngroup->inputs.first, sock=gnode->inputs.first; gsock; gsock=gsock->next, sock=sock->next) {
+		gsock->locx = rect->xmin;
+		sock->locx = rect->xmin - NODE_GROUP_FRAME;
+		sock->locy = gsock->locy = dy;
+		dy -= 2*NODE_DY;
 	}
 	
-	/* input sockets */
-	for(nsock= gnode->inputs.first; nsock; nsock= nsock->next) {
-		nsock->locx= rect->xmin;
-		nsock->locy= nsock->tosock->locy;
+	/* output sockets */
+	dy = 0.5f*(rect->ymin+rect->ymax) + NODE_DY*(BLI_countlist(&gnode->outputs)-1);
+	for(gsock=ngroup->outputs.first, sock=gnode->outputs.first; gsock; gsock=gsock->next, sock=sock->next) {
+		gsock->locx = rect->xmax;
+		sock->locx = rect->xmax + NODE_GROUP_FRAME;
+		sock->locy = gsock->locy = dy - NODE_DYS;
+		dy -= 2*NODE_DY;
 	}
 }
 
@@ -549,29 +566,6 @@ static void node_sync_cb(bContext *UNUSED(C), void *snode_v, void *node_v)
 
 /* **************  Socket callbacks *********** */
 
-/* NOTE: this is a block-menu, needs 0 events, otherwise the menu closes */
-static uiBlock *socket_vector_menu(bContext *C, ARegion *ar, void *socket_v)
-{
-	bNodeSocket *sock= socket_v;
-	uiBlock *block;
-	
-	SpaceNode *snode= CTX_wm_space_node(C);
-	bNodeTree *ntree = snode->nodetree;
-	PointerRNA ptr;
-	uiLayout *layout;
-	
-	RNA_pointer_create(&ntree->id, &RNA_NodeSocket, sock, &ptr);
-	
-	block= uiBeginBlock(C, ar, "socket menu", UI_EMBOSS);
-	uiBlockSetFlag(block, UI_BLOCK_KEEP_OPEN);
-	
-	layout= uiLayoutColumn(uiBlockLayout(block, UI_LAYOUT_VERTICAL, UI_LAYOUT_PANEL, sock->locx, sock->locy-8, 140, 20, U.uistyles.first), 0);
-	
-	uiItemR(layout, &ptr, "default_value", UI_ITEM_R_EXPAND, "", ICON_NULL);
-	
-	return block;
-}
-
 /* not a callback */
 static void node_draw_preview(bNodePreview *preview, rctf *prv)
 {
@@ -626,16 +620,91 @@ static void node_draw_preview(bNodePreview *preview, rctf *prv)
 	
 }
 
+typedef struct SocketVectorMenuArgs {
+	PointerRNA ptr;
+	int x, y, width;
+	uiButHandleFunc cb;
+	void *arg1, *arg2;
+} SocketVectorMenuArgs;
+
+/* NOTE: this is a block-menu, needs 0 events, otherwise the menu closes */
+static uiBlock *socket_vector_menu(bContext *C, ARegion *ar, void *args_v)
+{
+	SocketVectorMenuArgs *args= (SocketVectorMenuArgs*)args_v;
+	uiBlock *block;
+	uiLayout *layout;
+	
+	block= uiBeginBlock(C, ar, "socket menu", UI_EMBOSS);
+	uiBlockSetFlag(block, UI_BLOCK_KEEP_OPEN);
+	
+	layout= uiLayoutColumn(uiBlockLayout(block, UI_LAYOUT_VERTICAL, UI_LAYOUT_PANEL, args->x, args->y+2, args->width, 20, U.uistyles.first), 0);
+	
+	uiItemR(layout, &args->ptr, "default_value", UI_ITEM_R_EXPAND, "", ICON_NULL);
+	
+	return block;
+}
+
+static void node_draw_socket_button(bNodeTree *ntree, bNodeSocket *sock, const char *name,
+									uiBlock *block, int x, int y, int width,
+									uiButHandleFunc cb, void *arg1, void *arg2)
+{
+	uiBut *bt= NULL;
+	PointerRNA ptr;
+	int labelw;
+	SocketVectorMenuArgs *args;
+	
+	RNA_pointer_create(&ntree->id, &RNA_NodeSocket, sock, &ptr);
+	
+	switch (sock->type) {
+	case SOCK_VALUE:
+		bt=uiDefButR(block, NUM, B_NODE_EXEC, name,
+					 x, y+1, width, 17, 
+					 &ptr, "default_value", 0, sock->ns.min, sock->ns.max, -1, -1, NULL);
+		if (cb)
+			uiButSetFunc(bt, cb, arg1, arg2);
+		break;
+		
+	case SOCK_VECTOR:
+		args= MEM_callocN(sizeof(SocketVectorMenuArgs), "SocketVectorMenuArgs");
+	
+		args->ptr = ptr;
+		args->x = x;
+		args->y = y;
+		args->width = width;
+		args->cb = cb;
+		args->arg1 = arg1;
+		args->arg2 = arg2;
+		
+		uiDefBlockButN(block, socket_vector_menu, args, name, 
+					   x, y+1, width, 17, 
+					   "");
+		break;
+		
+	case SOCK_RGBA:
+		labelw= width - 40;
+		
+		bt=uiDefButR(block, COL, B_NODE_EXEC, "",
+					 x, y+2, (labelw>0 ? 40 : width), 15, 
+					 &ptr, "default_value", 0, sock->ns.min, sock->ns.max, -1, -1, NULL);
+		if (cb)
+			uiButSetFunc(bt, cb, arg1, arg2);
+		
+		if (name[0]!='\0' && labelw>0)
+			uiDefBut(block, LABEL, 0, name, 
+					 x + 40, y+2, labelw, 15, 
+					 NULL, 0, 0, 0, 0, "");
+		break;
+	}
+}
+
 static void node_draw_basis(const bContext *C, ARegion *ar, SpaceNode *snode, bNodeTree *ntree, bNode *node)
 {
 	bNodeSocket *sock;
-	uiBut *bt;
 	rctf *rct= &node->totr;
-	float /*slen,*/ iconofs;
-	int /*ofs,*/ color_id= node_get_colorid(node);
+	float iconofs;
+	int color_id= node_get_colorid(node);
 	char showname[128]; /* 128 used below */
 	View2D *v2d = &ar->v2d;
-	PointerRNA ptr;
 	
 	/* hurmf... another candidate for callback, have to see how this works first */
 	if(node->id && node->block && snode->treetype==NTREE_SHADER)
@@ -765,38 +834,10 @@ static void node_draw_basis(const bContext *C, ARegion *ar, SpaceNode *snode, bN
 		if(!(sock->flag & (SOCK_HIDDEN|SOCK_UNAVAIL))) {
 			socket_circle_draw(sock, NODE_SOCKSIZE);
 			
-			RNA_pointer_create(&ntree->id, &RNA_NodeSocket, sock, &ptr);
-			
 			if(node->block && sock->link==NULL) {
-			
-				if(sock->type==SOCK_VALUE) {
-					bt=uiDefButR(node->block, NUM, B_NODE_EXEC, sock->name,
-							 (short)sock->locx+NODE_DYS, (short)(sock->locy)-9, (short)node->width-NODE_DY, 17, 
-							  &ptr, "default_value", 0, sock->ns.min, sock->ns.max, -1, -1, NULL);
-					uiButSetFunc(bt, node_sync_cb, snode, node);
-				}
-				else if(sock->type==SOCK_VECTOR) {
-					uiDefBlockBut(node->block, socket_vector_menu, sock, sock->name, 
-						  (short)sock->locx+NODE_DYS, (short)sock->locy-9, (short)node->width-NODE_DY, 17, 
-						  "");
-				}
-				else if(node->block && sock->type==SOCK_RGBA) {
-					short labelw= (short)node->width-NODE_DY-40, width;
-					
-					if(labelw>0) width= 40; else width= (short)node->width-NODE_DY;
-					
-					bt=uiDefButR(node->block, COL, B_NODE_EXEC, "",
-								 (short)sock->locx+NODE_DYS, (short)(sock->locy)-8, width, 15, 
-								 &ptr, "default_value", 0, sock->ns.min, sock->ns.max, -1, -1, NULL);
-					uiButSetFunc(bt, node_sync_cb, snode, node);
-					
-					if(labelw>0) uiDefBut(node->block, LABEL, 0, sock->name, 
-										   (short)(sock->locx+NODE_DYS) + 40, (short)sock->locy-8, labelw, 15, 
-										   NULL, 0, 0, 0, 0, "");
-				}
+				node_draw_socket_button(ntree, sock, sock->name, node->block, sock->locx+NODE_DYS, sock->locy-NODE_DYS, node->width-NODE_DY, node_sync_cb, snode, node);
 			}
 			else {
-				
 				uiDefBut(node->block, LABEL, 0, sock->name, (short)(sock->locx+7), (short)(sock->locy-9.0f), 
 						 (short)(node->width-NODE_DY), NODE_DY,  NULL, 0, 0, 0, 0, "");
 			}
@@ -969,39 +1010,11 @@ static void node_draw_nodetree(const bContext *C, ARegion *ar, SpaceNode *snode,
 	}	
 }
 
-/* fake links from groupnode to internal nodes */
-static void node_draw_group_links(View2D *v2d, SpaceNode *snode, bNode *gnode)
+static void group_verify_cb(bContext *UNUSED(C), void *UNUSED(snode_v), void *ngroup_v)
 {
-	bNodeLink fakelink;
-	bNodeSocket *sock;
-	
-	glEnable(GL_BLEND);
-	glEnable(GL_LINE_SMOOTH);
-	
-	fakelink.tonode= fakelink.fromnode= gnode;
-	
-	for(sock= gnode->inputs.first; sock; sock= sock->next) {
-		if(!(sock->flag & (SOCK_HIDDEN|SOCK_UNAVAIL))) {
-			if(sock->tosock) {
-				fakelink.fromsock= sock;
-				fakelink.tosock= sock->tosock;
-				node_draw_link(v2d, snode, &fakelink);
-			}
-		}
-	}
-	
-	for(sock= gnode->outputs.first; sock; sock= sock->next) {
-		if(!(sock->flag & (SOCK_HIDDEN|SOCK_UNAVAIL))) {
-			if(sock->tosock) {
-				fakelink.tosock= sock;
-				fakelink.fromsock= sock->tosock;
-				node_draw_link(v2d, snode, &fakelink);
-			}
-		}
-	}
+	bNodeTree *ngroup= (bNodeTree*)ngroup_v;
 	
-	glDisable(GL_BLEND);
-	glDisable(GL_LINE_SMOOTH);
+	nodeVerifyGroup(ngroup);
 }
 
 /* groups are, on creation, centered around 0,0 */
@@ -1010,25 +1023,51 @@ static void node_draw_group(const bContext *C, ARegion *ar, SpaceNode *snode, bN
 	bNodeTree *ngroup= (bNodeTree *)gnode->id;
 	bNodeSocket *sock;
 	rctf rect= gnode->totr;
+	int index;
 	uiLayout *layout;
 	PointerRNA ptr;
+	uiBut *bt;
 	
 	/* backdrop header */
 	glEnable(GL_BLEND);
 	uiSetRoundBox(3);
 	UI_ThemeColorShadeAlpha(TH_NODE_GROUP, 0, -70);
-	uiDrawBox(GL_POLYGON, rect.xmin, rect.ymax, rect.xmax, rect.ymax+26, BASIS_RAD);
+	uiDrawBox(GL_POLYGON, rect.xmin-NODE_GROUP_FRAME, rect.ymax, rect.xmax+NODE_GROUP_FRAME, rect.ymax+26, BASIS_RAD);
 	
 	/* backdrop body */
 	UI_ThemeColorShadeAlpha(TH_BACK, -8, -70);
-	uiSetRoundBox(12);
+	uiSetRoundBox(0);
 	uiDrawBox(GL_POLYGON, rect.xmin, rect.ymin, rect.xmax, rect.ymax, BASIS_RAD);
-	
-	/* selection outline */
+
+	/* input column */
+	UI_ThemeColorShadeAlpha(TH_BACK, 10, -50);
+	uiSetRoundBox(8);
+	uiDrawBox(GL_POLYGON, rect.xmin-NODE_GROUP_FRAME, rect.ymin, rect.xmin, rect.ymax, BASIS_RAD);
+
+	/* output column */
+	UI_ThemeColorShadeAlpha(TH_BACK, 10, -50);
+	uiSetRoundBox(4);
+	uiDrawBox(GL_POLYGON, rect.xmax, rect.ymin, rect.xmax+NODE_GROUP_FRAME, rect.ymax, BASIS_RAD);
+
+	/* input column separator */
+	glColor4ub(200, 200, 200, 140);
+	glBegin(GL_LINES);
+	glVertex2f(rect.xmin, rect.ymin);
+	glVertex2f(rect.xmin, rect.ymax);
+	glEnd();
+
+	/* output column separator */
+	glColor4ub(200, 200, 200, 140);
+	glBegin(GL_LINES);
+	glVertex2f(rect.xmax, rect.ymin);
+	glVertex2f(rect.xmax, rect.ymax);
+	glEnd();
+
+	/* group node outline */
 	uiSetRoundBox(15);
 	glColor4ub(200, 200, 200, 140);
 	glEnable( GL_LINE_SMOOTH );
-	uiDrawBox(GL_LINE_LOOP, rect.xmin, rect.ymin, rect.xmax, rect.ymax+26, BASIS_RAD);
+	uiDrawBox(GL_LINE_LOOP, rect.xmin-NODE_GROUP_FRAME, rect.ymin, rect.xmax+NODE_GROUP_FRAME, rect.ymax+26, BASIS_RAD);
 	glDisable( GL_LINE_SMOOTH );
 	glDisable(GL_BLEND);
 	
@@ -1041,26 +1080,101 @@ static void node_draw_group(const bContext *C, ARegion *ar, SpaceNode *snode, bN
 	uiTemplateIDBrowse(layout, (bContext*)C, &ptr, "node_tree", NULL, NULL, NULL);
 	uiBlockLayoutResolve(gnode->block, NULL, NULL);
 
-	uiEndBlock(C, gnode->block);
-	uiDrawBlock(C, gnode->block);
-	gnode->block= NULL;
-
+	/* draw the internal tree nodes and links */
+	node_draw_nodetree(C, ar, snode, ngroup);
 
-	/* links from groupsockets to the internal nodes */
-	node_draw_group_links(&ar->v2d, snode, gnode);
-	
 	/* group sockets */
-	for(sock= gnode->inputs.first; sock; sock= sock->next)
-		if(!(sock->flag & (SOCK_HIDDEN|SOCK_UNAVAIL)))
-			socket_circle_draw(sock, NODE_SOCKSIZE);
-	for(sock= gnode->outputs.first; sock; sock= sock->next)
-		if(!(sock->flag & (SOCK_HIDDEN|SOCK_UNAVAIL)))
-			socket_circle_draw(sock, NODE_SOCKSIZE);
-
-
+	for(sock=ngroup->inputs.first, index=0; sock; sock=sock->next, ++index) {
+		socket_circle_draw(sock, NODE_SOCKSIZE);
+		/* small hack to use socket_circle_draw function with offset */
+		sock->locx -= NODE_GROUP_FRAME;
+		socket_circle_draw(sock, NODE_SOCKSIZE);
+		sock->locx += NODE_GROUP_FRAME;
+
+		bt = uiDefBut(gnode->block, TEX, 0, "", 
+					  sock->locx-114, sock->locy+1, 72, NODE_DY,
+					  sock->name, 0, 31, 0, 0, "");
+		uiButSetFunc(bt, group_verify_cb, snode, ngroup);
+		
+		node_draw_socket_button(ngroup, sock, "", gnode->block,
+								sock->locx-114, sock->locy-NODE_DY, 72,
+								NULL, NULL, NULL);
+
+		uiBlockSetDirection(gnode->block, UI_TOP);
+		uiBlockBeginAlign(gnode->block);
+		bt = uiDefIconButO(gnode->block, BUT, "NODE_OT_group_socket_move_up", 0, ICON_TRIA_UP,
+						   sock->locx-40, sock->locy, 16, 16, "");
+		if (!sock->prev)
+			uiButSetFlag(bt, UI_BUT_DISABLED);
+		RNA_int_set(uiButGetOperatorPtrRNA(bt), "index", index);
+		RNA_enum_set(uiButGetOperatorPtrRNA(bt), "in_out", SOCK_IN);
+		bt = uiDefIconButO(gnode->block, BUT, "NODE_OT_group_socket_move_down", 0, ICON_TRIA_DOWN,
+						   sock->locx-40, sock->locy-16, 16, 16, "");
+		if (!sock->next)
+			uiButSetFlag(bt, UI_BUT_DISABLED);
+		RNA_int_set(uiButGetOperatorPtrRNA(bt), "index", index);
+		RNA_enum_set(uiButGetOperatorPtrRNA(bt), "in_out", SOCK_IN);
+		uiBlockEndAlign(gnode->block);
+		uiBlockSetDirection(gnode->block, 0);
+		
+		uiBlockSetEmboss(gnode->block, UI_EMBOSSN);
+		bt = uiDefIconButO(gnode->block, BUT, "NODE_OT_group_socket_remove", 0, ICON_X,
+						   sock->locx-22, sock->locy-8, 16, 16, "");
+		RNA_int_set(uiButGetOperatorPtrRNA(bt), "index", index);
+		RNA_enum_set(uiButGetOperatorPtrRNA(bt), "in_out", SOCK_IN);
+		uiBlockSetEmboss(gnode->block, UI_EMBOSS);
+	}
 	
-	/* and finally the whole tree */
-	node_draw_nodetree(C, ar, snode, ngroup);
+	for(sock=ngroup->outputs.first, index=0; sock; sock=sock->next, ++index) {
+		socket_circle_draw(sock, NODE_SOCKSIZE);
+		/* small hack to use socket_circle_draw function with offset */
+		sock->locx += NODE_GROUP_FRAME;
+		socket_circle_draw(sock, NODE_SOCKSIZE);
+		sock->locx -= NODE_GROUP_FRAME;
+		
+		uiBlockSetEmboss(gnode->block, UI_EMBOSSN);
+		bt = uiDefIconButO(gnode->block, BUT, "NODE_OT_group_socket_remove", 0, ICON_X,
+						   sock->locx+6, sock->locy-8, 16, 16, "");
+		RNA_int_set(uiButGetOperatorPtrRNA(bt), "index", index);
+		RNA_enum_set(uiButGetOperatorPtrRNA(bt), "in_out", SOCK_OUT);
+		uiBlockSetEmboss(gnode->block, UI_EMBOSS);
+		
+		uiBlockSetDirection(gnode->block, UI_TOP);
+		uiBlockBeginAlign(gnode->block);
+		bt = uiDefIconButO(gnode->block, BUT, "NODE_OT_group_socket_move_up", 0, ICON_TRIA_UP,
+						   sock->locx+24, sock->locy, 16, 16, "");
+		if (!sock->prev)
+			uiButSetFlag(bt, UI_BUT_DISABLED);
+		RNA_int_set(uiButGetOperatorPtrRNA(bt), "index", index);
+		RNA_enum_set(uiButGetOperatorPtrRNA(bt), "in_out", SOCK_OUT);
+		bt = uiDefIconButO(gnode->block, BUT, "NODE_OT_group_socket_move_down", 0, ICON_TRIA_DOWN,
+						   sock->locx+24, sock->locy-16, 16, 16, "");
+		if (!sock->next)
+			uiButSetFlag(bt, UI_BUT_DISABLED);
+		RNA_int_set(uiButGetOperatorPtrRNA(bt), "index", index);
+		RNA_enum_set(uiButGetOperatorPtrRNA(bt), "in_out", SOCK_OUT);
+		uiBlockEndAlign(gnode->block);
+		uiBlockSetDirection(gnode->block, 0);
+		
+		if (sock->link) {
+			bt = uiDefBut(gnode->block, TEX, 0, "", 
+						  sock->locx+42, sock->locy-NODE_DYS+1, 72, NODE_DY,
+						  sock->name, 0, 31, 0, 0, "");
+			uiButSetFunc(bt, group_verify_cb, snode, ngroup);
+		}
+		else {
+			bt = uiDefBut(gnode->block, TEX, 0, "", 
+						  sock->locx+42, sock->locy+1, 72, NODE_DY,
+						  sock->name, 0, 31, 0, 0, "");
+			uiButSetFunc(bt, group_verify_cb, snode, ngroup);
+			
+			node_draw_socket_button(ngroup, sock, "", gnode->block, sock->locx+42, sock->locy-NODE_DY, 72, NULL, NULL, NULL);
+		}
+	}
+	
+	uiEndBlock(C, gnode->block);
+	uiDrawBlock(C, gnode->block);
+	gnode->block= NULL;
 }
 
 void drawnodespace(const bContext *C, ARegion *ar, View2D *v2d)
diff --git a/source/blender/editors/space_node/node_edit.c b/source/blender/editors/space_node/node_edit.c
index 385dfbcd9a8..c0c1b7dc72b 100644
--- a/source/blender/editors/space_node/node_edit.c
+++ b/source/blender/editors/space_node/node_edit.c
@@ -77,10 +77,15 @@
 
 #include "IMB_imbuf.h"
 
+#include "RNA_enum_types.h"
+
 #include "node_intern.h"
 
-#define SOCK_IN		1
-#define SOCK_OUT	2
+static EnumPropertyItem socket_in_out_items[] = {
+	{ SOCK_IN, "IN", 0, "In", "" },
+	{ SOCK_OUT, "OUT", 0, "Out", "" },
+	{ 0, NULL, 0, NULL, NULL}
+};
 
 /* ***************** composite job manager ********************** */
 
@@ -634,6 +639,241 @@ void NODE_OT_group_edit(wmOperatorType *ot)
 	ot->flag = OPTYPE_REGISTER|OPTYPE_UNDO;
 }
 
+/* ***************** Add Group Socket operator ************* */
+
+static int node_group_socket_add_exec(bContext *C, wmOperator *op)
+{
+	SpaceNode *snode = CTX_wm_space_node(C);
+	int in_out= -1;
+	char name[32]= "";
+	int type= SOCK_VALUE;
+	bNodeTree *ngroup= snode->edittree;
+	bNodeSocket *sock;
+	
+	ED_preview_kill_jobs(C);
+	
+	if (RNA_property_is_set(op->ptr, "name"))
+		RNA_string_get(op->ptr, "name", name);
+	
+	if (RNA_property_is_set(op->ptr, "type"))
+		type = RNA_enum_get(op->ptr, "type");
+	
+	if (RNA_property_is_set(op->ptr, "in_out"))
+		in_out = RNA_enum_get(op->ptr, "in_out");
+	else
+		return OPERATOR_CANCELLED;
+	
+	sock = nodeAddGroupSocket(ngroup, name, type, in_out);
+	
+	node_tree_verify_groups(snode->nodetree);
+	
+	snode_notify(C, snode);
+	
+	return OPERATOR_FINISHED;
+}
+
+void NODE_OT_group_socket_add(wmOperatorType *ot)
+{
+	/* identifiers */
+	ot->name = "Add Group Socket";
+	ot->description = "Add node group socket";
+	ot->idname = "NODE_OT_group_socket_add";
+	
+	/* api callbacks */
+	ot->exec = node_group_socket_add_exec;
+	ot->poll = ED_operator_node_active;
+	
+	/* flags */
+	ot->flag = OPTYPE_REGISTER|OPTYPE_UNDO;
+	
+	RNA_def_enum(ot->srna, "in_out", socket_in_out_items, SOCK_IN, "Socket Type", "Input or Output");
+	RNA_def_string(ot->srna, "name", "", 32, "Name", "Group socket name");
+	RNA_def_enum(ot->srna, "type", node_socket_type_items, SOCK_VALUE, "Type", "Type of the group socket");
+}
+
+/* ***************** Remove Group Socket operator ************* */
+
+static int node_group_socket_remove_exec(bContext *C, wmOperator *op)
+{
+	SpaceNode *snode = CTX_wm_space_node(C);
+	int index= -1;
+	int in_out= -1;
+	bNodeTree *ngroup= snode->edittree;
+	bNodeSocket *sock;
+	
+	ED_preview_kill_jobs(C);
+	
+	if (RNA_property_is_set(op->ptr, "index"))
+		index = RNA_int_get(op->ptr, "index");
+	else
+		return OPERATOR_CANCELLED;
+	
+	if (RNA_property_is_set(op->ptr, "in_out"))
+		in_out = RNA_enum_get(op->ptr, "in_out");
+	else
+		return OPERATOR_CANCELLED;
+	
+	sock = (bNodeSocket*)BLI_findlink(in_out==SOCK_IN ? &ngroup->inputs : &ngroup->outputs, index);
+	if (sock) {
+		nodeRemGroupSocket(ngroup, sock, in_out);
+		node_tree_verify_groups(snode->nodetree);
+		
+		snode_notify(C, snode);
+	}
+	
+	return OPERATOR_FINISHED;
+}
+
+void NODE_OT_group_socket_remove(wmOperatorType *ot)
+{
+	/* identifiers */
+	ot->name = "Remove Group Socket";
+	ot->description = "Removed node group socket";
+	ot->idname = "NODE_OT_group_socket_remove";
+	
+	/* api callbacks */
+	ot->exec = node_group_socket_remove_exec;
+	ot->poll = ED_operator_node_active;
+	
+	/* flags */
+	ot->flag = OPTYPE_REGISTER|OPTYPE_UNDO;
+	
+	RNA_def_int(ot->srna, "index", 0, 0, INT_MAX, "Index", "", 0, INT_MAX);
+	RNA_def_enum(ot->srna, "in_out", socket_in_out_items, SOCK_IN, "Socket Type", "Input or Output");
+}
+
+/* ***************** Move Group Socket Up operator ************* */
+
+static int node_group_socket_move_up_exec(bContext *C, wmOperator *op)
+{
+	SpaceNode *snode = CTX_wm_space_node(C);
+	int index= -1;
+	int in_out= -1;
+	bNodeTree *ngroup= snode->edittree;
+	bNodeSocket *sock, *prev;
+	
+	ED_preview_kill_jobs(C);
+	
+	if (RNA_property_is_set(op->ptr, "index"))
+		index = RNA_int_get(op->ptr, "index");
+	else
+		return OPERATOR_CANCELLED;
+	
+	if (RNA_property_is_set(op->ptr, "in_out"))
+		in_out = RNA_enum_get(op->ptr, "in_out");
+	else
+		return OPERATOR_CANCELLED;
+	
+	/* swap */
+	if (in_out==SOCK_IN) {
+		sock = (bNodeSocket*)BLI_findlink(&ngroup->inputs, index);
+		prev = sock->prev;
+		/* can't move up the first socket */
+		if (!prev)
+			return OPERATOR_CANCELLED;
+		BLI_remlink(&ngroup->inputs, sock);
+		BLI_insertlinkbefore(&ngroup->inputs, prev, sock);
+	}
+	else if (in_out==SOCK_OUT) {
+		sock = (bNodeSocket*)BLI_findlink(&ngroup->outputs, index);
+		prev = sock->prev;
+		/* can't move up the first socket */
+		if (!prev)
+			return OPERATOR_CANCELLED;
+		BLI_remlink(&ngroup->outputs, sock);
+		BLI_insertlinkbefore(&ngroup->outputs, prev, sock);
+	}
+	node_tree_verify_groups(snode->nodetree);
+	
+	snode_notify(C, snode);
+	
+	return OPERATOR_FINISHED;
+}
+
+void NODE_OT_group_socket_move_up(wmOperatorType *ot)
+{
+	/* identifiers */
+	ot->name = "Move Group Socket Up";
+	ot->description = "Move up node group socket";
+	ot->idname = "NODE_OT_group_socket_move_up";
+	
+	/* api callbacks */
+	ot->exec = node_group_socket_move_up_exec;
+	ot->poll = ED_operator_node_active;
+	
+	/* flags */
+	ot->flag = OPTYPE_REGISTER|OPTYPE_UNDO;
+	
+	RNA_def_int(ot->srna, "index", 0, 0, INT_MAX, "Index", "", 0, INT_MAX);
+	RNA_def_enum(ot->srna, "in_out", socket_in_out_items, SOCK_IN, "Socket Type", "Input or Output");
+}
+
+/* ***************** Move Group Socket Up operator ************* */
+
+static int node_group_socket_move_down_exec(bContext *C, wmOperator *op)
+{
+	SpaceNode *snode = CTX_wm_space_node(C);
+	int index= -1;
+	int in_out= -1;
+	bNodeTree *ngroup= snode->edittree;
+	bNodeSocket *sock, *next;
+	
+	ED_preview_kill_jobs(C);
+	
+	if (RNA_property_is_set(op->ptr, "index"))
+		index = RNA_int_get(op->ptr, "index");
+	else
+		return OPERATOR_CANCELLED;
+	
+	if (RNA_property_is_set(op->ptr, "in_out"))
+		in_out = RNA_enum_get(op->ptr, "in_out");
+	else
+		return OPERATOR_CANCELLED;
+	
+	/* swap */
+	if (in_out==SOCK_IN) {
+		sock = (bNodeSocket*)BLI_findlink(&ngroup->inputs, index);
+		next = sock->next;
+		/* can't move down the last socket */
+		if (!next)
+			return OPERATOR_CANCELLED;
+		BLI_remlink(&ngroup->inputs, sock);
+		BLI_insertlinkafter(&ngroup->inputs, next, sock);
+	}
+	else if (in_out==SOCK_OUT) {
+		sock = (bNodeSocket*)BLI_findlink(&ngroup->outputs, index);
+		next = sock->next;
+		/* can't move down the last socket */
+		if (!next)
+			return OPERATOR_CANCELLED;
+		BLI_remlink(&ngroup->outputs, sock);
+		BLI_insertlinkafter(&ngroup->outputs, next, sock);
+	}
+	node_tree_verify_groups(snode->nodetree);
+	
+	snode_notify(C, snode);
+	
+	return OPERATOR_FINISHED;
+}
+
+void NODE_OT_group_socket_move_down(wmOperatorType *ot)
+{
+	/* identifiers */
+	ot->name = "Move Group Socket Down";
+	ot->description = "Move down node group socket";
+	ot->idname = "NODE_OT_group_socket_move_down";
+	
+	/* api callbacks */
+	ot->exec = node_group_socket_move_down_exec;
+	ot->poll = ED_operator_node_active;
+	
+	/* flags */
+	ot->flag = OPTYPE_REGISTER|OPTYPE_UNDO;
+	
+	RNA_def_int(ot->srna, "index", 0, 0, INT_MAX, "Index", "", 0, INT_MAX);
+	RNA_def_enum(ot->srna, "in_out", socket_in_out_items, SOCK_IN, "Socket Type", "Input or Output");
+}
+
 /* ******************** Ungroup operator ********************** */
 
 static int node_group_ungroup_exec(bContext *C, wmOperator *op)
@@ -1433,6 +1673,33 @@ static int find_indicated_socket(SpaceNode *snode, bNode **nodep, bNodeSocket **
 			}
 		}
 	}
+	
+	/* check group sockets
+	 * NB: using ngroup->outputs as input sockets and vice versa here!
+	 */
+	if(in_out & SOCK_IN) {
+		for(sock= snode->edittree->outputs.first; sock; sock= sock->next) {
+			if(!(sock->flag & (SOCK_HIDDEN|SOCK_UNAVAIL))) {
+				if(BLI_in_rctf(&rect, sock->locx, sock->locy)) {
+					*nodep= NULL;	/* NULL node pointer indicates group socket */
+					*sockp= sock;
+					return 1;
+				}
+			}
+		}
+	}
+	if(in_out & SOCK_OUT) {
+		for(sock= snode->edittree->inputs.first; sock; sock= sock->next) {
+			if(!(sock->flag & (SOCK_HIDDEN|SOCK_UNAVAIL))) {
+				if(BLI_in_rctf(&rect, sock->locx, sock->locy)) {
+					*nodep= NULL;	/* NULL node pointer indicates group socket */
+					*sockp= sock;
+					return 1;
+				}
+			}
+		}
+	}
+	
 	return 0;
 }
 
@@ -1473,6 +1740,16 @@ static int node_socket_hilights(SpaceNode *snode, int in_out)
 	return redraw;
 }
 
+static int outside_group_rect(SpaceNode *snode)
+{
+	bNode *gnode= node_tree_get_editgroup(snode->nodetree);
+	if (gnode) {
+		return (snode->mx < gnode->totr.xmin || snode->mx >= gnode->totr.xmax
+				|| snode->my < gnode->totr.ymin || snode->my >= gnode->totr.ymax);
+	}
+	return 0;
+}
+
 /* ****************** Add *********************** */
 
 
@@ -1701,7 +1978,7 @@ static int node_duplicate_exec(bContext *C, wmOperator *UNUSED(op))
 	last = ntree->nodes.last;
 	for(node= ntree->nodes.first; node; node= node->next) {
 		if(node->flag & SELECT) {
-			newnode = nodeCopyNode(ntree, node, 1);
+			newnode = nodeCopyNode(ntree, node);
 			
 			/* deselect old node, select the copy instead */
 			node->flag &= ~(NODE_SELECT|NODE_ACTIVE);
@@ -1767,19 +2044,24 @@ static void node_remove_extra_links(SpaceNode *snode, bNodeSocket *tsock, bNodeL
 				break;
 		}
 		if(tlink) {
-			/* is there a free input socket with same type? */
-			for(sock= tlink->tonode->inputs.first; sock; sock= sock->next) {
-				if(sock->type==tlink->fromsock->type)
-					if(nodeCountSocketLinks(snode->edittree, sock) < sock->limit)
-						break;
-			}
-			if(sock) {
-				tlink->tosock= sock;
-				sock->flag &= ~SOCK_HIDDEN;
+			/* try to move the existing link to the next available socket */
+			if (tlink->tonode) {
+				/* is there a free input socket with same type? */
+				for(sock= tlink->tonode->inputs.first; sock; sock= sock->next) {
+					if(sock->type==tlink->fromsock->type)
+						if(nodeCountSocketLinks(snode->edittree, sock) < sock->limit)
+							break;
+				}
+				if(sock) {
+					tlink->tosock= sock;
+					sock->flag &= ~SOCK_HIDDEN;
+				}
+				else {
+					nodeRemLink(snode->edittree, tlink);
+				}
 			}
-			else {
+			else
 				nodeRemLink(snode->edittree, tlink);
-			}
 		}
 	}
 }
@@ -1810,7 +2092,7 @@ static int node_link_modal(bContext *C, wmOperator *op, wmEvent *event)
 			if(in_out==SOCK_OUT) {
 				if(find_indicated_socket(snode, &tnode, &tsock, SOCK_IN)) {
 					if(nodeFindLink(snode->edittree, sock, tsock)==NULL) {
-						if(tnode!=node  && link->tonode!=tnode && link->tosock!= tsock) {
+						if( link->tosock!= tsock && (!tnode || (tnode!=node && link->tonode!=tnode)) ) {
 							link->tonode= tnode;
 							link->tosock= tsock;
 							ntreeSolveOrder(snode->edittree);	/* for interactive red line warning */
@@ -1826,7 +2108,7 @@ static int node_link_modal(bContext *C, wmOperator *op, wmEvent *event)
 				if(find_indicated_socket(snode, &tnode, &tsock, SOCK_OUT)) {
 					if(nodeFindLink(snode->edittree, sock, tsock)==NULL) {
 						if(nodeCountSocketLinks(snode->edittree, tsock) < tsock->limit) {
-							if(tnode!=node && link->fromnode!=tnode && link->fromsock!= tsock) {
+							if( link->fromsock!= tsock && (!tnode || (tnode!=node && link->fromnode!=tnode)) ) {
 								link->fromnode= tnode;
 								link->fromsock= tsock;
 								ntreeSolveOrder(snode->edittree);	/* for interactive red line warning */
@@ -1847,19 +2129,28 @@ static int node_link_modal(bContext *C, wmOperator *op, wmEvent *event)
 		case LEFTMOUSE:
 		case RIGHTMOUSE:
 		case MIDDLEMOUSE:
-	
-			/* remove link? */
-			if(link->tonode==NULL || link->fromnode==NULL) {
-				nodeRemLink(snode->edittree, link);
-			}
-			else {
+			if(link->tosock && link->fromsock) {
 				/* send changed events for original tonode and new */
-				if(link->tonode) 
+				if(link->tonode)
 					NodeTagChanged(snode->edittree, link->tonode);
 				
 				/* we might need to remove a link */
-				if(in_out==SOCK_OUT) node_remove_extra_links(snode, link->tosock, link);
+				if(in_out==SOCK_OUT)
+					node_remove_extra_links(snode, link->tosock, link);
 			}
+			else if (outside_group_rect(snode) && (link->tonode || link->fromnode)) {
+				/* automatically add new group socket */
+				if (link->tonode && link->tosock) {
+					link->fromsock = nodeAddGroupSocketCopy(snode->edittree, link->tosock, SOCK_IN);
+					link->fromnode = NULL;
+				}
+				else if (link->fromnode && link->fromsock) {
+					link->tosock = nodeAddGroupSocketCopy(snode->edittree, link->fromsock, SOCK_OUT);
+					link->tonode = NULL;
+				}
+			}
+			else
+				nodeRemLink(snode->edittree, link);
 			
 			ntreeSolveOrder(snode->edittree);
 			node_tree_verify_groups(snode->nodetree);
diff --git a/source/blender/editors/space_node/node_intern.h b/source/blender/editors/space_node/node_intern.h
index eb7e1209801..532ca58905e 100644
--- a/source/blender/editors/space_node/node_intern.h
+++ b/source/blender/editors/space_node/node_intern.h
@@ -103,6 +103,10 @@ void NODE_OT_links_cut(struct wmOperatorType *ot);
 void NODE_OT_group_make(struct wmOperatorType *ot);
 void NODE_OT_group_ungroup(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);
+void NODE_OT_group_socket_move_up(struct wmOperatorType *ot);
+void NODE_OT_group_socket_move_down(struct wmOperatorType *ot);
 
 void NODE_OT_mute_toggle(struct wmOperatorType *ot);
 void NODE_OT_hide_toggle(struct wmOperatorType *ot);
diff --git a/source/blender/editors/space_node/node_ops.c b/source/blender/editors/space_node/node_ops.c
index 9c2b6f0274d..be5436cc2c8 100644
--- a/source/blender/editors/space_node/node_ops.c
+++ b/source/blender/editors/space_node/node_ops.c
@@ -74,6 +74,10 @@ void node_operatortypes(void)
 	WM_operatortype_append(NODE_OT_group_make);
 	WM_operatortype_append(NODE_OT_group_ungroup);
 	WM_operatortype_append(NODE_OT_group_edit);
+	WM_operatortype_append(NODE_OT_group_socket_add);
+	WM_operatortype_append(NODE_OT_group_socket_remove);
+	WM_operatortype_append(NODE_OT_group_socket_move_up);
+	WM_operatortype_append(NODE_OT_group_socket_move_down);
 	
 	WM_operatortype_append(NODE_OT_link_viewer);
 	
diff --git a/source/blender/editors/space_node/node_select.c b/source/blender/editors/space_node/node_select.c
index b53ac07aab3..48e70be51e5 100644
--- a/source/blender/editors/space_node/node_select.c
+++ b/source/blender/editors/space_node/node_select.c
@@ -288,7 +288,7 @@ static int node_select_linked_to_exec(bContext *C, wmOperator *UNUSED(op))
 		node->flag &= ~NODE_TEST;
 
 	for (link=snode->edittree->links.first; link; link=link->next) {
-		if (link->fromnode->flag & NODE_SELECT)
+		if (link->fromnode && link->tonode && (link->fromnode->flag & NODE_SELECT))
 			link->tonode->flag |= NODE_TEST;
 	}
 	
@@ -328,7 +328,7 @@ static int node_select_linked_from_exec(bContext *C, wmOperator *UNUSED(op))
 		node->flag &= ~NODE_TEST;
 
 	for(link=snode->edittree->links.first; link; link=link->next) {
-		if(link->tonode->flag & NODE_SELECT)
+		if(link->fromnode && link->tonode && (link->tonode->flag & NODE_SELECT))
 			link->fromnode->flag |= NODE_TEST;
 	}