diff --git a/Changes.md b/Changes.md index 993271fad06..31ba89c09f2 100644 --- a/Changes.md +++ b/Changes.md @@ -15,6 +15,12 @@ Fixes - Cycles : Fixed rendering to the Catalogue using the batch Render node (#5905). Note that rendering a mixture of Catalogue and file outputs is still not supported, and in this case any file outputs will be ignored. - CodeWidget : Fixed bug that could prevent changes from being committed while the completion menu was visible. +- Loop : Fixed handling of empty `indexVariable`. This now disables the Loop instead of creating an unnamed context variable. + +API +--- + +- Loop : Added `nextIterationContext()` method. API --- diff --git a/include/Gaffer/Loop.h b/include/Gaffer/Loop.h index 4a88296f16c..f0990dc296b 100644 --- a/include/Gaffer/Loop.h +++ b/include/Gaffer/Loop.h @@ -81,6 +81,10 @@ class GAFFER_API Loop : public ComputeNode Gaffer::Plug *correspondingInput( const Gaffer::Plug *output ) override; const Gaffer::Plug *correspondingInput( const Gaffer::Plug *output ) const override; + /// Returns the context that will be used to evaluate `nextPlug()` in + /// the next iteration of the loop (relative to the current context). + ContextPtr nextIterationContext() const; + void affects( const Plug *input, DependencyNode::AffectedPlugsContainer &outputs ) const override; protected : diff --git a/python/GafferTest/LoopTest.py b/python/GafferTest/LoopTest.py index 0cf4d975f19..d9ba6877d4b 100644 --- a/python/GafferTest/LoopTest.py +++ b/python/GafferTest/LoopTest.py @@ -307,5 +307,53 @@ def testHashAliasingDeadlock( self ) : # if not handled appropriately. script["loop"]["out"].getValue() + def testEmptyLoopVariable( self ) : + + loop = self.intLoop() + + loopBody = GafferTest.AddNode() + loopBody["op1"].setInput( loop["previous"] ) + loopBody["op2"].setValue( 2 ) + loop["next"].setInput( loopBody["sum"] ) + loop["iterations"].setValue( 4 ) + + self.assertEqual( loop["out"].getValue(), 8 ) + + loop["indexVariable"].setValue( "" ) + self.assertEqual( loop["out"].getValue(), 0 ) + + def testNextLoopIteration( self ) : + + loop = Gaffer.Loop() + self.assertIsNone( loop.nextIterationContext() ) + + loop = self.intLoop() + loop["next"].setInput( loop["previous"] ) + loop["iterations"].setValue( 10 ) + + index = 0 + context = Gaffer.Context() + while context is not None : + with context : + context = loop.nextIterationContext() + if context is not None : + self.assertEqual( context["loop:index"], index ) + index += 1 + + self.assertEqual( index, loop["iterations"].getValue() ) + + loop["iterations"].setValue( 0 ) + self.assertIsNone( loop.nextIterationContext() ) + + loop["iterations"].setValue( 2 ) + self.assertIsNotNone( loop.nextIterationContext() ) + loop["enabled"].setValue( False ) + self.assertIsNone( loop.nextIterationContext() ) + + loop["enabled"].setValue( True ) + self.assertIsNotNone( loop.nextIterationContext() ) + loop["indexVariable"].setValue( "" ) + self.assertIsNone( loop.nextIterationContext() ) + if __name__ == "__main__": unittest.main() diff --git a/src/Gaffer/Loop.cpp b/src/Gaffer/Loop.cpp index 7c43b2dcd8e..beca06e7144 100644 --- a/src/Gaffer/Loop.cpp +++ b/src/Gaffer/Loop.cpp @@ -160,6 +160,39 @@ const Gaffer::Plug *Loop::correspondingInput( const Gaffer::Plug *output ) const return output == outPlug() ? inPlug() : nullptr; } +ContextPtr Loop::nextIterationContext() const +{ + const Plug *in = inPlug(); + if( !in ) + { + return nullptr; + } + + const Context *context = Context::current(); + ContextAlgo::GlobalScope globalScope( context, in ); + + if( !enabledPlug()->getValue() ) + { + return nullptr; + } + + const IECore::InternedString indexVariable = indexVariablePlug()->getValue(); + if( indexVariable.string().empty() ) + { + return nullptr; + } + + const int index = context->get( indexVariable, -1 ); + if( index + 1 < iterationsPlug()->getValue() ) + { + ContextPtr result = new Context( *context ); + result->set( indexVariable, index + 1 ); + return result; + } + + return nullptr; +} + void Loop::affects( const Plug *input, DependencyNode::AffectedPlugsContainer &outputs ) const { ComputeNode::affects( input, outputs ); @@ -354,7 +387,7 @@ const ValuePlug *Loop::sourcePlug( const ValuePlug *output, const Context *conte if( ancestor == previousPlug() ) { const int index = context->get( indexVariable, 0 ); - if( index >= 1 && enabledPlug()->getValue() ) + if( index >= 1 && !indexVariable.string().empty() && enabledPlug()->getValue() ) { sourceLoopIndex = index - 1; return descendantPlug( nextPlug(), relativeName ); @@ -367,7 +400,7 @@ const ValuePlug *Loop::sourcePlug( const ValuePlug *output, const Context *conte else if( ancestor == outPlug() ) { const int iterations = iterationsPlug()->getValue(); - if( iterations > 0 && enabledPlug()->getValue() ) + if( iterations > 0 && !indexVariable.string().empty() && enabledPlug()->getValue() ) { sourceLoopIndex = iterations - 1; return descendantPlug( nextPlug(), relativeName ); diff --git a/src/GafferModule/ContextProcessorBinding.cpp b/src/GafferModule/ContextProcessorBinding.cpp index 77d1353feb8..a83b3b1ab19 100644 --- a/src/GafferModule/ContextProcessorBinding.cpp +++ b/src/GafferModule/ContextProcessorBinding.cpp @@ -70,6 +70,12 @@ void setupLoop( Loop &n, const ValuePlug &plug ) n.setup( &plug ); } +ContextPtr nextIterationContextWrapper( Loop &loop ) +{ + IECorePython::ScopedGILRelease gilRelease; + return loop.nextIterationContext(); +} + ContextPtr inPlugContext( const ContextProcessor &n ) { IECorePython::ScopedGILRelease gilRelease; @@ -189,6 +195,7 @@ void GafferModule::bindContextProcessor() DependencyNodeClass() .def( "setup", &setupLoop ) + .def( "nextIterationContext", &nextIterationContextWrapper ) ; DependencyNodeClass()