From 897b83e500d599e9b3dbff95bde1ce5438a837e4 Mon Sep 17 00:00:00 2001 From: Daniel Dresser Date: Mon, 28 Sep 2026 17:35:59 -0700 Subject: [PATCH 1/7] 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 f8641dd054b6e70eb1688b10a0b8e7063378d91f Mon Sep 17 00:00:00 2001 From: Daniel Dresser Date: Thu, 17 Sep 2026 17:48:40 -0700 Subject: [PATCH 2/7] KDTree : Add enclosedPoints() method that takes a list of half-spaces --- include/IECore/KDTree.h | 12 +++ include/IECore/KDTree.inl | 118 +++++++++++++++++++++++++++++ include/IECore/VectorOps.h | 3 + include/IECore/VectorOps.inl | 15 ++++ src/IECorePython/KDTreeBinding.cpp | 22 +++++- test/IECore/KDTree.py | 70 +++++++++++++++++ 6 files changed, 238 insertions(+), 2 deletions(-) diff --git a/include/IECore/KDTree.h b/include/IECore/KDTree.h index a9969cc371..8ab21febda 100644 --- a/include/IECore/KDTree.h +++ b/include/IECore/KDTree.h @@ -109,6 +109,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 @@ -142,6 +149,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..0cd6befd5c 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 @@ -235,6 +245,28 @@ 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" ); + } + workingData.resize( normals.size() ); + for( size_t i = 0; i < normals.size(); i++ ) + { + workingData[i].normal = normals[i]; + workingData[i].threshold = normals[i].dot( origins[i] ); + workingData[i].currentInnermost = Point( std::numeric_limits::max() ); + } + + enclosedPointsHalfSpacesWalk( rootIndex(), workingData, functor ); +} + template unsigned int KDTree::nearestNNeighbours( const Point &p, unsigned int numNeighbours, std::vector &nearNeighbours ) const { @@ -428,6 +460,92 @@ void KDTree::enclosedPointsWalk( NodeIndex nodeIndex, const Box & } } +template +template +void KDTree::enclosedPointsHalfSpacesWalk( NodeIndex nodeIndex, std::vector &working, F &&functor ) const +{ + // TODO - normalize somewhere before this? + 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 ) + { + reject |= pp.dot( halfSpace.normal ) < halfSpace.threshold; + } + + 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..91b0320c29 100644 --- a/include/IECore/VectorOps.inl +++ b/include/IECore/VectorOps.inl @@ -262,6 +262,21 @@ inline T vecCross( const T &v1, const T &v2 ) } +template +inline typename VectorTraits::BaseType vecSumElements( const T &v ) +{ + // TODO - the conventions for VectorTraits for 1 dimensional vectors are weird ... + // maybe instead of worrying about that, maybe I should just make this private? + if constexpr( T::dimensions() == 2 ) + { + return v[0] + v[1]; + } + else if constexpr( T::dimensions() == 3 ) + { + return v[0] + v[1] + v[2]; + } +} + } // 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() From ff58e83dc4e2420eeb7db28ff7e7ba6f85ff97d4 Mon Sep 17 00:00:00 2001 From: Daniel Dresser Date: Thu, 24 Sep 2026 16:23:58 -0700 Subject: [PATCH 3/7] KDTree : Store bound, make new enclosedPoints more accurate --- include/IECore/KDTree.h | 10 ++++++ include/IECore/KDTree.inl | 69 ++++++++++++++++++++++++++++++++------- 2 files changed, 67 insertions(+), 12 deletions(-) diff --git a/include/IECore/KDTree.h b/include/IECore/KDTree.h index 8ab21febda..071f527413 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 @@ -139,9 +140,18 @@ class KDTree class AxisSort; + // -- Utilities used when building the tree -- + + // Compute min/max of a list of points + Imath::Box bound( PermutationConstIterator permFirst, PermutationConstIterator permLast ); + // Return which axis of the bounding box is largest unsigned char majorAxis( PermutationConstIterator permFirst, PermutationConstIterator permLast ); + // Recursively build the tree void build( NodeIndex nodeIndex, PermutationIterator permFirst, PermutationIterator permLast ); + + // -- Walk functions that implement the recursive searches -- + 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; diff --git a/include/IECore/KDTree.inl b/include/IECore/KDTree.inl index 0cd6befd5c..df53ca50f2 100644 --- a/include/IECore/KDTree.inl +++ b/include/IECore/KDTree.inl @@ -132,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; @@ -145,32 +145,59 @@ void KDTree::init( PointIterator first, PointIterator last, int m /// \todo Can we reserve() enough space for m_nodes before doing this? build( rootIndex(), m_perm.begin(), m_perm.end() ); + + // 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 ). + Imath::Box totalBound = bound( m_perm.begin(), m_perm.end() ); + + // \todo : This 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.min[i]; + m_nodes.push_back( Node() ); + m_nodes.back().m_cutValue = totalBound.max[i]; + } } template -unsigned char KDTree::majorAxis( PermutationConstIterator permFirst, PermutationConstIterator permLast ) +Imath::Box::Point> KDTree::bound( PermutationConstIterator permFirst, PermutationConstIterator permLast ) { - Point min, max; + Imath::Box result; for( unsigned char i=0; i::dimensions(); i++ ) { - min[i] = std::numeric_limits::max(); - max[i] = std::numeric_limits::lowest(); + result.min[i] = std::numeric_limits::max(); + result.max[i] = std::numeric_limits::lowest(); } for( PermutationConstIterator it=permFirst; it!=permLast; it++ ) { for( unsigned char i=0; i::dimensions(); i++ ) { - if( (**it)[i] < min[i] ) + if( (**it)[i] < result.min[i] ) { - min[i] = (**it)[i]; + result.min[i] = (**it)[i]; } - if( (**it)[i] > max[i] ) + if( (**it)[i] > result.max[i] ) { - max[i] = (**it)[i]; + result.max[i] = (**it)[i]; } } } + return result; +} + +template +unsigned char KDTree::majorAxis( PermutationConstIterator permFirst, PermutationConstIterator permLast ) +{ unsigned char major = 0; - Point size = max - min; + Point size = bound( permFirst, permLast ).size(); for( unsigned char i=1; i::dimensions(); i++ ) { if( size[i] > size[major] ) @@ -193,7 +220,7 @@ void KDTree::build( NodeIndex nodeIndex, PermutationIterator perm if( permLast - permFirst > m_maxLeafSize ) { unsigned int cutAxis = majorAxis( permFirst, permLast ); - PermutationIterator permMid = permFirst + (permLast - permFirst)/2; + PermutationIterator permMid = permFirst + (permLast - permFirst)/2; std::nth_element( permFirst, permMid, permLast, AxisSort( cutAxis ) ); BaseType cutValue = (**permMid)[cutAxis]; // insert node @@ -256,12 +283,30 @@ void KDTree::enclosedPoints( { 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; + Imath::Box totalBound; + + for( unsigned char i=0; i::dimensions(); i++ ) + { + totalBound.min[i] = m_nodes[dummyNodesStartOffset + 2 * i ].m_cutValue; + totalBound.max[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 = normals[i].dot( origins[i] ); - workingData[i].currentInnermost = Point( std::numeric_limits::max() ); + + + for( unsigned char j=0; j::dimensions(); j++ ) + { + workingData[i].currentInnermost[j] = std::max( normals[i][j] * totalBound.min[j], normals[i][j] * totalBound.max[j] ); + } } enclosedPointsHalfSpacesWalk( rootIndex(), workingData, functor ); From fec40b0e0e4e7af82c8118132395ccb3b37e2811 Mon Sep 17 00:00:00 2001 From: Daniel Dresser Date: Fri, 25 Sep 2026 11:21:56 -0700 Subject: [PATCH 4/7] KDTree : Update previous function signatures to prefer functors signatures taking OutputIterators are now deprecated --- include/IECore/KDTree.h | 35 +++++++++++++++++++++++++--------- include/IECore/KDTree.inl | 40 +++++++++++++++++++++++++++------------ 2 files changed, 54 insertions(+), 21 deletions(-) diff --git a/include/IECore/KDTree.h b/include/IECore/KDTree.h index 071f527413..67657dfbf5 100644 --- a/include/IECore/KDTree.h +++ b/include/IECore/KDTree.h @@ -36,6 +36,7 @@ #define IE_CORE_KDTREE_H #include "IECore/Export.h" +#include "IECore/TypeTraits.h" #include "IECore/VectorTraits.h" IECORE_PUSH_DEFAULT_VISIBILITY @@ -49,6 +50,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 +104,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 +116,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 +170,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 df53ca50f2..dc9e9e226c 100644 --- a/include/IECore/KDTree.inl +++ b/include/IECore/KDTree.inl @@ -255,21 +255,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 @@ -371,7 +386,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() ) @@ -384,7 +400,7 @@ void KDTree::nearestNeighboursWalk( NodeIndex nodeIndex, const Po if (dist2 < r2 ) { - nearNeighbours.push_back( *perm ); + functor( *perm ); } } } @@ -404,10 +420,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 ); } } } @@ -475,8 +491,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]; @@ -488,7 +504,7 @@ void KDTree::enclosedPointsWalk( NodeIndex nodeIndex, const Box & const Point &pp = **perm; if( boxIntersects( bound, pp ) ) { - *it++ = *perm; + functor( *perm ); } } } @@ -496,11 +512,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 55af527f6de90f5aa19d64538a92d0119a0ee6a2 Mon Sep 17 00:00:00 2001 From: Daniel Dresser Date: Fri, 25 Sep 2026 18:53:30 -0700 Subject: [PATCH 5/7] 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( From e4fbffc72677652163e43686f2ff5754076a3ada Mon Sep 17 00:00:00 2001 From: Daniel Dresser Date: Mon, 28 Sep 2026 18:02:29 -0700 Subject: [PATCH 6/7] KDTree : Clean up storage of m_bound, requires ABI break --- include/IECore/KDTree.h | 1 + include/IECore/KDTree.inl | 31 ++----------------------------- 2 files changed, 3 insertions(+), 29 deletions(-) diff --git a/include/IECore/KDTree.h b/include/IECore/KDTree.h index 67657dfbf5..38cc3a873a 100644 --- a/include/IECore/KDTree.h +++ b/include/IECore/KDTree.h @@ -187,6 +187,7 @@ class KDTree NodeVector m_nodes; int m_maxLeafSize; PointIterator m_lastPoint; + Imath::Box< Point > m_bound; }; diff --git a/include/IECore/KDTree.inl b/include/IECore/KDTree.inl index dc9e9e226c..f1a80269f6 100644 --- a/include/IECore/KDTree.inl +++ b/include/IECore/KDTree.inl @@ -150,22 +150,7 @@ void KDTree::init( PointIterator first, PointIterator last, int m // 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 ). - Imath::Box totalBound = bound( m_perm.begin(), m_perm.end() ); - - // \todo : This 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.min[i]; - m_nodes.push_back( Node() ); - m_nodes.back().m_cutValue = totalBound.max[i]; - } + m_bound = bound( m_perm.begin(), m_perm.end() ); } template @@ -299,18 +284,6 @@ void KDTree::enclosedPoints( 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; - Imath::Box totalBound; - - for( unsigned char i=0; i::dimensions(); i++ ) - { - totalBound.min[i] = m_nodes[dummyNodesStartOffset + 2 * i ].m_cutValue; - totalBound.max[i] = m_nodes[dummyNodesStartOffset + 2 * i + 1 ].m_cutValue; - } - workingData.resize( normals.size() ); for( size_t i = 0; i < normals.size(); i++ ) { @@ -320,7 +293,7 @@ void KDTree::enclosedPoints( for( unsigned char j=0; j::dimensions(); j++ ) { - workingData[i].currentInnermost[j] = std::max( normals[i][j] * totalBound.min[j], normals[i][j] * totalBound.max[j] ); + workingData[i].currentInnermost[j] = std::max( normals[i][j] * m_bound.min[j], normals[i][j] * m_bound.max[j] ); } } From 4eddcdb75b86220da1f8de2dec3075ad3d315117 Mon Sep 17 00:00:00 2001 From: Daniel Dresser Date: Mon, 28 Sep 2026 18:23:35 -0700 Subject: [PATCH 7/7] KDTree : Remove deprecated function signatures --- include/IECore/KDTree.h | 8 +------- include/IECore/KDTree.inl | 20 +------------------- test/IECore/KDTreeTest.inl | 7 +++++-- 3 files changed, 7 insertions(+), 28 deletions(-) diff --git a/include/IECore/KDTree.h b/include/IECore/KDTree.h index 38cc3a873a..87fc52d4b2 100644 --- a/include/IECore/KDTree.h +++ b/include/IECore/KDTree.h @@ -108,8 +108,6 @@ class KDTree /// 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; /// Populates the passed vector with the N closest neighbours to p, sorted with the closest first. Returns the number found. @@ -119,12 +117,8 @@ class KDTree /// 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::value, bool > = true> + template 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 // functor which must take a PointIterator. diff --git a/include/IECore/KDTree.inl b/include/IECore/KDTree.inl index f1a80269f6..4a63a69284 100644 --- a/include/IECore/KDTree.inl +++ b/include/IECore/KDTree.inl @@ -248,30 +248,12 @@ void KDTree::nearestNeighbours( const Point &p, BaseType r, F &&f } template -unsigned int KDTree::nearestNeighbours( const Point &p, BaseType r, std::vector &nearNeighbours ) const -{ - nearNeighbours.clear(); - - nearestNeighbours( p, r, [&nearNeighbours]( PointIterator &it ){ nearNeighbours.push_back( it ); } ); - - return nearNeighbours.size(); -} - -template -template::value, bool >> +template 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 -{ - enclosedPoints( bound, [&it]( PointIterator &p ){ *it++ = p; } ); -} - template template void KDTree::enclosedPoints( diff --git a/test/IECore/KDTreeTest.inl b/test/IECore/KDTreeTest.inl index a1ae16eef6..4d0e186d4b 100644 --- a/test/IECore/KDTreeTest.inl +++ b/test/IECore/KDTreeTest.inl @@ -87,9 +87,12 @@ void KDTreeTest::testNearestNeighbours() for( typename Tree::Iterator it=m_points.begin(); it!=m_points.end(); it++ ) { typename T::BaseType radius = 0.05; - unsigned int numNeighbours = m_tree->nearestNeighbours( *it, radius, nearNeighbours ); + nearNeighbours.clear(); + m_tree->nearestNeighbours( *it, radius, [&nearNeighbours]( const typename Tree::Iterator &i ){ + nearNeighbours.push_back( i ); + } ); - BOOST_CHECK(numNeighbours <= m_numPoints); + BOOST_CHECK(nearNeighbours.size() <= m_numPoints); typename IteratorVector::const_iterator nit = nearNeighbours.begin(); for (; nit != nearNeighbours.end(); ++nit)