Merge pull request #5912 from johnhaddon/loopImprovements

Loop improvements

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
 ---

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 :

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()

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<int>( 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<int>( 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 );

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<Loop>()
 		.def( "setup", &setupLoop )
+		.def( "nextIterationContext", &nextIterationContextWrapper )
 	;
 
 	DependencyNodeClass<ContextProcessor>()