Merge pull request #5524 from johnhaddon/prefixWithLayerName

CollectImages : Add `addLayerPrefix` plug

Changes.md

@@ -14,6 +14,7 @@ Improvements
 
 - LightTool : Changed spot light and quad light edge tool tip locations so that they follow the cone and edge during drag.
 - Arnold : Improved speed of translation of encapsulated scenes when using many threads.
+- CollectImages : Added `addLayerPrefix` plug, to allow the layer prefix to be omitted in the case that the input images are already prefixed.
 
 Fixes
 -----

include/GafferImage/CollectImages.h

@@ -59,13 +59,19 @@ class GAFFERIMAGE_API CollectImages : public ImageProcessor
 		Gaffer::StringPlug *layerVariablePlug();
 		const Gaffer::StringPlug *layerVariablePlug() const;
 
+		Gaffer::BoolPlug *addLayerPrefixPlug();
+		const Gaffer::BoolPlug *addLayerPrefixPlug() const;
+
 		Gaffer::BoolPlug *mergeMetadataPlug();
 		const Gaffer::BoolPlug *mergeMetadataPlug() 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 hashViewNames( const GafferImage::ImagePlug *parent, const Gaffer::Context *context, IECore::MurmurHash &h ) const override;
 		IECore::ConstStringVectorDataPtr computeViewNames( const Gaffer::Context *context, const ImagePlug *parent ) const override;
 
@@ -92,6 +98,9 @@ class GAFFERIMAGE_API CollectImages : public ImageProcessor
 
 	private :
 
+		Gaffer::ObjectPlug *mappingPlug();
+		const Gaffer::ObjectPlug *mappingPlug() const;
+
 		static size_t g_firstPlugIndex;
 
 };

python/GafferImageTest/CollectImagesTest.py

@@ -151,10 +151,24 @@ def testLayerMapping( self ) :
 		self.assertEqual( sampler["color"].getValue(), imath.Color4f( 0.1, 0.2, 0.3, 0.4 ) )
 
 		# Test simple duplicate
+
+		def assertExpectedMessages( messageHandler, layerName, channelNames ) :
+
+			self.assertEqual( [ m.level for m in messageHandler.messages ], [ IECore.Msg.Level.Warning ] * len( channelNames ) )
+			self.assertEqual( [ m.context for m in messageHandler.messages ], [ "CollectImages" ] * len( channelNames ) )
+
+			self.assertEqual(
+				[ m.message for m in messageHandler.messages ],
+				[ "Ignoring duplicate channel \"{}\" from layer \"{}\"".format( c, layerName ) for c in channelNames ]
+			)
+
 		collect["rootLayers"].setValue( IECore.StringVectorData( [ 'A', 'A' ] ) )
 
