Skip to content

mesh_sampling tool: Add support for colors - #2257

Merged
taketwo merged 2 commits into
PointCloudLibrary:masterfrom
mintar:mesh_sampling_colors
Apr 7, 2018
Merged

mesh_sampling tool: Add support for colors#2257
taketwo merged 2 commits into
PointCloudLibrary:masterfrom
mintar:mesh_sampling_colors

Conversation

@mintar

@mintar mintar commented Mar 20, 2018

Copy link
Copy Markdown
Contributor

No description provided.

@taketwo taketwo left a comment

Copy link
Copy Markdown
Member

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.

Comment thread tools/mesh_sampling.cpp Outdated

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
Copy Markdown
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)

Comment thread tools/mesh_sampling.cpp Outdated
Eigen::Vector4f p;
Eigen::Vector3f n;
randPSurface (polydata, &cumulativeAreas, totalArea, p, calc_normal, n);
Eigen::Vector3i c;

Copy link
Copy Markdown
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

mintar commented Apr 6, 2018

Copy link
Copy Markdown
Contributor Author

@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

taketwo commented Apr 7, 2018

Copy link
Copy Markdown
Member

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