From c073f2f1c9f8faca78c0820ebab7b7fb86a2c12c Mon Sep 17 00:00:00 2001 From: Daniel Dresser Date: Mon, 28 Sep 2026 17:35:59 -0700 Subject: [PATCH 1/5] KDTree : Test for false negatives for nearestNeighbours in testCorePython --- test/IECore/KDTree.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/test/IECore/KDTree.py b/test/IECore/KDTree.py index c32362568a..d3ce334cea 100644 --- a/test/IECore/KDTree.py +++ b/test/IECore/KDTree.py @@ -61,15 +61,14 @@ def doNearestNeighbours(self, numPoints): for i in range(0, numPoints): for r in self.radii: - pIdxArray = self.tree.nearestNeighbours( self.points[i], r ) + result = self.tree.nearestNeighbours( self.points[i], r ) - for pIdx in pIdxArray: - self.assertTrue(pIdx >= 0) - self.assertTrue(pIdx < numPoints) - nearestPt = self.points[pIdx] - distToNearest = (self.points[pIdx] - self.points[i]).length() + expectedResult = [] + for j in range( self.points.size() ) : + if ( self.points[j] - self.points[i] ).length() < r: + expectedResult.append( j ) - self.assertTrue( distToNearest <= r ) + self.assertEqual( sorted( result ), expectedResult ) def doNearestNNeighbours(self, numPoints): From 2ef6259e9ea8a36fb4c7c30d9fbde100ca852b0c Mon Sep 17 00:00:00 2001 From: Daniel Dresser Date: Tue, 29 Sep 2026 14:32:01 -0700 Subject: [PATCH 2/5] KDTreeTest : Document our actual requirements for the types we support --- test/IECore/KDTreeTest.h | 25 +++++++++++++++++++++++++ test/IECore/KDTreeTest.inl | 6 +++--- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/test/IECore/KDTreeTest.h b/test/IECore/KDTreeTest.h index 0af7b5dff6..15ed072b28 100644 --- a/test/IECore/KDTreeTest.h +++ b/test/IECore/KDTreeTest.h @@ -84,6 +84,30 @@ class KDTreeTest }; +// I'm trying to use this test to document the current requirements we have for the types we support. +// It's not sufficient to specialize VectorTraits, we also require that the vector type behave mostly +// like an Imath vector. This requirement just arose over time because we only actually needed support +// for Imath vectors, and Maya's MPoint/MVector, both of which support some standard interface for +// vectors, like operator[] and operator-. +struct TestVecType +{ + float v[3] = { 0, 0, 0 }; + + float &operator[]( int i ){ return v[i]; }; + const float &operator[]( int i ) const { return v[i]; }; + + TestVecType operator-( const TestVecType &a ) { return { v[0] - a.v[0], v[1] - a.v[1], v[2] - a.v[2] }; }; +}; + +template<> +struct VectorTraits +{ + typedef float BaseType; + static unsigned int dimensions() { return 3; }; + static double get( const TestVecType &v, unsigned int i ) { return v.v[i]; }; + static void set( TestVecType &v, unsigned int i, float x ) { v.v[i] = x; }; +}; + template struct KDTreeTestSuite : public boost::unit_test::test_suite { @@ -94,6 +118,7 @@ struct KDTreeTestSuite : public boost::unit_test::test_suite addTest( "V3d" ); addTest( "V2f" ); addTest( "V2d" ); + addTest( "TestVecType" ); } template diff --git a/test/IECore/KDTreeTest.inl b/test/IECore/KDTreeTest.inl index a1ae16eef6..2cd246cd1a 100644 --- a/test/IECore/KDTreeTest.inl +++ b/test/IECore/KDTreeTest.inl @@ -86,7 +86,7 @@ void KDTreeTest::testNearestNeighbours() IteratorVector nearNeighbours; for( typename Tree::Iterator it=m_points.begin(); it!=m_points.end(); it++ ) { - typename T::BaseType radius = 0.05; + typename VectorTraits::BaseType radius = 0.05; unsigned int numNeighbours = m_tree->nearestNeighbours( *it, radius, nearNeighbours ); BOOST_CHECK(numNeighbours <= m_numPoints); @@ -167,8 +167,8 @@ void KDTreeTest::testNearestNNeighbours() if( !found ) { - typename T::BaseType distanceToRandomPt = vecDistance2(*randomPt, *it); - typename T::BaseType distanceToFurthestNeighbour = vecDistance2(*furthest, *it); + typename VectorTraits::BaseType distanceToRandomPt = vecDistance2(*randomPt, *it); + typename VectorTraits::BaseType distanceToFurthestNeighbour = vecDistance2(*furthest, *it); BOOST_CHECK( distanceToRandomPt >= distanceToFurthestNeighbour); } From ab14ffb9eb2e10d2611ed61b408499cdf684f167 Mon Sep 17 00:00:00 2001 From: Daniel Dresser Date: Thu, 17 Sep 2026 17:48:40 -0700 Subject: [PATCH 3/5] KDTree : Add enclosedPoints() method that takes a list of half-spaces --- Changes | 10 ++ include/IECore/KDTree.h | 26 +++- include/IECore/KDTree.inl | 197 +++++++++++++++++++++++++++-- include/IECore/VectorOps.h | 3 + include/IECore/VectorOps.inl | 12 ++ src/IECorePython/KDTreeBinding.cpp | 22 +++- test/IECore/KDTree.py | 70 ++++++++++ test/IECore/KDTreeTest.h | 5 + test/IECore/KDTreeTest.inl | 30 +++++ 9 files changed, 363 insertions(+), 12 deletions(-) diff --git a/Changes b/Changes index ca61026c06..132ffca65d 100644 --- a/Changes +++ b/Changes @@ -1,11 +1,21 @@ 10.7.x.x (relative to 10.7.1.3) ======== +Features +-------- + +- KDTree : Added enclosedPoints method that takes list of half-spaces defined by origins and normals. + Fixes ----- - SceneCacheFileFormat : Fixed `Unsupported Spec Type for ` error when a scene cache is added as a sublayer of a live stage. +API +------------ + +- KDTree : Updated methods taking OutputIterator to take functors instead. + 10.7.1.3 (relative to 10.7.1.2) ======== diff --git a/include/IECore/KDTree.h b/include/IECore/KDTree.h index a9969cc371..212c435c06 100644 --- a/include/IECore/KDTree.h +++ b/include/IECore/KDTree.h @@ -40,6 +40,7 @@ IECORE_PUSH_DEFAULT_VISIBILITY #include "Imath/ImathVec.h" +#include "Imath/ImathBox.h" IECORE_POP_DEFAULT_VISIBILITY #include @@ -109,6 +110,13 @@ class KDTree template void enclosedPoints( const Box &bound, OutputIterator it ) const; + // Finds all the points contained within a set of half-spaces, passing them to the given + // functor which must take a PointIterator. + // A half-space is specified with an origin and a plane normal ( the normal points towards the region that + // is included ) + template + void enclosedPoints( const std::vector &normals, const std::vector &origins, F &&functor ) const; + /// Returns the number of nodes in the tree. inline NodeIndex numNodes() const; /// Returns the specified Node of the tree. See rootIndex(), lowChildIndex() and highChildIndex() for @@ -132,8 +140,17 @@ class KDTree class AxisSort; - unsigned char majorAxis( PermutationConstIterator permFirst, PermutationConstIterator permLast ); - void build( NodeIndex nodeIndex, PermutationIterator permFirst, PermutationIterator permLast ); + // -- Utilities used when building the tree -- + + // Compute min/max of a list of points + std::pair bound( PermutationConstIterator permFirst, PermutationConstIterator permLast ); + // Return which axis of the bounding box is largest + unsigned char majorAxis( const std::pair &bound ); + // Recursively build the tree + void build( NodeIndex nodeIndex, PermutationIterator permFirst, PermutationIterator permLast, int preComputedAxis = -1 ); + + + // -- Walk functions that implement the recursive searches -- void nearestNeighbourWalk( NodeIndex nodeIndex, const Point &p, PointIterator &closestPoint, BaseType &distSquared ) const; @@ -142,6 +159,11 @@ class KDTree template void enclosedPointsWalk( NodeIndex nodeIndex, const Box &bound, OutputIterator it ) const; + struct HalfSpaceWorkingData; + + template + void enclosedPointsHalfSpacesWalk( NodeIndex nodeIndex, std::vector &working, F &&functor ) const; + void nearestNNeighboursWalk( NodeIndex nodeIndex, const Point &p, unsigned int numNeighbours, std::vector &nearNeighbours, BaseType &maxDistSquared ) const; Permutation m_perm; diff --git a/include/IECore/KDTree.inl b/include/IECore/KDTree.inl index 73b3b71f6c..8d9540197a 100644 --- a/include/IECore/KDTree.inl +++ b/include/IECore/KDTree.inl @@ -33,9 +33,11 @@ ////////////////////////////////////////////////////////////////////////// #include "IECore/BoxOps.h" +#include "IECore/Exception.h" #include "IECore/VectorOps.h" #include +#include "boost/container/small_vector.hpp" namespace IECore { @@ -108,6 +110,14 @@ class KDTree::AxisSort const unsigned int m_axis; }; +template +struct KDTree::HalfSpaceWorkingData +{ + Point normal; + Point currentInnermost; + BaseType threshold; +}; + // initialisation template @@ -122,7 +132,7 @@ KDTree::KDTree( PointIterator first, PointIterator last, int maxL } template -void KDTree::init( PointIterator first, PointIterator last, int maxLeafSize ) +void KDTree::init( PointIterator first, PointIterator last, int maxLeafSize ) { m_maxLeafSize = maxLeafSize; m_lastPoint = last; @@ -133,14 +143,40 @@ void KDTree::init( PointIterator first, PointIterator last, int m m_perm[i++] = it; } + // The cut planes we store only limit the size of each Node within the interior of the KDTree. + // If we need accurate sizes for Nodes on the exterior of the tree ( rather than treating them as + // infinite ), we need to include the bound as well ( this can be particularly important when the + // data is an axis-aligned plane, where every node on the "exterior" in the Z axis ). + std::pair totalBound = bound( m_perm.begin(), m_perm.end() ); + + // We've got a special case for the first level - we need to compute the overall bound anyway, + // so we precompute the major axis to avoid recomputing this bound at the first level. + int precomputedAxis = majorAxis( totalBound ); + /// \todo Can we reserve() enough space for m_nodes before doing this? - build( rootIndex(), m_perm.begin(), m_perm.end() ); + build( rootIndex(), m_perm.begin(), m_perm.end(), precomputedAxis ); + + // \todo : The total bound should be stored as an m_bound member variable, but that requires waiting for + // a major version, so we need to stash it somewhere else for now. Since the tree has now been fully + // built, and ends with leaf nodes that will stop further traversal, no one will notice if we stick + // some dummy nodes on the end of the list to store this bound. + + m_nodes.reserve( m_nodes.size() + VectorTraits::dimensions() * 2 ); + + for( unsigned char i=0; i::dimensions(); i++ ) + { + m_nodes.push_back( Node() ); + m_nodes.back().m_cutValue = totalBound.first[i]; + m_nodes.push_back( Node() ); + m_nodes.back().m_cutValue = totalBound.second[i]; + } } template -unsigned char KDTree::majorAxis( PermutationConstIterator permFirst, PermutationConstIterator permLast ) +std::pair::Point, typename KDTree::Point> KDTree::bound( PermutationConstIterator permFirst, PermutationConstIterator permLast ) { - Point min, max; + Point min; + Point max; for( unsigned char i=0; i::dimensions(); i++ ) { min[i] = std::numeric_limits::max(); max[i] = std::numeric_limits::lowest(); @@ -159,8 +195,14 @@ unsigned char KDTree::majorAxis( PermutationConstIterator permFir } } } + return { min, max }; +} + +template +unsigned char KDTree::majorAxis( const std::pair &bound ) +{ unsigned char major = 0; - Point size = max - min; + Point size = vecSub( bound.second, bound.first ); for( unsigned char i=1; i::dimensions(); i++ ) { if( size[i] > size[major] ) @@ -172,7 +214,7 @@ unsigned char KDTree::majorAxis( PermutationConstIterator permFir } template -void KDTree::build( NodeIndex nodeIndex, PermutationIterator permFirst, PermutationIterator permLast ) +void KDTree::build( NodeIndex nodeIndex, PermutationIterator permFirst, PermutationIterator permLast, int precomputedAxis ) { // make room for the new node if( nodeIndex>=m_nodes.size() ) @@ -182,8 +224,18 @@ void KDTree::build( NodeIndex nodeIndex, PermutationIterator perm if( permLast - permFirst > m_maxLeafSize ) { - unsigned int cutAxis = majorAxis( permFirst, permLast ); - PermutationIterator permMid = permFirst + (permLast - permFirst)/2; + unsigned int cutAxis; + if( precomputedAxis == -1 ) + { + std::pair b = bound( permFirst, permLast ); + cutAxis = majorAxis( b ); + } + else + { + cutAxis = precomputedAxis; + } + + PermutationIterator permMid = permFirst + (permLast - permFirst)/2; std::nth_element( permFirst, permMid, permLast, AxisSort( cutAxis ) ); BaseType cutValue = (**permMid)[cutAxis]; // insert node @@ -235,6 +287,46 @@ void KDTree::enclosedPoints( const Box &bound, OutputIterator it enclosedPointsWalk( rootIndex(), bound, it ); } +template +template +void KDTree::enclosedPoints( + const std::vector &normals, const std::vector &origins, F &&functor +) const +{ + std::vector workingData; + if( normals.size() != origins.size() ) + { + throw IECore::Exception( "Mismatched normals and origins passed to enclosedPoints" ); + } + + // \todo : We should be accessing this bound from an m_bound member variable, but since + // we can't add a member variable yet, we're awkwardly pulling this data from some dummy + // nodes stuck to the end of the node list. + size_t dummyNodesStartOffset = m_nodes.size() - VectorTraits::dimensions() * 2; + std::pair totalBound; + + for( unsigned char i=0; i::dimensions(); i++ ) + { + totalBound.first[i] = m_nodes[dummyNodesStartOffset + 2 * i ].m_cutValue; + totalBound.second[i] = m_nodes[dummyNodesStartOffset + 2 * i + 1 ].m_cutValue; + } + + workingData.resize( normals.size() ); + for( size_t i = 0; i < normals.size(); i++ ) + { + workingData[i].normal = normals[i]; + workingData[i].threshold = vecDot( normals[i], origins[i] ); + + + for( unsigned char j=0; j::dimensions(); j++ ) + { + workingData[i].currentInnermost[j] = std::max( normals[i][j] * totalBound.first[j], normals[i][j] * totalBound.second[j] ); + } + } + + enclosedPointsHalfSpacesWalk( rootIndex(), workingData, functor ); +} + template unsigned int KDTree::nearestNNeighbours( const Point &p, unsigned int numNeighbours, std::vector &nearNeighbours ) const { @@ -428,6 +520,95 @@ void KDTree::enclosedPointsWalk( NodeIndex nodeIndex, const Box & } } +template +template +void KDTree::enclosedPointsHalfSpacesWalk( NodeIndex nodeIndex, std::vector &working, F &&functor ) const +{ + const Node &node = m_nodes[nodeIndex]; + + if( node.isLeaf() ) + { + PointIterator *permLast = node.permLast(); + for( PointIterator *perm = node.permFirst(); perm!=permLast; perm++ ) + { + const Point &pp = **perm; + bool reject = false; + for( HalfSpaceWorkingData &halfSpace : working ) + { + if( vecDot( pp, halfSpace.normal ) < halfSpace.threshold ) + { + reject = true; + break; + } + } + + if( !reject ) + { + functor( *perm ); + } + } + } + else + { + unsigned char cutAxis = node.cutAxis(); + BaseType cutValue = node.cutValue(); + + boost::container::small_vector restoreInnermost; + restoreInnermost.reserve( working.size() ); + + for( HalfSpaceWorkingData &halfSpace : working ) + { + restoreInnermost.push_back( halfSpace.currentInnermost[ cutAxis ] ); + } + + bool rejectLow = false; + for( HalfSpaceWorkingData &halfSpace : working ) + { + if( halfSpace.normal[ cutAxis ] > BaseType( 0 ) ) + { + halfSpace.currentInnermost[ cutAxis ] = halfSpace.normal[ cutAxis ] * cutValue; + if( vecSumElements( halfSpace.currentInnermost ) < halfSpace.threshold ) + { + rejectLow = true; + } + } + } + + if( !rejectLow ) + { + enclosedPointsHalfSpacesWalk( lowChildIndex( nodeIndex ), working, functor ); + } + + bool rejectHigh = false; + for( size_t i = 0; i < working.size(); i++ ) + { + HalfSpaceWorkingData &halfSpace = working[i]; + if( halfSpace.normal[ cutAxis ] < BaseType( 0 ) ) + { + halfSpace.currentInnermost[ cutAxis ] = halfSpace.normal[ cutAxis ] * cutValue; + if( vecSumElements( halfSpace.currentInnermost ) < halfSpace.threshold ) + { + rejectHigh = true; + } + } + else + { + halfSpace.currentInnermost[ cutAxis ] = restoreInnermost[i]; + } + } + + if( !rejectHigh ) + { + enclosedPointsHalfSpacesWalk( highChildIndex( nodeIndex ), working, functor ); + } + + for( size_t i = 0; i < working.size(); i++ ) + { + working[i].currentInnermost[ cutAxis ] = restoreInnermost[i]; + } + } +} + template inline typename KDTree::NodeIndex KDTree::numNodes() const { diff --git a/include/IECore/VectorOps.h b/include/IECore/VectorOps.h index 4e47e40165..2f8ccb98e1 100644 --- a/include/IECore/VectorOps.h +++ b/include/IECore/VectorOps.h @@ -164,6 +164,9 @@ inline T vecConstruct( const typename VectorTraits::BaseType *components ); template inline T vecCross( const T &v1, const T &v2 ); +template +inline typename VectorTraits::BaseType vecSumElements( const T &v ); + } // namespace IECore #include "IECore/VectorOps.inl" diff --git a/include/IECore/VectorOps.inl b/include/IECore/VectorOps.inl index f8f3639951..915b870191 100644 --- a/include/IECore/VectorOps.inl +++ b/include/IECore/VectorOps.inl @@ -262,6 +262,18 @@ inline T vecCross( const T &v1, const T &v2 ) } +template +inline typename VectorTraits::BaseType vecSumElements( const T &v ) +{ + typename VectorTraits::BaseType result = 0; + for( unsigned int i=0; i::dimensions(); i++ ) + { + result += VectorTraits::get( v, i ); + } + + return result; +} + } // namespace IECore diff --git a/src/IECorePython/KDTreeBinding.cpp b/src/IECorePython/KDTreeBinding.cpp index 37f0a7d6ac..6b179687c1 100644 --- a/src/IECorePython/KDTreeBinding.cpp +++ b/src/IECorePython/KDTreeBinding.cpp @@ -75,10 +75,10 @@ struct KDTreeWrapper PointDataPtr m_points; - KDTreeWrapper(PointDataPtr points) + KDTreeWrapper(PointDataPtr points, int maxLeafSize = 4) { m_points = points->copy(); - m_tree = new T(m_points->readable().begin(), m_points->readable().end()); + m_tree = new T(m_points->readable().begin(), m_points->readable().end(), maxLeafSize); } virtual ~KDTreeWrapper() @@ -162,6 +162,22 @@ struct KDTreeWrapper return indices; } + IntVectorDataPtr enclosedPointsWithHalfSpaces( + const typename TypedData< std::vector >::ConstPtr &normals, + const typename TypedData< std::vector >::ConstPtr &origins + ) + { + IntVectorDataPtr indicesData = new IntVectorData(); + + auto &indices = indicesData->writable(); + m_tree->enclosedPoints( + normals->readable(), origins->readable(), + [&indices, this]( const typename T::Iterator i ){ indices.push_back( std::distance( m_points->readable().begin(), i ) ); } + ); + + return indicesData; + } + }; @@ -170,10 +186,12 @@ void bindKDTree(const char *bindName) { class_, boost::noncopyable>(bindName, no_init) .def(init< typename KDTreeWrapper::PointDataPtr >() ) + .def(init< typename KDTreeWrapper::PointDataPtr, int >() ) .def("nearestNeighbour", &KDTreeWrapper::nearestNeighbour ) .def("nearestNeighbours", &KDTreeWrapper::nearestNeighbours ) .def("nearestNNeighbours", &KDTreeWrapper::nearestNNeighbours ) .def("enclosedPoints", &KDTreeWrapper::enclosedPoints ) + .def("enclosedPoints", &KDTreeWrapper::enclosedPointsWithHalfSpaces ) ; } diff --git a/test/IECore/KDTree.py b/test/IECore/KDTree.py index d3ce334cea..399ea32784 100644 --- a/test/IECore/KDTree.py +++ b/test/IECore/KDTree.py @@ -128,6 +128,40 @@ def doEnclosedPoints( self, numPoints ) : else : self.assertFalse( i in s ) + def doEnclosedPointsHalfSpaces( self, numPoints ) : + + self.makeTree( numPoints ) + + for i in range( 0, 50 ) : + if i < 10: + # Test with a single random halfspace + normals = type( self.points )( [ (2 * self.randomVec() - 1) ] ) + origins = type( self.points )( [ self.randomVec() ] ) + elif i < 20: + # Test with 6 halfspaces that are guaranteed to contain some actual volume + # ( since they are chosen to be tangent to the surface of a sphere ) + normals = type( self.points )( [ (2 * self.randomVec() - 1).normalized() for i in range( 6 ) ] ) + origins = type( self.points )( [ -n * 0.25 + 0.5 for n in normals ] ) + else: + # Test with three totally random halfspaces that may or may not intersect + # to contain anything + normals = type( self.points )( [ (2 * self.randomVec() - 1) for i in range( 3 ) ] ) + origins = type( self.points )( [ self.randomVec() for i in range( 3 ) ] ) + + result = self.tree.enclosedPoints( normals, origins ) + + expectedResult = [] + for i in range( self.points.size() ) : + reject = False + for n,o in zip( normals, origins ): + if ( self.points[i] - o ).dot( n ) < 0: + reject = True + + if not reject: + expectedResult.append( i ) + + self.assertEqual( sorted( result ), expectedResult ) + class TestKDTreeV2f(unittest.TestCase, TestKDTree): @@ -147,6 +181,9 @@ def randomBox( self ) : max = min + imath.V2f( random.random(), random.random() ) return imath.Box2f( min, max ) + def randomVec( self ): + return imath.V2f( random.random(), random.random() ) + def testConstructors(self): """Test KDTreeV2f constructors""" @@ -177,6 +214,12 @@ def testEnclosedPoints(self): for t in self.treeSizes: self.doEnclosedPoints(t) + def testEnclosedPointsHalfSpaces(self): + """Test KDTreeV2f enclosedPointsHalfSpaces""" + + for t in self.treeSizes: + self.doEnclosedPointsHalfSpaces(t) + class TestKDTreeV2d(unittest.TestCase, TestKDTree): def makeTree(self, numPoints): @@ -195,6 +238,9 @@ def randomBox( self ) : max = min + imath.V2d( random.random(), random.random() ) return imath.Box2d( min, max ) + def randomVec( self ): + return imath.V2d( random.random(), random.random() ) + def testConstructors(self): """Test KDTreeV2d constructors""" @@ -225,6 +271,12 @@ def testEnclosedPoints(self): for t in self.treeSizes: self.doEnclosedPoints(t) + def testEnclosedPointsHalfSpaces(self): + """Test KDTreeV2d enclosedPointsHalfSpaces""" + + for t in self.treeSizes: + self.doEnclosedPointsHalfSpaces(t) + class TestKDTreeV3f(unittest.TestCase, TestKDTree): def makeTree(self, numPoints): @@ -243,6 +295,9 @@ def randomBox( self ) : max = min + imath.V3f( random.random(), random.random(), random.random() ) return imath.Box3f( min, max ) + def randomVec( self ): + return imath.V3f( random.random(), random.random(), random.random() ) + def testConstructors(self): """Test KDTreeV3f constructors""" @@ -273,6 +328,12 @@ def testEnclosedPoints(self): for t in self.treeSizes: self.doEnclosedPoints(t) + def testEnclosedPointsHalfSpaces(self): + """Test KDTreeV3f enclosedPointsHalfSpaces""" + + for t in self.treeSizes: + self.doEnclosedPointsHalfSpaces(t) + class TestKDTreeV3d(unittest.TestCase, TestKDTree): def makeTree(self, numPoints): @@ -291,6 +352,9 @@ def randomBox( self ) : max = min + imath.V3d( random.random(), random.random(), random.random() ) return imath.Box3d( min, max ) + def randomVec( self ): + return imath.V3d( random.random(), random.random(), random.random() ) + def testConstructors(self): """Test KDTreeV3d constructors""" @@ -321,6 +385,12 @@ def testEnclosedPoints(self): for t in self.treeSizes: self.doEnclosedPoints(t) + def testEnclosedPointsHalfSpaces(self): + """Test KDTreeV3d enclosedPointsHalfSpaces""" + + for t in self.treeSizes: + self.doEnclosedPointsHalfSpaces(t) + if __name__ == "__main__": unittest.main() diff --git a/test/IECore/KDTreeTest.h b/test/IECore/KDTreeTest.h index 15ed072b28..bd101aba4f 100644 --- a/test/IECore/KDTreeTest.h +++ b/test/IECore/KDTreeTest.h @@ -65,6 +65,7 @@ class KDTreeTest void testNearestNeighbour(); void testNearestNeighbours(); void testNearestNNeighbours(); + void testEnclosedPointsHalfSpaces(); private: @@ -137,6 +138,10 @@ struct KDTreeTestSuite : public boost::unit_test::test_suite test = BOOST_CLASS_TEST_CASE( &KDTreeTest::testNearestNNeighbours, instance ); test->p_name.set( test->p_name.get() + nameSuffix ); add( test ); + + test = BOOST_CLASS_TEST_CASE( &KDTreeTest::testEnclosedPointsHalfSpaces, instance ); + test->p_name.set( test->p_name.get() + nameSuffix ); + add( test ); } }; diff --git a/test/IECore/KDTreeTest.inl b/test/IECore/KDTreeTest.inl index 2cd246cd1a..6fef2f3daa 100644 --- a/test/IECore/KDTreeTest.inl +++ b/test/IECore/KDTreeTest.inl @@ -111,6 +111,36 @@ void KDTreeTest::testNearestNeighbours() } } +template +void KDTreeTest::testEnclosedPointsHalfSpaces() +{ + T origin; + T normal; + + for ( unsigned int i = 0; i < VectorTraits< T >::dimensions(); i++) + { + origin[ i ] = m_randGen.nextf() * 0.1f; + normal[ i ] = 2.0f * m_randGen.nextf() - 1.0f; + } + + std::vector result; + m_tree->enclosedPoints( { normal }, { origin }, [&result, this]( auto &it ){ result.push_back( it - m_points.begin() ); } ); + + std::vector expectedResult; + + for( size_t i = 0; i < m_points.size(); i++ ) + { + if( vecDot( vecSub( m_points[i], origin ), normal ) > 0.0f ) + { + expectedResult.push_back( i ); + } + } + + std::sort( result.begin(), result.end() ); + + BOOST_CHECK( result == expectedResult ); +} + template void KDTreeTest::testNearestNNeighbours() { From ed436a7298efa867f432ca6b43680aca81837ac3 Mon Sep 17 00:00:00 2001 From: Daniel Dresser Date: Fri, 25 Sep 2026 11:21:56 -0700 Subject: [PATCH 4/5] KDTree : Update previous function signatures to prefer functors signatures taking OutputIterators are now deprecated --- include/IECore/KDTree.h | 34 ++++++++++++++++++++++++--------- include/IECore/KDTree.inl | 40 +++++++++++++++++++++++++++------------ 2 files changed, 53 insertions(+), 21 deletions(-) diff --git a/include/IECore/KDTree.h b/include/IECore/KDTree.h index 212c435c06..7d18a80981 100644 --- a/include/IECore/KDTree.h +++ b/include/IECore/KDTree.h @@ -49,6 +49,15 @@ IECORE_POP_DEFAULT_VISIBILITY namespace IECore { +namespace Detail +{ + // \todo - ugly machinery needed until we deprecate the old signature of enclosedPoints + template + struct IsIterator : std::false_type {}; + template + struct IsIterator>>::iterator_category >> : std::true_type {}; +} + /// The KDTree class provides accelerated searching of pointsets. It is /// templated so that it can operate on a wide variety of datatypes, and uses /// the VectorTraits.h and VectorOps.h functionality to assist in this. @@ -94,10 +103,11 @@ class KDTree /// \threading May be called by multiple concurrent threads. PointIterator nearestNeighbour( const Point &p, BaseType &distSquared ) const; - /// Populates the passed vector of iterators with the neighbours of point p which are closer than radius r. Returns the number of points found. - /// \todo There should be a form where nearNeighbours is an output iterator, to allow any container to be filled. - /// See enclosedPoints for an example of this form. - /// \threading May be called by multiple concurrent threads provided they are each using a different vector for the result. + /// Call a functor for each neighbour of point p which is closer than radius r. + /// The functor must take a PointIterator. + template + void nearestNeighbours( const Point &p, BaseType r, F &&functor ) const; + /// \deprecated - use the form above that takes a functor, rather than this version that populates a vector. unsigned int nearestNeighbours( const Point &p, BaseType r, std::vector &nearNeighbours ) const; class Neighbour; @@ -105,9 +115,14 @@ class KDTree /// \threading May be called by multiple concurrent threads provided they are each using a different vector for the result. unsigned int nearestNNeighbours( const Point &p, unsigned int numNeighbours, std::vector &nearNeighbours ) const; - /// Finds all the points contained by the specified bound, outputting them to the specified iterator. + /// Finds all the points contained by the specified bound, outputting them to the specified functor, + /// which must take a PointIterator. /// \threading May be called by multiple concurrent threads. - template + template::value, bool > = true> + void enclosedPoints( const Box &bound, F &&functor ) const; + /// \deprecated - use the form above that takes a functor ( once we get rid of this deprecated signature, + /// we can get rid of the ugly enable_if guard above ). + template::value, bool > = true> void enclosedPoints( const Box &bound, OutputIterator it ) const; // Finds all the points contained within a set of half-spaces, passing them to the given @@ -154,10 +169,11 @@ class KDTree void nearestNeighbourWalk( NodeIndex nodeIndex, const Point &p, PointIterator &closestPoint, BaseType &distSquared ) const; - void nearestNeighboursWalk( NodeIndex nodeIndex, const Point &p, BaseType r2, std::vector &nearNeighbours ) const; + template + void nearestNeighboursWalk( NodeIndex nodeIndex, const Point &p, BaseType r2, F &&functor ) const; - template - void enclosedPointsWalk( NodeIndex nodeIndex, const Box &bound, OutputIterator it ) const; + template + void enclosedPointsWalk( NodeIndex nodeIndex, const Box &bound, F &&functor ) const; struct HalfSpaceWorkingData; diff --git a/include/IECore/KDTree.inl b/include/IECore/KDTree.inl index 8d9540197a..e2df309a07 100644 --- a/include/IECore/KDTree.inl +++ b/include/IECore/KDTree.inl @@ -270,21 +270,36 @@ PointIterator KDTree::nearestNeighbour( const Point &p, BaseType return closestPoint; } +template +template +void KDTree::nearestNeighbours( const Point &p, BaseType r, F &&functor ) const +{ + nearestNeighboursWalk(rootIndex(), p, r*r, functor ); +} + template unsigned int KDTree::nearestNeighbours( const Point &p, BaseType r, std::vector &nearNeighbours ) const { nearNeighbours.clear(); - nearestNeighboursWalk(rootIndex(), p, r*r, nearNeighbours ); + nearestNeighbours( p, r, [&nearNeighbours]( PointIterator &it ){ nearNeighbours.push_back( it ); } ); return nearNeighbours.size(); } template -template +template::value, bool >> +void KDTree::enclosedPoints( const Box &bound, F &&functor ) const +{ + enclosedPointsWalk( rootIndex(), bound, functor ); +} + +// \deprecated wrapper +template +template::value, bool >> void KDTree::enclosedPoints( const Box &bound, OutputIterator it ) const { - enclosedPointsWalk( rootIndex(), bound, it ); + enclosedPoints( bound, [&it]( PointIterator &p ){ *it++ = p; } ); } template @@ -386,7 +401,8 @@ void KDTree::nearestNeighbourWalk( NodeIndex nodeIndex, const Poi } template -void KDTree::nearestNeighboursWalk( NodeIndex nodeIndex, const Point &p, BaseType r2, std::vector &nearNeighbours ) const +template +void KDTree::nearestNeighboursWalk( NodeIndex nodeIndex, const Point &p, BaseType r2, F &&functor ) const { const Node &node = m_nodes[nodeIndex]; if( node.isLeaf() ) @@ -399,7 +415,7 @@ void KDTree::nearestNeighboursWalk( NodeIndex nodeIndex, const Po if (dist2 < r2 ) { - nearNeighbours.push_back( *perm ); + functor( *perm ); } } } @@ -419,10 +435,10 @@ void KDTree::nearestNeighboursWalk( NodeIndex nodeIndex, const Po secondChild = highChildIndex( nodeIndex ); } - nearestNeighboursWalk( firstChild, p, r2, nearNeighbours ); + nearestNeighboursWalk( firstChild, p, r2, functor ); if( d*d < r2 ) { - nearestNeighboursWalk( secondChild, p, r2, nearNeighbours ); + nearestNeighboursWalk( secondChild, p, r2, functor ); } } } @@ -490,8 +506,8 @@ void KDTree::nearestNNeighboursWalk( NodeIndex nodeIndex, const P } template -template -void KDTree::enclosedPointsWalk( NodeIndex nodeIndex, const Box &bound, OutputIterator it ) const +template +void KDTree::enclosedPointsWalk( NodeIndex nodeIndex, const Box &bound, F &&functor ) const { const Node &node = m_nodes[nodeIndex]; @@ -503,7 +519,7 @@ void KDTree::enclosedPointsWalk( NodeIndex nodeIndex, const Box & const Point &pp = **perm; if( boxIntersects( bound, pp ) ) { - *it++ = *perm; + functor( *perm ); } } } @@ -511,11 +527,11 @@ void KDTree::enclosedPointsWalk( NodeIndex nodeIndex, const Box & { if( vecGet( BoxTraits::min( bound ), node.cutAxis() ) <= node.cutValue() ) { - enclosedPointsWalk( lowChildIndex( nodeIndex ), bound, it ); + enclosedPointsWalk( lowChildIndex( nodeIndex ), bound, functor ); } if( vecGet( BoxTraits::max( bound ), node.cutAxis() ) >= node.cutValue() ) { - enclosedPointsWalk( highChildIndex( nodeIndex ), bound, it ); + enclosedPointsWalk( highChildIndex( nodeIndex ), bound, functor ); } } } From b75464f6c95fc88f9d4e791c91f623e509d0f17e Mon Sep 17 00:00:00 2001 From: Daniel Dresser Date: Fri, 25 Sep 2026 18:53:30 -0700 Subject: [PATCH 5/5] KDTreeBinding : Avoid using deprecated signatures --- src/IECorePython/KDTreeBinding.cpp | 42 ++++++++++-------------------- 1 file changed, 14 insertions(+), 28 deletions(-) diff --git a/src/IECorePython/KDTreeBinding.cpp b/src/IECorePython/KDTreeBinding.cpp index 6b179687c1..b387a32598 100644 --- a/src/IECorePython/KDTreeBinding.cpp +++ b/src/IECorePython/KDTreeBinding.cpp @@ -100,22 +100,15 @@ struct KDTreeWrapper { assert(m_tree); - typedef std::vector PointArray; - - PointArray points; - - unsigned int num = m_tree->nearestNeighbours(p, r, points); - - IntVectorDataPtr indices = new IntVectorData(); - - indices->writable().reserve( num ); + IntVectorDataPtr indicesData = new IntVectorData(); - for (typename PointArray::const_iterator it = points.begin(); it != points.end(); ++it) - { - indices->writable().push_back( std::distance( m_points->readable().begin(), *it ) ); - } + auto &indices = indicesData->writable(); + m_tree->nearestNeighbours( + p, r, + [&indices, this]( const typename T::Iterator i ){ indices.push_back( std::distance( m_points->readable().begin(), i ) ); } + ); - return indices; + return indicesData; } @@ -144,22 +137,15 @@ struct KDTreeWrapper IntVectorDataPtr enclosedPoints( const Box &bound ) { - typedef std::vector PointArray; - - PointArray points; - - m_tree->enclosedPoints( bound, std::back_insert_iterator( points ) ); - - IntVectorDataPtr indices = new IntVectorData(); - - indices->writable().reserve( points.size() ); + IntVectorDataPtr indicesData = new IntVectorData(); - for (typename PointArray::const_iterator it = points.begin(); it != points.end(); ++it) - { - indices->writable().push_back( std::distance( m_points->readable().begin(), *it ) ); - } + auto &indices = indicesData->writable(); + m_tree->enclosedPoints( + bound, + [&indices, this]( const typename T::Iterator i ){ indices.push_back( std::distance( m_points->readable().begin(), i ) ); } + ); - return indices; + return indicesData; } IntVectorDataPtr enclosedPointsWithHalfSpaces(