Fix T40986: crash on using the viewer node inside of group nodes.
authorLukas Tönne <lukas.toenne@gmail.com>
Tue, 8 Jul 2014 10:48:41 +0000 (12:48 +0200)
committerLukas Tönne <lukas.toenne@gmail.com>
Tue, 8 Jul 2014 10:51:35 +0000 (12:51 +0200)
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.

source/blender/compositor/intern/COM_NodeConverter.cpp
source/blender/compositor/intern/COM_NodeConverter.h
source/blender/compositor/intern/COM_NodeGraph.cpp
source/blender/compositor/intern/COM_NodeOperationBuilder.cpp
source/blender/compositor/intern/COM_NodeOperationBuilder.h
source/blender/compositor/nodes/COM_SplitViewerNode.cpp
source/blender/compositor/nodes/COM_ViewerNode.cpp

index 5965eade389ed665335ce39b146f59d0da2b7d86..81a10a58cc57ade19c0617336f3083e9b44e1d0d 100644 (file)
@@ -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();
+}
index cad8408a2bf18b00568345a7f4d64154a25fd986..e5e7629f39a7a844870d613a719ef7409f78a95a 100644 (file)
@@ -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;
index 21b88797233341a96f0e0ddaf035bb6907e45454..cbe47238249567e9c3f8ee3833d8954d56e97d70 100644 (file)
@@ -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) {
index a90bac847c056100fbb9b5c3c230f5f148c41c70..6554926991cbc8f716824317ebf5d2ebd504b866 100644 (file)
@@ -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 ****
  ****************************/
index ab890282babc486403e9eec0bbe1b7dd81e0357c..633ddc9152aea7d37aaed345f71066b49640807c 100644 (file)
@@ -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);
index 8eb1b76e89029d4ca33c60c4da44490620160431..15eca0a97e5442b0f43aeb4224f84cd62be22e69 100644 (file)
@@ -35,8 +35,7 @@ SplitViewerNode::SplitViewerNode(bNode *editorNode) : Node(editorNode)
 void SplitViewerNode::convertToOperations(NodeConverter &converter, const CompositorContext &context) const
 {
        bNode *editorNode = this->getbNode();
-       bool is_active = ((editorNode->flag & NODE_DO_OUTPUT_RECALC || context.isRendering()) &&
-                         (editorNode->flag & NODE_DO_OUTPUT) && this->isInActiveGroup());
+       bool do_output = (editorNode->flag & NODE_DO_OUTPUT_RECALC || context.isRendering()) && (editorNode->flag & NODE_DO_OUTPUT);
 
        NodeInput *image1Socket = this->getInputSocket(0);
        NodeInput *image2Socket = this->getInputSocket(1);
@@ -54,7 +53,6 @@ void SplitViewerNode::convertToOperations(NodeConverter &converter, const Compos
        ViewerOperation *viewerOperation = new ViewerOperation();
        viewerOperation->setImage(image);
        viewerOperation->setImageUser(imageUser);
-       viewerOperation->setActive(is_active);
        viewerOperation->setViewSettings(context.getViewSettings());
        viewerOperation->setDisplaySettings(context.getDisplaySettings());
 
@@ -68,4 +66,7 @@ void SplitViewerNode::convertToOperations(NodeConverter &converter, const Compos
        converter.addLink(splitViewerOperation->getOutputSocket(), viewerOperation->getInputSocket(0));
 
        converter.addPreview(splitViewerOperation->getOutputSocket());
+
+       if (do_output)
+               converter.registerViewer(viewerOperation);
 }
index 09a3cea2da187cb4a4898a7d871778bcfc19a085..07aa960c4d9defa9263047d3d7bede7852f4da22 100644 (file)
@@ -34,8 +34,7 @@ ViewerNode::ViewerNode(bNode *editorNode) : Node(editorNode)
 void ViewerNode::convertToOperations(NodeConverter &converter, const CompositorContext &context) const
 {
        bNode *editorNode = this->getbNode();
-       bool is_active = (editorNode->flag & NODE_DO_OUTPUT_RECALC || context.isRendering()) &&
-                        ((editorNode->flag & NODE_DO_OUTPUT) && this->isInActiveGroup());
+       bool do_output = (editorNode->flag & NODE_DO_OUTPUT_RECALC || context.isRendering()) && (editorNode->flag & NODE_DO_OUTPUT);
        bool ignore_alpha = editorNode->custom2 & CMP_NODE_OUTPUT_IGNORE_ALPHA;
 
        NodeInput *imageSocket = this->getInputSocket(0);
@@ -47,7 +46,6 @@ void ViewerNode::convertToOperations(NodeConverter &converter, const CompositorC
        viewerOperation->setbNodeTree(context.getbNodeTree());
        viewerOperation->setImage(image);
        viewerOperation->setImageUser(imageUser);
-       viewerOperation->setActive(is_active);
        viewerOperation->setChunkOrder((OrderOfChunks)editorNode->custom1);
        viewerOperation->setCenterX(editorNode->custom3);
        viewerOperation->setCenterY(editorNode->custom4);
@@ -74,4 +72,7 @@ void ViewerNode::convertToOperations(NodeConverter &converter, const CompositorC
        converter.mapInputSocket(depthSocket, viewerOperation->getInputSocket(2));
 
        converter.addNodeInputPreview(imageSocket);
+
+       if (do_output)
+               converter.registerViewer(viewerOperation);
 }