Skip to content

Commit

Permalink
fixup! USDLayerWriter : Refactor using parallelReduceLocations
Browse files Browse the repository at this point in the history
- Add unit test to expose bug.
- Fix child merging operation, which was using path prefix from the children rather than the parent.
- Rename merging functions to make the problem/solution clearer.
- Remove unnecessary `path` argument.
  • Loading branch information
johnhaddon committed Feb 11, 2025
1 parent 8135f7e commit a33e1a7
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 19 deletions.
30 changes: 30 additions & 0 deletions python/GafferUSDTest/USDLayerWriterTest.py
Original file line number Diff line number Diff line change
Expand Up @@ -418,5 +418,35 @@ def testDispatch( self ) :

self.assertTrue( ( self.temporaryDirectory() / "test.usda" ).is_file() )

def testIdenticalNamesInDifferentGroups( self ) :

# /plane <- modified in layer
# /group
# /plane <- identical

plane = GafferScene.Plane()

group = GafferScene.Group()
group["in"][0].setInput( plane["out"] )

parent = GafferScene.Parent()
parent["in"].setInput( plane["out"] )
parent["children"][0].setInput( group["out"] )
parent["parent"].setValue( "/" )

pathFilter = GafferScene.PathFilter()
pathFilter["paths"].setValue( IECore.StringVectorData( [ "/plane" ] ) )

primitiveVariables = GafferScene.PrimitiveVariables()
primitiveVariables["in"].setInput( parent["out"] )
primitiveVariables["filter"].setInput( pathFilter["out"] )
primitiveVariables["primitiveVariables"].addChild( Gaffer.NameValuePlug( "a", 10 ) )

layerFileName, compositionFileName = self.__writeLayerAndComposition( parent["out"], primitiveVariables["out"] )

reader = GafferScene.SceneReader()
reader["fileName"].setValue( compositionFileName )
self.assertScenesEqual( primitiveVariables["out"], reader["out"], checks = self.allSceneChecks - { "sets" } )

if __name__ == "__main__":
unittest.main()
35 changes: 16 additions & 19 deletions src/GafferUSD/USDLayerWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,22 +85,20 @@ struct Filters
PathMatcher deleteAttributes;
ScenePlug::ScenePath localPath;

// Merges in filters from sibling locations.
void add( const Filters &other )
void mergeSibling( const Filters &sibling )
{
fullyPruned = fullyPruned && other.fullyPruned;
prune.addPaths( other.prune );
deleteAttributes.addPaths( other.deleteAttributes );
deleteObject.addPaths( other.deleteObject );
fullyPruned = fullyPruned && sibling.fullyPruned;
prune.addPaths( sibling.prune );
deleteAttributes.addPaths( sibling.deleteAttributes );
deleteObject.addPaths( sibling.deleteObject );
}

// Merges in filters from child locations.
void addWithPrefix( const Filters &other )
void mergeChildren( const Filters &children )
{
fullyPruned = fullyPruned && other.fullyPruned;
prune.addPaths( other.prune, other.localPath );
deleteAttributes.addPaths( other.deleteAttributes, other.localPath );
deleteObject.addPaths( other.deleteObject, other.localPath );
fullyPruned = fullyPruned && children.fullyPruned;
prune.addPaths( children.prune, localPath );
deleteAttributes.addPaths( children.deleteAttributes, localPath );
deleteObject.addPaths( children.deleteObject, localPath );
}

};
Expand All @@ -110,7 +108,7 @@ struct Filters
// - Objects with identical hashes will be included in `Filter::deleteObject`.
// - Attributes with identical hashes will be included in `Filter::deleteAttributes`.
// - Subtrees which are identical in all properties will be included in `Filter::prune`.
Filters buildFilters( const GafferScene::ScenePlug *baseScene, const GafferScene::ScenePlug *layerScene, const std::vector<float> &frames, const ScenePlug::ScenePath &path )
Filters buildFilters( const GafferScene::ScenePlug *baseScene, const GafferScene::ScenePlug *layerScene, const std::vector<float> &frames )
{
Context::EditableScope childNamesScope( Context::current() );

Expand Down Expand Up @@ -181,17 +179,16 @@ Filters buildFilters( const GafferScene::ScenePlug *baseScene, const GafferScene
},
[]( Filters &result, const Filters &childrenResult )
{
result.addWithPrefix( childrenResult );
result.mergeChildren( childrenResult );
if( result.fullyPruned )
{
result.prune.addPath( result.localPath );
}
},
[]( Filters &result, const Filters &sibling )
[]( Filters &result, const Filters &siblingResult )
{
result.add( sibling );
},
path
result.mergeSibling( siblingResult );
}
);
}

Expand Down Expand Up @@ -521,7 +518,7 @@ void USDLayerWriter::executeSequence( const std::vector<float> &frames ) const

// Figure out the filters for our Prune, DeleteObject and DeleteAttribute
// nodes.
const Filters filters = buildFilters( basePlug(), layerPlug(), frames, ScenePlug::ScenePath() );
const Filters filters = buildFilters( basePlug(), layerPlug(), frames );

// Pass the filter settings via context variables since we can't call
// `Plug::setValue()` from `executeSequence()` because it would violate
Expand Down

0 comments on commit a33e1a7

Please sign in to comment.