-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
Fix PLY save bug: vertex_index -> vertex_indices. #579
Conversation
Could you please also check why this test passes? |
@taketwo +1 it is a bug on my side. |
test_ply_mesh_io.cpp does save with savePLYFile, and read with loadPolygonFilePLY. loadPolygonFilePLY uses ply reader from vtk, which may take vertex_index, but loadPLYFile doesn't. It will be better if loadPLYFile can be added to the test. |
@yangyanli Could you add a simple regression test for the bug? |
But wait, how come that you changed the format that |
loadPolygonFilePLY is more tolerant than loadPLYFile. loadPLYFile only takes vertex_indices, loadPolygonFilePLY takes both. I do not have test build on my machines, and have no experience on the regression test, so it might take a while for me to do that. |
@yangyanli Well "regression test" is just a fancy name. Basically we just need to make sure that there is a test which fails with old code and succeeds with new code. In this particular case it is sufficient to add Also, you do not need to build all tests. Simply enable them (e.g. using |
Great! Merging this. |
Fix PLY save bug: vertex_index -> vertex_indices.
PLYReader does not take the *.ply files generated by PLYWriter due to the mismatch between vertex_index and vertex_indices. And it seems to be the fault of PLYWriter, so change vertex_index to vertex_indices.