Skip to content
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

CropHull::applyFilter3D speedup using AABB tree #5300

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yasamoka
Copy link
Contributor

@yasamoka yasamoka commented Jun 18, 2022

This pull request provides a tested AABB tree implementation using CGAL (Computational Geometry Algorithms Library).

This implementation is used to speed up CropHull::applyFilter3D which shoots rays into a scene and checks for intersections with the triangle mesh. On top of that, the function is also parallelized by setting the number of threads using CropHull::setNumberOfThreads.

A quick benchmark showing the difference in performance is provided below.

CPU: Intel Core i5 6300HQ
RAM: 16GB DDR4-2133 (dual channel)
Laptop: Dell XPS 15 9550

Mesh: David (Michelangelo) mesh (https://commons.wikimedia.org/wiki/File:David_(Michelangelo).stl)
Input cloud size: 10,000 points (randomly generated)
Number of threads: 4 (only applicable for new implementation)

The mesh is simplified using CGAL's surface mesh simplification.
The numbers for the AABB tree implementation include tree construction time which is automatically performed the first time applyFilter3D is called (if tree construction is not called explicitly first).

Implementation # Triangles Time Speedup
Default 10,000 5779 ms
AABB tree + OMP 10,000 34.3 ms 168.48x
Default 100,000 58549 ms
AABB tree + OMP 100,000 160 ms 365.93x

For simpler geometry, the speedup is lower but should still be respectable.

I have added a new module (aabb_tree) to the documentation. Please let me know if there are any improvements I could add there.

I would also appreciate feedback regarding the modifications to the CMake scripts that were required to make this work.

Thank you very much!

@oysstu
Copy link

oysstu commented Dec 15, 2022

Not a developer of PCL, but I saw this and thought I should mention that the AABB module in CGAL is licensed under GPL.

@yasamoka
Copy link
Contributor Author

Not a developer of PCL, but I saw this and thought I should mention that the AABB module in CGAL is licensed under GPL.

Thank you for the heads-up! Much appreciated.

Does this change anything in this pull request, though? As far as I know, this is not a derivative work of CGAL, so the project is not obliged to release any source code under the same GPL license. The CGAL libraries are built independently and are dynamically linked - no different than if a commercial project were, for instance, to use CGAL.

I hope I'm not missing something here.

@oysstu
Copy link

oysstu commented Dec 15, 2022

AFAIK linking with a GPL library means that the derivative work must fulfill the obligations of the GPL license. LGPL on the other hand permits linking without any obligations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants