Skip to content

Fix UniformSampling filter by correcting distance calculation to voxel center#4328

Merged
mvieth merged 8 commits intoPointCloudLibrary:masterfrom
Micalson:bug_fix
Jul 25, 2022
Merged

Fix UniformSampling filter by correcting distance calculation to voxel center#4328
mvieth merged 8 commits intoPointCloudLibrary:masterfrom
Micalson:bug_fix

Conversation

@Micalson
Copy link
Copy Markdown
Contributor

UniformSampling uses the closest point to every voxel center as the sample point. But it uses the point closest to the voxel index as the sample point in the repo.

float diff_cur = ((*input_)[(*indices_)[cp]].getVector4fMap () - ijk.cast<float> ()).squaredNorm ();

}

// Compute the voxel center
Eigen::Vector4f voxel_center((ijk[0] + 0.5) * leaf_size_[0], (ijk[1] + 0.5) * leaf_size_[1], (ijk[2] + 0.5) * leaf_size_[2], 1);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if you're supposed to use the voxel center, but if you are:

ijk[3] = 0.5;
const Eigen::Vector4f voxel_center = (ijk + 0.5) * leaf_size_;

PS: Does voxel_center[3] have to be 1 or 0?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yours is the easier.
In theory, voxel_center[3] should be equal to 1 because of the following (data[3] = 1.0f;).


But if voxel_center[3] is equal to 0, you should also get the right results .

Regards,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested the code you suggested, but it was compiled incorrectly and displayed scalar cannot be added to the Vector4i type. I modify the code, and commit it, please check it. @kunaltyagi

@SergioRAgostinho
Copy link
Copy Markdown
Member

I'm just gonna add this here #4327 (comment)

You understood it right. The documentation is misleading and should be updated to reflect the actual implementation.

There's no point in implementing what the documentation describes because the VoxelGrid filter already has that functionality.

@Micalson
Copy link
Copy Markdown
Contributor Author

According to the source code, VoxelGrid uses the points' centroid in every voxel as the sampling point, and it is different from UniformSampling.

@SergioRAgostinho
Copy link
Copy Markdown
Member

Stack a RandomSample filter after the VoxelGrid filter.

Use an easier way to fix bugs
@stale
Copy link
Copy Markdown

stale Bot commented Sep 13, 2020

Marking this as stale due to 30 days of inactivity. Commenting or adding a new commit to the pull request will revert this.

@stale stale Bot added the status: stale label Sep 13, 2020
@mvieth
Copy link
Copy Markdown
Member

mvieth commented Jul 22, 2022

Closing and reopening to rerun checks ...

@mvieth mvieth closed this Jul 22, 2022
@mvieth mvieth reopened this Jul 22, 2022
@stale stale Bot removed the status: stale label Jul 22, 2022
@mvieth mvieth added module: filters changelog: fix Meta-information for changelog generation labels Jul 23, 2022
@mvieth mvieth changed the title Fix UniformSampling error, it selects error point as sample point Fix UniformSampling filter by correcting distance calculation to voxel center Jul 23, 2022
Copy link
Copy Markdown
Member

@mvieth mvieth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@mvieth mvieth merged commit 22940e4 into PointCloudLibrary:master Jul 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog: fix Meta-information for changelog generation module: filters

Projects

None yet

4 participants