Skip to content

KdTree : Changes to Make In The Next Major Version #1566

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
danieldresser-ie wants to merge 2 commits into
base: main
Choose a base branch
from
+12 −68
Open

KdTree : Changes to Make In The Next Major Version #1566

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2dbf4a4
KDTree : Clean up storage of m_bound, requires ABI break
danieldresser-ie Sep 29, 2026
cf4cb0e
KDTree : Remove deprecated function signatures
danieldresser-ie Sep 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Changes
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ Breaking Changes
----------------

- MeshAlgo : Changed the output of `reverseWinding()`.
- KDTree : Removed deprecated function signatures.

10.7.2.0 (relative to 10.7.1.3)
========
Expand Down
18 changes: 2 additions & 16 deletions include/IECore/KDTree.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,6 @@ IECORE_POP_DEFAULT_VISIBILITY
namespace IECore
{

namespace Detail
{
// \todo - ugly machinery needed until we deprecate the old signature of enclosedPoints
template <typename, typename = void>
struct IsIterator : std::false_type {};
template <typename T>
struct IsIterator<T, std::void_t< typename std::iterator_traits<std::remove_cv_t<std::remove_reference_t<T>>>::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.
Expand Down Expand Up @@ -107,8 +98,6 @@ class KDTree
/// The functor must take a PointIterator.
template<typename F>
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<PointIterator> &nearNeighbours ) const;

class Neighbour;
/// Populates the passed vector with the N closest neighbours to p, sorted with the closest first. Returns the number found.
Expand All @@ -118,12 +107,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<typename Box, typename F, std::enable_if_t< !Detail::IsIterator<F>::value, bool > = true>
template<typename Box, typename F>
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<typename Box, typename OutputIterator, std::enable_if_t< Detail::IsIterator<OutputIterator>::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.
Expand Down Expand Up @@ -186,6 +171,7 @@ class KDTree
NodeVector m_nodes;
int m_maxLeafSize;
PointIterator m_lastPoint;
std::pair< Point, Point > m_bound;

};

Expand Down
54 changes: 4 additions & 50 deletions include/IECore/KDTree.inl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,29 +147,14 @@ void KDTree<PointIterator>::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 ).
std::pair<Point,Point> totalBound = bound( m_perm.begin(), m_perm.end() );
m_bound = 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 );
int precomputedAxis = majorAxis( m_bound );

/// \todo Can we reserve() enough space for m_nodes before doing this?
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<Point>::dimensions() * 2 );

for( unsigned char i=0; i<VectorTraits<Point>::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<class PointIterator>
Expand Down Expand Up @@ -278,30 +263,12 @@ void KDTree<PointIterator>::nearestNeighbours( const Point &p, BaseType r, F &&f
}

template<class PointIterator>
unsigned int KDTree<PointIterator>::nearestNeighbours( const Point &p, BaseType r, std::vector<PointIterator> &nearNeighbours ) const
{
nearNeighbours.clear();

nearestNeighbours( p, r, [&nearNeighbours]( PointIterator &it ){ nearNeighbours.push_back( it ); } );

return nearNeighbours.size();
}

template<class PointIterator>
template<typename Box, typename F, std::enable_if_t< !Detail::IsIterator<F>::value, bool >>
template<typename Box, typename F >
void KDTree<PointIterator>::enclosedPoints( const Box &bound, F &&functor ) const
{
enclosedPointsWalk( rootIndex(), bound, functor );
}

// \deprecated wrapper
template<class PointIterator>
template<typename Box, typename OutputIterator, std::enable_if_t< Detail::IsIterator<OutputIterator>::value, bool >>
void KDTree<PointIterator>::enclosedPoints( const Box &bound, OutputIterator it ) const
{
enclosedPoints( bound, [&it]( PointIterator &p ){ *it++ = p; } );
}

template<class PointIterator>
template<typename F>
void KDTree<PointIterator>::enclosedPoints(
Expand All @@ -314,28 +281,15 @@ void KDTree<PointIterator>::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<Point>::dimensions() * 2;
std::pair<Point,Point> totalBound;

for( unsigned char i=0; i<VectorTraits<Point>::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<VectorTraits<Point>::dimensions(); j++ )
{
workingData[i].currentInnermost[j] = std::max( normals[i][j] * totalBound.first[j], normals[i][j] * totalBound.second[j] );
workingData[i].currentInnermost[j] = std::max( normals[i][j] * m_bound.first[j], normals[i][j] * m_bound.second[j] );
}
}

Expand Down
7 changes: 5 additions & 2 deletions test/IECore/KDTreeTest.inl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,12 @@ void KDTreeTest<T>::testNearestNeighbours()
for( typename Tree::Iterator it=m_points.begin(); it!=m_points.end(); it++ )
{
typename VectorTraits<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)
Expand Down
Loading