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

[io] Specify no face elements in PLY files (from point cloud) to make them interoperable with VTK #4774

Merged
merged 1 commit into from
Jun 1, 2021

Conversation

tin1254
Copy link
Contributor

@tin1254 tin1254 commented May 25, 2021

Fix loading ply file by loadPolygonFilePLY as vtk requires face element as mentioned in #1470

I tested the fix with ply file here and the following code, it doesn't throw Cannot read geometry from vtk now. And the saved ply can be opened by meshlab without issue

#include <pcl/io/ply_io.h>
#include <pcl/io/vtk_lib_io.h>

int main(){
    pcl::PointCloud<pcl::PointXYZ> test_ply;
    pcl::PolygonMesh test_mesh;
    pcl::io::loadPLYFile("XXX.ply",test_ply);
    pcl::io::savePLYFile("XXX.ply",test_ply);
    pcl::io::loadPolygonFilePLY("XXX.ply",test_mesh);
    return 0;
}

@kunaltyagi kunaltyagi added changelog: enhancement Meta-information for changelog generation module: io labels May 25, 2021
@kunaltyagi kunaltyagi changed the title Add writing empty face element in ply writer [io] Specify no face elements in PLY files (from point cloud) to make them interoperable with Meshlab May 25, 2021
kunaltyagi
kunaltyagi previously approved these changes May 25, 2021
Copy link
Member

@kunaltyagi kunaltyagi left a comment

Choose a reason for hiding this comment

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

6 years for a fix... I think the record is 8 years

@larshg
Copy link
Contributor

larshg commented May 25, 2021

Should we add a inline comment to why it has to be there?

@kunaltyagi kunaltyagi dismissed their stale review May 25, 2021 12:18

lars makes a good point

Fix loading ply file by loadPolygonFilePLY as vtk requires face element
@tin1254
Copy link
Contributor Author

tin1254 commented May 25, 2021

Should we add a inline comment to why it has to be there?

Done

Copy link
Member

@mvieth mvieth left a comment

Choose a reason for hiding this comment

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

Great, thank you!
Minor side note: the written files were only incompatible with the VTK reader, right? Meshlab was never the problem (unlike the title suggests)?

@tin1254
Copy link
Contributor Author

tin1254 commented May 27, 2021

Great, thank you!
Minor side note: the written files were only incompatible with the VTK reader, right? Meshlab was never the problem (unlike the title suggests)?

Yeah, Meshlab was only used to ensure I didn't break the ply writer, I'll change the title

@tin1254 tin1254 changed the title [io] Specify no face elements in PLY files (from point cloud) to make them interoperable with Meshlab [io] Specify no face elements in PLY files (from point cloud) to make them interoperable with VTK May 27, 2021
@kunaltyagi kunaltyagi merged commit 202cbc2 into PointCloudLibrary:master Jun 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: enhancement Meta-information for changelog generation module: io
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants