TweakPlug : Possible way to expose functionality needed by PrimitiveVariableTweaks

include/Gaffer/TweakPlug.h

@@ -123,12 +123,22 @@ class GAFFER_API TweakPlug : public Gaffer::ValuePlug
 			MissingMode missingMode = MissingMode::Error
 		) const;
 
+		template< typename T >
+		static T applyNumericTweak(
+			const T &source,
+			const T &tweak,
+			TweakPlug::Mode mode,
+			const std::string &tweakName
+		);
+
+		static const char *modeToString( Gaffer::TweakPlug::Mode mode );
+
 	private :
 
 		Gaffer::ValuePlug *valuePlugInternal();
 		const Gaffer::ValuePlug *valuePlugInternal() const;
 
-		void applyNumericTweak(
+		void applyNumericDataTweak(
 			const IECore::Data *sourceData,
 			const IECore::Data *tweakData,
 			IECore::Data *destData,
@@ -144,9 +154,13 @@ class GAFFER_API TweakPlug : public Gaffer::ValuePlug
 			const std::string &tweakName
 		) const;
 
-		void applyReplaceTweak( const IECore::Data *sourceData, IECore::Data *tweakData ) const;
+		template<typename T>
+		static T vectorAwareMin( const T &v1, const T &v2 );
 
-		static const char *modeToString( Gaffer::TweakPlug::Mode mode );
+		template<typename T>
+		static T vectorAwareMax( const T &v1, const T &v2 );
+
+		void applyReplaceTweak( const IECore::Data *sourceData, IECore::Data *tweakData ) const;
 
 };
 

include/Gaffer/TweakPlug.inl

@@ -38,6 +38,8 @@
 
 #include "Gaffer/PlugAlgo.h"
 
+#include "IECore/TypeTraits.h"
+
 #include "fmt/format.h"
 
 namespace Gaffer
@@ -136,7 +138,7 @@ bool TweakPlug::applyTweak(
 		mode == Gaffer::TweakPlug::Max
 	)
 	{
-		applyNumericTweak( currentValue, newData.get(), newData.get(), mode, name );
+		applyNumericDataTweak( currentValue, newData.get(), newData.get(), mode, name );
 	}
 	else if(
 		mode == TweakPlug::ListAppend ||
@@ -179,4 +181,100 @@ bool TweaksPlug::applyTweaks(
 	return tweakApplied;
 }
 
+template<typename T>
+T TweakPlug::vectorAwareMin( const T &v1, const T &v2 )
+{
+	if constexpr( IECore::TypeTraits::IsVec<T>::value || IECore::TypeTraits::IsColor<T>::value )
+	{
+		T result;
+		for( size_t i = 0; i < T::dimensions(); ++i )
+		{
+			result[i] = std::min( v1[i], v2[i] );
+		}
+		return result;
+	}
+	else
+	{
+		return std::min( v1, v2 );
+	}
+}
+
+template<typename T>
+T TweakPlug::vectorAwareMax( const T &v1, const T &v2 )
+{
+	if constexpr( IECore::TypeTraits::IsVec<T>::value || IECore::TypeTraits::IsColor<T>::value )
+	{
+		T result;
+		for( size_t i = 0; i < T::dimensions(); ++i )
+		{
+			result[i] = std::max( v1[i], v2[i] );
+		}
+		return result;
+	}
+	else
+	{
+		return std::max( v1, v2 );
+	}
+}
+
+template< typename T >
+T TweakPlug::applyNumericTweak(
+	const T &source,
+	const T &tweak,
+	TweakPlug::Mode mode,
+	const std::string &tweakName
+)
+{
+	if constexpr(
+		( std::is_arithmetic_v<T> && !std::is_same_v< T, bool > ) ||
+		IECore::TypeTraits::IsVec<T>::value ||
+		IECore::TypeTraits::IsColor<T>::value
+	)
+	{
+		switch( mode )
+		{
+			case TweakPlug::Add :
+				return source + tweak;
+			case TweakPlug::Subtract :
+				return source - tweak;
+			case TweakPlug::Multiply :
+				return source * tweak;
+			case TweakPlug::Min :
+				return vectorAwareMin( source, tweak );
+			case TweakPlug::Max :
+				return vectorAwareMax( source, tweak );
+			case TweakPlug::ListAppend :
+			case TweakPlug::ListPrepend :
+			case TweakPlug::ListRemove :
+			case TweakPlug::Replace :
+			case TweakPlug::Remove :
+			case TweakPlug::Create :
+			case TweakPlug::CreateIfMissing :
+				throw IECore::Exception(
+					fmt::format(
+						"Cannot apply tweak with mode {} using applyNumericTweak.",
+						modeToString( mode )
+					)
+				);
+			default:
+				throw IECore::Exception( fmt::format( "Not a valid tweak mode: {}.", mode ) );
+		}
+	}
+	else
+	{
+		// NOTE: If we are operating on variables that aren't actually stored in a Data, then the
+		// data type reported here may not be technically correct - for example, we might want to
+		// call this on elements of a StringVectorData, in which case this would report a type of
+		// "StringData", but there is nothing of actual type "StringData". This message still
+		// communicates the actual problem though ( we don't support arithmetic on strings ).
+
+		throw IECore::Exception(
+			fmt::format(
+				"Cannot apply tweak with mode {} to \"{}\" : Data type {} not supported.",
+				modeToString( mode ), tweakName, IECore::TypedData<T>::staticTypeName()
+			)
+		);
+	}
+}
+
 } // namespace Gaffer

