Merge pull request #3055 from johnhaddon/invalidLocationFixes

CollectScenes/Duplicate : Don't error when given invalid locations
GafferHQ · Feb 28, 2019 · 6536bcc · 6536bcc
2 parents dd22bc5 + 3c77da5
commit 6536bcc
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 13 deletions.
diff --git a/python/GafferSceneTest/CollectScenesTest.py b/python/GafferSceneTest/CollectScenesTest.py
@@ -272,5 +272,29 @@ def testLoadFromVersion0_48( self ) :
 
 		self.assertTrue( s["CollectScenes"]["in"].getInput(), s["Sphere"]["out"] )
 
+	def testCollectInvalidLocation( self ) :
+
+		sphere = GafferScene.Sphere()
+		sphere["sets"].setValue( "set1" )
+
+		group = GafferScene.Group()
+		group["in"][0].setInput( sphere["out"] )
+
+		collect = GafferScene.CollectScenes()
+		collect["rootNames"].setValue( IECore.StringVectorData( [ "A" ] ) )
+		collect["in"].setInput( group["out"] )
+
+		self.assertSceneValid( collect["out"] )
+		self.assertEqual( collect["out"].childNames( "/" ), IECore.InternedStringVectorData( [ "A" ] ) )
+		self.assertEqual( collect["out"].childNames( "/A" ), IECore.InternedStringVectorData( [ "group" ] ) )
+		self.assertEqual( collect["out"].childNames( "/A/group" ), IECore.InternedStringVectorData( [ "sphere" ] ) )
+		self.assertEqual( collect["out"].childNames( "/A/group/sphere" ), IECore.InternedStringVectorData() )
+
+		collect["sourceRoot"].setValue( "iDontExist" )
+
+		self.assertSceneValid( collect["out"] )
+		self.assertEqual( collect["out"].childNames( "/" ), IECore.InternedStringVectorData( [ "A" ] ) )
+		self.assertEqual( collect["out"].childNames( "/A" ), IECore.InternedStringVectorData() )
+
 if __name__ == "__main__":
 	unittest.main()
diff --git a/python/GafferSceneTest/DuplicateTest.py b/python/GafferSceneTest/DuplicateTest.py
@@ -106,7 +106,8 @@ def testInvalidTarget( self ) :
 		d["in"].setInput( s["out"] )
 		d["target"].setValue( "/cube" )
 
-		self.assertRaises( RuntimeError, d["out"].childNames, "/" )
+		self.assertSceneValid( d["out"] )
+		self.assertScenesEqual( d["out"], d["in"] )
 
 	def testNamePlug( self ) :
 

diff --git a/src/GafferScene/CollectScenes.cpp b/src/GafferScene/CollectScenes.cpp
@@ -36,7 +36,8 @@
 
 #include "GafferScene/CollectScenes.h"
 
-#include "Gaffer/ArrayPlug.h"
+#include "GafferScene/SceneAlgo.h"
+
 #include "Gaffer/Context.h"
 #include "Gaffer/StringPlug.h"
 
@@ -290,6 +291,15 @@ void CollectScenes::hashChildNames( const ScenePath &path, const Gaffer::Context
 	else
 	{
 		SceneScope sceneScope( context, rootNameVariablePlug()->getValue(), path, sourceRootPlug() );
+		if( path.size() == 1 )
+		{
+			const auto &upstreamPath = Context::current()->get<ScenePlug::ScenePath>( ScenePlug::scenePathContextName );
+			if( !SceneAlgo::exists( inPlug(), upstreamPath ) )
+			{
+				h = inPlug()->childNamesPlug()->defaultValue()->Object::hash();
+				return;
+			}
+		}
 		h = inPlug()->childNamesPlug()->hash();
 	}
 }
@@ -322,6 +332,14 @@ IECore::ConstInternedStringVectorDataPtr CollectScenes::computeChildNames( const
 	else
 	{
 		SceneScope sceneScope( context, rootNameVariablePlug()->getValue(), path, sourceRootPlug() );
+		if( path.size() == 1 )
+		{
+			const auto &upstreamPath = Context::current()->get<ScenePlug::ScenePath>( ScenePlug::scenePathContextName );
+			if( !SceneAlgo::exists( inPlug(), upstreamPath ) )
+			{
+				return inPlug()->childNamesPlug()->defaultValue();
+			}
+		}
 		return inPlug()->childNamesPlug()->getValue();
 	}
 }

diff --git a/src/GafferScene/Duplicate.cpp b/src/GafferScene/Duplicate.cpp
@@ -36,6 +36,8 @@
 
 #include "GafferScene/Duplicate.h"
 
+#include "GafferScene/SceneAlgo.h"
+
 #include "Gaffer/StringPlug.h"
 
 #include "IECore/StringAlgo.h"
@@ -172,7 +174,14 @@ void Duplicate::hash( const ValuePlug *output, const Context *context, IECore::M
 	}
 	else if( output == childNamesPlug() )
 	{
-		targetPlug()->hash( h );
+		ScenePath target;
+		ScenePlug::stringToPath( targetPlug()->getValue(), target );
+		if( !SceneAlgo::exists( inPlug(), target ) )
+		{
+			h = childNamesPlug()->defaultValue()->Object::hash();
+			return;
+		}
+		h.append( target.data(), target.size() );
 		copiesPlug()->hash( h );
 		namePlug()->hash( h );
 	}
@@ -198,19 +207,13 @@ void Duplicate::compute( ValuePlug *output, const Context *context ) const
 	}
 	else if( output == childNamesPlug() )
 	{
-		// get the path to our target.
+		// Get the path to our target, and check it exists.
 		ScenePath target;
 		ScenePlug::stringToPath( targetPlug()->getValue(), target );
-
-		// throw if the target path doesn't exist in the input. we need to compute the input child names at the
-		// parent for this, but it's not necessary to represent that in the hash, because it doesn't actually
-		// affect our result (if we throw we will have no result).
-		ScenePath parent( target ); parent.pop_back();
-		ConstInternedStringVectorDataPtr parentChildNamesData = inPlug()->childNames( parent );
-		vector<InternedString> parentChildNames = parentChildNamesData->readable();
-		if( find( parentChildNames.begin(), parentChildNames.end(), target.back() ) == parentChildNames.end() )
+		if( !SceneAlgo::exists( inPlug(), target ) )
 		{
-			throw Exception( boost::str( boost::format( "Target \"%s\" does not exist" ) % target.back().string() ) );
+			output->setToDefault();
+			return;
 		}
 
 		// go ahead and generate our childnames.