PLYLoader fails to parse PLYExporter output

[dbkaplun]

The error:

    TypeError: Cannot read property 'properties' of undefined
      33 |       .toEqual(plyData.toString('utf8'));
      34 | 
    > 35 |     const plyGeom = new THREE.PLYLoader().parse(plyData);
      36 |     expect(plyGeom.getAttribute('position').count)
      37 |       .toEqual(geom.vertices.length);
      38 |   });
      
      at parseASCII (node_modules/three/examples/js/loaders/PLYLoader.js:273:72)
      at Object.<anonymous>.THREE.PLYLoader.parse (node_modules/three/examples/js/loaders/PLYLoader.js:471:15)
      at _callee2$ (test/VFFLoader.test.js:35:43)

Here's the repro.

    const plyData = await fixture('airway.vff.ply');
    expect(new THREE.PLYExporter().parse(exportMesh))
      .toEqual(plyData.toString('utf8'));

    const plyGeom = new THREE.PLYLoader().parse(plyData); // throws error
    expect(plyGeom.getAttribute('position').count)
      .toEqual(geom.vertices.length);

To test: (sorry for the annoying repro steps)

git clone git@github.com:CustomOrthopaedics/vff-loader.git
cd vff-loader
git checkout 766457
npm install
npm test

[Mugen87]

I'm unable to load your file airway.vff.ply in Blender. And Sketchfab produces a weird result:

@dbkaplun Can you please export airway.vff as glTF and share the file in this thread?

[dbkaplun]

@Mugen87 that output actually looks correct. The input file contains points only and no faces. Presumably the BufferGeometry infers faces from the points and that's why it looks so wonky.

The bug is that the THREEjs-exported PLY file cannot be read by the PLY reader. For reference, I am using this to unit test a custom loader, by comparing loaded data against a pre-exported PLY.

Actually I would rather use another format for testing, like STL (because Github supports STL previews), but most of them are incapable of exporting points-only objects. So this is low priority for me.

[donmccurdy]

Another option would be GLTFExporter -> GLTFLoader, both of which do support point data.

[Mugen87]

Presumably the BufferGeometry infers faces from the points and that's why it looks so wonky.

This was a picture of Sketchfab. They are not using three.js.

[Mugen87]

/ping @gkjohnson

[gkjohnson]

Hey @dbkaplun! It sounds like VFF is a point cloud format, which the ply exporter doesn't currently handle. I'd be happy to help with updating the exporter so it does, though.

I'm not able to run the repro steps at the moment, but it looks like the error on import might be because the face count is exported as a fractional number, which is obviously bad (and should probably throw an error when exporting).

Ultimately the exporter should be updated to handle point data export by omitting the element and face property fields in the header. I think the right solution would be to exclude the face list completely when exporting geometry with no index array, but that gets a bit more complicated when exporting an Object3D hierarchy with multiple meshes of different types. At the very least I can add that to the list of limitations of the exporter.

I'll have to think about how to handle multiple meshes a bit more and look into what the PLY format offers to support that. I'll add a couple issues in the PLY exporter repo when I get a chance, as well.

Thanks!
Garrett

[gkjohnson]

So it doesn't look like PLY supports exporting multiple mesh types or vertex formats in a single file, which makes the case of exporting an object with multiple different mesh types a bit more complicated:

Exported Object3D
└ Point Cloud Mesh
└ Triangle Mesh w/o Normals
└ Triangle Mesh w/  Normals

I think the best we can do is to default to the common set of properties needed to represent all the meshes. So if one mesh in the set requires normal data or an index array, then it will be provided / generated for all meshes / vertices. If this causes issues with one of the meshes (number of positions is not divisible by 3) then we can throw an error or print a warning. This should only ever be a problem if an Objec3D containing multiple children is being exported. But that should serve this use case just fine.

@dbkaplun @Mugen87 thoughts?

Thanks!
Garrett

[Mugen87]

I think this sounds reasonable. 👍

[gkjohnson]

@dbkaplun It still needs a little more testing, but here's a branch that will auto exclude unused properties in the exported file, if you want to give it a try:

https://github.com/gkjohnson/ply-exporter-js/tree/dev/auto-exclude-properties

There's a binary exporter there, which I haven't PR'd into THREE, yet, as well.

[dbkaplun]

@gkjohnson the branch looks fine to me. I filed this ticket for three.js's sake so it is low priority for me like I said.