python/GafferTest/TweakPlugTest.py

@@ -345,6 +345,47 @@ def testTweakInternedString( self ) :
 		self.assertTrue( result )
 		self.assertEqual( data["a"], IECore.InternedStringData( "stringValue" ) )
 
+	def testInvalidNumeric( self ) :
+
+		parameters = IECore.CompoundData(
+			{
+				"a" : "foo",
+				"b" : IECore.IntVectorData( [ 0, 1, 2 ] ),
+				"c" : IECore.StringVectorData( [ "gouda", "cheddar", "cheddar", "swiss" ] ),
+			}
+		)
+
+		tweaks = Gaffer.TweaksPlug()
+		tweaks.addChild( Gaffer.TweakPlug( "a", "foo", Gaffer.TweakPlug.Mode.Add ) )
+		#tweaks.addChild( Gaffer.TweakPlug( "b", imath.Color3f( 1.0, 2.0, 3.0 ) ) )
+		#tweaks.addChild( Gaffer.TweakPlug( "c", imath.V2f( 1.0, 2.0 ) ) )
+		#tweaks.addChild( Gaffer.TweakPlug( "d", IECore.StringVectorData( [ "brie" ] ) ) )
+		#tweaks.addChild( Gaffer.TweakPlug( "f", IECore.StringVectorData( [] ), Gaffer.TweakPlug.Mode.ListAppend ) )
+
+		with self.assertRaisesRegex( Exception,
+			'Cannot apply tweak with mode Add to "a" : Data type StringData not supported.'
+		) :
+			tweaks.applyTweaks( parameters )
+
+		del tweaks[0]
+
+		tweaks.addChild( Gaffer.TweakPlug( "b", IECore.IntVectorData( [ 0, 1, 2 ] ), Gaffer.TweakPlug.Mode.Multiply ) )
+
+		with self.assertRaisesRegex( Exception,
+			'Cannot apply tweak with mode Multiply to "b" : Data type IntVectorData not supported.'
+		) :
+			tweaks.applyTweaks( parameters )
+
+		del tweaks[0]
+
+		tweaks.addChild( Gaffer.TweakPlug( "c", IECore.StringVectorData( [ "foo" ] ), Gaffer.TweakPlug.Mode.Subtract ) )
+
+		with self.assertRaisesRegex( Exception,
+			'Cannot apply tweak with mode Subtract to "c" : Data type StringVectorData not supported.'
+		) :
+			tweaks.applyTweaks( parameters )
+
+
 	def testPathMatcherListOperations( self ) :
 
 		data = IECore.CompoundData(

src/Gaffer/TweakPlug.cpp

@@ -63,50 +63,6 @@ using namespace Gaffer;
 namespace
 {
 
-/// \todo - if these make sense, I guess they should be pushed back to cortex
-
-// IsColorTypedData
-template< typename T > struct IsColorTypedData : boost::mpl::and_< TypeTraits::IsTypedData<T>, TypeTraits::IsColor< typename TypeTraits::ValueType<T>::type > > {};
-
-// SupportsArithmeticData
-template< typename T > struct SupportsArithData : boost::mpl::or_<  TypeTraits::IsNumericSimpleTypedData<T>, TypeTraits::IsVecTypedData<T>, IsColorTypedData<T>> {};
-
-template<typename T>
-T vectorAwareMin( const T &v1, const T &v2 )
-{
-	if constexpr( TypeTraits::IsVec<T>::value || TypeTraits::IsColor<T>::value )
-	{
-		T result;
-		for( size_t i = 0; i < T::dimensions(); ++i )
-		{
-			result[i] = std::min( v1[i], v2[i] );
-		}
-		return result;
-	}
-	else
-	{
-		return std::min( v1, v2 );
-	}
-}
-
-template<typename T>
-T vectorAwareMax( const T &v1, const T &v2 )
-{
-	if constexpr( TypeTraits::IsVec<T>::value || TypeTraits::IsColor<T>::value )
-	{
-		T result;
-		for( size_t i = 0; i < T::dimensions(); ++i )
-		{
-			result[i] = std::max( v1[i], v2[i] );
-		}
-		return result;
-	}
-	else
-	{
-		return std::max( v1, v2 );
-	}
-}
-
 template<typename T>
 vector<T> tweakedList( const std::vector<T> &source, const std::vector<T> &tweak, TweakPlug::Mode mode )
 {
@@ -301,7 +257,39 @@ bool TweakPlug::applyTweak( IECore::CompoundData *parameters, MissingMode missin
 	);
 }
 
-void TweakPlug::applyNumericTweak(
+const char *TweakPlug::modeToString( Gaffer::TweakPlug::Mode mode )
+{
+	switch( mode )
+	{
+		case Gaffer::TweakPlug::Replace :
+			return "Replace";
+		case Gaffer::TweakPlug::Add :
+			return "Add";
+		case Gaffer::TweakPlug::Subtract :
+			return "Subtract";
+		case Gaffer::TweakPlug::Multiply :
+			return "Multiply";
+		case Gaffer::TweakPlug::Remove :
+			return "Remove";
+		case Gaffer::TweakPlug::Create :
+			return  "Create";
+		case Gaffer::TweakPlug::Min :
+			return "Min";
+		case Gaffer::TweakPlug::Max :
+			return "Max";
+		case Gaffer::TweakPlug::ListAppend :
+			return "ListAppend";
+		case Gaffer::TweakPlug::ListPrepend :
+			return "ListPrepend";
+		case Gaffer::TweakPlug::ListRemove :
+			return "ListRemove";
+		case Gaffer::TweakPlug::CreateIfMissing :
+			return "CreateIfMissing";
+	}
+	return  "Invalid";
+}
+
+void TweakPlug::applyNumericDataTweak(
 	const IECore::Data *sourceData,
 	const IECore::Data *tweakData,
 	IECore::Data *destData,
@@ -317,48 +305,13 @@ void TweakPlug::applyNumericTweak(
 
 			using DataType = typename std::remove_pointer<decltype( data )>::type;
 
-			if constexpr( SupportsArithData<DataType>::value ) {
-
+			if constexpr( TypeTraits::IsTypedData< DataType >::value )
+			{
 				const DataType *sourceDataCast = runTimeCast<const DataType>( sourceData );
 				const DataType *tweakDataCast = runTimeCast<const DataType>( tweakData );
 
-				switch( mode )
-				{
-					case TweakPlug::Add :
-						data->writable() = sourceDataCast->readable() + tweakDataCast->readable();
-						break;
-					case TweakPlug::Subtract :
-						data->writable() = sourceDataCast->readable() - tweakDataCast->readable();
-						break;
-					case TweakPlug::Multiply :
-						data->writable() = sourceDataCast->readable() * tweakDataCast->readable();
-						break;
-					case TweakPlug::Min :
-						data->writable() = vectorAwareMin( sourceDataCast->readable(), tweakDataCast->readable() );
-						break;
-					case TweakPlug::Max :
-						data->writable() = vectorAwareMax( sourceDataCast->readable(), tweakDataCast->readable() );
-						break;
-					case TweakPlug::ListAppend :
-					case TweakPlug::ListPrepend :
-					case TweakPlug::ListRemove :
-					case TweakPlug::Replace :
-					case TweakPlug::Remove :
-					case TweakPlug::Create :
-					case TweakPlug::CreateIfMissing :
-						// These cases are unused - we handle them outside of numericTweak.
-						// But the compiler gets unhappy if we don't handle some cases.
-						assert( false );
-						break;
-				}
-			}
-			else
-			{
-				throw IECore::Exception(
-					fmt::format(
-						"Cannot apply tweak with mode {} to \"{}\" : Data type {} not supported.",
-						modeToString( mode ), tweakName, sourceData->typeName()
-					)
+				data->writable() = applyNumericTweak(
+					sourceDataCast->readable(), tweakDataCast->readable(), mode, tweakName
 				);
 			}
 		}
@@ -436,38 +389,6 @@ void TweakPlug::applyReplaceTweak( const IECore::Data *sourceData, IECore::Data
 	}
 }
 
-const char *TweakPlug::modeToString( Gaffer::TweakPlug::Mode mode )
-{
-	switch( mode )
-	{
-		case Gaffer::TweakPlug::Replace :
-			return "Replace";
-		case Gaffer::TweakPlug::Add :
-			return "Add";
-		case Gaffer::TweakPlug::Subtract :
-			return "Subtract";
-		case Gaffer::TweakPlug::Multiply :
-			return "Multiply";
-		case Gaffer::TweakPlug::Remove :
-			return "Remove";
-		case Gaffer::TweakPlug::Create :
-			return  "Create";
-		case Gaffer::TweakPlug::Min :
-			return "Min";
-		case Gaffer::TweakPlug::Max :
-			return "Max";
-		case Gaffer::TweakPlug::ListAppend :
-			return "ListAppend";
-		case Gaffer::TweakPlug::ListPrepend :
-			return "ListPrepend";
-		case Gaffer::TweakPlug::ListRemove :
-			return "ListRemove";
-		case Gaffer::TweakPlug::CreateIfMissing :
-			return "CreateIfMissing";
-	}
-	return  "Invalid";
-}
-
 //////////////////////////////////////////////////////////////////////////
 // TweaksPlug
 //////////////////////////////////////////////////////////////////////////