Merge pull request #5469 from johnhaddon/removeDeprecations

Remove deprecations

Changes.md

@@ -5,6 +5,9 @@ Breaking Changes
 ----------------
 
 - Dispatcher : Removed `createMatching()` method.
+- Process : Removed non-const variant of the `handleException()` method.
+- StringPlug : Removed deprecated `precomputedHash` argument from `getValue()` method.
+- OpenColorIOContext : Removed `configEnabledPlug()`, `configValuePlug()`, `workingSpaceEnabledPlug()` and `workingSpaceValuePlug()` methods. Use the OptionalValuePlug child accessors instead.
 
 1.3.x.x (relative to 1.3.3.0)
 =======

include/Gaffer/Process.h

@@ -98,8 +98,6 @@ class GAFFER_API Process : private ThreadState::Scope
 		/// and rethrow the exception for propagation back to
 		/// the original caller.
 		[[noreturn]] void handleException() const;
-		/// \todo This just exists for ABI compatibility. Remove it.
-		void handleException();
 
 	private :
 

include/Gaffer/StringPlug.h

@@ -118,10 +118,8 @@ class GAFFER_API StringPlug : public ValuePlug
 		/// > (which would be `\` on Windows).
 		/// \undoable
 		void setValue( const std::filesystem::path &value );
-		/// Returns the value. The `precomputedHash` argument is deprecated, and
-		/// will be removed in a future release.
-		/// \todo Remove `precomputedHash` argument.
-		std::string getValue( const IECore::MurmurHash *precomputedHash = nullptr ) const;
+		/// Returns the value.
+		std::string getValue() const;
 
 		void setFrom( const ValuePlug *other ) override;
 

include/GafferImage/OpenColorIOContext.h

@@ -41,6 +41,7 @@
 
 #include "Gaffer/CompoundDataPlug.h"
 #include "Gaffer/ContextProcessor.h"
+#include "Gaffer/OptionalValuePlug.h"
 #include "Gaffer/TypedObjectPlug.h"
 
 namespace GafferImage
@@ -56,25 +57,11 @@ class GAFFERIMAGE_API OpenColorIOContext : public Gaffer::ContextProcessor
 		explicit OpenColorIOContext( const std::string &name=GraphComponent::defaultName<OpenColorIOContext>() );
 		~OpenColorIOContext() override;
 
-		/// \todo Return OptionalValuePlug, and remove `configEnabledPlug()` and
-		/// `configValuePlug()` methods. Do the same for `workingSpace` plugs.
-		Gaffer::ValuePlug *configPlug();
-		const Gaffer::ValuePlug *configPlug() const;
+		Gaffer::OptionalValuePlug *configPlug();
+		const Gaffer::OptionalValuePlug *configPlug() const;
 
-		Gaffer::BoolPlug *configEnabledPlug();
-		const Gaffer::BoolPlug *configEnabledPlug() const;
-
-		Gaffer::StringPlug *configValuePlug();
-		const Gaffer::StringPlug *configValuePlug() const;
-
-		Gaffer::ValuePlug *workingSpacePlug();
-		const Gaffer::ValuePlug *workingSpacePlug() const;
-
-		Gaffer::BoolPlug *workingSpaceEnabledPlug();
-		const Gaffer::BoolPlug *workingSpaceEnabledPlug() const;
-
-		Gaffer::StringPlug *workingSpaceValuePlug();
-		const Gaffer::StringPlug *workingSpaceValuePlug() const;
+		Gaffer::OptionalValuePlug *workingSpacePlug();
+		const Gaffer::OptionalValuePlug *workingSpacePlug() const;
 
 		Gaffer::ValuePlug *variablesPlug();
 		const Gaffer::ValuePlug *variablesPlug() const;

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/StringPlugTest.py

@@ -304,13 +304,11 @@ def testSubstitutionsFromExpressionInput( self ) :
 
 			self.assertEqual( s["substitionsOn"]["out"].getValue(), "test.1.exr" )
 			substitutionsOnHash1 = s["substitionsOn"]["out"].hash()
-			self.assertEqual( s["substitionsOn"]["out"].getValue( _precomputedHash = substitutionsOnHash1 ), "test.1.exr" )
 
 			# We should get sequences out of the non-substituting node.
 
 			self.assertEqual( s["substitionsOff"]["out"].getValue(), "test.#.exr" )
 			substitutionsOffHash1 = s["substitionsOff"]["out"].hash()
-			self.assertEqual( s["substitionsOff"]["out"].getValue( _precomputedHash = substitutionsOffHash1 ), "test.#.exr" )
 			self.assertNotEqual( substitutionsOnHash1, substitutionsOffHash1 )
 
 			# We shouldn't get frame numbers out of the third node, because the
@@ -320,7 +318,6 @@ def testSubstitutionsFromExpressionInput( self ) :
 
 			self.assertEqual( s["substitionsOnIndirectly"]["out"].getValue(), "test.#.exr" )
 			substitionsOnIndirectlyHash1 = s["substitionsOnIndirectly"]["out"].hash()
