-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
Add OpenMP to radius outlier removal #6182
Merged
larshg
merged 9 commits into
PointCloudLibrary:master
from
larshg:AddOpenMPRadiusOutlierRemoval
Dec 6, 2024
Merged
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
bcc9ed9
Set out.is_dense after copying the points, since else out.is_dense is…
larshg 0b04d31
Added OMP support to RadiusOutlierRemoval.
larshg c93bae9
Fix missing sharing attributes.
larshg fc3af6a
Remove the need for #pragma critical, by saving if a point should be …
larshg 28efda6
Preset to keep all points in radius outlier removal.
larshg 62779a5
Fix filtering to work correctly with negative_ and extract_removed_in…
larshg 23f7300
Make the negative test in the paralleled for loop.
larshg e1011b6
Improve per review.
larshg c23f6ba
Added radius_outlier_removal benchmark.
larshg File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Why is it necessary to put this last?
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.
When we copy from the original pointcloud, it also copy is_dense from the original cloud, which could be false, but after this filtering it should be true.
But I'm not sure if all filters should remove nan values if doesn't have to keep the output cloud organized.
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 checked and not all subclasses of
FilterIndices
guarantee thatindices
contain no invalid/nan points. E.g.RandomSample
may sample invalid points. Also for some other classes this might not be guaranteed ifnegative_==true
. Maybe it is best if we leave the code in this file unchanged (or deleteoutput.is_dense = true;
), and in the future we can consider modifyingcopyPointCloud
to check if the output cloud is now dense.I think it makes sense to remove nan values to stay consistent with
keep_organized_==false
. Also,user_filter_value_
may be anything (not necessarily nan).