diff --git a/Changes.md b/Changes.md index 927854660c1..99450d650b8 100644 --- a/Changes.md +++ b/Changes.md @@ -13,6 +13,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 ----- diff --git a/include/GafferImage/CollectImages.h b/include/GafferImage/CollectImages.h index c040f74601e..ee3bb3791a1 100644 --- a/include/GafferImage/CollectImages.h +++ b/include/GafferImage/CollectImages.h @@ -59,6 +59,9 @@ 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; @@ -66,6 +69,9 @@ class GAFFERIMAGE_API CollectImages : public ImageProcessor 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; }; diff --git a/python/GafferImageTest/CollectImagesTest.py b/python/GafferImageTest/CollectImagesTest.py index d239720f0e2..cd1bb61fce8 100644 --- a/python/GafferImageTest/CollectImagesTest.py +++ b/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() diff --git a/python/GafferImageUI/CollectImagesUI.py b/python/GafferImageUI/CollectImagesUI.py index 4c59c6b767d..97d4f115611 100644 --- a/python/GafferImageUI/CollectImagesUI.py +++ b/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", diff --git a/src/GafferImage/CollectImages.cpp b/src/GafferImage/CollectImages.cpp index b4acdbd02b1..1253e1ecb97 100644 --- a/src/GafferImage/CollectImages.cpp +++ b/src/GafferImage/CollectImages.cpp @@ -43,6 +43,8 @@ #include "Gaffer/ArrayPlug.h" #include "Gaffer/Context.h" +#include "IECore/NullObject.h" + #include "fmt/format.h" #include @@ -78,28 +80,75 @@ void copyRegion( const float *fromBuffer, const Box2i &fromWindow, const Box2i & } } -void sourceLayerAndChannel( const string &destChannel, const vector &rootLayers, string &srcLayer, string &srcChannel ) +class MappingData : public IECore::Data { - for( unsigned int i = 0; i < rootLayers.size(); i++ ) - { - if( - destChannel.size() >= rootLayers[i].size() + 2 && - destChannel.compare( 0, rootLayers[i].size(), rootLayers[i] ) == 0 && - destChannel[rootLayers[i].size()] == '.' ) + + public : + + MappingData( bool addLayerPrefix ) + : m_addLayerPrefix( addLayerPrefix ), m_outputChannelNames( new StringVectorData ) { - srcLayer = rootLayers[i]; - srcChannel = destChannel.substr( rootLayers[i].size() + 1 ); + } - // Note that if multiple layer names could match this channel, we just take the first one. - // This would only apply in strange circumstances, and in this case of ambiguity, just taking the - // first one seems reasonable - return; + void addLayer( const string &layerName, const vector &channelNames ) + { + for( const auto &channelName : channelNames ) + { + const string outputChannelName = m_addLayerPrefix ? ImageAlgo::channelName( layerName, channelName ) : channelName; + const Input input = { layerName, channelName }; + if( m_mapping.try_emplace( outputChannelName, input ).second ) + { + m_outputChannelNames->writable().push_back( outputChannelName ); + } + else + { + // Duplicate channel names could arise for several reasons : + // + // - The user entered the same layer name twice. + // - Names overlap due to complex hierachical naming, such as a layer named `A` with + // a channel named `B.R` and a layer named `A.B` with a channel named `R`. + // - `addLayerPrefix` is off, but the input channels do not have a suitable + // prefix of their own. + // + // In all cases we take the first channel and ignore the duplicate, but we + // also emit a warning so the user can fix the setup. + msg( + Msg::Warning, "CollectImages", + fmt::format( "Ignoring duplicate channel \"{}\" from layer \"{}\"", outputChannelName, layerName ) + ); + } + } } - } - srcLayer = ""; - srcChannel = destChannel; -} + const StringVectorData *outputChannelNames() const { return m_outputChannelNames.get(); } + + struct Input + { + const string layerName; + const string channelName; + }; + + const Input &input( const string &outputChannelName ) const + { + auto it = m_mapping.find( outputChannelName ); + if( it == m_mapping.end() ) + { + throw IECore::Exception( fmt::format( "Invalid output channel {}", outputChannelName ) ); + } + return it->second; + } + + private : + + const bool m_addLayerPrefix; + StringVectorDataPtr m_outputChannelNames; + + using Map = unordered_map; + Map m_mapping; + +}; + +IE_CORE_DECLAREPTR( MappingData ) } // namespace @@ -118,7 +167,9 @@ CollectImages::CollectImages( const std::string &name ) addChild( new StringVectorDataPlug( "rootLayers", Plug::In, new StringVectorData ) ); addChild( new StringPlug( "layerVariable", Plug::In, "collect:layerName" ) ); + addChild( new BoolPlug( "addLayerPrefix", Plug::In, true ) ); addChild( new BoolPlug( "mergeMetadata", Plug::In ) ); + addChild( new ObjectPlug( "__mapping", Plug::Out, NullObject::defaultNullObject() ) ); } CollectImages::~CollectImages() @@ -145,20 +196,66 @@ const Gaffer::StringPlug *CollectImages::layerVariablePlug() const return getChild( g_firstPlugIndex + 1 ); } -Gaffer::BoolPlug *CollectImages::mergeMetadataPlug() +Gaffer::BoolPlug *CollectImages::addLayerPrefixPlug() { return getChild( g_firstPlugIndex + 2 ); } -const Gaffer::BoolPlug *CollectImages::mergeMetadataPlug() const +const Gaffer::BoolPlug *CollectImages::addLayerPrefixPlug() const { return getChild( g_firstPlugIndex + 2 ); } +Gaffer::BoolPlug *CollectImages::mergeMetadataPlug() +{ + return getChild( g_firstPlugIndex + 3 ); +} + +const Gaffer::BoolPlug *CollectImages::mergeMetadataPlug() const +{ + return getChild( g_firstPlugIndex + 3 ); +} + +Gaffer::ObjectPlug *CollectImages::mappingPlug() +{ + return getChild( g_firstPlugIndex + 4 ); +} + +const Gaffer::ObjectPlug *CollectImages::mappingPlug() const +{ + return getChild( g_firstPlugIndex + 4 ); +} + void CollectImages::affects( const Gaffer::Plug *input, AffectedPlugsContainer &outputs ) const { ImageProcessor::affects( input, outputs ); + if( + input == addLayerPrefixPlug() || + input == layerVariablePlug() || + input == rootLayersPlug() || + input == inPlug()->channelNamesPlug() + ) + { + outputs.push_back( mappingPlug() ); + } + + if( input == mappingPlug() ) + { + outputs.push_back( outPlug()->channelNamesPlug() ); + } + + if( + input == mappingPlug() || + input == layerVariablePlug() || + input == inPlug()->deepPlug() || + input == inPlug()->dataWindowPlug() || + input == inPlug()->channelDataPlug() + ) + { + outputs.push_back( outPlug()->channelDataPlug() ); + } + const ImagePlug *imagePlug = input->parent(); if( imagePlug && imagePlug == inPlug() ) { @@ -167,16 +264,6 @@ void CollectImages::affects( const Gaffer::Plug *input, AffectedPlugsContainer & outputs.push_back( outPlug()->dataWindowPlug() ); } - if( input == imagePlug->channelNamesPlug() ) - { - outputs.push_back( outPlug()->channelNamesPlug() ); - } - - if( input == imagePlug->channelDataPlug() ) - { - outputs.push_back( outPlug()->channelDataPlug() ); - } - if( input == imagePlug->formatPlug() ) { outputs.push_back( outPlug()->formatPlug() ); @@ -201,8 +288,6 @@ void CollectImages::affects( const Gaffer::Plug *input, AffectedPlugsContainer & } else if( input == rootLayersPlug() || input == layerVariablePlug() ) { - outputs.push_back( outPlug()->channelNamesPlug() ); - outputs.push_back( outPlug()->channelDataPlug() ); outputs.push_back( outPlug()->dataWindowPlug() ); outputs.push_back( outPlug()->formatPlug() ); outputs.push_back( outPlug()->metadataPlug() ); @@ -216,6 +301,57 @@ void CollectImages::affects( const Gaffer::Plug *input, AffectedPlugsContainer & } +void CollectImages::hash( const Gaffer::ValuePlug *output, const Gaffer::Context *context, IECore::MurmurHash &h ) const +{ + if( output == mappingPlug() ) + { + ImageProcessor::hash( output, context, h ); + + addLayerPrefixPlug()->hash( h ); + + const std::string layerVariable = layerVariablePlug()->getValue(); + Context::EditableScope layerScope( context ); + + ConstStringVectorDataPtr rootLayersData = rootLayersPlug()->getValue(); + for( auto &rootLayer : rootLayersData->readable() ) + { + h.append( rootLayer ); + layerScope.set( layerVariable, &rootLayer ); + inPlug()->channelNamesPlug()->hash( h ); + } + } + else + { + ImageProcessor::hash( output, context, h ); + } +} + +void CollectImages::compute( Gaffer::ValuePlug *output, const Gaffer::Context *context ) const +{ + if( output == mappingPlug() ) + { + MappingDataPtr mapping = new MappingData( addLayerPrefixPlug()->getValue() ); + + const std::string layerVariable = layerVariablePlug()->getValue(); + Context::EditableScope layerScope( context ); + + ConstStringVectorDataPtr rootLayersData = rootLayersPlug()->getValue(); + for( auto &rootLayer : rootLayersData->readable() ) + { + layerScope.set( layerVariable, &rootLayer ); + ConstStringVectorDataPtr inputChannelNamesData = inPlug()->channelNamesPlug()->getValue(); + mapping->addLayer( rootLayer, inputChannelNamesData->readable() ); + } + + static_cast( output )->setValue( mapping ); + } + else + { + ImageProcessor::compute( output, context ); + } +} + + void CollectImages::hashViewNames( const GafferImage::ImagePlug *output, const Gaffer::Context *context, IECore::MurmurHash &h ) const { ImageProcessor::hashViewNames( output, context, h ); @@ -543,78 +679,30 @@ Imath::Box2i CollectImages::computeDataWindow( const Gaffer::Context *context, c void CollectImages::hashChannelNames( const GafferImage::ImagePlug *output, const Gaffer::Context *context, IECore::MurmurHash &h ) const { ImageProcessor::hashChannelNames( output, context, h ); - - const std::string layerVariable = layerVariablePlug()->getValue(); - - ConstStringVectorDataPtr rootLayersData = rootLayersPlug()->getValue(); - const vector &rootLayers = rootLayersData->readable(); - - h.append( &(rootLayers[0]), rootLayers.size() ); - - Context::EditableScope editScope( context ); - for( unsigned int i = 0; i < rootLayers.size(); i++ ) - { - editScope.set( layerVariable, &( rootLayers[i] ) ); - inPlug()->channelNamesPlug()->hash( h ); - } + mappingPlug()->hash( h ); } IECore::ConstStringVectorDataPtr CollectImages::computeChannelNames( const Gaffer::Context *context, const ImagePlug *parent ) const { - StringVectorDataPtr channelNamesData = new StringVectorData(); - std::vector &outChannelNames = channelNamesData->writable(); - - const std::string layerVariable = layerVariablePlug()->getValue(); - - ConstStringVectorDataPtr rootLayersData = rootLayersPlug()->getValue(); - const vector &rootLayers = rootLayersData->readable(); - - Context::EditableScope editScope( context ); - for( unsigned int i = 0; i < rootLayers.size(); i++ ) - { - editScope.set( layerVariable, &( rootLayers[i] ) ); - ConstStringVectorDataPtr layerChannelsData = inPlug()->channelNamesPlug()->getValue(); - const std::vector &layerChannels = layerChannelsData->readable(); - - for( unsigned int j = 0; j < layerChannels.size(); j++ ) - { - std::string curName = GafferImage::ImageAlgo::channelName( rootLayers[i], layerChannels[j] ); - - // Duplicate channel names could arise because either: - // * The user entered the same layer name twice ( just ignore one of them ) - // * A name overlap due to complex hierachical naming: - // ie. both a layer named A with a channel named B.R - // and a layer named A.B with a channel named R - // ( In this complicated and unlikely case, we just take the one listed first in rootLayers ) - if( find( outChannelNames.begin(), outChannelNames.end(), curName ) == outChannelNames.end() ) - { - outChannelNames.push_back( curName ); - } - } - } - - return channelNamesData; + auto mapping = boost::static_pointer_cast( mappingPlug()->getValue() ); + return mapping->outputChannelNames(); } void CollectImages::hashChannelData( const GafferImage::ImagePlug *parent, const Gaffer::Context *context, IECore::MurmurHash &h ) const { - ConstStringVectorDataPtr rootLayersData; + ConstMappingDataPtr mapping; string layerVariable; { ImagePlug::GlobalScope c( context ); - rootLayersData = rootLayersPlug()->getValue(); + mapping = boost::static_pointer_cast( mappingPlug()->getValue() ); layerVariable = layerVariablePlug()->getValue(); } - const vector &rootLayers = rootLayersData->readable(); - - const std::string &channelName = context->get( ImagePlug::channelNameContextName ); - std::string srcLayer, srcChannel; - sourceLayerAndChannel( channelName, rootLayers, srcLayer, srcChannel ); + const MappingData::Input &input = mapping->input( context->get( ImagePlug::channelNameContextName ) ); Context::EditableScope editScope( context ); - editScope.set( ImagePlug::channelNameContextName, &srcChannel ); - editScope.set( layerVariable, &srcLayer ); + editScope.set( ImagePlug::channelNameContextName, &input.channelName ); + editScope.set( layerVariable, &input.layerName ); const V2i tileOrigin = context->get( ImagePlug::tileOriginContextName ); const Box2i tileBound( tileOrigin, tileOrigin + V2i( ImagePlug::tileSize() ) ); @@ -646,21 +734,18 @@ void CollectImages::hashChannelData( const GafferImage::ImagePlug *parent, const IECore::ConstFloatVectorDataPtr CollectImages::computeChannelData( const std::string &channelName, const Imath::V2i &tileOrigin, const Gaffer::Context *context, const ImagePlug *parent ) const { - ConstStringVectorDataPtr rootLayersData; + ConstMappingDataPtr mapping; string layerVariable; { ImagePlug::GlobalScope c( context ); - rootLayersData = rootLayersPlug()->getValue(); + mapping = boost::static_pointer_cast( mappingPlug()->getValue() ); layerVariable = layerVariablePlug()->getValue(); } - const vector &rootLayers = rootLayersData->readable(); - - std::string srcLayer, srcChannel; - sourceLayerAndChannel( channelName, rootLayers, srcLayer, srcChannel ); + const MappingData::Input &input = mapping->input( channelName ); Context::EditableScope editScope( context ); - editScope.set( layerVariable, &srcLayer ); + editScope.set( layerVariable, &input.layerName ); // First use this EditableScope as a global scope editScope.remove( ImagePlug::channelNameContextName ); @@ -676,7 +761,7 @@ IECore::ConstFloatVectorDataPtr CollectImages::computeChannelData( const std::st } // Then set up the scope to evaluate the input channel data - editScope.set( ImagePlug::channelNameContextName, &srcChannel ); + editScope.set( ImagePlug::channelNameContextName, &input.channelName ); editScope.set( ImagePlug::tileOriginContextName, &tileOrigin ); ConstFloatVectorDataPtr inputData = inPlug()->channelDataPlug()->getValue();