Multi File Output Node

Adds a new node type for saving multiple image files from a single node.

Unlike the existing file output node this node has an arbitrary number of possible input slots. It has a base path string that can be set to a general base folder. Every input socket then uses its name as an extension of the base path for file organization. This can include further subfolders on top of the base path. Example:

Base path: '/home/user/myproject'
Input 1: 'Compo'
Input 2: 'Diffuse/'
Input 3: 'details/Normals'

would create output files
in /home/user/myproject: Compo0001.png, Compo0002.png, ...
in /home/user/myproject/Diffuse: 0001.png, 0002.png, ... (no filename base given)
in /home/user/myproject/details: Normals0001.png, Normals0002.png, ...

Most settings for the node can be found in the sidebar (NKEY). New input sockets can be added with the "Add Input" button. There is a list of input sockets and below that the details for each socket can be changed, including the sub-path and filename. Sockets can be removed here as well. By default each socket uses the render settings file output format, but each can use its own format if necessary.

To my knowledge this is the first node making use of such dynamic sockets in trunk. So this is also a design test, other nodes might use this in the future.

Adding operator buttons on top of a node is a bit unwieldy atm, because all node operators generally work on selected and/or active node(s). The operator button would therefore either have to make sure the node is  activated before the operator is called (block callback maybe?) OR it has to store the node name (risky, weak reference). For now it is only used in the sidebar, where only the active node's buttons are displayed.

Also adds a new struct_type value to bNodeSocket, in order to distinguish different socket types with the same data type (file inputs are SOCK_RGBA color sockets). Would be nicer to use data type only for actual data evaluation, but used in too many places, this works ok for now.

[brechtvl, 2012-02-21 12:27:53]

Is there any particular reason to create a new node rather than extending the
File Output node? It seems like a natural extension of that, only issue I can
see is that it requires some version patching.

http://codereview.appspot.com/5666047/diff/1/source/blender/editors/space_nod...
File source/blender/editors/space_node/node_header.c (right):

http://codereview.appspot.com/5666047/diff/1/source/blender/editors/space_nod...
source/blender/editors/space_node/node_header.c:117: ntemp.r = &scene->r;
Instead of passing RenderData, I'd just pass in Main + Scene, that's sort of the
typical context you get in RNA too, which is less specific.

http://codereview.appspot.com/5666047/diff/1/source/blender/editors/space_nod...
source/blender/editors/space_node/node_header.c:162: ntemp.r = &scene.r;
scene.r => scene->r

http://codereview.appspot.com/5666047/diff/1/source/blender/makesdna/DNA_node...
File source/blender/makesdna/DNA_node_types.h (right):

http://codereview.appspot.com/5666047/diff/1/source/blender/makesdna/DNA_node...
source/blender/makesdna/DNA_node_types.h:80: short struct_type;			/* optional
identifier for RNA struct subtype */
Why add this struct_type mechanism instead of adding a new socket type? Maybe it
is the right thing to do but I'm not sure, what are the other possible use cases
here?

If it was just a socket type, at least it would fit more cleanly, and it also
makes some sense conceptually, as you can output files with either
color/vector/float values.

http://codereview.appspot.com/5666047/diff/1/source/blender/makesdna/DNA_node...
source/blender/makesdna/DNA_node_types.h:370: ImageFormatData format;
Does this need to be a per socket setting? The use case is not entirely clear to
me, maybe this was requested, but it seems inconvenient having to change the
image settings on all sockets rather than once for a node.

The other reason is also, it might make sense to add EXR multi layer output
eventually, and than it makes even more sense to have one per node.

http://codereview.appspot.com/5666047/diff/1/source/blender/nodes/composite/n...
File source/blender/nodes/composite/nodes/node_composite_outputFile.c (right):

http://codereview.appspot.com/5666047/diff/1/source/blender/nodes/composite/n...
source/blender/nodes/composite/nodes/node_composite_outputFile.c:236: #if 0	/*
XXX is this needed? if so, how to integrate z buffer output in the UI? */
I don't think this is needed, some old file formats can save a Z buffer in the
same file, but that isn't really in use anymore as far as I know, multilayer EXR
would be used instead.

[lukas.toenne, 2012-02-22 11:56:37]

http://codereview.appspot.com/5666047/diff/1/source/blender/editors/space_nod...
File source/blender/editors/space_node/node_header.c (right):

http://codereview.appspot.com/5666047/diff/1/source/blender/editors/space_nod...
source/blender/editors/space_node/node_header.c:117: ntemp.r = &scene->r;
On 2012/02/21 12:27:53, brechtvl wrote:
> Instead of passing RenderData, I'd just pass in Main + Scene, that's sort of
the
> typical context you get in RNA too, which is less specific.

Done.

http://codereview.appspot.com/5666047/diff/1/source/blender/editors/space_nod...
source/blender/editors/space_node/node_header.c:162: ntemp.r = &scene.r;
On 2012/02/21 12:27:53, brechtvl wrote:
> scene.r => scene->r

Done.

http://codereview.appspot.com/5666047/diff/1/source/blender/makesdna/DNA_node...
File source/blender/makesdna/DNA_node_types.h (right):

