Merge pull request #6069 from johnhaddon/pythonCommandBatching

PythonCommand : Add `framesMode` plug

Changes.md

@@ -21,6 +21,7 @@ Improvements
 - NodeEditor : Improved performance when showing a node with many colour plugs. Showing the Arnold `standard_surface` shader is now almost 2x faster. [^1]
 - GraphEditor : Added colour coding to the strike-throughs drawn for disabled nodes. Black indicates that the node is always disabled, and yellow indicates that its `enabled` plug has an input connection, and therefore might be context-sensitive.
 - ListContainer : Adding a child widget with non-default alignment no longer causes the container to take up all available space.
+- PythonCommand : Added a `framesMode` plug which determines if the command is called once for each frame, once for each batch of frames, or once for each complete sequence.
 
 Fixes
 -----
@@ -46,6 +47,7 @@ Breaking Changes
 - IECoreArnold : Added `messageContext` argument to `NodeAlgo::Converter` and `NodeAlgo::MotionConverter`.
 - Instancer : Renamed `encapsulateInstanceGroups` plug to `encapsulate`. Encapsulation now produces a single capsule at the `.../instances` location, instead of capsules at each `.../instances/<prototypeName>` location.
 - GraphGadget : Moved <kbd>D</kbd> shortcut handling to GraphEditor.
+- PythonCommand : Removed `sequence` plug. Settings from old files are remapped automatically to the new `framesMode` plug on loading.
 
 [^1]: To be omitted from 1.5.0.0 release notes.
 

python/GafferDispatch/PythonCommand.py

@@ -35,6 +35,7 @@
 ##########################################################################
 
 import ast
+import enum
 
 import IECore
 import imath
@@ -44,6 +45,8 @@
 
 class PythonCommand( GafferDispatch.TaskNode ) :
 
+	FramesMode = enum.IntEnum( "FramesMode", [ "Single", "Sequence", "Batch" ], start = 0 )
+
 	def __init__( self, name = "PythonCommand" ) :
 
 		GafferDispatch.TaskNode.__init__( self, name )
@@ -53,7 +56,7 @@ def __init__( self, name = "PythonCommand" ) :
 		# directly anyway.
 		self["command"] = Gaffer.StringPlug( substitutions = IECore.StringAlgo.Substitutions.NoSubstitutions )
 		self["variables"] = Gaffer.CompoundDataPlug()
-		self["sequence"] = Gaffer.BoolPlug()
+		self["framesMode"] = Gaffer.IntPlug( minValue = int( self.FramesMode.Single ), maxValue = int( self.FramesMode.Batch ) )
 
 	def hash( self, context ) :
 
@@ -78,7 +81,7 @@ def hash( self, context ) :
 
 		self["variables"].hash( h )
 
-		if self.requiresSequenceExecution() :
+		if self["framesMode"].getValue() != self.FramesMode.Single :
 			h.append( context.getFrame() )
 
 		return h
@@ -91,12 +94,16 @@ def execute( self ) :
 
 	def executeSequence( self, frames ) :
 
-		if not self.requiresSequenceExecution() :
-			## \todo It'd be nice if the dispatcher didn't call
-			# executeSequence() if requiresSequenceExecution() was False.
-			# At the same time we could look into properly supporting
-			# varying results for requiresSequenceExecution(), with sequences
-			# going into their own batch independent of non-sequence batches.
+		sequence = False
+		with Gaffer.Context( Gaffer.Context.current() ) as frameContext :
+			for frame in frames :
+				frameContext.setFrame( frame )
+				if self["framesMode"].getValue() != self.FramesMode.Single :
+					sequence = True
+					break
+
+		if not sequence :
+			# Calls `execute()`.
 			GafferDispatch.TaskNode.executeSequence( self, frames )
 			return
 
@@ -106,7 +113,7 @@ def executeSequence( self, frames ) :
 
 	def requiresSequenceExecution( self ) :
 
-		return self["sequence"].getValue()
+		return self["framesMode"].getValue() == self.FramesMode.Sequence
 
 	# Protected rather than private to allow access by PythonCommandUI.
 	# Not for general use.

python/GafferDispatchTest/DispatcherTest.py