-			self.assertEqual( s["substitionsOnIndirectly"]["out"].getValue( _precomputedHash = substitionsOnIndirectlyHash1 ), "test.#.exr" )
 
 			# Frame 2
 			#########
@@ -339,23 +336,20 @@ def testSubstitutionsFromExpressionInput( self ) :
 
 			self.assertEqual( s["substitionsOn"]["out"].getValue(), "test.2.exr" )
 			substitutionsOnHash2 = s["substitionsOn"]["out"].hash()
-			self.assertEqual( s["substitionsOn"]["out"].getValue( _precomputedHash = substitutionsOnHash2 ), "test.2.exr" )
 			self.assertNotEqual( substitutionsOnHash2, substitutionsOnHash1 )
 
 			# We should still get sequences out of the non-substituting node,
 			# and it should have the same output hash as it had on frame 1.
 
 			self.assertEqual( s["substitionsOff"]["out"].getValue(), "test.#.exr" )
 			substitutionsOffHash2 = s["substitionsOff"]["out"].hash()
-			self.assertEqual( s["substitionsOff"]["out"].getValue( _precomputedHash = substitutionsOffHash2 ), "test.#.exr" )
 			self.assertEqual( substitutionsOffHash1, substitutionsOffHash2 )
 			self.assertNotEqual( substitutionsOnHash2, substitutionsOffHash2 )
 
 			# The third node should still be non-substituting.
 
 			self.assertEqual( s["substitionsOnIndirectly"]["out"].getValue(), "test.#.exr" )
 			substitionsOnIndirectlyHash2 = s["substitionsOnIndirectly"]["out"].hash()
-			self.assertEqual( s["substitionsOnIndirectly"]["out"].getValue( _precomputedHash = substitionsOnIndirectlyHash2 ), "test.#.exr" )
 			self.assertEqual( substitionsOnIndirectlyHash2, substitionsOnIndirectlyHash1 )
 
 	def testHashUsesValue( self ) :

python/GafferTest/TypedPlugTest.py

@@ -200,25 +200,60 @@ def testNoChildrenAccepted( self ) :
 
 	def testPrecomputedHash( self ) :
 
-		n = GafferTest.StringInOutNode()
-		n["in"].setValue( "hi" )
+		class MatrixMultiplyNode( Gaffer.ComputeNode ) :
 
-		self.assertEqual( n["out"].getValue(), "hi" )
+			def __init__( self, name = "MatrixMultiply" ) :
+
+				Gaffer.ComputeNode.__init__( self, name )
+
+				self["in1"] = Gaffer.M44fPlug()
+				self["in2"] = Gaffer.M44fPlug()
+				self["out"] = Gaffer.M44fPlug( direction = Gaffer.Plug.Direction.Out )
+
+				self.numComputeCalls = 0
+				self.numHashCalls = 0
+
+			def affects( self, input ) :
+
+				outputs = Gaffer.ComputeNode.affects( self, input )
+				if input.isSame( self["in1"] ) or input.isSame( self["in2"] ) :
+					outputs.append( self.getChild( "out" ) )
+
+				return outputs
+
+			def hash( self, output, context, h ) :
+
+				assert( output.isSame( self.getChild( "out" ) ) )
+
+				self["in1"].hash( h )
+				self["in2"].hash( h )
+
+				self.numHashCalls += 1
+
+			def compute( self, output, context ) :
+
+				assert( output.isSame( self.getChild( "out" ) ) )
+				output.setValue( self["in1"].getValue() * self["in2"].getValue() )
+
+				self.numComputeCalls += 1
+
+		IECore.registerRunTimeTyped( MatrixMultiplyNode )
+
+		n = MatrixMultiplyNode()
+
+		self.assertEqual( n["out"].getValue(), imath.M44f() )
 		self.assertEqual( n.numHashCalls, 1 )
 		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.
-		self.assertEqual( n["out"].getValue( _precomputedHash = h ), "hi" )
-		self.assertEqual( n.numHashCalls, numHashCalls )
+		# 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 ), imath.M44f() )
+		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/Process.cpp

@@ -161,11 +161,6 @@ void Process::handleException() const
 	}
 }
 
-void Process::handleException()
-{
-	const_cast<const Process *>( this )->handleException();
-}
-
 void Process::emitError( const std::string &error, const Plug *source ) const
 {
 	const Plug *plug = m_destinationPlug;

src/Gaffer/StringPlug.cpp

@@ -106,9 +106,9 @@ void StringPlug::setValue( const std::filesystem::path &value )
 	setValue( value.generic_string() );
 }
 
