Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CollectScenes/Duplicate : Don't error when given invalid locations #3055

Merged
merged 2 commits into from
Feb 28, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 24 additions & 0 deletions python/GafferSceneTest/CollectScenesTest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
3 changes: 2 additions & 1 deletion python/GafferSceneTest/DuplicateTest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 ) :

Expand Down
20 changes: 19 additions & 1 deletion src/GafferScene/CollectScenes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@

#include "GafferScene/CollectScenes.h"

#include "Gaffer/ArrayPlug.h"
#include "GafferScene/SceneAlgo.h"

#include "Gaffer/Context.h"
#include "Gaffer/StringPlug.h"

Expand Down Expand Up @@ -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();
}
}
Expand Down Expand Up @@ -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();
}
}
Expand Down
25 changes: 14 additions & 11 deletions src/GafferScene/Duplicate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@

#include "GafferScene/Duplicate.h"

#include "GafferScene/SceneAlgo.h"

#include "Gaffer/StringPlug.h"

#include "IECore/StringAlgo.h"
Expand Down Expand Up @@ -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 );
}
Expand All @@ -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.
Expand Down