http://codereview.appspot.com/5666047/diff/1/source/blender/makesdna/DNA_node...
source/blender/makesdna/DNA_node_types.h:80: short struct_type;			/* optional
identifier for RNA struct subtype */
On 2012/02/21 12:27:53, brechtvl wrote:
> Why add this struct_type mechanism instead of adding a new socket type? Maybe
it
> is the right thing to do but I'm not sure, what are the other possible use
cases
> here?
> 
> If it was just a socket type, at least it would fit more cleanly, and it also
> makes some sense conceptually, as you can output files with either
> color/vector/float values.

These sockets really are color sockets. They just use some additional
information (image format, paths in name), which requires customized draw
functions and RNA refine. Socket types OTOH are currently based only on the data
type of the socket. If we add a new data type for each socket with slightly
different behavior it means we have to update data handling and conversions for
all existing node systems as well, which is really not necessary. It would be
nicer to actually distinguish general socket type and data type better, but for
now this kind of "subtype" seems an acceptable solution.

http://codereview.appspot.com/5666047/diff/1/source/blender/makesdna/DNA_node...
source/blender/makesdna/DNA_node_types.h:370: ImageFormatData format;
On 2012/02/21 12:27:53, brechtvl wrote:
> Does this need to be a per socket setting? The use case is not entirely clear
to
> me, maybe this was requested, but it seems inconvenient having to change the
> image settings on all sockets rather than once for a node.
> 
> The other reason is also, it might make sense to add EXR multi layer output
> eventually, and than it makes even more sense to have one per node.

Yes, this was an explicit request. Note that by default all sockets use the
render settings (use_render_format flag), setting custom socket format is
optional. We also discussed using a file format per node instead of "global"
render settings, but this seems to work well for now.

http://codereview.appspot.com/5666047/diff/1/source/blender/nodes/composite/n...
File source/blender/nodes/composite/nodes/node_composite_outputFile.c (right):

http://codereview.appspot.com/5666047/diff/1/source/blender/nodes/composite/n...
source/blender/nodes/composite/nodes/node_composite_outputFile.c:236: #if 0	/*
XXX is this needed? if so, how to integrate z buffer output in the UI? */
On 2012/02/21 12:27:53, brechtvl wrote:
> I don't think this is needed, some old file formats can save a Z buffer in the
> same file, but that isn't really in use anymore as far as I know, multilayer
EXR
> would be used instead.

Agreed, i would rather add another new node type specifically for writing
multi-layer exr. Can work the same way as this node, just writing to layers
instead individual files and folders.

[lukas.toenne, 2012-02-22 11:57:33]

Implemented some proposed changes

[lukas.toenne, 2012-02-22 12:11:02]

Added mute functions to allow quick disabling of file output nodes

[brechtvl, 2012-02-22 12:55:35]

You committed this now, but I still do not agree with the design decisions.

File format per socket still seems overkill to me, I'd like to understand the
rationale for that. Of course it's most flexible and that is what users will
request, but I think they will regret it when they created dozes of sockets and
then have to go change all of them.

I also still do not think it's a good idea to make this a new node type. My
point was that a single node type could do everything.

I also do not think that the sockets really should be color sockets, exactly
because it should be possible to save out colors/vectors/floats. For EXR, you
want to be able to output them as such, not all as color.

[lukas.toenne, 2012-02-22 16:34:43]

On 2012/02/22 12:55:35, brechtvl wrote:
> You committed this now, but I still do not agree with the design decisions.

Sorry, maybe that was a bit rushed :/ 

> File format per socket still seems overkill to me, I'd like to understand the
> rationale for that. Of course it's most flexible and that is what users will
> request, but I think they will regret it when they created dozes of sockets
and
> then have to go change all of them.

Main reason for this node was the ability to have a single base path (that was
Francesco's request anyway). If all sockets on a node _have_ to use the same
format you would need a second, third, etc. node again to occasionally store to
different formats, and in turn have to deal with changing paths. I cannot really
say how important each of these issues is in comparison. Would it be enough to
use a per-node format setting by default (instead of global render format), but
keep the optional per-socket format?

> I also still do not think it's a good idea to make this a new node type. My
> point was that a single node type could do everything.

Fair enough, we can replace the old file output implementation with this one.
Would require a bit of do_versions and deprecate the new node type again, but i
don't mind.

> I also do not think that the sockets really should be color sockets, exactly
> because it should be possible to save out colors/vectors/floats. For EXR, you
> want to be able to output them as such, not all as color.

Ok, have been thinking about this a bit, but i still think we should not add a
new socket type for this purpose. The idea is that the sock->type (i.e. data
type) defines what kind of a data a socket accepts/generates. The struct_type
OTOH defines the RNA subclass, what additional info the socket stores
(sock->storage). We don't have multiple inheritance in RNA, so the data subclass
should be defined on top of the struct type. A cleaner approach would be to
define the input data as a separate pointer property instead, though this is a
more radical change of the API and will likely break lots of scripts.

I would suggest that instead of making a new socket type, the file node inputs
change their type (sock->type) automatically according to what is connected to
them. This data type is what is then actually written into the file. Might also
allow users to set and explicit data type or make this dependent on the selected
file format. The sock->struct_type stays the same all the time, only the data
type changes. This is an additional feature though (old output node doesn't
support this either). This would be very similar to the group interface output
nodes i made recently: 

http://www.youtube.com/watch?v=Jk_Kk5yOYvA#t=408

[lukas.toenne, 2012-02-29 16:29:12]

Implemented discussed changes, see following mail

[lukas.toenne, 2012-02-29 16:39:41]

I have implemented the changes discussed so far.

1) Old CMP_NODE_OUTPUT_FILE and CMP_NODE_OUTPUT_MULTI_FILE have been merged,
only CMP_NODE_OUTPUT_FILE remains. All functions renamed accordingly.

