Merge pull request #5590 from johnhaddon/shuffle

Shuffle improvements

Changes.md

@@ -15,6 +15,16 @@ Improvements
 - Cache : Increased default computation cache size to 8Gb. Call `Gaffer.ValuePlug.setCacheMemoryLimit()` from a startup file to override this.
 - Dispatcher : Reduced internal overhead of `dispatch()` call, with one benchmark showing around a 3x speedup.
 - ScriptWindow : Added "Save" option to dialogue shown when closing a window containing unsaved changes.
+- Shuffle :
+  - Reimplemented to match ShuffleAttributes and ShufflePrimitiveVariables.
+    - Any number of shuffles can be added using the UI.
+    - Wildcards can be used to match multiple source channels, and expressions can be used to map them to destination channels.
+    - Source channels can optionally be deleted after shuffling.
+    - Overwriting of destination channels can optionally be avoided.
+  - Added `missingSourceMode` plug to determine behaviour when a source channel doesn't exist.
+- NodeEditor : Improved image channel selectors :
+  - Added "Custom" option, to allow strings to be entered manually.
+  - Added right-click context menu.
 
 Fixes
 -----
@@ -49,6 +59,10 @@ API
 - GafferTractorTest : Added `tractorAPI()` method which returns a mock API if Tractor is not available. This allows the GafferTractor module to be tested without Tractor being installed.
 - ParallelAlgo : Added `canCallOnUIThread()` function.
 - Label : Added `textSelectable` constructor argument.
+- ShufflesPlug :
+  - Added `ignoreMissingSource` argument to `shuffle()`.
+  - Added `shuffleWithExtraSources()` method.
+- ShufflePlugValueWidget : Widgets for the `source` and `destination` plugs can now be customised using standard `plugValueWidget:type` metadata.
 
 Breaking Changes
 ----------------
@@ -78,6 +92,10 @@ Breaking Changes
 - OpenColorIOContext : Removed `configEnabledPlug()`, `configValuePlug()`, `workingSpaceEnabledPlug()` and `workingSpaceValuePlug()` methods. Use the OptionalValuePlug child accessors instead.
 - Windows launch script : Removed the hardcoded `/debugexe` switch used when `GAFFER_DEBUG` is enabled, making it possible to use debuggers other than Visual Studio. Debug switches can be added to the `GAFFER_DEBUGGER` environment variable instead.
 - Enums : Replaced `IECore.Enum` types with standard Python types from the `enum` module.
+- Shuffle :
+  - Removed ChannelPlug type. Use `Gaffer.ShufflePlug` instead.
+  - Renamed `channels` plug to `shuffles` plug, matching nodes such as ShuffleAttributes and ShufflePrimitiveVariables.
+- ShuffleUI : Removed `nodeMenuCreateCommand()`.
 
 Build
 -----

include/Gaffer/ShufflePlug.h

@@ -96,8 +96,23 @@ class GAFFER_API ShufflesPlug : public ValuePlug
 
 		/// Shuffles the sources into a destination container. The container type should have a map
 		/// compatible interface with string-compatible keys (eg std::string, IECore::InternedString).
+		/// If `ignoreMissingSource` is false, then an exception will be thrown if a source is not
+		/// found.
 		template<typename T>
-		T shuffle( const T &sourceContainer ) const;
+		T shuffle( const T &sourceContainer, bool ignoreMissingSource = true ) const;
+		/// As above, but using `extraSources` to provide fallback values for sources not
+		/// found in `sourceContainer`. A special key, `*`, may be included to provide a fallback
+		/// for _any_ source.
+		/// > Note : The `additionalSources` container is only searched for exact matches, _not_
+		/// > for wildcard matches.
+		template<typename T>
+		T shuffleWithExtraSources( const T &sourceContainer, const T &extraSources, bool ignoreMissingSource = true ) const;
+
+	private :
+
+		template<typename T>
+		T shuffleInternal( const T &sourceContainer, const T *extraSources, bool ignoreMissingSource ) const;
+
 };
 
 } // namespace Gaffer

include/Gaffer/ShufflePlug.inl

