Revise Voxel functionalities in VoxelHashMap and Preprocessing

[tizianoGuadagnino]

This unify the usage of Voxel and VoxelHash between Preprocessing and VoxelHashMap, so that all the using are in one and one place only.

[tizianoGuadagnino]

This now potentially invalidates #360, but I feel it is probably what we want.

[benemer]

I am a bit confused now, because we now have 3 or even 4 downsampling strategies with corresponding PRs 😆

How about first unifying the Voxel indexing by having one and only one PointToVoxel from the VoxelHashMap? We can still discuss together how we want to do the downsampling.

[benemer]

Ah, no, my confusion. The downsampled result will be the same.

This removes a lot of boilerplate code, nice!

I just don't like providing a dummy max_distance_ that we never use 🤔

[nachovizzo]

For the sake of records, once we also did this. But we decided to split it to avoid inter dependencies between the modules. With this change of I want to compile the preprocessing module I first need the voxel hash map. Additionally, we also wanted to leave the door open for people who might want to borrow the solely one module and thus most of the pieces are isolated (from a build perspective).

That said, we might revise this idea now 💡 as I don't think it was a the most use case of the project.

We could do this but I would avoid this "hacky" thing, as if one reads it is not clear why you are creating a "map"

If we want to merge the implementations I see 2 options

1. extract the voxelization utility into a separate module (hashing point to voxel, etc) I think we even did this moons ago 🤔
2. create a static method in the voxel hash map that can be used without creating an unnecessary object of the hash map class

Wdyt?

[nachovizzo]

@tizianoGuadagnino is it too much to ask now for splitting this into 2 PR's? 90% of the changes are style changes and not the original idea we had for this PR?

I will like to have 2 commits in the history, 1 for the moving the existing utilities to a separate header, and a follow up changing style, raw for loops for for each, etc

[tizianoGuadagnino]

@tizianoGuadagnino is it too much to ask now for splitting this into 2 PR's? 90% of the changes are style changes and not the original idea we had for this PR?

I will like to have 2 commits in the history, 1 for the moving the existing utilities to a separate header, and a follow up changing style, raw for loops for for each, etc

@nachovizzo I can, but spiritually, these two changes are actually one, which is we revise everything that regards Voxels. This includes VoxelUtils, VoxelHash, VoxelHashMap and Voxeldownsample. Or maybe you mean we do a separate PR for for to std::for_each?. If you have a strong opinion of this, I can do it, but can I consider all the changes accepted?

In the meantime, I will change the name of the PR to be consistent.

[nachovizzo]

@tizianoGuadagnino I updated this PR with the only changes that should be, without any style change

Let's follow up on the style changes in #364

[benemer]

Awesome, thank you guys :)