Fix T40986: crash on using the viewer node inside of group nodes.

Viewers were activated both inside the active group as well as the top
level tree (the latter being a quick fix for getting a fallback viewer).
This caused a race condition on the shared viewer image.

Now the active viewer is defined at node conversion time in the converter
so that only one can be active at a time without each node having to
follow complicated rules for exclusion.

5 files changed, 52 insertions, 3 deletions

diff --git a/source/blender/compositor/intern/COM_NodeConverter.cpp b/source/blender/compositor/intern/COM_NodeConverter.cpp
index 5965eade389..81a10a58cc5 100644
--- a/source/blender/compositor/intern/COM_NodeConverter.cpp
+++ b/source/blender/compositor/intern/COM_NodeConverter.cpp
@@ -156,3 +156,13 @@ void NodeConverter::addOutputVector(NodeOutput *output, const float value[3])
 	m_builder->addOperation(operation);
 	m_builder->mapOutputSocket(output, operation->getOutputSocket());
 }
+
+void NodeConverter::registerViewer(ViewerOperation *viewer)
+{
+	m_builder->registerViewer(viewer);
+}
+
+ViewerOperation *NodeConverter::active_viewer() const
+{
+	return m_builder->active_viewer();
+}
diff --git a/source/blender/compositor/intern/COM_NodeConverter.h b/source/blender/compositor/intern/COM_NodeConverter.h
index cad8408a2bf..e5e7629f39a 100644
--- a/source/blender/compositor/intern/COM_NodeConverter.h
+++ b/source/blender/compositor/intern/COM_NodeConverter.h
@@ -34,6 +34,8 @@ class NodeOperationInput;
 class NodeOperationOutput;
 class NodeOperationBuilder;
 
+class ViewerOperation;
+
 /** Interface type for converting a \a Node into \a NodeOperation.
  *  This is passed to \a Node::convertToOperation methods and allows them
  *  to register any number of operations, create links between them,
@@ -102,6 +104,11 @@ public:
 	 */
 	NodeOperation *setInvalidOutput(NodeOutput *output);
 	
+	/** Define a viewer operation as the active output, if possible */
+	void registerViewer(ViewerOperation *viewer);
+	/** The currently active viewer output operation */
+	ViewerOperation *active_viewer() const;
+	
 private:
 	/** The internal builder for storing the results of the graph construction. */
 	NodeOperationBuilder *m_builder;
diff --git a/source/blender/compositor/intern/COM_NodeGraph.cpp b/source/blender/compositor/intern/COM_NodeGraph.cpp
index 21b88797233..cbe47238249 100644
--- a/source/blender/compositor/intern/COM_NodeGraph.cpp
+++ b/source/blender/compositor/intern/COM_NodeGraph.cpp
@@ -101,8 +101,7 @@ void NodeGraph::add_bNodeTree(const CompositorContext &context, int nodes_start,
 	const bNodeTree *basetree = context.getbNodeTree();
 	
 	/* update viewers in the active edittree as well the base tree (for backdrop) */
-	bool is_active_group = ((parent_key.value == basetree->active_viewer_key.value) ||
-	                        (tree == basetree));
+	bool is_active_group = (parent_key.value == basetree->active_viewer_key.value);
 	
 	/* add all nodes of the tree to the node list */
 	for (bNode *node = (bNode *)tree->nodes.first; node; node = node->next) {
diff --git a/source/blender/compositor/intern/COM_NodeOperationBuilder.cpp b/source/blender/compositor/intern/COM_NodeOperationBuilder.cpp
index a90bac847c0..6554926991c 100644
--- a/source/blender/compositor/intern/COM_NodeOperationBuilder.cpp
+++ b/source/blender/compositor/intern/COM_NodeOperationBuilder.cpp
@@ -38,12 +38,14 @@ extern "C" {
 #include "COM_SocketProxyOperation.h"
 #include "COM_ReadBufferOperation.h"
 #include "COM_WriteBufferOperation.h"
+#include "COM_ViewerOperation.h"
 
 #include "COM_NodeOperationBuilder.h" /* own include */
 
 NodeOperationBuilder::NodeOperationBuilder(const CompositorContext *context, bNodeTree *b_nodetree) :
     m_context(context),
-    m_current_node(NULL)
+    m_current_node(NULL),
+    m_active_viewer(NULL)
 {
 	m_graph.from_bNodeTree(*context, b_nodetree);
 }
@@ -239,6 +241,25 @@ void NodeOperationBuilder::addNodeInputPreview(NodeInput *input)
 	}
 }
 
+void NodeOperationBuilder::registerViewer(ViewerOperation *viewer)
+{
+	if (m_active_viewer) {
+		if (m_current_node->isInActiveGroup()) {
+			/* deactivate previous viewer */
+			m_active_viewer->setActive(false);
+			
+			m_active_viewer = viewer;
+			viewer->setActive(true);
+		}
+	}
+	else {
+		if (m_current_node->getbNodeTree() == m_context->getbNodeTree()) {
+			m_active_viewer = viewer;
+			viewer->setActive(true);
+		}
+	}
+}
+
 /****************************
  **** Optimization Steps ****
  ****************************/
diff --git a/source/blender/compositor/intern/COM_NodeOperationBuilder.h b/source/blender/compositor/intern/COM_NodeOperationBuilder.h
index ab890282bab..633ddc9152a 100644
--- a/source/blender/compositor/intern/COM_NodeOperationBuilder.h
+++ b/source/blender/compositor/intern/COM_NodeOperationBuilder.h
@@ -44,6 +44,7 @@ class NodeOperationOutput;
 
 class PreviewOperation;
 class WriteBufferOperation;
+class ViewerOperation;
 
 class NodeOperationBuilder {
 public:
@@ -87,6 +88,12 @@ private:
 	
 	Node *m_current_node;
 	
+	/** Operation that will be writing to the viewer image
+	 *  Only one operation can occupy this place at a time,
+	 *  to avoid race conditions
+	 */
+	ViewerOperation *m_active_viewer;
+	
 public:
 	NodeOperationBuilder(const CompositorContext *context, bNodeTree *b_nodetree);
 	~NodeOperationBuilder();
@@ -110,6 +117,11 @@ public:
 	/** Add a preview operation for a node input */
 	void addNodeInputPreview(NodeInput *input);
 	
+	/** Define a viewer operation as the active output, if possible */
+	void registerViewer(ViewerOperation *viewer);
+	/** The currently active viewer output operation */
+	ViewerOperation *active_viewer() const { return m_active_viewer; }
+	
 protected:
 	static NodeInput *find_node_input(const InputSocketMap &map, NodeOperationInput *op_input);
 	static const OpInputs &find_operation_inputs(const OpInputInverseMap &map, NodeInput *node_input);