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

mesh_sampling tool: Add support for colors #2257

Merged
merged 2 commits into from
Apr 7, 2018

Conversation

mintar
Copy link
Contributor

@mintar mintar commented Mar 20, 2018

No description provided.

Copy link
Member

@taketwo taketwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides from inline comments, LGTM.

@@ -81,7 +79,18 @@ randomPointTriangle (float a1, float a2, float a3, float b1, float b2, float b3,
}

inline void
randPSurface (vtkPolyData * polydata, std::vector<double> * cumulativeAreas, double totalArea, Eigen::Vector4f& p, bool calcNormal, Eigen::Vector3f& n)
randomPointTriangle (float a1, float a2, float a3, float b1, float b2, float b3, float c1, float c2, float c3,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this overload? (see next comment)

@@ -135,7 +169,8 @@ uniform_sampling (vtkSmartPointer<vtkPolyData> polydata, size_t n_samples, bool
{
Eigen::Vector4f p;
Eigen::Vector3f n;
randPSurface (polydata, &cumulativeAreas, totalArea, p, calc_normal, n);
Eigen::Vector3i c;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can as well be Eigen::Vector4f c, which can be passed directly to the existing version of randomPointTriangle().

@taketwo taketwo added needs: author reply Specify why not closed/merged yet module: tools labels Mar 20, 2018
@mintar
Copy link
Contributor Author

mintar commented Apr 6, 2018

@taketwo : I've updated the PR according to your comments.

@taketwo taketwo merged commit 4d1ab04 into PointCloudLibrary:master Apr 7, 2018
@taketwo taketwo added this to the pcl-1.9.0 milestone Apr 7, 2018
@taketwo taketwo removed the needs: author reply Specify why not closed/merged yet label Apr 7, 2018
@taketwo
Copy link
Member

taketwo commented Apr 7, 2018

Thanks!

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

Successfully merging this pull request may close these issues.

2 participants