Merge pull request #5522 from johnhaddon/precomputedHashFix

ValuePlug : Stop `getValueInternal()` ignoring `precomputedHash`

python/GafferTest/NumericPlugTest.py

@@ -450,17 +450,14 @@ def testPrecomputedHash( self ) :
 		self.assertEqual( n.numComputeCalls, 1 )
 
 		h = n["sum"].hash()
-		numHashCalls = n.numHashCalls
-		# Accept either 1 or 2 - it would be reasonable for the ValuePlug
-		# to have either cached the hash or not, but that's not what we're
-		# testing here.
-		self.assertTrue( numHashCalls == 1 or numHashCalls == 2 )
+		self.assertEqual( n.numHashCalls, 1 )
 		self.assertEqual( n.numComputeCalls, 1 )
 
-		# What we care about is that calling getValue() with a precomputed hash
-		# definitely doesn't recompute the hash again.
+		# Calling `getValue()`` with a precomputed hash shouldn't recompute the
+		# hash again, even if it has been cleared from the cache.
+		Gaffer.ValuePlug.clearHashCache()
 		self.assertEqual( n["sum"].getValue( _precomputedHash = h ), 30 )
-		self.assertEqual( n.numHashCalls, numHashCalls )
+		self.assertEqual( n.numHashCalls, 1 )
 		self.assertEqual( n.numComputeCalls, 1 )
 
 	def testIsSetToDefault( self ) :

python/GafferTest/TypedPlugTest.py

@@ -208,17 +208,14 @@ def testPrecomputedHash( self ) :
 		self.assertEqual( n.numComputeCalls, 1 )
 
 		h = n["out"].hash()
-		numHashCalls = n.numHashCalls
-		# Accept either 1 or 2 - it would be reasonable for the ValuePlug
-		# to have either cached the hash or not, but that's not what we're
-		# testing here.
-		self.assertTrue( numHashCalls == 1 or numHashCalls == 2 )
+		self.assertEqual( n.numHashCalls, 1 )
 		self.assertEqual( n.numComputeCalls, 1 )
 
-		# What we care about is that calling getValue() with a precomputed hash
-		# definitely doesn't recompute the hash again.
+		# Calling `getValue()` with a precomputed hash shouldn't recompute the
+		# hash again, even if it has been cleared from the cache.
+		Gaffer.ValuePlug.clearHashCache()
 		self.assertEqual( n["out"].getValue( _precomputedHash = h ), "hi" )
-		self.assertEqual( n.numHashCalls, numHashCalls )
+		self.assertEqual( n.numHashCalls, 1 )
 		self.assertEqual( n.numComputeCalls, 1 )
 
 	def testBoolPlugStringConnections( self ) :

python/GafferTest/ValuePlugTest.py

@@ -286,22 +286,18 @@ def testPrecomputedHashOptimisation( self ) :
 		self.assertEqual( a1, IECore.StringData( "a" ) )
 		self.assertEqual( n.numHashCalls, 1 )
 
-		# We apply some leeway in our test for how many hash calls are
-		# made - a good ValuePlug implementation will probably avoid
-		# unecessary repeated calls in most cases, but it's not
-		# what this unit test is about.
 		a2 = n["out"].getValue( _copy = False )
 		self.assertTrue( a2.isSame( a1 ) )
-		self.assertTrue( n.numHashCalls == 1 or n.numHashCalls == 2 )
+		self.assertEqual( n.numHashCalls, 1 )
 
 		h = n["out"].hash()
-		self.assertTrue( n.numHashCalls >= 1 and n.numHashCalls <= 3 )
-		numHashCalls = n.numHashCalls
+		self.assertEqual( n.numHashCalls, 1 )
 
-		# What we care about is that calling getValue() with a precomputed hash
-		# definitely doesn't recompute the hash again.
+		# Calling `getValue()` with a precomputed hash shouldn't recompute the
+		# hash again, even if it has been cleared from the cache.
+		Gaffer.ValuePlug.clearHashCache()
 		a3 = n["out"].getValue( _copy = False, _precomputedHash = h )
-		self.assertEqual( n.numHashCalls, numHashCalls )
+		self.assertEqual( n.numHashCalls, 1 )
 		self.assertTrue( a3.isSame( a1 ) )
 
 	def testSerialisationOfChildValues( self ) :

src/Gaffer/ValuePlug.cpp

@@ -567,7 +567,7 @@ class ValuePlug::ComputeProcess : public Process
 			// > `StringPlug::hash()` account for additional processing (such as
 			// > substitutions) performed in public `getValue()` methods _after_
 			// > calling `getValueInternal()`.
-			const IECore::MurmurHash hash = p->ValuePlug::hash();
+			const IECore::MurmurHash hash = precomputedHash ? *precomputedHash : p->ValuePlug::hash();
 
 			if( !Process::forceMonitoring( threadState, plug, staticType ) )
 			{