Page MenuHome

Nameable, explicit group node sockets
Closed, ArchivedPublicPATCH

Description

This patch adds the ability to rename the external sockets of node groups. This feature makes node groups much more usable by providing meaning to the external sockets, which is not directly obvious when using the internal node sockets names (e.g. an arbitrary "Factor" input of a mix node could be renamed to "Amplitude" or whatever parameter it provides to the groups functionality). Note that only the external group sockets are renamed, not the internal nodes in the group tree!

The patch also improves the user interface for editing group nodes. On the left and right side of the opened group there are now two columns for the external input/output sockets. There an ordered list of text buttons with the socket names is displayed (this also helps to identify the order of sockets). When renaming a group socket, all the instances are updated.

There were some deeper changes necessary to make this work, in particular with reading/writing blend files. Since type info structs for nodes are not part of the DNA and are not stored in blend files, the group type info would be lost when reloading a file. Instead of regenerating the group node type each time from scratch, a new struct bNodeGroupSocket was added for external group sockets. This replaces the bNodeSocketType list in the node group types (which are now unused). Instead the list of external sockets is directly part of the bNodeTree struct (empty for non-group, local trees).

There are a couple of additional features that can be implemented to further improve group nodes:
* Explicit selection of external sockets, instead of just using all unhidden, unlinked sockets.
* Reordering of group node sockets.
These will be provided as separate patches.

Event Timeline

UPDATE: Fixed a few minor bugs and removed old code.

UPDATE2: Node sockets are now by default internal, i.e. don't get a group socket counterpart. I modified the linking operator for groups, so that a socket can be exposed to the group by dragging a link outside of the group edit rectangle. By dragging and dropping on the rectangle an external socket can be made internal again. The code for automatic socket exposure is still there, it could be enabled by a user setting.

This second improvement could also be applied as a separate patch if necessary. The latest patch provided here contains both features.

The implementation and functionality looks good to me.

* When you create a group node now, with some inputs/outputs connected to other nodes, those connections are lost. I think it's ok to not create group sockets for all inputs/outputs, but sockets that are already connected to external nodes should be exposed.

* I'm getting a crashes removing group input/output sockets when they are connected to other nodes, tested with compositing nodes and simple mix node in a group between render layer and output. Maybe it's a threading issue, but I don't simultaneous access to the data, so perhaps it's not.

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000088
0x0000000100682172 in ntreeSolveOrder (ntree=0x119ecf1a8) at /Users/brecht/dev/blender-2.5/blender/source/blender/blenkernel/intern/node.c:1862
1862 link->tosock->link= link;
(gdb) bt
#0 0x0000000100682172 in ntreeSolveOrder (ntree=0x119ecf1a8) at /Users/brecht/dev/blender-2.5/blender/source/blender/blenkernel/intern/node.c:1862
#1 0x0000000100680a53 in ntreeCopyTree (ntree=0x118f78e28, internal_select=0) at /Users/brecht/dev/blender-2.5/blender/source/blender/blenkernel/intern/node.c:1245
#2 0x0000000100684075 in ntreeLocalize (ntree=0x118f78e28) at /Users/brecht/dev/blender-2.5/blender/source/blender/blenkernel/intern/node.c:2677
#3 0x00000001001d8693 in compo_initjob (cjv=0x119ecef48) at /Users/brecht/dev/blender-2.5/blender/source/blender/editors/space_node/node_edit.c:124
#4 0x00000001001621fa in WM_jobs_start (wm=0x104557ca8, steve=0x119ecf048) at /Users/brecht/dev/blender-2.5/blender/source/blender/windowmanager/intern/wm_jobs.c:288
#5 0x00000001001d88f9 in snode_composite_job (C=0x10418c038, sa=0x10456da18) at /Users/brecht/dev/blender-2.5/blender/source/blender/editors/space_node/node_edit.c:191
#6 0x00000001001e05db in node_area_refresh (C=0x10418c038, sa=0x10456da18) at /Users/brecht/dev/blender-2.5/blender/source/blender/editors/space_node/space_node.c:240
#7 0x00000001003f31c0 in ED_area_do_refresh (C=0x10418c038, sa=0x10456da18) at /Users/brecht/dev/blender-2.5/blender/source/blender/editors/screen/area.c:140
#8 0x00000001001596ca in wm_event_do_notifiers (C=0x10418c038) at /Users/brecht/dev/blender-2.5/blender/source/blender/windowmanager/intern/wm_event_system.c:294
#9 0x0000000100155d12 in WM_main (C=0x10418c038) at /Users/brecht/dev/blender-2.5/blender/source/blender/windowmanager/intern/wm.c:345
#10 0x00000001001554fd in main (argc=1, argv=0x7fff5fbff228) at /Users/brecht/dev/blender-2.5/blender/source/creator/creator.c:1194


Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000066
[Switching to process 12159]
0x0000000100682ede in ntreeBeginExecTree (ntree=0x119966b28) at /Users/brecht/dev/blender-2.5/blender/source/blender/blenkernel/intern/node.c:2229
2229 ns= ntree->stack + sock->link->fromsock->stack_index;
(gdb) bt
#0 0x0000000100682ede in ntreeBeginExecTree (ntree=0x119966b28) at /Users/brecht/dev/blender-2.5/blender/source/blender/blenkernel/intern/node.c:2229
#1 0x0000000100683cd8 in ntreeCompositExecTree (ntree=0x119966b28, rd=0x104ee5f20, do_preview=1) at /Users/brecht/dev/blender-2.5/blender/source/blender/blenkernel/intern/node.c:2576
#2 0x00000001001d87d5 in compo_startjob (cjv=0x119965058, stop=0x119966a3c, do_update=0x119966a3a, progress=0x119966a40) at /Users/brecht/dev/blender-2.5/blender/source/blender/editors/space_node/node_edit.c:165
#3 0x0000000100162052 in do_job_thread (job_v=0x1199669c8) at /Users/brecht/dev/blender-2.5/blender/source/blender/windowmanager/intern/wm_jobs.c:238
#4 0x00000001007d4d53 in tslot_thread_start (tslot_p=0x1169282e8) at /Users/brecht/dev/blender-2.5/blender/source/blender/blenlib/intern/threads.c:213
#5 0x00007fff8235c456 in _pthread_start ()
#6 0x00007fff8235c309 in thread_start ()
(gdb) p sock->link
$4 = (struct bNodeLink *) 0x1199650d8
(gdb) p sock->link->fromsock
$5 = (bNodeSocket *) 0x0
(gdb)

