-
Notifications
You must be signed in to change notification settings - Fork 320
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.
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
Remove GetPoints #389
Remove GetPoints #389
Conversation
Co-authored-by: Ignacio Vizzo <[email protected]>
…edundant_copy_of_points
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really get why we do this, but I'm also happy to proceed. I left some comments mainly on readability.
I'd also refactor the PR description as we don't remove the GetPoints
function but absorve its functionality in VoxelHashMap::GetClosestNeighbor
. Also I didn't understand the comment with the GetAdjacentVoxels
at all 🤣
Nope bruda, I literally remove it, as it was concatenating the inner points of each voxel into a single Thanks for the comments ❤️ I will integrate them now. About the |
Hi guys, it is me again. I just removed the GetPoints function, as it is now redundant because we moved the nn search to the
VoxelHashMap
, which has the same performance but less code.To follow up, move the does the voxel exist check from the NN search to the
AdjacentVoxels
function. This opens a lot of additional filtering that can be done on the NN search, which should (hopefully) reduce the compute.