diff --git a/Changes.md b/Changes.md index 75e0458c52..85f7919576 100644 --- a/Changes.md +++ b/Changes.md @@ -9,6 +9,11 @@ Fixes - Fixed "colour tearing", where updates to some image channels became visible before updates to others. - Fixed unnecessary texture updates when specific image tiles don't change. +Breaking Changes +---------------- + +- Instancer : `encapsulate` now produces a single Capsule at the `.../instances` location, instead of the previous `encapsulateInstanceGroups` plug which produced separate capsules at each `.../instances/` location. Outputting a single capsule is slightly less flexible for users, but is much faster to render in Arnold ( Arnold is currently unable to deal effectively with large numbers of fully overlapping procedurals ). This change also allowed us to simplify the code slightly, reducing the time spent in Gaffer when rendering an encapsulated Instancer by around 20%. + [^1]: To be omitted from 1.5.0.0 release notes. 1.5.0.0a1 (relative to 1.4.13.0) @@ -138,7 +143,6 @@ Breaking Changes - ImageGadget : Remove non-const variant of `getContext()`. - LazyMethod : `deferUntilPlaybackStops` now requires that the Widget has a `scriptNode()` method rather than a `context()` method. - Python : Gaffer now disables the user site-packages directory by setting `PYTHONNOUSERSITE=1`. To revert to the previous behaviour, set `PYTHONNOUSERSITE=0` before launching Gaffer. -- Instancer : `encapsulateInstanceGroups` now produces a single Capsule at the `.../instances` location, instead of producing separate capsules at each `.../instances/` location. Outputting a single capsule is slightly less flexible for users, but is much faster to render in Arnold ( Arnold is currently unable to deal effectively with large numbers of fully overlapping procedurals ). This change also allowed us to simplify the code slightly, reducing the time spent in Gaffer when rendering an encapsulated Instancer by around 20%. Build ----- diff --git a/include/GafferScene/Instancer.h b/include/GafferScene/Instancer.h index 74ec532526..3a01bead8b 100644 --- a/include/GafferScene/Instancer.h +++ b/include/GafferScene/Instancer.h @@ -144,8 +144,8 @@ class GAFFERSCENE_API Instancer : public BranchCreator Gaffer::StringPlug *attributePrefixPlug(); const Gaffer::StringPlug *attributePrefixPlug() const; - Gaffer::BoolPlug *encapsulateInstanceGroupsPlug(); - const Gaffer::BoolPlug *encapsulateInstanceGroupsPlug() const; + Gaffer::BoolPlug *encapsulatePlug(); + const Gaffer::BoolPlug *encapsulatePlug() const; Gaffer::BoolPlug *seedEnabledPlug(); const Gaffer::BoolPlug *seedEnabledPlug() const; @@ -246,6 +246,7 @@ class GAFFERSCENE_API Instancer : public BranchCreator void engineHash( const ScenePath &sourcePath, const Gaffer::Context *context, IECore::MurmurHash &h ) const; ConstEngineSplitPrototypesDataPtr engineSplitPrototypes( const ScenePath &sourcePath, const Gaffer::Context *context ) const; + void engineSplitPrototypesHash( const ScenePath &sourcePath, const Gaffer::Context *context, IECore::MurmurHash &h ) const; struct PrototypeScope : public Gaffer::Context::EditableScope { diff --git a/python/GafferSceneTest/InstancerTest.py b/python/GafferSceneTest/InstancerTest.py index eea249caf8..818fad3dea 100644 --- a/python/GafferSceneTest/InstancerTest.py +++ b/python/GafferSceneTest/InstancerTest.py @@ -70,7 +70,7 @@ def assertEncapsulatedRendersSame( self, instancer ): continue corresponding.setValue( i.getValue() ) - encapInstancer["encapsulateInstanceGroups"].setValue( True ) + encapInstancer["encapsulate"].setValue( True ) self.assertScenesRenderSame( instancer["out"], encapInstancer["out"], expandProcedurals = True, ignoreLinks = True ) @@ -171,7 +171,7 @@ def test( self ) : encapInstancer["prototypes"].setInput( instanceInput["out"] ) encapInstancer["parent"].setValue( "/seeds" ) encapInstancer["name"].setValue( "instances" ) - encapInstancer["encapsulateInstanceGroups"].setValue( True ) + encapInstancer["encapsulate"].setValue( True ) # Test an edge case, and make sure while we're at it that we're actually getting an InstancerCapsule # ( Because it's private, it's bound to Python in a sorta weird way that means its typeName() will @@ -880,7 +880,7 @@ def testAnimation( self ) : with self.assertRaisesRegex( RuntimeError, 'Instancer.out.transform : Instance id "2" is invalid, instancer produces only 2 children. Topology may have changed during shutter.' ): self.assertScenesRenderSame( instancer["out"], instancer["out"], expandProcedurals = True, ignoreLinks = True ) - instancer["encapsulateInstanceGroups"].setValue( True ) + instancer["encapsulate"].setValue( True ) with testContext: with self.assertRaisesRegex( RuntimeError, 'Instance id "2" is invalid, instancer produces only 2 children. Topology may have changed during shutter.' ): @@ -1432,7 +1432,7 @@ def testSets( self ) : encapInstancer["prototypes"].setInput( instances["out"] ) encapInstancer["parent"].setValue( "/object" ) encapInstancer["prototypeIndex"].setValue( "index" ) - encapInstancer["encapsulateInstanceGroups"].setValue( True ) + encapInstancer["encapsulate"].setValue( True ) unencapFilter = GafferScene.PathFilter() unencapFilter["paths"].setValue( IECore.StringVectorData( [ "/..." ] ) ) @@ -2594,7 +2594,7 @@ def testContextSet( self ): # When encapsulating, we shouldn't pay any time cost for evaluating the set, even with a huge # number of instances plane["divisions"].setValue( imath.V2i( 1000 ) ) - instancer["encapsulateInstanceGroups"].setValue( True ) + instancer["encapsulate"].setValue( True ) t = time.time() instancer["out"].set( "set" ) totalTime = time.time() - t @@ -2836,7 +2836,7 @@ def testPrototypePropertiesAffectCapsule( self ) : with self.subTest( encapsulate = encapsulate ) : - instancer["encapsulateInstanceGroups"].setValue( encapsulate ) + instancer["encapsulate"].setValue( encapsulate ) # Prototype properties used by the capsule should be reflected # in the object hash. For the hash to be updated successfully, @@ -3115,7 +3115,7 @@ def testChildNamesPerfWithPrototypesAndIds( self ): @GafferTest.TestRunner.PerformanceTestMethod() def testEncapsulatedRenderPerf( self ): nodes = self.initSimpleInstancer( withPrototypes = True, withIds = False ) - nodes["instancer"]["encapsulateInstanceGroups"].setValue( True ) + nodes["instancer"]["encapsulate"].setValue( True ) renderer = GafferScene.Private.IECoreScenePreview.CapturingRenderer( GafferScene.Private.IECoreScenePreview.Renderer.RenderType.Batch ) diff --git a/python/GafferSceneUI/InstancerUI.py b/python/GafferSceneUI/InstancerUI.py index 2e89a97dec..d2791ebdf0 100644 --- a/python/GafferSceneUI/InstancerUI.py +++ b/python/GafferSceneUI/InstancerUI.py @@ -576,11 +576,11 @@ def __init__( self, headings, toolTipOverride = "" ) : ], - "encapsulateInstanceGroups" : [ + "encapsulate" : [ "description", """ - Converts each group of instances into a capsule, which won't + Converts instances into a capsule, which won't be expanded until you Unencapsulate or render. When keeping these locations encapsulated, downstream nodes can't see the instance locations, which prevents editing but improves @@ -588,8 +588,10 @@ def __init__( self, headings, toolTipOverride = "" ) : Encapsulate node because it has the following benefits : - Substantially improved performance when the prototypes - define sets. + define sets. - Fewer unnecessary updates during interactive rendering. + - Faster performance in renderer backends with special + instancer capsule support ( ie. Arnold ) """, "label", "Instance Groups", diff --git a/src/GafferScene/Instancer.cpp b/src/GafferScene/Instancer.cpp index 231dcf077b..a6f6647160 100644 --- a/src/GafferScene/Instancer.cpp +++ b/src/GafferScene/Instancer.cpp @@ -342,7 +342,8 @@ class Instancer::EngineData : public Data m_orientations( nullptr ), m_scales( nullptr ), m_uniformScales( nullptr ), - m_prototypeContextVariables( prototypeContextVariables ) + m_prototypeContextVariables( prototypeContextVariables ), + m_hasDuplicates( false ) { if( !m_primitive ) { @@ -395,7 +396,6 @@ class Instancer::EngineData : public Data } } - m_hasDuplicates = false; if( m_ids ) { for( size_t i = 0, e = numPoints(); i < e; ++i ) @@ -1202,7 +1202,7 @@ Instancer::Instancer( const std::string &name ) addChild( new StringPlug( "scale", Plug::In ) ); addChild( new StringPlug( "attributes", Plug::In ) ); addChild( new StringPlug( "attributePrefix", Plug::In ) ); - addChild( new BoolPlug( "encapsulateInstanceGroups", Plug::In ) ); + addChild( new BoolPlug( "encapsulate", Plug::In ) ); addChild( new BoolPlug( "seedEnabled", Plug::In ) ); addChild( new StringPlug( "seedVariable", Plug::In, "seed" ) ); addChild( new IntPlug( "seeds", Plug::In, 10, 1 ) ); @@ -1361,12 +1361,12 @@ const Gaffer::StringPlug *Instancer::attributePrefixPlug() const return getChild( g_firstPlugIndex + 12 ); } -Gaffer::BoolPlug *Instancer::encapsulateInstanceGroupsPlug() +Gaffer::BoolPlug *Instancer::encapsulatePlug() { return getChild( g_firstPlugIndex + 13 ); } -const Gaffer::BoolPlug *Instancer::encapsulateInstanceGroupsPlug() const +const Gaffer::BoolPlug *Instancer::encapsulatePlug() const { return getChild( g_firstPlugIndex + 13 ); } @@ -1532,8 +1532,8 @@ void Instancer::affects( const Plug *input, AffectedPlugsContainer &outputs ) co // For the affects of our output plug, we can mostly rely on BranchCreator's mechanism driven // by affectsBranchObject etc., but for these 3 plugs, we have an overridden hash/compute // which in addition to everything that BranchCreator handles, are also affected by - // encapsulateInstanceGroupsPlug() - if( input == encapsulateInstanceGroupsPlug() ) + // encapsulatePlug() + if( input == encapsulatePlug() ) { outputs.push_back( outPlug()->objectPlug() ); outputs.push_back( outPlug()->childNamesPlug() ); @@ -1543,7 +1543,7 @@ void Instancer::affects( const Plug *input, AffectedPlugsContainer &outputs ) co if( input->parent() == prototypesPlug() && input != prototypesPlug()->globalsPlug() && - !encapsulateInstanceGroupsPlug()->isSetToDefault() + !encapsulatePlug()->isSetToDefault() ) { outputs.push_back( outPlug()->objectPlug() ); @@ -1605,7 +1605,7 @@ void Instancer::hash( const Gaffer::ValuePlug *output, const Gaffer::Context *co scalePlug()->hash( h ); attributesPlug()->hash( h ); attributePrefixPlug()->hash( h ); - encapsulateInstanceGroupsPlug()->hash( h ); + encapsulatePlug()->hash( h ); seedEnabledPlug()->hash( h ); seedVariablePlug()->hash( h ); @@ -2289,7 +2289,7 @@ IECore::ConstObjectPtr Instancer::computeBranchObject( const ScenePath &sourcePa void Instancer::hashObject( const ScenePath &path, const Gaffer::Context *context, const ScenePlug *parent, IECore::MurmurHash &h ) const { - if( parent != capsuleScenePlug() && encapsulateInstanceGroupsPlug()->getValue() ) + if( parent != capsuleScenePlug() && encapsulatePlug()->getValue() ) { // Handling this special case here means an extra call to sourceAndBranchPaths // when we're encapsulating and we're not inside a branch - this is a small @@ -2322,7 +2322,7 @@ void Instancer::hashObject( const ScenePath &path, const Gaffer::Context *contex IECore::ConstObjectPtr Instancer::computeObject( const ScenePath &path, const Gaffer::Context *context, const ScenePlug *parent ) const { - if( parent != capsuleScenePlug() && encapsulateInstanceGroupsPlug()->getValue() ) + if( parent != capsuleScenePlug() && encapsulatePlug()->getValue() ) { ScenePath sourcePath, branchPath; parentAndBranchPaths( path, sourcePath, branchPath ); @@ -2368,7 +2368,7 @@ void Instancer::hashBranchChildNames( const ScenePath &sourcePath, const ScenePa { // "/instances/" BranchCreator::hashBranchChildNames( sourcePath, branchPath, context, h ); - engineHash( sourcePath, context, h ); + engineSplitPrototypesHash( sourcePath, context, h ); h.append( branchPath.back() ); } else @@ -2444,7 +2444,7 @@ IECore::ConstInternedStringVectorDataPtr Instancer::computeBranchChildNames( con void Instancer::hashChildNames( const ScenePath &path, const Gaffer::Context *context, const ScenePlug *parent, IECore::MurmurHash &h ) const { - if( parent != capsuleScenePlug() && encapsulateInstanceGroupsPlug()->getValue() ) + if( parent != capsuleScenePlug() && encapsulatePlug()->getValue() ) { ScenePath sourcePath, branchPath; parentAndBranchPaths( path, sourcePath, branchPath ); @@ -2460,7 +2460,7 @@ void Instancer::hashChildNames( const ScenePath &path, const Gaffer::Context *co IECore::ConstInternedStringVectorDataPtr Instancer::computeChildNames( const ScenePath &path, const Gaffer::Context *context, const ScenePlug *parent ) const { - if( parent != capsuleScenePlug() && encapsulateInstanceGroupsPlug()->getValue() ) + if( parent != capsuleScenePlug() && encapsulatePlug()->getValue() ) { ScenePath sourcePath, branchPath; parentAndBranchPaths( path, sourcePath, branchPath ); @@ -2584,7 +2584,7 @@ IECore::ConstPathMatcherDataPtr Instancer::computeBranchSet( const ScenePath &so void Instancer::hashSet( const IECore::InternedString &setName, const Gaffer::Context *context, const ScenePlug *parent, IECore::MurmurHash &h ) const { - if( parent != capsuleScenePlug() && encapsulateInstanceGroupsPlug()->getValue() ) + if( parent != capsuleScenePlug() && encapsulatePlug()->getValue() ) { h = inPlug()->setPlug()->hash(); return; @@ -2595,7 +2595,7 @@ void Instancer::hashSet( const IECore::InternedString &setName, const Gaffer::Co IECore::ConstPathMatcherDataPtr Instancer::computeSet( const IECore::InternedString &setName, const Gaffer::Context *context, const ScenePlug *parent ) const { - if( parent != capsuleScenePlug() && encapsulateInstanceGroupsPlug()->getValue() ) + if( parent != capsuleScenePlug() && encapsulatePlug()->getValue() ) { return inPlug()->setPlug()->getValue(); } @@ -2621,6 +2621,12 @@ Instancer::ConstEngineSplitPrototypesDataPtr Instancer::engineSplitPrototypes( c return boost::static_pointer_cast( engineSplitPrototypesPlug()->getValue() ); } +void Instancer::engineSplitPrototypesHash( const ScenePath &sourcePath, const Gaffer::Context *context, IECore::MurmurHash &h ) const +{ + ScenePlug::PathScope scope( context, &sourcePath ); + engineSplitPrototypesPlug()->hash( h ); +} + const std::type_info &Instancer::instancerCapsuleTypeInfo() { return typeid( InstancerCapsule ); @@ -2918,9 +2924,9 @@ void Instancer::InstancerCapsule::render( IECoreScenePreview::Renderer *renderer std::vector< std::string > names( engines[0]->numValidPrototypes() ); std::vector< int > namePrefixLengths( engines[0]->numValidPrototypes() ); - for( size_t i = r.begin(); i != r.end(); ++i ) + for( size_t pointIndex = r.begin(); pointIndex != r.end(); ++pointIndex ) { - int protoIndex = engines[0]->prototypeIndex( i ); + int protoIndex = engines[0]->prototypeIndex( pointIndex ); if( protoIndex == -1 ) { // Invalid prototype @@ -2948,7 +2954,7 @@ void Instancer::InstancerCapsule::render( IECoreScenePreview::Renderer *renderer // general instead of assuming that frame is the only variable for which offsetMode may be set? ) prototypeScope.setFrame( onFrameTime ); - engines[0]->setPrototypeContextVariables( i, prototypeScope ); + engines[0]->setPrototypeContextVariables( pointIndex, prototypeScope ); proto = prototypeCache.get( PrototypeCacheGetterKey( protoIndex, prototypeScope.context() ) ).get(); } @@ -2971,7 +2977,7 @@ void Instancer::InstancerCapsule::render( IECoreScenePreview::Renderer *renderer // careful not to modify them though! currentAttributes->members() = proto->m_attributes->members(); - engines[0]->instanceAttributes( i, *currentAttributes ); + engines[0]->instanceAttributes( pointIndex, *currentAttributes ); attribsStorage = renderer->attributes( currentAttributes.get() ); attribs = attribsStorage.get(); } @@ -2980,7 +2986,7 @@ void Instancer::InstancerCapsule::render( IECoreScenePreview::Renderer *renderer attribs = proto->m_rendererAttributes.get(); } - int instanceId = engines[0]->instanceId( i ); + int instanceId = engines[0]->instanceId( pointIndex ); if( !namePrefixLengths[protoIndex] ) @@ -3020,14 +3026,14 @@ void Instancer::InstancerCapsule::render( IECoreScenePreview::Renderer *renderer if( sampleTimes.size() == 1 ) { - objectInterface->transform( proto->m_transforms[0] * engines[0]->instanceTransform( i ) ); + objectInterface->transform( proto->m_transforms[0] * engines[0]->instanceTransform( pointIndex ) ); } else { - for( unsigned int j = 0; j < engines.size(); j++ ) + for( unsigned int i = 0; i < engines.size(); i++ ) { - int curPointIndex = j == 0 ? i : engines[j]->pointIndex( instanceId ); - pointTransforms[j] = proto->m_transforms[j] * engines[j]->instanceTransform( curPointIndex ); + int curPointIndex = i == 0 ? pointIndex : engines[i]->pointIndex( instanceId ); + pointTransforms[i] = proto->m_transforms[i] * engines[i]->instanceTransform( curPointIndex ); } objectInterface->transform( pointTransforms, sampleTimes );