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.

1 files changed, 232 insertions, 118 deletions

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)