Compositing reroute converts floats to red color #41573

New Issue

Damien Picard · 2014-08-25T16:08:23+02:00

Damien Picard commented

2014-08-25 16:08:23 +02:00

System Information
Ubuntu 14.04
GTX 660M

Blender Version
Broken: 2.71.5 ca1bca4
Worked: 2.71

When viewing a float reroute node, instead of converting the value to RGB, only the red canal is set, resulting in red image.

Viewer connected to the RenderLayer alpha output:

Viewer connected to the reroute output:

Exact steps for others to reproduce the error

Open file.
Render
bug_reroute.blend

**System Information** Ubuntu 14.04 GTX 660M **Blender Version** Broken: 2.71.5 ca1bca4 Worked: 2.71 When viewing a float reroute node, instead of converting the value to RGB, only the red canal is set, resulting in red image. Viewer connected to the RenderLayer alpha output: ![bug_reroute1.jpg](https://archive.blender.org/developer/F106655/bug_reroute1.jpg) Viewer connected to the reroute output: ![bug_reroute.jpg](https://archive.blender.org/developer/F106656/bug_reroute.jpg) **Exact steps for others to reproduce the error** * Open file. * Render [bug_reroute.blend](https://archive.blender.org/developer/F106653/bug_reroute.blend)

Damien Picard commented

2014-08-25 16:08:23 +02:00

Changed status to: 'Open'

Damien Picard commented

2014-08-25 16:08:23 +02:00

Added subscriber: @pioverfour

blender-admin commented

2014-08-25 16:08:23 +02:00

#41724 was marked as duplicate of this issue

Lukas Tönne was assigned by Sergey Sharybin

2014-08-25 16:54:52 +02:00

Sergey Sharybin commented

2014-08-25 16:54:52 +02:00

Added subscribers: @LukasTonne, @Sergey

Sergey Sharybin commented

2014-08-25 16:54:52 +02:00

@LukasTonne, mind having a look?

Bastien Montagne commented

2014-08-26 15:21:44 +02:00

Added subscriber: @mont29

Sergey Sharybin commented

2014-09-05 13:36:24 +02:00

Added subscriber: @sebastian_k

Sergey Sharybin commented

2014-09-05 13:36:24 +02:00

◀ Merged tasks: #41724.

Bastien Montagne commented

2014-09-07 22:42:43 +02:00

Ok, so issue goes back (at least) to 09874df135

It affects both reroute and muted nodes (try e.g. a NOP math node between alpha input and viewer, no problem, but mute it, you get red again).

This simple patch fixes it all, but would not swear this is the right solution, since I do not exactly fully understand that 'convert' parameter…

P140: (An Untitled Masterwork)

diff --git a/source/blender/compositor/intern/COM_NodeGraph.cpp b/source/blender/compositor/intern/COM_NodeGraph.cpp
index 2dcf419..d7ad5ec 100644
--- a/source/blender/compositor/intern/COM_NodeGraph.cpp
+++ b/source/blender/compositor/intern/COM_NodeGraph.cpp
@@ -202,7 +202,7 @@ void NodeGraph::add_bNodeLink(const NodeRange &node_range, bNodeLink *b_nodelink
 void NodeGraph::add_proxies_mute(bNodeTree *b_ntree, bNode *b_node, bNodeInstanceKey key, bool is_active_group)
 {
 	for (bNodeLink *b_link = (bNodeLink *)b_node->internal_links.first; b_link; b_link = b_link->next) {
-		SocketProxyNode *proxy = new SocketProxyNode(b_node, b_link->fromsock, b_link->tosock, false);
+		SocketProxyNode *proxy = new SocketProxyNode(b_node, b_link->fromsock, b_link->tosock, true);
 		add_node(proxy, b_ntree, key, is_active_group);
 	}
 }
@@ -294,6 +294,6 @@ void NodeGraph::add_proxies_group(const CompositorContext &context, bNode *b_nod
 
 void NodeGraph::add_proxies_reroute(bNodeTree *b_ntree, bNode *b_node, bNodeInstanceKey key, bool is_active_group)
 {
-	SocketProxyNode *proxy = new SocketProxyNode(b_node, (bNodeSocket *)b_node->inputs.first, (bNodeSocket *)b_node->outputs.first, false);
+	SocketProxyNode *proxy = new SocketProxyNode(b_node, (bNodeSocket *)b_node->inputs.first, (bNodeSocket *)b_node->outputs.first, true);
 	add_node(proxy, b_ntree, key, is_active_group);
 }

Lukas, definitively your call now. ;)

Ok, so issue goes back (at least) to 09874df135 It affects both reroute and muted nodes (try e.g. a NOP math node between alpha input and viewer, no problem, but mute it, you get red again). This simple patch fixes it all, but would not swear this is the right solution, since I do not exactly fully understand that 'convert' parameter… [P140: (An Untitled Masterwork)](https://archive.blender.org/developer/P140.txt) ``` diff --git a/source/blender/compositor/intern/COM_NodeGraph.cpp b/source/blender/compositor/intern/COM_NodeGraph.cpp index 2dcf419..d7ad5ec 100644 --- a/source/blender/compositor/intern/COM_NodeGraph.cpp +++ b/source/blender/compositor/intern/COM_NodeGraph.cpp @@ -202,7 +202,7 @@ void NodeGraph::add_bNodeLink(const NodeRange &node_range, bNodeLink *b_nodelink void NodeGraph::add_proxies_mute(bNodeTree *b_ntree, bNode *b_node, bNodeInstanceKey key, bool is_active_group) { for (bNodeLink *b_link = (bNodeLink *)b_node->internal_links.first; b_link; b_link = b_link->next) { - SocketProxyNode *proxy = new SocketProxyNode(b_node, b_link->fromsock, b_link->tosock, false); + SocketProxyNode *proxy = new SocketProxyNode(b_node, b_link->fromsock, b_link->tosock, true); add_node(proxy, b_ntree, key, is_active_group); } } @@ -294,6 +294,6 @@ void NodeGraph::add_proxies_group(const CompositorContext &context, bNode *b_nod void NodeGraph::add_proxies_reroute(bNodeTree *b_ntree, bNode *b_node, bNodeInstanceKey key, bool is_active_group) { - SocketProxyNode *proxy = new SocketProxyNode(b_node, (bNodeSocket *)b_node->inputs.first, (bNodeSocket *)b_node->outputs.first, false); + SocketProxyNode *proxy = new SocketProxyNode(b_node, (bNodeSocket *)b_node->inputs.first, (bNodeSocket *)b_node->outputs.first, true); add_node(proxy, b_ntree, key, is_active_group); } ``` Lukas, definitively your call now. ;)

Lukas Tönne commented

2014-09-08 09:42:49 +02:00

@mont29 the use_conversion parameter for proxies controls whether data gets converted to the proxies own type in every case or not. This is useful e.g. for node groups, where you expect the proxies to have the specified interface type. So for example when a group socket is defined as "Float" and an RGBA is passed in it always gets converted to Float anyway, even when used as RGBA internally. Otherwise the behavior of the group could vary depending on the external nodes.

For the reroute sockets a conversion should not be necessary - in fact they change their type in the UI to indicate that the input type is passed on unmodified! The problem then is why the reroute node actually causes the buffer to become an "RGBA" (with only the red channel). The viewer node should only see a Float input and use the first (R) channel as the value, instead of interpreting it as a full RGBA color value ...

@mont29 the `use_conversion` parameter for proxies controls whether data gets converted to the proxies own type in every case or not. This is useful e.g. for node groups, where you expect the proxies to have the specified interface type. So for example when a group socket is defined as "Float" and an RGBA is passed in it always gets converted to Float anyway, even when used as RGBA internally. Otherwise the behavior of the group could vary depending on the external nodes. For the reroute sockets a conversion should not be necessary - in fact they change their type in the UI to indicate that the input type is passed on unmodified! The problem then is why the reroute node actually causes the buffer to become an "RGBA" (with only the red channel). The viewer node should only see a Float input and use the first (R) channel as the value, instead of interpreting it as a full RGBA color value ...

Bastien Montagne commented

2014-09-08 11:23:56 +02:00

@LukasTonne imho, issue is that there is no conversion here, precisely.

Placing prints in NodeOperationBuilder::add_datatype_conversions(), for this file I got the following:

Input alpha to output: COM_DT_VALUE -> COM_DT_COLOR (use_conversion, use_conversion)
Input alpha to reroute: COM_DT_VALUE -> COM_DT_VALUE (use_conversion, not_use_conversion)
Reroute to viewer: COM_DT_VALUE -> COM_DT_COLOR (not_use_conversion, use_conversion)

So imho, either we use conversions for reroute/mute proxies too, or we change that part of NodeOperationBuilder::add_datatype_conversions():

		if (!from_op->useDatatypeConversion() || !to_op->useDatatypeConversion())
			continue;

to:

		if (!(from_op->useDatatypeConversion() || to_op->useDatatypeConversion()))
			continue;

So that if an operation at one point of the link requires conversion, we do it, even if the other does not require it.

@LukasTonne imho, issue is that there is no conversion here, precisely. Placing prints in `NodeOperationBuilder::add_datatype_conversions()`, for this file I got the following: ``` Input alpha to output: COM_DT_VALUE -> COM_DT_COLOR (use_conversion, use_conversion) Input alpha to reroute: COM_DT_VALUE -> COM_DT_VALUE (use_conversion, not_use_conversion) Reroute to viewer: COM_DT_VALUE -> COM_DT_COLOR (not_use_conversion, use_conversion) ``` So imho, either we use conversions for reroute/mute proxies too, or we change that part of `NodeOperationBuilder::add_datatype_conversions()`: ``` if (!from_op->useDatatypeConversion() || !to_op->useDatatypeConversion()) continue; ``` to: ``` if (!(from_op->useDatatypeConversion() || to_op->useDatatypeConversion())) continue; ``` So that if an operation at one point of the link requires conversion, we do it, even if the other does not require it.

Lukas Tönne commented

2014-09-08 12:23:02 +02:00

Sounds good