@@ -59,7 +59,19 @@ namespace Gaffer
 {
 
 template<typename T>
-T ShufflesPlug::shuffle( const T &sourceContainer ) const
+T ShufflesPlug::shuffle( const T &sourceContainer, bool ignoreMissingSource ) const
+{
+	return shuffleInternal( sourceContainer, static_cast<const T *>( nullptr ), ignoreMissingSource );
+}
+
+template<typename T>
+T ShufflesPlug::shuffleWithExtraSources( const T &sourceContainer, const T &extraSources, bool ignoreMissingSource ) const
+{
+	return shuffleInternal( sourceContainer, &extraSources, ignoreMissingSource );
+}
+
+template<typename T>
+T ShufflesPlug::shuffleInternal( const T &sourceContainer, const T *extraSources, bool ignoreMissingSource ) const
 {
 	using NameContainer = std::unordered_set< typename T::key_type >;
 
@@ -103,8 +115,26 @@ T ShufflesPlug::shuffle( const T &sourceContainer ) const
 			// NOTE : No wildcards in source so shuffle is one move.
 
 			const std::string &srcName = srcPattern;
-			const typename T::const_iterator sIt = sourceContainer.find( srcName );
+			const typename T::mapped_type *srcValue = nullptr;
+			typename T::const_iterator sIt = sourceContainer.find( srcName );
 			if( sIt != sourceContainer.end() )
+			{
+				srcValue = &sIt->second;
+			}
+			else if( extraSources )
+			{
+				sIt = extraSources->find( srcName );
+				if( sIt == extraSources->end() )
+				{
+					sIt = extraSources->find( "*" );
+				}
+				if( sIt != extraSources->end() )
+				{
+					srcValue = &sIt->second;
+				}
+			}
+
+			if( srcValue )
 			{
 				Gaffer::Context::EditableScope scope( Gaffer::Context::current() );
 				scope.set<std::string>( g_sourceVariable, &srcName );
@@ -116,7 +146,7 @@ T ShufflesPlug::shuffle( const T &sourceContainer ) const
 					{
 						if( dstReplace || ( destinationContainer.find( dstName ) == destinationContainer.end() ) )
 						{
-							destinationContainer[ dstName ] = sIt->second;
+							destinationContainer[ dstName ] = *srcValue;
 							names.insert( dstName );
 						}
 
@@ -126,6 +156,9 @@ T ShufflesPlug::shuffle( const T &sourceContainer ) const
 						}
 					}
 				}
+			} else if( !ignoreMissingSource )
+			{
+				throw IECore::Exception( fmt::format( "Source \"{}\" does not exist", srcName ) );
 			}
 		}
 		else

include/GafferImage/Shuffle.h

@@ -38,12 +38,11 @@
 
 #include "GafferImage/ImageProcessor.h"
 