@@ -1663,7 +1663,7 @@ def testScaling( self ) :
 
 			perSequence = GafferDispatch.PythonCommand()
 			perSequence["command"].setValue( "pass" )
-			perSequence["sequence"].setValue( True )
+			perSequence["framesMode"].setValue( perSequence.FramesMode.Sequence )
 			perSequence["preTasks"][0].setInput( perFrame["task"] )
 			s["perSequence%d" % i] = perSequence
 

python/GafferDispatchTest/ExecuteApplicationTest.py

@@ -350,9 +350,9 @@ def testCanSerialiseFrameDependentPlugs( self ) :
 			""" )
 		)
 
-		def validate( sequence ) :
+		def validate( framesMode ) :
 
-			s["PythonCommand"]["sequence"].setValue( sequence )
+			s["PythonCommand"]["framesMode"].setValue( framesMode )
 
 			s["fileName"].setValue( self.__scriptFileName )
 			s.context().setFrame( 10 )
@@ -369,8 +369,8 @@ def validate( sequence ) :
 			# we must retain the non-substituted value
 			self.assertEqual( ss["t"]["fileName"].getValue(), "{}/test.####.txt".format( self.temporaryDirectory().as_posix() ) )
 
-		validate( sequence = True )
-		validate( sequence = False )
+		validate( framesMode = GafferDispatch.PythonCommand.FramesMode.Sequence )
+		validate( framesMode = GafferDispatch.PythonCommand.FramesMode.Single )
 
 if __name__ == "__main__":
 	unittest.main()

python/GafferDispatchTest/LocalDispatcherTest.py

@@ -856,7 +856,7 @@ def testScaling( self ) :
 
 			perSequence = GafferDispatch.PythonCommand()
 			perSequence["command"].setValue( "pass" )
-			perSequence["sequence"].setValue( True )
+			perSequence["framesMode"].setValue( perSequence.FramesMode.Sequence )
 			perSequence["preTasks"][0].setInput( perFrame["task"] )
 			s["perSequence%d" % i] = perSequence
 

python/GafferDispatchTest/PythonCommandTest.py

@@ -216,9 +216,12 @@ def testRequiresSequenceExecution( self ) :
 		n = GafferDispatch.PythonCommand()
 		self.assertFalse( n.requiresSequenceExecution() )
 
-		n["sequence"].setValue( True )
+		n["framesMode"].setValue( n.FramesMode.Sequence )
 		self.assertTrue( n.requiresSequenceExecution() )
 
+		n["framesMode"].setValue( n.FramesMode.Batch )
+		self.assertFalse( n.requiresSequenceExecution() )
+
 	def testFramesNotAvailableInNonSequenceMode( self ) :
 
 		s = Gaffer.ScriptNode()
@@ -236,7 +239,7 @@ def testSequenceMode( self ) :
 
 		s = Gaffer.ScriptNode()
 		s["n"] = GafferDispatch.PythonCommand()
-		s["n"]["sequence"].setValue( True )
+		s["n"]["framesMode"].setValue( GafferDispatch.PythonCommand.FramesMode.Sequence )
 
 		s["n"]["command"].setValue( inspect.cleandoc(
 			"""
@@ -260,7 +263,7 @@ def testSequenceModeVariable( self ) :
 		s = Gaffer.ScriptNode()
 
 		s["n"] = GafferDispatch.PythonCommand()
-		s["n"]["sequence"].setValue( True )
+		s["n"]["framesMode"].setValue( GafferDispatch.PythonCommand.FramesMode.Sequence )
 		s["n"]["variables"].addChild( Gaffer.NameValuePlug( "testInt", 42 ) )
 
 		s["e"] = Gaffer.Expression()
@@ -304,7 +307,7 @@ def testSequenceModeStaticVariable( self ) :
 		s = Gaffer.ScriptNode()
 
 		s["n"] = GafferDispatch.PythonCommand()
