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

Moves UniformSampling to filters, it's not a Keypoint #1411

Merged
merged 1 commit into from
Nov 3, 2015

Conversation

fran6co
Copy link
Contributor

@fran6co fran6co commented Oct 30, 2015

Fixes #1404

@gedikli
Copy link
Member

gedikli commented Nov 1, 2015

There is a reason why we did the single pass Mean and Covar estimation -> speed.
If the double pass is required, use those methods directly or call the single-pass with Eigen::Matrix3d matrices, which uses double as accumulator. That should also increase the accuracy.

@fran6co
Copy link
Contributor Author

fran6co commented Nov 1, 2015

Not sure how that relates to the UniformSampling move to filter. Did you meant to post it here => #1407 ?

@taketwo
Copy link
Member

taketwo commented Nov 2, 2015

LGTM
Regarding API break, I think we should keep the old header file, otherwise those who use this class won't be able to compile their code and will have no clue what happened and why the file just disappeared. So maybe wipe the old file, insert a warning macro with a descriptive message and include the file from it's new location?

@fran6co
Copy link
Contributor Author

fran6co commented Nov 2, 2015

Is it ok like this?

@taketwo
Copy link
Member

taketwo commented Nov 2, 2015

👍 but we'll need a yes from @jspricke

jspricke added a commit that referenced this pull request Nov 3, 2015
Moves UniformSampling to filters, it's not a Keypoint
@jspricke jspricke merged commit 887b3ea into PointCloudLibrary:master Nov 3, 2015
@mpflueger
Copy link

With this change some tutorials now require pcl 1.8 (I'm looking at correspondence_grouping, but there may be others), but the ubuntu PPA is still at 1.7. Perhaps the website needs a dropdown to select your pcl version when looking at a tutorial? Or at least a warning and link to the repo to find old versions?

@fran6co fran6co deleted the uniform branch June 30, 2016 23:46
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.

5 participants