2) do_versions code for converting single-file output nodes into multi-file
output nodes. If a Z buffer input is used, the node is made into a multilayer
exr with two inputs. (see below). Also re-identifies multi-file output nodes
with the CMP_NODE_OUTPUT_FILE type.

3) "Global" format is stored in node now. By default this overrides any
per-socket settings.

4) Multilayer EXR output implemented. When M.EXR format is selected for node
format, all socket format details are ignored. Socket names are used for layer
names.

5) Input buffer types are used as-is when possible, i.e. stored as B/W, RGB or
RGBA. In regular file output the format dictates the number of actual channels,
so the CompBuf is typechecked to the right type first. For multilayer EXR the
number of channels is more flexible, so an input buffer will store only the
channels it actually uses.

6) The editor socket type is updated from linked sockets as an indicator of the
actual data written to files. This may not be totally accurate for regular file
output though, due to restrictions of format setting.

[brechtvl, 2012-02-29 17:02:48]

http://codereview.appspot.com/5666047/diff/10001/source/blender/nodes/composi...
File source/blender/nodes/composite/nodes/node_composite_outputFile.c (right):

http://codereview.appspot.com/5666047/diff/10001/source/blender/nodes/composi...
source/blender/nodes/composite/nodes/node_composite_outputFile.c:204: * It's an
ugly mess, hopefully Tile will do this stuff right ...
I don't really see this as a code issue, imo we shouldn't even be trying to
match the render result size, it should just be based on the incoming buffers, I
don't think it needs to be fixed.

http://codereview.appspot.com/5666047/diff/10001/source/blender/nodes/composi...
source/blender/nodes/composite/nodes/node_composite_outputFile.c:210: switch
(nimf->format.planes) {
This option doesn't really make much sense in this context, in my opinion, just
ignore it.

http://codereview.appspot.com/5666047/diff/10001/source/blender/nodes/composi...
source/blender/nodes/composite/nodes/node_composite_outputFile.c:235:
printf("Error Size mismatch\n");
In this error message, it should specify that it's a file output node that
generated the error, and that it expects all sizes to be the same.

http://codereview.appspot.com/5666047/diff/10001/source/blender/nodes/composi...
source/blender/nodes/composite/nodes/node_composite_outputFile.c:295: #if 0	/*
XXX not used yet */
I guess this could just use the first input?

[lukas.toenne, 2012-03-01 07:41:33]

Also fixed the socket display for non-color file output sockets and a minor
update issue in node_edit.

http://codereview.appspot.com/5666047/diff/10001/source/blender/nodes/composi...
File source/blender/nodes/composite/nodes/node_composite_outputFile.c (right):

http://codereview.appspot.com/5666047/diff/10001/source/blender/nodes/composi...
source/blender/nodes/composite/nodes/node_composite_outputFile.c:204: * It's an
ugly mess, hopefully Tile will do this stuff right ...
On 2012/02/29 17:02:48, brechtvl wrote:
> I don't really see this as a code issue, imo we shouldn't even be trying to
> match the render result size, it should just be based on the incoming buffers,
I
> don't think it needs to be fixed.

Done.

http://codereview.appspot.com/5666047/diff/10001/source/blender/nodes/composi...
source/blender/nodes/composite/nodes/node_composite_outputFile.c:210: switch
(nimf->format.planes) {
On 2012/02/29 17:02:48, brechtvl wrote:
> This option doesn't really make much sense in this context, in my opinion,
just
> ignore it.

Done.

http://codereview.appspot.com/5666047/diff/10001/source/blender/nodes/composi...
source/blender/nodes/composite/nodes/node_composite_outputFile.c:235:
printf("Error Size mismatch\n");
On 2012/02/29 17:02:48, brechtvl wrote:
> In this error message, it should specify that it's a file output node that
> generated the error, and that it expects all sizes to be the same.

Done.

http://codereview.appspot.com/5666047/diff/10001/source/blender/nodes/composi...
source/blender/nodes/composite/nodes/node_composite_outputFile.c:295: #if 0	/*
XXX not used yet */
On 2012/02/29 17:02:48, brechtvl wrote:
> I guess this could just use the first input?

Done.

[brechtvl, 2012-03-01 10:57:39]

LGTM :)