Merge pull request #553 from johnhaddon/nodeGadgetCreatorNullReturn

Fixed GraphGadget for NULL return from NodeGadget::create().
GafferHQ · Sep 26, 2013 · 3eb68db · 3eb68db
2 parents 758d816 + 65d6a08
commit 3eb68db
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 7 deletions.
diff --git a/include/GafferUI/GraphGadget.h b/include/GafferUI/GraphGadget.h
@@ -169,7 +169,9 @@ class GraphGadget : public ContainerGadget
 		void offsetNodes( Gaffer::Set *nodes, const Imath::V2f &offset );
 
 		void updateGraph();
-		void addNodeGadget( Gaffer::Node *node );
+		/// May return NULL if NodeGadget::create() returns NULL, signifying that
+		/// someone has registered a creator in order to hide all nodes of a certain type.
+		NodeGadget *addNodeGadget( Gaffer::Node *node );
 		void removeNodeGadget( const Gaffer::Node *node );
 		NodeGadget *findNodeGadget( const Gaffer::Node *node ) const;
 		void updateNodeGadgetTransform( NodeGadget *nodeGadget );

diff --git a/python/GafferUITest/NodeGraphTest.py b/python/GafferUITest/NodeGraphTest.py
@@ -845,6 +845,40 @@ def testConnectionMinimisation( self ) :
 
 		self.assertFalse( c1.getMinimised() )
 		self.assertFalse( c2.getMinimised() )
+
+	def testNodeGadgetCreatorReturningNull( self ) :
+
+		class InvisibleNode( GafferTest.AddNode ) :
+
+			def __init__( self, name = "InvisibleNode" ) :
+
+				GafferTest.AddNode.__init__( self, name )
+
+		IECore.registerRunTimeTyped( InvisibleNode )
+
+		GafferUI.NodeGadget.registerNodeGadget( InvisibleNode.staticTypeId(), lambda node : None )
+
+		script = Gaffer.ScriptNode()
+		g = GafferUI.GraphGadget( script )
+
+		script["n1"] = InvisibleNode()
+		script["n2"] = InvisibleNode()
+
+		self.assertEqual( g.nodeGadget( script["n1"] ), None )
+		self.assertEqual( g.nodeGadget( script["n2"] ), None )
+
+		script["n2"]["op1"].setInput( script["n1"]["sum"] )
+
+		self.assertEqual( g.connectionGadget( script["n2"]["op1"] ), None )
+
+		# in case it wasn't clear, hiding the nodes has zero
+		# effect on their computations.
+
+		script["n1"]["op1"].setValue( 12 )
+		script["n1"]["op2"].setValue( 13 )
+		script["n2"]["op2"].setValue( 100 )
+
+		self.assertEqual( script["n2"]["sum"].getValue(), 125 )
 
 if __name__ == "__main__":
 	unittest.main()
diff --git a/src/GafferUI/GraphGadget.cpp b/src/GafferUI/GraphGadget.cpp
@@ -536,8 +536,11 @@ void GraphGadget::rootChildAdded( Gaffer::GraphComponent *root, Gaffer::GraphCom
 	if( node && ( !m_filter || m_filter->contains( node ) ) )
 	{
 		if( !findNodeGadget( node ) )
-		{	addNodeGadget( node );
-			addConnectionGadgets( node );   
+		{
+			if( addNodeGadget( node ) )
+			{
+				addConnectionGadgets( node );   
+			}
 		}
 	}
 }
@@ -557,8 +560,11 @@ void GraphGadget::filterMemberAdded( Gaffer::Set *set, IECore::RunTimeTyped *mem
 	if( node && node->parent<Gaffer::Node>() == m_root )
 	{
 		if( !findNodeGadget( node ) )
-		{	addNodeGadget( node );
-			addConnectionGadgets( node );   
+		{
+			if( addNodeGadget( node ) )
+			{
+				addConnectionGadgets( node );   
+			}
 		}
 	}
 }
@@ -1196,9 +1202,14 @@ void GraphGadget::updateGraph()
 
 }
 
-void GraphGadget::addNodeGadget( Gaffer::Node *node )
+NodeGadget *GraphGadget::addNodeGadget( Gaffer::Node *node )
 {	
 	NodeGadgetPtr nodeGadget = NodeGadget::create( node );
+	if( !nodeGadget )
+	{
+		return NULL;
+	}
+
 	addChild( nodeGadget );
 
 	NodeGadgetEntry nodeGadgetEntry;
@@ -1215,6 +1226,8 @@ void GraphGadget::addNodeGadget( Gaffer::Node *node )
 	}
 
 	updateNodeGadgetTransform( nodeGadget.get() );
+
+	return nodeGadget;
 }
 
 void GraphGadget::removeNodeGadget( const Gaffer::Node *node )
@@ -1264,6 +1277,10 @@ void GraphGadget::addConnectionGadgets( Gaffer::GraphComponent *plugParent )
 	Gaffer::Node *node = plugParent->isInstanceOf( Gaffer::Node::staticTypeId() ) ? static_cast<Gaffer::Node *>( plugParent ) : plugParent->ancestor<Gaffer::Node>();
 
 	NodeGadget *nodeGadget = findNodeGadget( node );
+	if( !nodeGadget )
+	{
+		return;
+	}
 
 	for( Gaffer::PlugIterator pIt( plugParent->children().begin(), plugParent->children().end() ); pIt!=pIt.end(); pIt++ )
 	{
@@ -1311,7 +1328,13 @@ void GraphGadget::addConnectionGadget( Gaffer::Plug *dstPlug )
 	}
 
 	Gaffer::Node *dstNode = dstPlug->node();
-	Nodule *dstNodule = findNodeGadget( dstNode )->nodule( dstPlug );
+	NodeGadget *dstNodeGadget = findNodeGadget( dstNode );
+	if( !dstNodeGadget )
+	{
+		return;
+	}
+
+	Nodule *dstNodule = dstNodeGadget->nodule( dstPlug );
 	if( !dstNodule )
 	{
 		// the destination connection point is not represented in the graph