Stanford (.ply) - Point cloud export
Needs ReviewPublic

Authored by Christian Brinkmann (poor) on Dec 7 2017, 5:57 PM.

Details

Summary

This patch adds the ability to export point clouds (loose geometry) via ply.

Cheers,
Christian

Diff Detail

Campbell Barton (campbellbarton) requested changes to this revision.EditedDec 13 2017, 12:46 PM

It would be best to implement this by exporting all loose vertices, if a mesh has a single face - no vertices will be exported.

Also, this patch removes vertex alpha support by accident.

Edit: It might be slow to detect loose vertices, this could be an option.

This revision now requires changes to proceed.Dec 13 2017, 12:46 PM

Minor style issue.

io_mesh_ply/export_ply.py
56

Pep 8 style: Multiple statements on one line

Christian Brinkmann (poor) updated this revision to Diff 9722.EditedDec 20 2017, 2:25 PM
Christian Brinkmann (poor) retitled this revision from Stanford (.ply) - Point cloud export to Stanford (.ply) - Point cloud export.
Christian Brinkmann (poor) edited the summary of this revision. (Show Details)

Thanks for your review @Campbell Barton (campbellbarton) . Good catch, I noticed that it's essentially the same for loose edges.

I've updated the patch to reflect both cases and attached a blend with all possible scenarios I can think of (mesh only, point cloud, mesh with loose edges and vertices etc.).

The exporter is now able to export all components of the geometry. However, I had to re-write the code to maintain the vertex indices for reference. Positive side effect is that in case the mesh has no uv's, the exporter now only writes single vertices an their attributes to the file, instead of writing all vertices per face. In my tests all my scenarios are working and the geometry came back to blender as expected. The importer seems to handle all that cases very great, although we could add removing possible doubles at the end, but I'm not sure whether that's good practice switching to edit mode and so on...

What do you think?

Edit: There are only 3 components per vertex color, right?

This patch makes too many unrelated changes. I was hoping this could be done by keeping existing code as is, with an optional block to write extra vertices.

From what I can tell this should be possible. Optionally loop over ply_verts, storing each used vertex in an array of booleans. Then loop over the array, adding any unused vertices to ply_verts with attributes (color, uv)

io_mesh_ply/export_ply.py
42

This change seems unnecessary and causes an extra dict lookup in tight loops.

100–109

UV's and normals are no longer de-duplicated.

123

Remove the square brackets, since this creates a list in memory instead of using an iterator.

For quick test: compare the performance of any([i for i in range(2**128)]) with any(i for i in range(2**128))

157

Alpha is still being removed in this patch.

170–190

Refactoring string formatting shouldn't be part of this patch, it complicates review. Best only make functional changes.

Other changes can be made later and reviewed separately.

Campbell Barton (campbellbarton) requested changes to this revision.Jan 26 2018, 12:34 AM
This revision now requires changes to proceed.Jan 26 2018, 12:34 AM
Christian Brinkmann (poor) updated this revision to Diff 10148.EditedMar 3 2018, 8:28 PM
Christian Brinkmann (poor) marked an inline comment as done.

Thanks for your review Campbell, it really helped me pushing the patch forward. I rolled back all unnecessary changes as requested and hope that this allows a better review.

Just adding extra vertices is an option of course, but as user I'd expect to get all loose edges as well, right?

diff --git a/io_mesh_ply/export_ply.py b/io_mesh_ply/export_ply.py
index 8de5d674..084c39b8 100644
--- a/io_mesh_ply/export_ply.py
+++ b/io_mesh_ply/export_ply.py
@@ -131,6 +131,13 @@ def save_mesh(filepath,
                 vert_count += 1
 
             pf.append(pf_vidx)
+    
+    # Loose vertices
+    ply_mesh_indices = set([v[0] for v in ply_verts])
+    if len(mesh_verts) > len(ply_mesh_indices):
+        for vert in mesh_verts:
+            if vert.index not in ply_mesh_indices:
+                ply_verts.append((vert.index, vert.normal[:], uvcoord, color))
 
     fw("ply\n")
     fw("format ascii 1.0\n")

By just appending the vertices to ply_verts we loose the actual vertex order. Problem is, without having that in place, I don't see any chance to support loose edges. Even if there is one single loose edge, then all edges (including that ones of the mesh) need to be written to the file as well as referencing to the corresponding vertices in the file, which is the main reason I'm using a dict, since we can get the order for free, also we can save some lines if the object has no UV's.

I also did some speed tests with this Ramose Murex Model (added one extra vert and one loose edge) and found out that the patch is even faster writing the file on my machine:

Current Exporter

UV'sElapsed timeFile size
No120.61 sec~657mb
Yes192.65 sec~860mb

Patch

Loose PropertyUV'sElapsed timeFile size
disabledNo52,17 sec~250mb
enabledNo64,35 sec~300mb
disabledYES123,61 sec~860mb
enabledYES133,98 sec~944mb

Anyway, just let me know if you have any better idea or I can do something else.

Cheers,
Christian