-		self.assertEqual( list(collect["out"]["channelNames"].getValue()), [ "A.R", "A.G", "A.B", "A.A" ] )
-		self.assertEqual( sampler["color"].getValue(), imath.Color4f( 0.1, 0.2, 0.3, 0.4 ) )
+		with IECore.CapturingMessageHandler() as mh :
+			self.assertEqual( list(collect["out"]["channelNames"].getValue()), [ "A.R", "A.G", "A.B", "A.A" ] )
+			self.assertEqual( sampler["color"].getValue(), imath.Color4f( 0.1, 0.2, 0.3, 0.4 ) )
+
+		assertExpectedMessages( mh, "A", [ "A.R", "A.G", "A.B", "A.A" ] )
 
 		collect["rootLayers"].setValue( IECore.StringVectorData( [ 'A', 'B' ] ) )
 		self.assertEqual( list(collect["out"]["channelNames"].getValue()), [
@@ -165,13 +179,24 @@ def testLayerMapping( self ) :
 		self.assertEqual( sampler["color"].getValue(), imath.Color4f( 0.2, 0.4, 0.6, 0.8 ) )
 
 		# Test overlapping names take the first layer
+
 		constant1["layer"].setValue( "B" )
 		collect["rootLayers"].setValue( IECore.StringVectorData( [ 'A', 'A.B' ] ) )
 		sampler["channels"].setValue( IECore.StringVectorData( [ "A.B.R", "A.B.G","A.B.B","A.B.A" ] ) )
-		self.assertEqual( list(collect["out"]["channelNames"].getValue()), [ "A.B.R", "A.B.G", "A.B.B", "A.B.A" ] )
+
+		with IECore.CapturingMessageHandler() as mh :
+			self.assertEqual( list(collect["out"]["channelNames"].getValue()), [ "A.B.R", "A.B.G", "A.B.B", "A.B.A" ] )
+
+		assertExpectedMessages( mh, "A.B", [ "A.B.R", "A.B.G", "A.B.B", "A.B.A" ] )
+
 		self.assertEqual( sampler["color"].getValue(), imath.Color4f( 0.1, 0.2, 0.3, 0.4 ) )
 		collect["rootLayers"].setValue( IECore.StringVectorData( [ 'A.B', 'A' ] ) )
-		self.assertEqual( list(collect["out"]["channelNames"].getValue()), [ "A.B.R", "A.B.G", "A.B.B", "A.B.A" ] )
+
+		with IECore.CapturingMessageHandler() as mh :
+			self.assertEqual( list(collect["out"]["channelNames"].getValue()), [ "A.B.R", "A.B.G", "A.B.B", "A.B.A" ] )
+
+		assertExpectedMessages( mh, "A", [ "A.B.R", "A.B.G", "A.B.B", "A.B.A" ] )
+
 		self.assertEqual( sampler["color"].getValue(), imath.Color4f( 0.2, 0.4, 0.6, 0.8 ) )
 
 	def testDeep( self ) :
@@ -261,5 +286,60 @@ def testMergeMetadata( self ) :
 			IECore.CompoundData( { str(i) : IECore.IntData(i+1) for i in range( 4 ) } )
 		)
 
+	@GafferTest.TestRunner.PerformanceTestMethod()
+	def testHighLayerCountPerformance( self ) :
+
+		constant = GafferImage.Constant()
+
+		collect = GafferImage.CollectImages()
+		collect["in"].setInput( constant["out"] )
+		collect["rootLayers"].setValue( IECore.StringVectorData( [ "layer{}".format( i ) for i in range( 0, 1000 ) ] ) )
+
+		with GafferTest.TestRunner.PerformanceScope() :
+			GafferImageTest.processTiles( collect["out"] )
+
+	def testLayerPrefix( self ) :
+
+		# By default, we add a layer name prefix to all channel names.
+
+		constant = GafferImage.Constant()
+
+		collect = GafferImage.CollectImages()
+		collect["in"].setInput( constant["out"] )
+		collect["rootLayers"].setValue( IECore.StringVectorData( [ "diffuse", "specular" ] ) )
+
+		self.assertEqual(
+			collect["out"].channelNames(),
+			IECore.StringVectorData( [
+				"diffuse.R", "diffuse.G", "diffuse.B", "diffuse.A",
+				"specular.R", "specular.G", "specular.B", "specular.A",
+			] )
+		)
+
+		# But that is inconvenient if you've already got the layer name in the
+		# channel name.
+
+		constant["layer"].setValue( "${collect:layerName}" )
+
+		self.assertEqual(
+			collect["out"].channelNames(),
+			IECore.StringVectorData( [
+				"diffuse.diffuse.R", "diffuse.diffuse.G", "diffuse.diffuse.B", "diffuse.diffuse.A",
+				"specular.specular.R", "specular.specular.G", "specular.specular.B", "specular.specular.A",
+			] )
+		)
+
+		# So we let people turn it off.
+
+		collect["addLayerPrefix"].setValue( False )
+
+		self.assertEqual(
+			collect["out"].channelNames(),
+			IECore.StringVectorData( [
+				"diffuse.R", "diffuse.G", "diffuse.B", "diffuse.A",
+				"specular.R", "specular.G", "specular.B", "specular.A",
+			] )
+		)
+
 if __name__ == "__main__":
 	unittest.main()

python/GafferImageUI/CollectImagesUI.py

@@ -66,7 +66,7 @@
 
 			"description",
 			"""
-			A list of the new layers to create.
+			A list of values for the `layerVariable`, defining the layers to be collected.
 			""",
 
 		],
@@ -81,6 +81,17 @@
 
 		],
 
+		"addLayerPrefix" : [
+
+			"description",
+			"""
+			When on, the output channel names are automatically prefixed with
+			the name of the layer being collected. Should be turned off when
+			the input channel names already contain the layer name.
+			""",
+
+		],
+
 		"mergeMetadata" : [
 
 			"description",