From bd4ff955291d2fbf149976f21ea4220fa519da8f Mon Sep 17 00:00:00 2001 From: John Haddon Date: Fri, 4 Oct 2024 12:14:54 +0100 Subject: [PATCH 1/2] PythonCommand : Add `framesMode` plug This replaces the previous `sequence` plug, adding a new `Batch` option which allows the command to be called once for each batch, as determined by the `dispatcher.batchSize` plug. Backwards compatibility with the old `sequence` plug is provided by mapping the old name to the new one, and ensuring the new enum values correspond to the old True/False values. --- Changes.md | 2 + python/GafferDispatch/PythonCommand.py | 25 ++++++---- .../GafferDispatchTest/PythonCommandTest.py | 25 ++++++++++ python/GafferDispatchUI/PythonCommandUI.py | 21 ++++++-- .../pythonCommandCompatibility.py | 50 +++++++++++++++++++ 5 files changed, 111 insertions(+), 12 deletions(-) create mode 100644 startup/GafferDispatch/pythonCommandCompatibility.py diff --git a/Changes.md b/Changes.md index 3ae3ce13101..5e6eb8184c2 100644 --- a/Changes.md +++ b/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/` location. - GraphGadget : Moved D 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. diff --git a/python/GafferDispatch/PythonCommand.py b/python/GafferDispatch/PythonCommand.py index 4e7c2ddb94b..dc33b49f658 100644 --- a/python/GafferDispatch/PythonCommand.py +++ b/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. diff --git a/python/GafferDispatchTest/PythonCommandTest.py b/python/GafferDispatchTest/PythonCommandTest.py index 2db7496d19e..173c02851fa 100644 --- a/python/GafferDispatchTest/PythonCommandTest.py +++ b/python/GafferDispatchTest/PythonCommandTest.py @@ -326,6 +326,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, diff --git a/python/GafferDispatchUI/PythonCommandUI.py b/python/GafferDispatchUI/PythonCommandUI.py index a444c2e4c45..7c267e15667 100644 --- a/python/GafferDispatchUI/PythonCommandUI.py +++ b/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, ), diff --git a/startup/GafferDispatch/pythonCommandCompatibility.py b/startup/GafferDispatch/pythonCommandCompatibility.py new file mode 100644 index 00000000000..f6ec25742f9 --- /dev/null +++ b/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__ ) From af68f5d28e2518cfbf8c6161c70f8c7f124d016c Mon Sep 17 00:00:00 2001 From: John Haddon Date: Fri, 4 Oct 2024 12:42:26 +0100 Subject: [PATCH 2/2] GafferDispatchTest : Adapt to `PythonCommand.framesMode` plug These tests already passed using the compatibility config, but we also want to update them to show people the new way. --- python/GafferDispatchTest/DispatcherTest.py | 2 +- python/GafferDispatchTest/ExecuteApplicationTest.py | 8 ++++---- python/GafferDispatchTest/LocalDispatcherTest.py | 2 +- python/GafferDispatchTest/PythonCommandTest.py | 11 +++++++---- 4 files changed, 13 insertions(+), 10 deletions(-) diff --git a/python/GafferDispatchTest/DispatcherTest.py b/python/GafferDispatchTest/DispatcherTest.py index dc17a9d1df7..8fb009e052a 100644 --- a/python/GafferDispatchTest/DispatcherTest.py +++ b/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 diff --git a/python/GafferDispatchTest/ExecuteApplicationTest.py b/python/GafferDispatchTest/ExecuteApplicationTest.py index db8b30d212d..897abce39a6 100644 --- a/python/GafferDispatchTest/ExecuteApplicationTest.py +++ b/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() diff --git a/python/GafferDispatchTest/LocalDispatcherTest.py b/python/GafferDispatchTest/LocalDispatcherTest.py index 38b85b70ed6..d22670bc42f 100644 --- a/python/GafferDispatchTest/LocalDispatcherTest.py +++ b/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 diff --git a/python/GafferDispatchTest/PythonCommandTest.py b/python/GafferDispatchTest/PythonCommandTest.py index 173c02851fa..4496b0792ee 100644 --- a/python/GafferDispatchTest/PythonCommandTest.py +++ b/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(