diff --git a/Changes.md b/Changes.md index 4efcfdd42f8..231b0a38c84 100644 --- a/Changes.md +++ b/Changes.md @@ -1,3 +1,12 @@ +0.61.x.x (relative to 0.61.11.0) +========= + +Fixes +----- + +- Image Node Mix : Fixed incorrect results outside mask data window, and incorrect results when changing inputs. + + 0.61.11.0 (relative to 0.61.10.0) ========= diff --git a/python/GafferImageTest/ImageTestCase.py b/python/GafferImageTest/ImageTestCase.py index c562457761b..641d14d7cb9 100644 --- a/python/GafferImageTest/ImageTestCase.py +++ b/python/GafferImageTest/ImageTestCase.py @@ -107,7 +107,7 @@ def assertImagesEqual( self, imageA, imageB, maxDifference = 0.0, ignoreMetadata stats = GafferImage.ImageStats() stats["in"].setInput( difference["out"] ) - stats["area"].setValue( imageA["format"].getValue().getDisplayWindow() ) + stats["area"].setValue( imageA["dataWindow"].getValue() ) for channelName in imageA["channelNames"].getValue() : diff --git a/python/GafferImageTest/ImageTransformTest.py b/python/GafferImageTest/ImageTransformTest.py index c532601ee9b..c27869deb8e 100644 --- a/python/GafferImageTest/ImageTransformTest.py +++ b/python/GafferImageTest/ImageTransformTest.py @@ -575,7 +575,7 @@ def testConcatenation( self ) : # show that it is legitimate : differences in filtering are that great. # The threshold is still significantly lower than the differences between # checker tiles, so does guarantee that tiles aren't getting out of alignment. - self.assertImagesEqual( tc2["out"], t2["out"], maxDifference = 0.11, ignoreDataWindow = True ) + self.assertImagesEqual( tc2["out"], t2["out"], maxDifference = 0.17, ignoreDataWindow = True ) def testDisabledAndNonConcatenating( self ) : diff --git a/python/GafferImageTest/MixTest.py b/python/GafferImageTest/MixTest.py index bebe03d2a1b..7a3e6e4ef15 100644 --- a/python/GafferImageTest/MixTest.py +++ b/python/GafferImageTest/MixTest.py @@ -51,6 +51,7 @@ class MixTest( GafferImageTest.ImageTestCase ) : rPath = os.path.expandvars( "$GAFFER_ROOT/python/GafferImageTest/images/redWithDataWindow.100x100.exr" ) gPath = os.path.expandvars( "$GAFFER_ROOT/python/GafferImageTest/images/greenWithDataWindow.100x100.exr" ) checkerPath = os.path.expandvars( "$GAFFER_ROOT/python/GafferImageTest/images/checkerboard.100x100.exr" ) + checkerNegativeDataWindowPath = os.path.expandvars( "$GAFFER_ROOT/python/GafferImageTest/images/checkerWithNegativeDataWindow.200x150.exr" ) checkerMixPath = os.path.expandvars( "$GAFFER_ROOT/python/GafferImageTest/images/checkerMix.100x100.exr" ) representativeDeepImagePath = os.path.expandvars( "$GAFFER_ROOT/python/GafferImageTest/images/representativeDeepImage.exr" ) radialPath = os.path.expandvars( "$GAFFER_ROOT/python/GafferImageTest/images/radial.exr" ) @@ -601,5 +602,94 @@ def testMismatchThrows( self ) : mix["in"][0].setInput( deep["out"] ) GafferImage.ImageAlgo.tiles( mix["out"] ) + def testFuzzDataWindows( self ): + + # A bunch of different test images with varying data windows + file1 = GafferImage.ImageReader() + file1["fileName"].setValue( self.checkerNegativeDataWindowPath ) + + file1Shuffle = GafferImage.Shuffle() + file1Shuffle["channels"].addChild( GafferImage.Shuffle.ChannelPlug( "A", "R" ) ) + file1Shuffle['in'].setInput( file1['out'] ) + + file2 = GafferImage.ImageReader() + file2["fileName"].setValue( self.checkerPath ) + + file2Shuffle = GafferImage.Shuffle() + file2Shuffle["channels"].addChild( GafferImage.Shuffle.ChannelPlug( "A", "R" ) ) + file2Shuffle['in'].setInput( file2['out'] ) + + largeConstant = GafferImage.Constant() + largeConstant["format"].setValue( GafferImage.Format( imath.Box2i( imath.V2i( 0 ), imath.V2i( 1920, 1080 ) ), 1 ) ) + largeConstant["color"].setValue( imath.Color4f( 0.7, 0.8, 0.9, 0.65 ) ) + + smallConstant = GafferImage.Constant() + smallConstant["format"].setValue( GafferImage.Format( imath.Box2i( imath.V2i( 554, 210 ), imath.V2i( 1265, 869 ) ), 1 ) ) + smallConstant["color"].setValue( imath.Color4f( 0.4, 0.5, 0.6, 0.45 ) ) + + leftConstant = GafferImage.Constant() + leftConstant["format"].setValue( GafferImage.Format( imath.Box2i( imath.V2i( -353, -561 ), imath.V2i( 915, 500 ) ), 1 ) ) + leftConstant["color"].setValue( imath.Color4f( 0.1, 0.2, 0.3, 0.75 ) ) + + rightConstant = GafferImage.Constant() + rightConstant["format"].setValue( GafferImage.Format( imath.Box2i( imath.V2i( 945, 557 ), imath.V2i( 2007, 1117 ) ), 1 ) ) + rightConstant["color"].setValue( imath.Color4f( 0.61, 0.62, 0.63, 0.35 ) ) + + Mix = GafferImage.Mix( "Mix" ) + + # Create a network using Merge that should match the result of Mix + MaskPromote = GafferImage.Shuffle( "MaskPromote" ) + MaskPromote["channels"].addChild( GafferImage.Shuffle.ChannelPlug( "R", "A" ) ) + MaskPromote["channels"].addChild( GafferImage.Shuffle.ChannelPlug( "G", "A" ) ) + MaskPromote["channels"].addChild( GafferImage.Shuffle.ChannelPlug( "B", "A" ) ) + MaskPromote["channels"].addChild( GafferImage.Shuffle.ChannelPlug( "A", "A" ) ) + MaskPromote['in'].setInput( Mix['mask'] ) + + Input0Mult = GafferImage.Merge( "Input0Mult" ) + Input0Mult["operation"].setValue( GafferImage.Merge.Operation.Multiply ) + Input0Mult['in'][0].setInput( Mix['in'][0] ) + Input0Mult["in"][1].setInput( MaskPromote["out"] ) + + Input0Subtract = GafferImage.Merge( "Input0Subtract" ) + Input0Subtract["operation"].setValue( GafferImage.Merge.Operation.Subtract ) + Input0Subtract["in"][0].setInput( Input0Mult["out"] ) + Input0Subtract['in'][1].setInput( Mix['in'][0] ) + + Input1Mult = GafferImage.Merge( "Input1Mult" ) + Input1Mult["operation"].setValue( GafferImage.Merge.Operation.Multiply ) + Input1Mult['in'][0].setInput( Mix['in'][1] ) + Input1Mult["in"][1].setInput( MaskPromote["out"] ) + + ReferenceMerge = GafferImage.Merge( "ReferenceMerge" ) + ReferenceMerge["operation"].setValue( GafferImage.Merge.Operation.Add ) + ReferenceMerge["in"][0].setInput( Input0Subtract["out"] ) + ReferenceMerge["in"][1].setInput( Input1Mult["out"] ) + + # Expand the data window of the reference to match the Mix, so we can compare + BlackBackground = GafferImage.Shuffle( "BlackBackground" ) + BlackBackground["channels"].addChild( GafferImage.Shuffle.ChannelPlug( "R", "__black" ) ) + BlackBackground["channels"].addChild( GafferImage.Shuffle.ChannelPlug( "G", "__black" ) ) + BlackBackground["channels"].addChild( GafferImage.Shuffle.ChannelPlug( "B", "__black" ) ) + BlackBackground["channels"].addChild( GafferImage.Shuffle.ChannelPlug( "A", "__black" ) ) + BlackBackground["in"].setInput( Mix["out"] ) + + ReferenceWithDataWindow = GafferImage.Merge( "ReferenceWithDataWindow" ) + ReferenceWithDataWindow["operation"].setValue( GafferImage.Merge.Operation.Over ) + ReferenceWithDataWindow["in"][0].setInput( BlackBackground["out"] ) + ReferenceWithDataWindow["in"][1].setInput( ReferenceMerge["out"] ) + + # For a more thorough test, use the full list of test images - but restricting to just 3 catches + # most failures, and takes just 2 seconds to test + #testImages = [ largeConstant, smallConstant, leftConstant, rightConstant, file1Shuffle, file2Shuffle ] + testImages = [ largeConstant, smallConstant, leftConstant ] + for input0 in testImages: + for input1 in testImages: + for mask in testImages: + Mix["in"][0].setInput( input0["out"] ) + Mix["in"][1].setInput( input1["out"] ) + Mix["mask"].setInput( mask["out"] ) + + self.assertImagesEqual( Mix["out"], ReferenceWithDataWindow["out"], maxDifference = 1e-7 ) + if __name__ == "__main__": unittest.main() diff --git a/src/GafferImage/Mix.cpp b/src/GafferImage/Mix.cpp index 3e3b20bf28c..d820f347e27 100644 --- a/src/GafferImage/Mix.cpp +++ b/src/GafferImage/Mix.cpp @@ -115,6 +115,11 @@ void Mix::affects( const Gaffer::Plug *input, AffectedPlugsContainer &outputs ) outputs.push_back( outPlug()->dataWindowPlug() ); outputs.push_back( outPlug()->channelNamesPlug() ); } + else if( input == maskPlug()->dataWindowPlug() ) + { + outputs.push_back( outPlug()->dataWindowPlug() ); + outputs.push_back( outPlug()->channelDataPlug() ); + } else if( input == maskPlug()->deepPlug() || input == maskPlug()->sampleOffsetsPlug() ) { outputs.push_back( outPlug()->channelDataPlug() ); @@ -128,6 +133,10 @@ void Mix::affects( const Gaffer::Plug *input, AffectedPlugsContainer &outputs ) if( inputImage->parent() == inPlugs() ) { outputs.push_back( outPlug()->getChild( input->getName() ) ); + if( input == inputImage->dataWindowPlug() ) + { + outputs.push_back( outPlug()->channelDataPlug() ); + } } } } @@ -173,6 +182,7 @@ Imath::Box2i Mix::computeDataWindow( const Gaffer::Context *context, const Image dataWindow.extendBy( (*it)->dataWindowPlug()->getValue() ); } + return dataWindow; } @@ -279,7 +289,8 @@ void Mix::hashChannelData( const GafferImage::ImagePlug *output, const Gaffer::C { maskValidBound = boxIntersection( tileBound, maskPlug()->dataWindowPlug()->getValue() ); } - h.append( maskValidBound ); + h.append( maskValidBound.min - tileOrigin ); + h.append( maskValidBound.max - tileOrigin ); for( int i = 0; i < 2; i++ ) { @@ -301,7 +312,12 @@ void Mix::hashChannelData( const GafferImage::ImagePlug *output, const Gaffer::C // input data windows, we may be using/revealing the invalid parts of a tile. We // deal with this in computeChannelData() by treating the invalid parts as black, // and must therefore hash in the valid bound here to take that into account. - h.append( validBound[i] ); + // + // Note that validBound only matters in relation to the channel data for this tile though, + // so we can hash it in relation to the tile origin, providing a small speedup by allowing + // reuse of constant tiles ( 20% speedup in testFuzzDataWindows ) + h.append( validBound[i].min - tileOrigin ); + h.append( validBound[i].max - tileOrigin ); } outPlug()->deepPlug()->hash( h ); maskPlug()->deepPlug()->hash( h ); @@ -351,6 +367,7 @@ IECore::ConstFloatVectorDataPtr Mix::computeChannelData( const std::string &chan std::string maskChannel; Box2i maskValidBound; + bool hasMask = false; bool maskDeep; { // Start by grabbing all the plug values we need that are global @@ -380,10 +397,21 @@ IECore::ConstFloatVectorDataPtr Mix::computeChannelData( const std::string &chan if( maskPlug()->getInput() && ImageAlgo::channelExists( maskPlug()->channelNamesPlug()->getValue()->readable(), maskChannel ) ) { + hasMask = true; maskValidBound = boxIntersection( tileBound, maskPlug()->dataWindowPlug()->getValue() ); if( BufferAlgo::empty( maskValidBound ) ) { - return inputs[ 0 ]->channelData( channelName, tileOrigin ); + if( BufferAlgo::empty( validBound[0] ) ) + { + return ImagePlug::blackTile(); + } + else if( validBound[0] == tileBound ) + { + // If the whole tile is within the input data window, we can just pass through + // the input. Otherwise, we need to hit the slower path below to trim out the + // part of the input which is outside the input's data window. + return inputs[ 0 ]->channelData( channelName, tileOrigin ); + } } } @@ -525,16 +553,12 @@ IECore::ConstFloatVectorDataPtr Mix::computeChannelData( const std::string &chan b = *B; } - float m = mix; + float m = hasMask ? 0.0f : mix; if( M ) { if( yValidMask && x >= maskValidBound.min.x && x < maskValidBound.max.x ) { - m *= std::max( 0.0f, std::min( 1.0f, *M ) ); - } - else - { - m = 0; + m = mix * std::max( 0.0f, std::min( 1.0f, *M ) ); } ++M; }