-std::string StringPlug::getValue( const IECore::MurmurHash *precomputedHash ) const
+std::string StringPlug::getValue() const
 {
-	ConstStringDataPtr s = getObjectValue<StringData>( precomputedHash );
+	ConstStringDataPtr s = getObjectValue<StringData>();
 
 	const bool performSubstitutions =
 		m_substitutions &&

src/GafferImage/OpenColorIOContext.cpp

@@ -67,64 +67,24 @@ OpenColorIOContext::~OpenColorIOContext()
 {
 }
 
-Gaffer::ValuePlug *OpenColorIOContext::configPlug()
+Gaffer::OptionalValuePlug *OpenColorIOContext::configPlug()
 {
-	return getChild<ValuePlug>( g_firstPlugIndex );
+	return getChild<OptionalValuePlug>( g_firstPlugIndex );
 }
 
-const Gaffer::ValuePlug *OpenColorIOContext::configPlug() const
+const Gaffer::OptionalValuePlug *OpenColorIOContext::configPlug() const
 {
-	return getChild<ValuePlug>( g_firstPlugIndex );
+	return getChild<OptionalValuePlug>( g_firstPlugIndex );
 }
 
-Gaffer::BoolPlug *OpenColorIOContext::configEnabledPlug()
+Gaffer::OptionalValuePlug *OpenColorIOContext::workingSpacePlug()
 {
-	return configPlug()->getChild<BoolPlug>( 0 );
+	return getChild<OptionalValuePlug>( g_firstPlugIndex + 1 );
 }
 
-const Gaffer::BoolPlug *OpenColorIOContext::configEnabledPlug() const
+const Gaffer::OptionalValuePlug *OpenColorIOContext::workingSpacePlug() const
 {
-	return configPlug()->getChild<BoolPlug>( 0 );
-}
-
-Gaffer::StringPlug *OpenColorIOContext::configValuePlug()
-{
-	return configPlug()->getChild<StringPlug>( 1 );
-}
-
-const Gaffer::StringPlug *OpenColorIOContext::configValuePlug() const
-{
-	return configPlug()->getChild<StringPlug>( 1 );
-}
-
-Gaffer::ValuePlug *OpenColorIOContext::workingSpacePlug()
-{
-	return getChild<ValuePlug>( g_firstPlugIndex + 1 );
-}
-
-const Gaffer::ValuePlug *OpenColorIOContext::workingSpacePlug() const
-{
-	return getChild<ValuePlug>( g_firstPlugIndex + 1 );
-}
-
-Gaffer::BoolPlug *OpenColorIOContext::workingSpaceEnabledPlug()
-{
-	return workingSpacePlug()->getChild<BoolPlug>( 0 );
-}
-
-const Gaffer::BoolPlug *OpenColorIOContext::workingSpaceEnabledPlug() const
-{
-	return workingSpacePlug()->getChild<BoolPlug>( 0 );
-}
-
-Gaffer::StringPlug *OpenColorIOContext::workingSpaceValuePlug()
-{
-	return workingSpacePlug()->getChild<StringPlug>( 1 );
-}
-
-const Gaffer::StringPlug *OpenColorIOContext::workingSpaceValuePlug() const
-{
-	return workingSpacePlug()->getChild<StringPlug>( 1 );
+	return getChild<OptionalValuePlug>( g_firstPlugIndex + 1 );
 }
 
 Gaffer::ValuePlug *OpenColorIOContext::variablesPlug()
@@ -197,14 +157,14 @@ void OpenColorIOContext::compute( ValuePlug *output, const Context *context ) co
 		IECore::CompoundDataPtr resultData = new IECore::CompoundData;
 		IECore::CompoundDataMap &result = resultData->writable();
 
-		if( configEnabledPlug()->getValue() )
+		if( configPlug()->enabledPlug()->getValue() )
 		{
-			result["ocio:config"] = new StringData( configValuePlug()->getValue() );
+			result["ocio:config"] = new StringData( configPlug()->valuePlug<StringPlug>()->getValue() );
 		}
 
-		if( workingSpaceEnabledPlug()->getValue() )
+		if( workingSpacePlug()->enabledPlug()->getValue() )
 		{
-			result["ocio:workingSpace"] = new StringData( workingSpaceValuePlug()->getValue() );
+			result["ocio:workingSpace"] = new StringData( workingSpacePlug()->valuePlug<StringPlug>()->getValue() );
 		}
 
 		ConstCompoundDataPtr extraVariables = extraVariablesPlug()->getValue();

src/GafferModule/StringPlugBinding.cpp

@@ -63,12 +63,12 @@ void setPathValue( StringPlug *plug, const std::filesystem::path &value )
 	plug->setValue( value );
 }
 
-std::string getValue( const StringPlug *plug, const IECore::MurmurHash *precomputedHash )
+std::string getValue( const StringPlug *plug )
 {
 	// Must release GIL in case computation spawns threads which need
 	// to reenter Python.
 	IECorePython::ScopedGILRelease r;
-	return plug->getValue( precomputedHash );
+	return plug->getValue();
 }
 
 std::string substitutionsRepr( unsigned substitutions )
@@ -161,7 +161,7 @@ void GafferModule::bindStringPlug()
 		// Must be registered before string-based `setValue()`, to give it weaker overloading precedence.
 		.def( "setValue", &setPathValue )
 		.def( "setValue", &setValue )
-		.def( "getValue", &getValue, ( boost::python::arg( "_precomputedHash" ) = object() ) )
+		.def( "getValue", &getValue )
 	;
 
 	s.attr( "ValueType" ) = boost::python::object( boost::python::handle<>( boost::python::borrowed( &PyUnicode_Type ) ) );