Fixed the mentioned issues and made a few simplifications:
* Invalid links after "internalising" sockets were caused by using the wrong node tree to check links (just a local variable shadowing ntree pointer)
* Linked sockets in a new group are now exposed, so corresponding links to the group are created
* Group tree verification can be simplified, since InitTypes does not have to be called. Node groups don't need an individual type now, they just have their own list of external sockets.
* Removed group socket update after readfile (ntreeMakeGroupSockets). This too is not necessary, since the group socket list is stored in the file.

With latest changes I can't find any more problems with this patch. Assigning to Ton.

Hey nice patch! I'm testing it and seems to work fine.

Note 1: when compiling I get an error at line 208 in rna_main_api.c where it looks like a ntreeMakeOwnType was forgot to be renamed into ntreeMakeGroupSockets.

Note 2: it would be nice if one could also change the order of inputs and outputs in the group in addition to which ones to show.

I've been playing a few minutes with it and have other ideas.

1) would be nice to have an X and arrow-down/arrow-up buttons next to ins/outs names in order to remove and re-order them.

2) also useful would be to be able to put any outs on the group's outs, even those that are already used within the group; for instance if inside the group you create a mask that you use in the group itself but that could be also used outside for other nodes.

Version 5 fixes the compile error with ntreeMakeOwnType (introduced in r33437) and adds a few improvements:
* When adding sockets to group input/output, previous sockets will keep their order, new sockets are attached at the end.
* In addition to dropping a link in empty background, an external socket can also be removed by the "x" button next to the socket name.
* Up/down buttons can be used to change the order of group sockets after adding.

Version 6:
* Fixed a sloppyness segfault bug
* Socket circles are now displayed on the groups outer border for cleaner display
* Removed name comparison from group node verification. Since group sockets are stored in the blend file, this is not necessary any more (still in place for non-group nodes though). Was causing links to be deleted when changing a socket name.

Thanks for the changes, looks great. The only problem is now when I compact the group the names are not reflected and instead I get the standard names.

Fixed this in version 7.
I earlier removed the name check from group socket verification to allow name changes without removing links, but the node instances' names then stayed unchanged. Now updating them from the group sockets.

groupnode_nameable_sockets_07.diff causes crash on win64 build.
quick fix:

static EnumPropertyItem socket_in_out_items[] = {
{ SOCK_IN, "IN", 0, "In", "" },
{ SOCK_OUT, "OUT", 0, "Out", "" },
{ -1, "", 0, NULL, NULL },
};

to

static EnumPropertyItem socket_in_out_items[] = {
{ SOCK_IN, "IN", 0, "In", "" },
{ SOCK_OUT, "OUT", 0, "Out", "" },
{ -1, "", 0, NULL, NULL },
{0, NULL, 0, NULL, NULL}
};

in node_edit.c.

Thanks for the report, fixed in version 8.

Version 9 adds RNA definitions for groups and functions for adding/removing group sockets. Credits for this go to Uncle_Entity!

Version 10 makes a bigger leap:
Group sockets are now stored as a linked list of standard bNodeSocket pointers, just like regular node sockets. This makes it easier to use the same mechanism for linking groups to internal nodes:
* A group input can be linked to multiple internal sockets.
* Internal sockets can be linked to multiple group outputs
* Group sockets can be unlinked. Inputs will be unused in that case, while outputs will use their constant value (just like unlinked regular node inputs).
* Group inputs can be linked to group outputs directly, without any intermediate internal nodes.

This is a radical simplification, since the same system is used for regular sockets as well as group sockets, but it means that the patch is likely to have a few remaining bugs. This version is only uploaded as a reference for further testing and should not be considered for trunk merging until fully stabilized!

Grop Node of version 10 doesn't works at least on Windows.
http://www.pasteall.org/pic/8823
http://www.pasteall.org/blend/5074

version 9 works fine.
http://www.pasteall.org/blend/5075
http://www.pasteall.org/pic/8824

Version 11 should fix most of the issues.

The buffers for group sockets are now inside the group node, instead of each instance as previously. These were freed after group execution, so the result did not show.

Also when making a new group from selected nodes, only one group socket per exposed internal node is now created instead of one for each branch.

Yet another version (12) uploaded (The only way to make it stop is by reviewing it! :D )
This time added RNA definitions for groups and fixed the operator for adding group nodes.

Version 13: Updated to svn trunk (34794) and made a few bug fixes, added RNA paths to group tree sockets and a pointer check in getExecutableNode.

issues of version 13:
- color of output node in material nodes isn't changed
- outer/inner parameters of group node doesn't sync

http://www.pasteall.org/blend/5195

I withdraw second issue (about parameters) .
it seems to be specification, not a issue.

Bastien Montagne (mont29) changed the task status from Unknown Status to Unknown Status.Aug 7 2014, 2:56 PM

Thinks it’s time to archive! :)