-		s["n"]["sequence"].setValue( True )
+		s["n"]["framesMode"].setValue( GafferDispatch.PythonCommand.FramesMode.Sequence )
 		s["n"]["variables"].addChild( Gaffer.NameValuePlug( "testInt", 42 ) )
 
 		commandLines = inspect.cleandoc(
@@ -326,6 +329,31 @@ def testSequenceModeStaticVariable( self ) :
 		self.assertEqual( s["n"].frames, [ 1, 2, 3, 4, 5 ] )
 		self.assertEqual( s["n"].numCalls, 1 )
 
+	def testBatchMode( self ) :
+
+		s = Gaffer.ScriptNode()
+
+		s["n"] = GafferDispatch.PythonCommand()
+		s["n"]["framesMode"].setValue( GafferDispatch.PythonCommand.FramesMode.Batch )
+		s["n"]["variables"].addChild( Gaffer.NameValuePlug( "testInt", 42 ) )
+		s["n"]["dispatcher"]["batchSize"].setValue( 5 )
+		s["n"].calls = []
+
+		s["n"]["command"].setValue( "self.calls.append( frames )" )
+
+		s["d"] = self.__dispatcher( frameRange = "1-20" )
+		s["d"]["tasks"][0].setInput( s["n"]["task"] )
+		s["d"]["task"].execute()
+		self.assertEqual(
+			s["n"].calls,
+			[
+				[ 1, 2, 3, 4, 5 ],
+				[ 6, 7, 8, 9, 10 ],
+				[ 11, 12, 13, 14, 15 ],
+				[ 16, 17, 18, 19, 20 ]
+			]
+		)
+
 	def testCannotAccessVariablesOutsideFrameRange( self ) :
 
 		# We don't want to allow access to variables outside the frame range,

python/GafferDispatchUI/PythonCommandUI.py

@@ -79,12 +79,19 @@
 
 		),
 
-		"sequence" : (
+		"framesMode" : (
 
 			"description",
 			"""
-			Calls the command once for each sequence, instead of once
-			per frame. In this mode, an additional variable called `frames`
+			Determines how tasks for different frames are distributed
+			between calls to the command :
+
+			- Single : The command will be called for a single frame at a time.
+			- Batch : The command will be called once for each batch of frames
+			  defined by `dispatcher.batchSize`.
+			- Sequence : The command will be called once for all frames.
+
+			In Batch and Sequences modes, an additional variable called `frames`
 			is available to the command, containing a list of all frame
 			numbers for which execution should be performed. The Context may
 			be updated to reference any frame from this list, and accessing
@@ -105,9 +112,17 @@
 			# Do some one-time finalization
 			...
 			```
+
+			> Note : In Single mode, the command will only be called for each
+			> frame if the inputs are animated. If the inputs are static
+			> then the command will only be called once.
 			""",
 
 			"layout:section", "Advanced",
+			"plugValueWidget:type", "GafferUI.PresetsPlugValueWidget",
+			"preset:Single", GafferDispatch.PythonCommand.FramesMode.Single,
+			"preset:Batch", GafferDispatch.PythonCommand.FramesMode.Batch,
+			"preset:Sequence", GafferDispatch.PythonCommand.FramesMode.Sequence,
 
 		),
 

startup/GafferDispatch/pythonCommandCompatibility.py

@@ -0,0 +1,50 @@
+##########################################################################
+#
+#  Copyright (c) 2024, Cinesite VFX Ltd. All rights reserved.
+#
+#  Redistribution and use in source and binary forms, with or without
+#  modification, are permitted provided that the following conditions are
+#  met:
+#
+#      * Redistributions of source code must retain the above
+#        copyright notice, this list of conditions and the following
+#        disclaimer.
+#
+#      * Redistributions in binary form must reproduce the above
+#        copyright notice, this list of conditions and the following
+#        disclaimer in the documentation and/or other materials provided with
+#        the distribution.
+#
+#      * Neither the name of John Haddon nor the names of
+#        any other contributors to this software may be used to endorse or
+#        promote products derived from this software without specific prior
+#        written permission.
+#
+#  THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS
+#  IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+#  THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+#  PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+#  CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+#  EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+#  PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+#  PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+#  LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+#  NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+#  SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+#
+##########################################################################
+
+import GafferDispatch
+
+def __getItemWrapper( originalGetItem ):
+
+	def getItem( self, key ):
+
+		if key == "sequence":
+			key = "framesMode"
+
+		return originalGetItem( self, key )
+
+	return getItem
+
+GafferDispatch.PythonCommand.__getitem__ = __getItemWrapper( GafferDispatch.PythonCommand.__getitem__ )