From ddab904d4b2a9898eda54b593317b13bbe164e58 Mon Sep 17 00:00:00 2001 From: Daniel Dresser Date: Thu, 21 Nov 2024 17:28:29 -0800 Subject: [PATCH 1/2] TweakPlug : Expose applyNumericTweak --- include/Gaffer/TweakPlug.h | 16 ++++- include/Gaffer/TweakPlug.inl | 100 ++++++++++++++++++++++++++++- python/GafferTest/TweakPlugTest.py | 41 ++++++++++++ src/Gaffer/TweakPlug.cpp | 89 ++----------------------- 4 files changed, 160 insertions(+), 86 deletions(-) diff --git a/include/Gaffer/TweakPlug.h b/include/Gaffer/TweakPlug.h index ab67d21cb5..d16933f59c 100644 --- a/include/Gaffer/TweakPlug.h +++ b/include/Gaffer/TweakPlug.h @@ -123,12 +123,20 @@ 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 + ); + 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,6 +152,12 @@ class GAFFER_API TweakPlug : public Gaffer::ValuePlug const std::string &tweakName ) const; + template + static T vectorAwareMin( const T &v1, const T &v2 ); + + template + static T vectorAwareMax( const T &v1, const T &v2 ); + void applyReplaceTweak( const IECore::Data *sourceData, IECore::Data *tweakData ) const; static const char *modeToString( Gaffer::TweakPlug::Mode mode ); diff --git a/include/Gaffer/TweakPlug.inl b/include/Gaffer/TweakPlug.inl index 57a195f603..f7d29a6294 100644 --- a/include/Gaffer/TweakPlug.inl +++ b/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 +T TweakPlug::vectorAwareMin( const T &v1, const T &v2 ) +{ + if constexpr( IECore::TypeTraits::IsVec::value || IECore::TypeTraits::IsColor::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 +T TweakPlug::vectorAwareMax( const T &v1, const T &v2 ) +{ + if constexpr( IECore::TypeTraits::IsVec::value || IECore::TypeTraits::IsColor::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 && !std::is_same_v< T, bool > ) || + IECore::TypeTraits::IsVec::value || + IECore::TypeTraits::IsColor::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::staticTypeName() + ) + ); + } +} + } // namespace Gaffer diff --git a/python/GafferTest/TweakPlugTest.py b/python/GafferTest/TweakPlugTest.py index 772fdd5584..faa3cc4a5b 100644 --- a/python/GafferTest/TweakPlugTest.py +++ b/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( diff --git a/src/Gaffer/TweakPlug.cpp b/src/Gaffer/TweakPlug.cpp index 7a1fb169f3..9b91f22f7f 100644 --- a/src/Gaffer/TweakPlug.cpp +++ b/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, TypeTraits::IsColor< typename TypeTraits::ValueType::type > > {}; - -// SupportsArithmeticData -template< typename T > struct SupportsArithData : boost::mpl::or_< TypeTraits::IsNumericSimpleTypedData, TypeTraits::IsVecTypedData, IsColorTypedData> {}; - -template -T vectorAwareMin( const T &v1, const T &v2 ) -{ - if constexpr( TypeTraits::IsVec::value || TypeTraits::IsColor::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 -T vectorAwareMax( const T &v1, const T &v2 ) -{ - if constexpr( TypeTraits::IsVec::value || TypeTraits::IsColor::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 vector tweakedList( const std::vector &source, const std::vector &tweak, TweakPlug::Mode mode ) { @@ -301,7 +257,7 @@ bool TweakPlug::applyTweak( IECore::CompoundData *parameters, MissingMode missin ); } -void TweakPlug::applyNumericTweak( +void TweakPlug::applyNumericDataTweak( const IECore::Data *sourceData, const IECore::Data *tweakData, IECore::Data *destData, @@ -317,48 +273,13 @@ void TweakPlug::applyNumericTweak( using DataType = typename std::remove_pointer::type; - if constexpr( SupportsArithData::value ) { - + if constexpr( TypeTraits::IsTypedData< DataType >::value ) + { const DataType *sourceDataCast = runTimeCast( sourceData ); const DataType *tweakDataCast = runTimeCast( 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 ); } } From 9453000083bd4b7414b8baeb366f30dedf71a6bf Mon Sep 17 00:00:00 2001 From: Daniel Dresser Date: Mon, 25 Nov 2024 18:21:20 -0800 Subject: [PATCH 2/2] TweakPlug : Expose modeToString --- include/Gaffer/TweakPlug.h | 4 +-- src/Gaffer/TweakPlug.cpp | 64 +++++++++++++++++++------------------- 2 files changed, 34 insertions(+), 34 deletions(-) diff --git a/include/Gaffer/TweakPlug.h b/include/Gaffer/TweakPlug.h index d16933f59c..99b65cee25 100644 --- a/include/Gaffer/TweakPlug.h +++ b/include/Gaffer/TweakPlug.h @@ -131,6 +131,8 @@ class GAFFER_API TweakPlug : public Gaffer::ValuePlug const std::string &tweakName ); + static const char *modeToString( Gaffer::TweakPlug::Mode mode ); + private : Gaffer::ValuePlug *valuePlugInternal(); @@ -160,8 +162,6 @@ class GAFFER_API TweakPlug : public Gaffer::ValuePlug void applyReplaceTweak( const IECore::Data *sourceData, IECore::Data *tweakData ) const; - static const char *modeToString( Gaffer::TweakPlug::Mode mode ); - }; IE_CORE_DECLAREPTR( TweakPlug ) diff --git a/src/Gaffer/TweakPlug.cpp b/src/Gaffer/TweakPlug.cpp index 9b91f22f7f..e9cdc8f4e1 100644 --- a/src/Gaffer/TweakPlug.cpp +++ b/src/Gaffer/TweakPlug.cpp @@ -257,6 +257,38 @@ bool TweakPlug::applyTweak( IECore::CompoundData *parameters, MissingMode missin ); } +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, @@ -357,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 //////////////////////////////////////////////////////////////////////////