Merge pull request #5532 from johnhaddon/instancerFix

Instancer : Fix dirty propagation from prototypes to capsule

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

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 ):

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<uint64_t>( 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 );