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

Add a new CentroidPoint class #586

Merged
merged 4 commits into from
Mar 29, 2014

Conversation

taketwo
Copy link
Member

@taketwo taketwo commented Mar 21, 2014

Originally a part of #473, this turned out to be a self-contained feature which might be useful not only inside octree module.

A generic class that computes the centroid of points fed to it.

Here by "centroid" we denote not just the mean of 3D point coordinates, but also mean of values in the other data fields. The general-purpose computeNDCentroid() function also implements this sort of functionality, however it does it in a "dumb" way, i.e. regardless of the semantics of the data inside a field it simply averages the values. In certain cases (e.g. for x, y, z, intensity fields) this behavior is reasonable, however in other cases (e.g. rgb, rgba, label fields) this does not lead to meaningful results.

This class is capable of computing the centroid in a "smart" way, i.e. taking into account the meaning of the data inside fields. Currently the following fields are supported:

  • XYZ (x, y, z)

    Separate average for each field.

  • Normal (normal_x, normal_y, normal_z)

    Separate average for each field, and the resulting vector is normalized.

  • Curvature (curvature)

    Average.

  • RGB/RGBA (rgb or rgba)

    Separate average for R, G, B, and alpha channels.

  • Intensity (intensity)

    Average.

  • Label (label)

    Majority vote.

The template parameter defines the type of points that may be accumulated with this class. This may be an arbitrary PCL point type, and centroid computation will happen only for the fields that are present in it and are supported.

Current centroid may be retrieved at any time using get(). Note that the function is templated on point type, so it is possible to fetch the centroid into a point type that differs from the type of points that are being accumulated. All the "extra" fields for which the centroid is not being calculated will be left untouched.

The class is accompanied by a couple of computeCentroid() functions (designed upon the existing compute3DCentroid() and computeNDCentroid()), plus a bunch of unit tests.

@jpapon
Copy link
Contributor

jpapon commented Mar 24, 2014

This looks very nice! Just one nitpicky little thing:
This is usually slower than using find.. It probably doesn't matter since the number of labels will be small, but using find lets you do it in one lookup instead of two. Though maybe the compiler figures out to do it anyways.

By the way, kudos, using boost::fusion::vector gives a very nice clean solution. I wonder what the overhead cost is of it. I imagine very little though- the vector is created when the template is instantiated, so it's just iterating through at run time, right?

The unit tests look good too.

There are quite a number of functions dealing with centroid, mean, and
covariance computation. They all are grouped in 'common/centroid.h'
header. Therefore it makes sense to group their corresponding test
functions in a separate file. Hence creation of 'test_centroid.cpp'.
@taketwo
Copy link
Member Author

taketwo commented Mar 24, 2014

This is usually slower than using find.. It probably doesn't matter since the number of labels will be small

Good point! Let's use faster code anyways.

I wonder what the overhead cost is of it. I imagine very little though- the vector is created when the template is instantiated, so it's just iterating through at run time, right?

I am not 100% sure, but I think there is no run-time for loop involved. The for_each construction is unfolded at compile-time into as many calls to GetPoint::operator() as there are elements in the vector (because the contents of the vector are known at compile-time).

jspricke added a commit that referenced this pull request Mar 29, 2014
@jspricke jspricke merged commit 3d393a9 into PointCloudLibrary:master Mar 29, 2014
@taketwo taketwo deleted the add-centroid-point branch March 30, 2014 09:27
@taketwo taketwo mentioned this pull request Mar 7, 2015
SergioRAgostinho added a commit to SergioRAgostinho/pcl that referenced this pull request Nov 8, 2015
@SergioRAgostinho SergioRAgostinho mentioned this pull request Nov 8, 2015
SergioRAgostinho added a commit to SergioRAgostinho/pcl that referenced this pull request Nov 12, 2015
Addresses PointCloudLibrary#1413 , based on PointCloudLibrary#1170, modified to make use of PointCloudLibrary#586. Includes extra tests for RGBA and for testing downsamplealldata=false
SergioRAgostinho added a commit to SergioRAgostinho/pcl that referenced this pull request Nov 12, 2015
Addresses PointCloudLibrary#1413 , based on PointCloudLibrary#1170, modified to make use of PointCloudLibrary#586.
Includes extra tests for RGBA and for testing downsamplealldata=false
SergioRAgostinho added a commit to SergioRAgostinho/pcl that referenced this pull request Nov 16, 2015
Addresses PointCloudLibrary#1413 , based on PointCloudLibrary#1170, modified to make use of PointCloudLibrary#586.
Includes extra tests for RGBA and for testing downsamplealldata=false
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.

4 participants