diff --git a/Changes.md b/Changes.md index 22f6dbdef9..c01b68fa6a 100644 --- a/Changes.md +++ b/Changes.md @@ -30,6 +30,9 @@ Improvements - Editor : The node graph is now evaluated in a context determined relative to the focus node. - LightEditor, RenderPassEditor : The "Disable Edit" right-click menu item and D shortcut now act as a toggle, where edits disabled in the current session via these actions can be reenabled with D or by selecting "Reenable Edit" from the right-click menu. - EditScope : Setting a Viewer or Editor's target edit scope to "None" will now prevent edits from being made within any upstream edit scope. To make edits in an edit scope, it must be set as the target. +- FreezeTransform : + - Improved performance for large meshes by using multithreading. + - Improved UI responsiveness by supporting cancellation of long computes. Fixes ----- diff --git a/include/GafferScene/FreezeTransform.h b/include/GafferScene/FreezeTransform.h index 8a253c31ae..98af375b60 100644 --- a/include/GafferScene/FreezeTransform.h +++ b/include/GafferScene/FreezeTransform.h @@ -58,6 +58,8 @@ class GAFFERSCENE_API FreezeTransform : public FilteredSceneProcessor void hash( const Gaffer::ValuePlug *output, const Gaffer::Context *context, IECore::MurmurHash &h ) const override; void compute( Gaffer::ValuePlug *output, const Gaffer::Context *context ) const override; + Gaffer::ValuePlug::CachePolicy computeCachePolicy( const Gaffer::ValuePlug *output ) const override; + void hashBound( const ScenePath &path, const Gaffer::Context *context, const ScenePlug *parent, IECore::MurmurHash &h ) const override; void hashTransform( const ScenePath &path, const Gaffer::Context *context, const ScenePlug *parent, IECore::MurmurHash &h ) const override; void hashObject( const ScenePath &path, const Gaffer::Context *context, const ScenePlug *parent, IECore::MurmurHash &h ) const override; @@ -72,6 +74,13 @@ class GAFFERSCENE_API FreezeTransform : public FilteredSceneProcessor Gaffer::M44fPlug *transformPlug(); const Gaffer::M44fPlug *transformPlug() const; + /// We compute the processed object on this internal plug rather than on + /// `out.object` directly. This allows us to use the TaskCollaboration + /// task policy for processing objects without paying the overhead when + /// we're just passing them through (when the filter doesn't match). + Gaffer::ObjectPlug *processedObjectPlug(); + const Gaffer::ObjectPlug *processedObjectPlug() const; + static size_t g_firstPlugIndex; }; diff --git a/python/GafferSceneTest/FreezeTransformTest.py b/python/GafferSceneTest/FreezeTransformTest.py index 37ddf04705..a8c32c528d 100644 --- a/python/GafferSceneTest/FreezeTransformTest.py +++ b/python/GafferSceneTest/FreezeTransformTest.py @@ -105,7 +105,8 @@ def testFilter( self ) : def testAffects( self ) : t = GafferScene.FreezeTransform() - self.assertEqual( set( t.affects( t["in"]["object"] ) ), set( [ t["out"]["object"] ] ) ) + self.assertEqual( set( t.affects( t["in"]["object"] ) ), set( [ t["out"]["object"], t["__processedObject"] ] ) ) + self.assertEqual( set( t.affects( t["__processedObject"] ) ), set( [ t["out"]["object"] ] ) ) def testSetFilter( self ) : diff --git a/src/GafferScene/FreezeTransform.cpp b/src/GafferScene/FreezeTransform.cpp index 2d5a9c2c7f..5c1c6accea 100644 --- a/src/GafferScene/FreezeTransform.cpp +++ b/src/GafferScene/FreezeTransform.cpp @@ -43,6 +43,7 @@ #include "IECore/DataAlgo.h" #include "IECore/TypeTraits.h" +#include "IECore/NullObject.h" using namespace std; using namespace Imath; @@ -60,6 +61,7 @@ FreezeTransform::FreezeTransform( const std::string &name ) { storeIndexOfNextChild( g_firstPlugIndex ); addChild( new M44fPlug( "__transform", Plug::Out ) ); + addChild( new ObjectPlug( "__processedObject", Plug::Out, NullObject::defaultNullObject() ) ); outPlug()->childBoundsPlug()->setFlags( Plug::AcceptsDependencyCycles, true ); @@ -85,6 +87,16 @@ const Gaffer::M44fPlug *FreezeTransform::transformPlug() const return getChild( g_firstPlugIndex ); } +Gaffer::ObjectPlug *FreezeTransform::processedObjectPlug() +{ + return getChild( g_firstPlugIndex + 1 ); +} + +const Gaffer::ObjectPlug *FreezeTransform::processedObjectPlug() const +{ + return getChild( g_firstPlugIndex + 1 ); +} + void FreezeTransform::affects( const Gaffer::Plug *input, AffectedPlugsContainer &outputs ) const { FilteredSceneProcessor::affects( input, outputs ); @@ -116,10 +128,18 @@ void FreezeTransform::affects( const Gaffer::Plug *input, AffectedPlugsContainer } if( - input == filterPlug() || input == inPlug()->objectPlug() || input == transformPlug() ) + { + outputs.push_back( processedObjectPlug() ); + } + + if( + input == filterPlug() || + input == inPlug()->objectPlug() || + input == processedObjectPlug() + ) { outputs.push_back( outPlug()->objectPlug() ); } @@ -131,9 +151,14 @@ void FreezeTransform::hash( const Gaffer::ValuePlug *output, const Gaffer::Conte if( output == transformPlug() ) { - const ScenePath &scenePath = context->get( ScenePlug::scenePathContextName ); - h.append( inPlug()->fullTransformHash( scenePath ) ); - h.append( outPlug()->fullTransformHash( scenePath ) ); + const ScenePath &path = context->get( ScenePlug::scenePathContextName ); + h.append( inPlug()->fullTransformHash( path ) ); + h.append( outPlug()->fullTransformHash( path ) ); + } + else if( output == processedObjectPlug() ) + { + inPlug()->objectPlug()->hash( h ); + transformPlug()->hash( h ); } } @@ -143,17 +168,49 @@ void FreezeTransform::compute( Gaffer::ValuePlug *output, const Gaffer::Context { /// \todo Would it speed things up if we computed this from the parent full transforms and /// the local transforms? So we don't traverse the full path at each location? - const ScenePath &scenePath = context->get( ScenePlug::scenePathContextName ); - const M44f inTransform = inPlug()->fullTransform( scenePath ); - const M44f outTransform = outPlug()->fullTransform( scenePath ); + const ScenePath &path = context->get( ScenePlug::scenePathContextName ); + const M44f inTransform = inPlug()->fullTransform( path ); + const M44f outTransform = outPlug()->fullTransform( path ); const M44f transform = inTransform * outTransform.inverse(); static_cast( output )->setValue( transform ); return; } + else if( output == processedObjectPlug() ) + { + ConstObjectPtr inputObject = inPlug()->objectPlug()->getValue(); + ConstObjectPtr result; + const Primitive *inputPrimitive = runTimeCast( inputObject.get() ); + if( !inputPrimitive ) + { + result = inputObject; + } + else + { + PrimitivePtr outputPrimitive = inputPrimitive->copy(); + const M44f transform = transformPlug()->getValue(); + IECoreScenePreview::PrimitiveAlgo::transformPrimitive( *outputPrimitive, transform, context->canceller() ); + result = std::move( outputPrimitive ); + } + + static_cast( output )->setValue( result ); + return; + } FilteredSceneProcessor::compute( output, context ); } +Gaffer::ValuePlug::CachePolicy FreezeTransform::computeCachePolicy( const Gaffer::ValuePlug *output ) const +{ + if( output == processedObjectPlug() ) + { + return ValuePlug::CachePolicy::TaskCollaboration; + } + else + { + return FilteredSceneProcessor::computeCachePolicy( output ); + } +} + void FreezeTransform::hashBound( const ScenePath &path, const Gaffer::Context *context, const ScenePlug *parent, IECore::MurmurHash &h ) const { const unsigned m = filterValue( context ); @@ -225,38 +282,22 @@ Imath::M44f FreezeTransform::computeTransform( const ScenePath &path, const Gaff void FreezeTransform::hashObject( const ScenePath &path, const Gaffer::Context *context, const ScenePlug *parent, IECore::MurmurHash &h ) const { - const unsigned m = filterValue( context ); - if( m & ( IECore::PathMatcher::AncestorMatch | IECore::PathMatcher::ExactMatch ) ) + if( filterValue( context ) & ( IECore::PathMatcher::AncestorMatch | IECore::PathMatcher::ExactMatch ) ) { - FilteredSceneProcessor::hashObject( path, context, parent, h ); - inPlug()->objectPlug()->hash( h ); - transformPlug()->hash( h ); + h = processedObjectPlug()->hash(); } else { + // pass through h = inPlug()->objectPlug()->hash(); } } IECore::ConstObjectPtr FreezeTransform::computeObject( const ScenePath &path, const Gaffer::Context *context, const ScenePlug *parent ) const { - const unsigned m = filterValue( context ); - if( m & ( IECore::PathMatcher::AncestorMatch | IECore::PathMatcher::ExactMatch ) ) + if( filterValue( context ) & ( IECore::PathMatcher::AncestorMatch | IECore::PathMatcher::ExactMatch ) ) { - ConstObjectPtr inputObject = inPlug()->objectPlug()->getValue(); - const Primitive *inputPrimitive = runTimeCast( inputObject.get() ); - if( !inputPrimitive ) - { - return inputObject; - } - - PrimitivePtr outputPrimitive = inputPrimitive->copy(); - - const M44f transform = transformPlug()->getValue(); - - IECoreScenePreview::PrimitiveAlgo::transformPrimitive( *outputPrimitive, transform, context->canceller() ); - - return outputPrimitive; + return processedObjectPlug()->getValue(); } else { diff --git a/src/GafferScene/IECoreScenePreview/PrimitiveAlgo.cpp b/src/GafferScene/IECoreScenePreview/PrimitiveAlgo.cpp index 09081fe6e2..a25f830f45 100644 --- a/src/GafferScene/IECoreScenePreview/PrimitiveAlgo.cpp +++ b/src/GafferScene/IECoreScenePreview/PrimitiveAlgo.cpp @@ -1381,14 +1381,17 @@ void PrimitiveAlgo::transformPrimitive( { std::vector< Imath::V3f >& writable = vecVar->writable(); - for( size_t i = 0; i < writable.size(); i++ ) - { - Canceller::check( canceller ); - transformPrimVarValue( - &writable[i], &writable[i], 1, - matrix, normalMatrix, interp - ); - }; + tbb::parallel_for( + tbb::blocked_range( 0, writable.size(), 10000 ), + [&]( tbb::blocked_range &range ) + { + Canceller::check( canceller ); + transformPrimVarValue( + &writable[range.begin()], &writable[range.begin()], range.end() - range.begin(), + matrix, normalMatrix, interp + ); + } + ); } else {