From 6dcbdc496532bd7ad168008d1c099131e3540627 Mon Sep 17 00:00:00 2001 From: John Haddon Date: Wed, 8 Nov 2023 15:05:07 +0000 Subject: [PATCH] Instancer : Fix dirty propagation from prototypes to capsule --- Changes.md | 3 + python/GafferSceneTest/InstancerTest.py | 79 +++++++++++++++++++++++++ src/GafferScene/Instancer.cpp | 21 +++++-- 3 files changed, 98 insertions(+), 5 deletions(-) diff --git a/Changes.md b/Changes.md index 2c360a99ae6..57ad7170f37 100644 --- a/Changes.md +++ b/Changes.md @@ -5,6 +5,9 @@ Fixes ----- - Catalogue : Fixed bugs which caused additional images to appear when changing light groups or crop in an Arnold render (#4267, #4633). +- Instancer : + - Fixed failure to update encapsulated instancers when prototype properties changed during interactive renders. + - Prevented unnecessary updates for encapsulated instancers when prototype globals changed. - Process : Fixed bug which caused a `No result found` exception to be thrown when a more descriptive exception should have been thrown instead. API diff --git a/python/GafferSceneTest/InstancerTest.py b/python/GafferSceneTest/InstancerTest.py index c0563d2d23b..c260cd0f6fc 100644 --- a/python/GafferSceneTest/InstancerTest.py +++ b/python/GafferSceneTest/InstancerTest.py @@ -2340,6 +2340,85 @@ def testEmptyPrototypes( self ) : self.assertEqual( instancer["variations"].getValue(), IECore.CompoundData( { "" : IECore.IntData( 0 ) } ) ) + def testPrototypePropertiesAffectCapsule( self ) : + + plane = GafferScene.Plane() + + planeFilter = GafferScene.PathFilter() + planeFilter["paths"].setValue( IECore.StringVectorData( [ "/plane" ] ) ) + + sphere = GafferScene.Sphere() + + sphereFilter = GafferScene.PathFilter() + sphereFilter["paths"].setValue( IECore.StringVectorData( [ "/sphere" ] ) ) + + sphereAttributes = GafferScene.CustomAttributes() + sphereAttributes["in"].setInput( sphere["out"] ) + sphereAttributes["filter"].setInput( sphereFilter["out"] ) + + sphereOptions = GafferScene.CustomOptions() + sphereOptions["in"].setInput( sphereAttributes["out"] ) + + instancer = GafferScene.Instancer() + instancer["in"].setInput( plane["out"] ) + instancer["prototypes"].setInput( sphereOptions["out"] ) + instancer["filter"].setInput( planeFilter["out"] ) + + for encapsulate in ( False, True ) : + + with self.subTest( encapsulate = encapsulate ) : + + instancer["encapsulateInstanceGroups"].setValue( encapsulate ) + + # Prototype properties used by the capsule should be reflected + # in the object hash. For the hash to be updated successfully, + # the `object` plug must also be dirtied (because we cache + # hashes). + + hashes = set() + hashes.add( instancer["out"].objectHash( "/plane/instances/sphere" ) ) + self.assertEqual( len( hashes ), 1 ) + + sphere["radius"].setValue( 2 + int( encapsulate ) ) + hashes.add( instancer["out"].objectHash( "/plane/instances/sphere" ) ) + self.assertEqual( len( hashes ), 2 if encapsulate else 1 ) + + dirtiedPlugs = GafferTest.CapturingSlot( instancer.plugDirtiedSignal() ) + + sphere["transform"]["translate"]["x"].setValue( 2 + int( encapsulate ) ) + hashes.add( instancer["out"].objectHash( "/plane/instances/sphere" ) ) + self.assertEqual( len( hashes ), 3 if encapsulate else 1 ) + + sphereAttributes["attributes"]["test"] = Gaffer.NameValuePlug( "test", encapsulate ) + hashes.add( instancer["out"].objectHash( "/plane/instances/sphere" ) ) + self.assertEqual( len( hashes ), 4 if encapsulate else 1 ) + + sphere["sets"].setValue( "testSet{}".format( encapsulate ) ) + hashes.add( instancer["out"].objectHash( "/plane/instances/sphere" ) ) + self.assertEqual( len( hashes ), 5 if encapsulate else 1 ) + + # When not encapsulating, there should be no unnecessary + # dirtying of the `object` plug. + + if not encapsulate : + self.assertNotIn( + instancer["out"]["object"], + { x[0] for x in dirtiedPlugs } + ) + + # And prototype globals shouldn't affect `object` in either + # mode, because they're not used by the capsule. + + del dirtiedPlugs[:] + + sphereOptions["options"]["test"] = Gaffer.NameValuePlug( "test", encapsulate ) + hashes.add( instancer["out"].objectHash( "/plane/instances/sphere" ) ) + self.assertEqual( len( hashes ), 5 if encapsulate else 1 ) + self.assertNotIn( + instancer["out"]["object"], + { x[0] for x in dirtiedPlugs } + ) + @unittest.skipIf( GafferTest.inCI(), "Performance not relevant on CI platform" ) @GafferTest.TestRunner.PerformanceTestMethod() def testContextSetPerfNoVariationsSingleEvaluate( self ): diff --git a/src/GafferScene/Instancer.cpp b/src/GafferScene/Instancer.cpp index 7b45495cc23..8698da4d0c5 100644 --- a/src/GafferScene/Instancer.cpp +++ b/src/GafferScene/Instancer.cpp @@ -1233,6 +1233,15 @@ void Instancer::affects( const Plug *input, AffectedPlugsContainer &outputs ) co outputs.push_back( outPlug()->setPlug() ); } + if( + input->parent() == prototypesPlug() && + input != prototypesPlug()->globalsPlug() && + !encapsulateInstanceGroupsPlug()->isSetToDefault() + ) + { + outputs.push_back( outPlug()->objectPlug() ); + } + // The capsule scene depends on all the same things as the regular output scene ( aside from not // being affected by the encapsulate plug, which always must be true when it's evaluated anyway ), // so we can leverage the logic in BranchCreator to drive it @@ -2022,11 +2031,13 @@ void Instancer::hashObject( const ScenePath &path, const Gaffer::Context *contex BranchCreator::hashBranchObject( sourcePath, branchPath, context, h ); h.append( reinterpret_cast( this ) ); /// We need to include anything that will affect how the capsule will expand. - /// \todo Once we fix motion blur behaviour so that Capsules don't - /// depend on the source scene's shutter settings, we should be able to omit - /// the `dirtyCount` for `prototypesPlug()->globalsPlug()`, by summing the - /// count for its siblings instead. - h.append( prototypesPlug()->dirtyCount() ); + for( const auto &prototypePlug : ValuePlug::Range( *prototypesPlug() ) ) + { + if( prototypePlug != prototypesPlug()->globalsPlug() ) + { + h.append( prototypePlug->dirtyCount() ); + } + } engineHash( sourcePath, context, h ); h.append( context->hash() ); outPlug()->boundPlug()->hash( h );