-#include "Gaffer/StringPlug.h"
+#include "Gaffer/ShufflePlug.h"
 
 namespace GafferImage
 {
 
-/// \todo: Refactor using Gaffer::ShufflesPlug
 class GAFFERIMAGE_API Shuffle : public ImageProcessor
 {
 
@@ -54,46 +53,26 @@ class GAFFERIMAGE_API Shuffle : public ImageProcessor
 
 		GAFFER_NODE_DECLARE_TYPE( GafferImage::Shuffle, ShuffleTypeId, ImageProcessor );
 
-		/// A custom plug to hold the name of an output channel and the
-		/// name of an input channel to shuffle into it. Add instances
-		/// of these to the Shuffle::channelsPlug() to define the shuffle.
-		class GAFFERIMAGE_API ChannelPlug : public Gaffer::ValuePlug
+		enum class MissingSourceMode
 		{
-
-			public :
-
-				GAFFER_PLUG_DECLARE_TYPE( GafferImage::Shuffle::ChannelPlug, ShuffleChannelPlugTypeId, Gaffer::ValuePlug );
-
-				// Standard constructor. This is needed for serialisation.
-				ChannelPlug(
-					const std::string &name = defaultName<ChannelPlug>(),
-					Direction direction=In,
-					unsigned flags = Default
-				);
-				// Convenience constructor defining a shuffle of the specified
-				// in channel to the specified out channel.
-				ChannelPlug( const std::string &out, const std::string &in );
-
-				Gaffer::StringPlug *outPlug();
-				const Gaffer::StringPlug *outPlug() const;
-
-				Gaffer::StringPlug *inPlug();
-				const Gaffer::StringPlug *inPlug() const;
-
-				bool acceptsChild( const Gaffer::GraphComponent *potentialChild ) const override;
-				Gaffer::PlugPtr createCounterpart( const std::string &name, Direction direction ) const override;
-
+			Ignore,
+			Error,
+			Black
 		};
 
-		IE_CORE_DECLAREPTR( ChannelPlug )
+		Gaffer::IntPlug *missingSourceModePlug();
+		const Gaffer::IntPlug *missingSourceModePlug() const;
 
-		Gaffer::ValuePlug *channelsPlug();
-		const Gaffer::ValuePlug *channelsPlug() const;
+		Gaffer::ShufflesPlug *shufflesPlug();
+		const Gaffer::ShufflesPlug *shufflesPlug() const;
 
 		void affects( const Gaffer::Plug *input, AffectedPlugsContainer &outputs ) const override;
 
 	protected :
 
+		void hash( const Gaffer::ValuePlug *output, const Gaffer::Context *context, IECore::MurmurHash &h ) const override;
+		void compute( Gaffer::ValuePlug *output, const Gaffer::Context *context ) const override;
+
 		void hashChannelNames( const GafferImage::ImagePlug *parent, const Gaffer::Context *context, IECore::MurmurHash &h ) const override;
 		void hashChannelData( const GafferImage::ImagePlug *parent, const Gaffer::Context *context, IECore::MurmurHash &h ) const override;
 
@@ -102,6 +81,9 @@ class GAFFERIMAGE_API Shuffle : public ImageProcessor
 
 	private :
 
+		Gaffer::ObjectPlug *mappingPlug();
+		const Gaffer::ObjectPlug *mappingPlug() const;
+
 		std::string inChannelName( const std::string &outChannelName ) const;
 
 		static size_t g_firstPlugIndex;

include/GafferImage/TypeIds.h

@@ -58,7 +58,7 @@ enum TypeId
 	GradeTypeId = 110763,
 	ShuffleTypeId = 110764,
 	ConstantTypeId = 110765,
-	ShuffleChannelPlugTypeId = 110766,
+	ShuffleChannelPlugTypeId = 110766, // Obsolete - available for reuse
 	ChannelMaskPlugTypeId = 110767,
 	WarpTypeId = 110768,
 	VectorWarpTypeId = 110769,

python/GafferArnoldTest/ArnoldTextureBakeTest.py

@@ -231,9 +231,7 @@ def testManyImages( self ):
 		shuffleRef["in"].setInput( resizeRef["out"] )
 		for layer in [ "beauty", "diffuse" ]:
 			for channel in [ "R", "G", "B" ]:
-				shuffleRef["channels"].addChild( GafferImage.Shuffle.ChannelPlug() )
-				shuffleRef["channels"][-1]["in"].setValue( channel )
-				shuffleRef["channels"][-1]["out"].setValue( layer + "." + channel )
+				shuffleRef["shuffles"].addChild( Gaffer.ShufflePlug( channel, f"{layer}.{channel}" ) )
 
 		differenceMerge = GafferImage.Merge()
 		differenceMerge["in"]["in0"].setInput( aovCollect["out"] )

python/GafferImage/BleedFill.py

@@ -181,7 +181,7 @@ def __init__(self, name = 'BleedFill' ) :
 		self["__unpremult"]["in"].setInput( self["__restoreDataSize"]["out"] )
 
 		self["__resetAlpha"] = GafferImage.Shuffle()
-		self["__resetAlpha"]["channels"].addChild( GafferImage.Shuffle.ChannelPlug( "A", "__white" ) )
+		self["__resetAlpha"]["shuffles"].addChild( Gaffer.ShufflePlug( "__white", "A" ) )
 		self["__resetAlpha"]["in"].setInput( self["__unpremult"]["out"] )
 
 		self["__disableSwitch"] = Gaffer.Switch()

python/GafferImageTest/AnaglyphTest.py

@@ -57,10 +57,10 @@ def test( self ) :
 
 		left = GafferImage.Shuffle()
 		left["in"].setInput( reader["out"] )
-		left["channels"].addChild( GafferImage.Shuffle.ChannelPlug( "R", "customRgba.R" ) )
-		left["channels"].addChild( GafferImage.Shuffle.ChannelPlug( "G", "customRgba.G" ) )
-		left["channels"].addChild( GafferImage.Shuffle.ChannelPlug( "B", "customRgba.B" ) )
-		left["channels"].addChild( GafferImage.Shuffle.ChannelPlug( "A", "customRgba.A" ) )
+		left["shuffles"].addChild( Gaffer.ShufflePlug( "customRgba.R", "R" ) )
+		left["shuffles"].addChild( Gaffer.ShufflePlug( "customRgba.G", "G" ) )
+		left["shuffles"].addChild( Gaffer.ShufflePlug( "customRgba.B", "B" ) )
+		left["shuffles"].addChild( Gaffer.ShufflePlug( "customRgba.A", "A" ) )
 
 		right = GafferImage.ImageTransform()
 		right["in"].setInput( left["out"] )

python/GafferImageTest/ColorSpaceTest.py

@@ -261,8 +261,8 @@ def testSingleChannelImage( self ) :
 
 		s = GafferImage.Shuffle()
 		s["in"].setInput( r["out"] )
-		s["channels"].addChild( s.ChannelPlug( "G", "R" ) )
-		s["channels"].addChild( s.ChannelPlug( "B", "R" ) )
+		s["shuffles"].addChild( Gaffer.ShufflePlug( "R", "G" ) )
+		s["shuffles"].addChild( Gaffer.ShufflePlug( "R", "B" ) )
 
 		# This test is primarily to check that the ColorSpace node doesn't pull
 		# on non-existent input channels, and can still transform a single-channel
@@ -289,10 +289,8 @@ def testUnpremultiplied( self ) :
 		i["fileName"].setValue( self.imagesPath() / "circles.exr" )
 
 		shuffleAlpha = GafferImage.Shuffle()
-		shuffleAlpha["channels"].addChild( GafferImage.Shuffle.ChannelPlug( "channel" ) )
+		shuffleAlpha["shuffles"].addChild( Gaffer.ShufflePlug( "R", "A" ) )
 		shuffleAlpha["in"].setInput( i["out"] )
-		shuffleAlpha["channels"]["channel"]["out"].setValue( 'A' )
-		shuffleAlpha["channels"]["channel"]["in"].setValue( 'R' )
 
 		gradeAlpha = GafferImage.Grade()
 		gradeAlpha["in"].setInput( shuffleAlpha["out"] )

python/GafferImageTest/DeepHoldoutTest.py

@@ -41,6 +41,7 @@
 
 import IECore
 
+import Gaffer
 import GafferTest
 import GafferImage
 import GafferImageTest
@@ -95,10 +96,10 @@ def testBasics( self ):
 		# For a more complex holdout, we can create a comparison manually using shuffles and a DeepMerge
 		preShuffle = GafferImage.Shuffle()
 		preShuffle["in"].setInput( representativeImage["out"] )
-		preShuffle["channels"].addChild( preShuffle.ChannelPlug( "holdoutR", "R" ) )
-		preShuffle["channels"].addChild( preShuffle.ChannelPlug( "holdoutG", "G" ) )
-		preShuffle["channels"].addChild( preShuffle.ChannelPlug( "holdoutB", "B" ) )
-		preShuffle["channels"].addChild( preShuffle.ChannelPlug( "holdoutA", "A" ) )
+		preShuffle["shuffles"].addChild( Gaffer.ShufflePlug( "R", "holdoutR" ) )
+		preShuffle["shuffles"].addChild( Gaffer.ShufflePlug( "G", "holdoutG" ) )
+		preShuffle["shuffles"].addChild( Gaffer.ShufflePlug( "B", "holdoutB" ) )
+		preShuffle["shuffles"].addChild( Gaffer.ShufflePlug( "A", "holdoutA" ) )
 
 		manualHoldoutMerge = GafferImage.DeepMerge()
 		manualHoldoutMerge["in"][0].setInput( preShuffle["out"] )
@@ -109,10 +110,10 @@ def testBasics( self ):
 
 		postShuffle = GafferImage.Shuffle()
 		postShuffle["in"].setInput( manualHoldoutFlatten["out"] )
-		postShuffle["channels"].addChild( postShuffle.ChannelPlug( "R", "holdoutR" ) )
-		postShuffle["channels"].addChild( postShuffle.ChannelPlug( "G", "holdoutG" ) )
-		postShuffle["channels"].addChild( postShuffle.ChannelPlug( "B", "holdoutB" ) )
-		postShuffle["channels"].addChild( postShuffle.ChannelPlug( "A", "holdoutA" ) )
+		postShuffle["shuffles"].addChild( Gaffer.ShufflePlug( "holdoutR", "R" ) )
+		postShuffle["shuffles"].addChild( Gaffer.ShufflePlug( "holdoutG", "G" ) )
+		postShuffle["shuffles"].addChild( Gaffer.ShufflePlug( "holdoutB", "B" ) )
+		postShuffle["shuffles"].addChild( Gaffer.ShufflePlug( "holdoutA", "A" ) )
 
 		channelCleanup = GafferImage.DeleteChannels()
 		channelCleanup["in"].setInput( postShuffle["out"] )
@@ -169,7 +170,7 @@ def testDirtyPropagation( self ):
 		self.assertIn( "out.channelData", dirtiedPlugs )
 		del cs[:]
 
-		aShuffle["channels"].addChild( bShuffle.ChannelPlug( "Z", "__white" ) )
+		aShuffle["shuffles"].addChild( Gaffer.ShufflePlug( "__white", "Z" ) )
 		dirtiedPlugs = { x[0].relativeName( holdout ) for x in cs }
 		self.assertIn( "__intermediateIn.channelData", dirtiedPlugs )
 		self.assertIn( "__flattened.channelData", dirtiedPlugs )
@@ -179,7 +180,7 @@ def testDirtyPropagation( self ):
 		self.assertIn( "out.channelNames", dirtiedPlugs )
 		del cs[:]
 
-		bShuffle["channels"].addChild( bShuffle.ChannelPlug( "Z", "__white" ) )
+		bShuffle["shuffles"].addChild( Gaffer.ShufflePlug( "__white", "Z" ) )
 		dirtiedPlugs = { x[0].relativeName( holdout ) for x in cs }
 		self.assertIn( "__flattened.channelData", dirtiedPlugs )
 		self.assertIn( "out.channelData", dirtiedPlugs )

python/GafferImageTest/DeepStateTest.py

@@ -960,7 +960,7 @@ def testMissingChannels( self ) :
 
 		referenceNoZBack = GafferImage.Shuffle()
 		referenceNoZBack["in"].setInput( deepMerge["out"] )
-		referenceNoZBack["channels"].addChild( referenceNoZBack.ChannelPlug( "ZBack", "Z" ) )
+		referenceNoZBack["shuffles"].addChild( Gaffer.ShufflePlug( "Z", "ZBack" ) )
 
 		referenceFlatten["in"].setInput( referenceNoZBack["out"] )
 

python/GafferImageTest/DilateTest.py

@@ -291,8 +291,8 @@ def testAgainstOIIO( self ):
 
 		driverShuffle = GafferImage.Shuffle()
 		driverShuffle["in"].setInput( imageReader["out"] )
-		driverShuffle["channels"].addChild( GafferImage.Shuffle.ChannelPlug( "driver", "R" ) )
-		driverShuffle["channels"].addChild( GafferImage.Shuffle.ChannelPlug( "driven", "R" ) )
+		driverShuffle["shuffles"].addChild( Gaffer.ShufflePlug( "R", "driver" ) )
+		driverShuffle["shuffles"].addChild( Gaffer.ShufflePlug( "R", "driven" ) )
 
 		driverDelete = GafferImage.DeleteChannels()
 		driverDelete["in"].setInput( driverShuffle["out"] )
@@ -306,8 +306,8 @@ def testAgainstOIIO( self ):
 
 		refDriverShuffle = GafferImage.Shuffle()
 		refDriverShuffle["in"].setInput( refReader["out"] )
-		refDriverShuffle["channels"].addChild( GafferImage.Shuffle.ChannelPlug( "driver", "R" ) )
-		refDriverShuffle["channels"].addChild( GafferImage.Shuffle.ChannelPlug( "driven", "R" ) )
+		refDriverShuffle["shuffles"].addChild( Gaffer.ShufflePlug( "R", "driver" ) )
+		refDriverShuffle["shuffles"].addChild( Gaffer.ShufflePlug( "R", "driven" ) )
 
 		refDriverDelete = GafferImage.DeleteChannels()
 		refDriverDelete["in"].setInput( refDriverShuffle["out"] )
@@ -354,7 +354,7 @@ def testAgainstOIIO( self ):
 
 
 				with self.subTest( refFile = ref, driverChannel = channelName ):
-					for channelPlug in [ i["in"] for i in driverShuffle["channels"].children() + refDriverShuffle["channels"].children() ] + [ driverDilate["masterChannel"] ]:
+					for channelPlug in [ i["source"] for i in driverShuffle["shuffles"].children() + refDriverShuffle["shuffles"].children() ] + [ driverDilate["masterChannel"] ]:
 						channelPlug.setValue( channelName )
 
 					self.assertImagesEqual( driverDilate["out"], refDriverDelete["out"], ignoreMetadata = True )