From 53b0bbc6a2edab08e4944a89b68d9b2374c59e89 Mon Sep 17 00:00:00 2001 From: Daniel Dresser Date: Tue, 5 Nov 2024 08:27:40 -0800 Subject: [PATCH 1/2] MeshAlgoSegmentTest : Revert changes to test results. --- test/IECoreScene/MeshAlgoSegmentTest.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/IECoreScene/MeshAlgoSegmentTest.py b/test/IECoreScene/MeshAlgoSegmentTest.py index fa5c52d6cb..a51e33f508 100644 --- a/test/IECoreScene/MeshAlgoSegmentTest.py +++ b/test/IECoreScene/MeshAlgoSegmentTest.py @@ -66,8 +66,8 @@ def testCanSegmentUsingIntegerPrimvar( self ) : p12 = imath.V3f( 1, 2, 0 ) p22 = imath.V3f( 2, 2, 0 ) - self.assertEqual( segments[0]["P"].data, IECore.V3fVectorData( [p00, p10, p11, p01, p20, p21], IECore.GeometricData.Interpretation.Point ) ) - self.assertEqual( segments[1]["P"].data, IECore.V3fVectorData( [p01, p11, p12, p02, p21, p22], IECore.GeometricData.Interpretation.Point ) ) + self.assertEqual( segments[0]["P"].data, IECore.V3fVectorData( [p00, p10, p20, p01, p11, p21], IECore.GeometricData.Interpretation.Point ) ) + self.assertEqual( segments[1]["P"].data, IECore.V3fVectorData( [p01, p11, p21, p02, p12, p22], IECore.GeometricData.Interpretation.Point ) ) def testCanSegmentUsingStringPrimvar( self ) : mesh = IECoreScene.MeshPrimitive.createPlane( imath.Box2f( imath.V2f( 0 ), imath.V2f( 2 ) ), imath.V2i( 2 ) ) @@ -95,8 +95,8 @@ def testCanSegmentUsingStringPrimvar( self ) : p12 = imath.V3f( 1, 2, 0 ) p22 = imath.V3f( 2, 2, 0 ) - self.assertEqual( segments[0]["P"].data, IECore.V3fVectorData( [p00, p10, p11, p01, p21, p22, p12], IECore.GeometricData.Interpretation.Point ) ) - self.assertEqual( segments[1]["P"].data, IECore.V3fVectorData( [p10, p20, p21, p11, p01, p12, p02], IECore.GeometricData.Interpretation.Point ) ) + self.assertEqual( segments[0]["P"].data, IECore.V3fVectorData( [p00, p10, p01, p11, p21, p12, p22], IECore.GeometricData.Interpretation.Point ) ) + self.assertEqual( segments[1]["P"].data, IECore.V3fVectorData( [p10, p20, p01, p11, p21, p02, p12], IECore.GeometricData.Interpretation.Point ) ) def testSegmentsFullyIfNoSegmentValuesGiven( self ) : mesh = IECoreScene.MeshPrimitive.createPlane( imath.Box2f( imath.V2f( 0 ), imath.V2f( 2 ) ), imath.V2i( 2 ) ) @@ -130,8 +130,8 @@ def testSegmentsFullyIfNoSegmentValuesGiven( self ) : s0 = segments[1] s1 = segments[0] - self.assertEqual( s0["P"].data, IECore.V3fVectorData( [p00, p10, p11, p01, p21, p22, p12], IECore.GeometricData.Interpretation.Point ) ) - self.assertEqual( s1["P"].data, IECore.V3fVectorData( [p10, p20, p21, p11, p01, p12, p02], IECore.GeometricData.Interpretation.Point ) ) + self.assertEqual( s0["P"].data, IECore.V3fVectorData( [p00, p10, p01, p11, p21, p12, p22], IECore.GeometricData.Interpretation.Point ) ) + self.assertEqual( s1["P"].data, IECore.V3fVectorData( [p10, p20, p01, p11, p21, p02, p12], IECore.GeometricData.Interpretation.Point ) ) def testRaisesExceptionIfSegmentKeysNotSameTypeAsPrimvar( self ) : @@ -184,7 +184,7 @@ def testSegmentSubset( self ) : p22 = imath.V3f( 2, 2, 0 ) self.assertEqual( len( segments ), 1 ) - self.assertEqual( segments[0]["P"].data, IECore.V3fVectorData( [p11, p21, p22, p12], IECore.GeometricData.Interpretation.Point ) ) + self.assertEqual( segments[0]["P"].data, IECore.V3fVectorData( [p11, p21, p12, p22], IECore.GeometricData.Interpretation.Point ) ) self.assertEqual( segments[0]["s"].data, IECore.StringVectorData( ["b"] ) ) From a5885efb9e9ddae41f403d69f4ca6e4c8f3602da Mon Sep 17 00:00:00 2001 From: Daniel Dresser Date: Mon, 4 Nov 2024 17:19:06 -0800 Subject: [PATCH 2/2] MeshAlgo::MeshSplitter : Preserve vertex order --- Changes | 5 ++ src/IECoreScene/MeshAlgoSplit.cpp | 68 ++++++++++++++++++++++----- test/IECoreScene/MeshAlgoSplitTest.py | 31 ++++++++---- 3 files changed, 84 insertions(+), 20 deletions(-) diff --git a/Changes b/Changes index 18564d6106..59af128adb 100644 --- a/Changes +++ b/Changes @@ -8,6 +8,11 @@ Improvements - IECoreHoudini : Updated to support Houdini 20.0 and 20.5. - IECoreMaya : Avoid compilation warnings with new gcc. +Fixes +----- + +- MeshAlgo::MeshSplitter/segment : Fixed so that we now preserve vertex order while splitting. This matches the old behvaviour before 1.4.6.0 when segment was optimized. This doesn't affect the correctness of the result, but is a better convention to match user expectations - when combining meshes followed by splitting, it's better if you get back the same vertex ids you started with. + Build ----- diff --git a/src/IECoreScene/MeshAlgoSplit.cpp b/src/IECoreScene/MeshAlgoSplit.cpp index cab5e58412..587e3e3304 100644 --- a/src/IECoreScene/MeshAlgoSplit.cpp +++ b/src/IECoreScene/MeshAlgoSplit.cpp @@ -490,7 +490,8 @@ class Reindexer m_newIndices( m_newIndicesData->writable() ), m_blockSize( blockSize ), m_fromOldIds( ( numOriginalIds - 1 ) / blockSize + 1 ), - m_numIdsUsed( 0 ) + m_numIdsUsed( 0 ), + m_indicesComputed( false ) { m_newIndices.reserve( numIndices ); } @@ -510,21 +511,21 @@ class Reindexer block = std::make_unique< std::vector >( m_blockSize, -1 ); } - int &r = (*block)[ subIndex ]; - if( r == -1 ) - { - // Id isn't used yet, we need to set this location in the block, and use it - r = m_numIdsUsed; - m_numIdsUsed++; - } + // We initially record that this index is used just by marking it with a 0, against the background of -1. + // Once computeIndices is called, the 0 will be replaced with a new index, only counting indices that are + // used. + (*block)[ subIndex ] = 0; - m_newIndices.push_back( r ); + m_newIndices.push_back( id ); + + m_indicesComputed = false; } // Don't add the index, but just test if it is a part of the reindex. If it is an // id which has already been added, return the new id, otherwise return -1 inline int testIndex( int id ) { + computeIndices(); int blockId = id / m_blockSize; int subIndex = id % m_blockSize; auto &block = m_fromOldIds[ blockId ]; @@ -541,6 +542,7 @@ class Reindexer // Get the new indices. Call after calling addIndex for every original index IntVectorDataPtr getNewIndices() { + computeIndices(); return m_newIndicesData; } @@ -550,6 +552,7 @@ class Reindexer template void remapData( const std::vector &in, std::vector &out ) { + computeIndices(); out.resize( m_numIdsUsed ); for( unsigned int i = 0; i < m_fromOldIds.size(); i++ ) { @@ -574,6 +577,7 @@ class Reindexer // original id corresponding to each id of the output void getDataRemapping( std::vector &dataRemap ) { + computeIndices(); dataRemap.resize( m_numIdsUsed ); for( unsigned int i = 0; i < m_fromOldIds.size(); i++ ) { @@ -595,6 +599,45 @@ class Reindexer } private: + + void computeIndices() + { + // Once indices have been added, and before using them, this function is called to + // compute the new indices. + if( m_indicesComputed ) + { + return; + } + + m_indicesComputed = true; + + for( unsigned int blockId = 0; blockId < m_fromOldIds.size(); blockId++ ) + { + auto &block = m_fromOldIds[ blockId ]; + if( !block ) + { + continue; + } + + for( int i = 0; i < m_blockSize; i++ ) + { + if( (*block)[i] != -1 ) + { + (*block)[i] = m_numIdsUsed; + m_numIdsUsed++; + } + } + } + + for( int &id : m_newIndices ) + { + int blockId = id / m_blockSize; + int subIndex = id % m_blockSize; + + id = (*m_fromOldIds[ blockId ])[subIndex]; + } + } + // IntVectorData to hold the new indices IntVectorDataPtr m_newIndicesData; std::vector< int > &m_newIndices; @@ -605,13 +648,16 @@ class Reindexer // Store the mapping from old ids to new ids. The outer vector holds a unique_ptr for each // block of m_blockSize ids in the original id range. These pointers are null if no ids from // that block have been used. Once a block is used, it is allocated with a vector that is set - // to -1 for ids which have not been used, and the new corresponding id for ids which have been - // used + // to -1 for ids which have not been used, and zeros for ids which have been used. When computeIndices() + // is called, all used elements get a new id assigned, relative to just the used ids. std::vector< std::unique_ptr< std::vector< int > > > m_fromOldIds; // How many unique ids have appeared in the indices added so far int m_numIdsUsed; + // Whether we have yet computed the new indices for each used index + bool m_indicesComputed; + }; struct ResamplePrimitiveVariableFunctor diff --git a/test/IECoreScene/MeshAlgoSplitTest.py b/test/IECoreScene/MeshAlgoSplitTest.py index f6556c7dca..5e6b66c412 100644 --- a/test/IECoreScene/MeshAlgoSplitTest.py +++ b/test/IECoreScene/MeshAlgoSplitTest.py @@ -170,16 +170,29 @@ def accumulateInPython2( iterable ): newVerticesPerFace = [] newVertIndices = [] mapFaceVert = [] - reverseIndices = [] + + usedIndices = set() for i in r: vpf = mesh.verticesPerFace[i] newVerticesPerFace.append( vpf ) for j in range( vpf ): origFaceVert = faceIndices[i] + j origVert = mesh.vertexIds[ origFaceVert ] - newIndex = reindex.setdefault( origVert, len( reindex ) ) - if len( reindex ) > len( reverseIndices ): - reverseIndices.append( origVert ) + + usedIndices.add( origVert ) + + usedIndices = sorted( usedIndices ) + + for i in range( len( usedIndices ) ): + reindex[ usedIndices[i] ] = i + + for i in r: + vpf = mesh.verticesPerFace[i] + for j in range( vpf ): + origFaceVert = faceIndices[i] + j + origVert = mesh.vertexIds[ origFaceVert ] + + newIndex = usedIndices.index( origVert ) newVertIndices.append( newIndex ) mapFaceVert.append( origFaceVert ) @@ -226,7 +239,7 @@ def accumulateInPython2( iterable ): if p.interpolation == interp.Constant: d = p.data.copy() elif p.interpolation in [ interp.Vertex, interp.Varying ]: - d = type( p.data )( [ pd[i] for i in reverseIndices] ) + d = type( p.data )( [ pd[i] for i in usedIndices] ) elif p.interpolation == interp.FaceVarying: d = type( p.data )( [ pd[i] for i in mapFaceVert ] ) elif p.interpolation == interp.Uniform: @@ -269,8 +282,8 @@ def testCanSplitUsingIntegerPrimvar( self ) : p12 = imath.V3f( 1, 2, 0 ) p22 = imath.V3f( 2, 2, 0 ) - self.assertEqual( s0["P"].data, IECore.V3fVectorData( [p00, p10, p11, p01, p20, p21], IECore.GeometricData.Interpretation.Point ) ) - self.assertEqual( s1["P"].data, IECore.V3fVectorData( [p01, p11, p12, p02, p21, p22], IECore.GeometricData.Interpretation.Point ) ) + self.assertEqual( s0["P"].data, IECore.V3fVectorData( [p00, p10, p20, p01, p11, p21], IECore.GeometricData.Interpretation.Point ) ) + self.assertEqual( s1["P"].data, IECore.V3fVectorData( [p01, p11, p21, p02, p12, p22], IECore.GeometricData.Interpretation.Point ) ) def testSplitsFully( self ) : mesh = IECoreScene.MeshPrimitive.createPlane( imath.Box2f( imath.V2f( 0 ), imath.V2f( 2 ) ), imath.V2i( 2 ) ) @@ -303,8 +316,8 @@ def testSplitsFully( self ) : if s0["s"].data[0] != 'a': s0,s1 = s1,s0 - self.assertEqual( s0["P"].data, IECore.V3fVectorData( [p00, p10, p11, p01, p21, p22, p12], IECore.GeometricData.Interpretation.Point ) ) - self.assertEqual( s1["P"].data, IECore.V3fVectorData( [p10, p20, p21, p11, p01, p12, p02], IECore.GeometricData.Interpretation.Point ) ) + self.assertEqual( s0["P"].data, IECore.V3fVectorData( [p00, p10, p01, p11, p21, p12, p22], IECore.GeometricData.Interpretation.Point ) ) + self.assertEqual( s1["P"].data, IECore.V3fVectorData( [p10, p20, p01, p11, p21, p02, p12], IECore.GeometricData.Interpretation.Point ) ) def testSplitUsingIndexedPrimitiveVariable( self ) :