Add: progressive morphological filter to extract ground returns #574
Conversation
|
|
||
| /** \brief Get the maximum window size to be used in filtering ground returns. */ | ||
| int | ||
| getMaxWindowSize () { return (max_window_size_); } |
|
I do not really like this organization with recursive calls to std::vector<int>
iteratePMF (const typename PointCloud::ConstPtr &source,
int iteration, float height_threshold, float window_size,
std::vector<int> indices);First is that a vector is used as a return type. Yes, most compilers will apply RVO, but as far as I know it is not guaranteed. Second is passing a vector inside by value, which will require a copy. |
|
@taketwo Let me know if you're more comfortable with this updated version. |
| window_size = cell_size_ * ( 2.0f * iteration++ * base_ + 1.0f ); | ||
| } | ||
|
|
||
| // Seed the indices vector with indices into the input point cloud. |
There was a problem hiding this comment.
This class derives from PCLBase which allows users to supply indices so that only a subset of points is processed. (Which is a quite useful feature actually.) So you should seed from indices_.
|
@taketwo Where do we stand on this? |
|
@chambbj Sorry got busy with other things. I will try to review this and the other request over the weekend. |
|
@taketwo Thanks. Both had issues with Travis (one failed and the other didn't even seem to trigger), so I can rebase with master and re-push to try and get clean builds prior to your review. |
|
Don't worry, I can build myself. I guess there will be some more rebases anyways so we'll see how Travis reacts. |
|
Still, I do not understand the need for recursion. It increases the memory usage and (IMHO) makes the code less comprehensible. Why can't we just use a good old |
| PCL_INFO ("[Batch processing mode] Added %s for processing.\n", itr->path ().string ().c_str ()); | ||
| } | ||
| } | ||
| //batchProcess (pcd_files, output_dir, resolution); |
|
@taketwo I believe all issues have been addressed with the latest commit. Let me know if you find anything else. |
|
Thanks, I think this version is so much better. I would ask you to add a space at line 116 and then I think we are ready to merge. Oh, and just in case. I do not know the details of the algorithm and the assumptions that you may make about float diff = cloud->points[p_idx].z - cloud_f->points[p_idx].z;
if (diff < height_thresholds[i])
pt_indices.push_back (ground[p_idx]);it rings a bell for potentially missing |
|
I'm comfortable with the difference and comparison. |
|
@taketwo One test timed out, but I think we may be able to merge now. Final thoughts? |
Add: progressive morphological filter to extract ground returns
No description provided.