unibas-gravis / scalismo

Scalable Image Analysis and Shape Modelling
https://scalismo.org
Apache License 2.0
247 stars 65 forks source link

Feat/native ply io #418

Closed madsendennis closed 9 months ago

madsendennis commented 10 months ago

Native scala implementation to read/write the PLY format. The implementation is ~20 times faster to write a PLY file and ~3 times faster to read.

Currently is still missing support for TexturedFile.

This is the first part of removing the VTK dependency in Scalismo, https://github.com/unibas-gravis/scalismo/issues/415

Andreas-Forster commented 10 months ago

There is an implementation in scalismo.faces for reading and writing plys. How does this implementation relates to the other one? Is it completely independent?

madsendennis commented 10 months ago

There is an implementation in scalismo.faces for reading and writing plys. How does this implementation relates to the other one? Is it completely independent?

Ohh damn. Should have looked in there first. This one is completely independent. Maybe we should just copy over the scalismo faces version instead. I found a few java implementations, but they all missed a few things, so ended up creating one from scratch.

madsendennis commented 10 months ago

The PLYMesh write/reader in scalismo.faces would also work. It has a lot more structure to it. And this library has been tested a lot more. The question is how to structure it such that we only move all the parts that has with the format supported in scalismo. And, the implementation is rather slow: half the speed on reading and ~15 times slower for writing (compared to the one suggested in the MR).

Andreas-Forster commented 10 months ago

Regarding the two options:

madsendennis commented 10 months ago

I'll have a closer look at how the file is read/written and see if we can do some improvement on the scalismo-faces version. I think it would look nicer to have an extension of the base-ply functionality in scalismo-faces. But speed is key for me - so only if it's not slower. For now I put the MR in draft mode.

madsendennis commented 9 months ago

Some more profiling between my simple method found in the PLY.scala file and the ported scalismo-faces ply reader found under the io/ply folder:

Facemesh with 53.466 vertices and 106014 triangles: Writing test (16 times faster)

Reading test (5.5 times faster)

For a larger mesh with 575.781 vertices and 1.147.466 triangles Writing test (40 times faster)

Reading test (3.3 times faster)

I did try to swap the reader in scalismo-faces with the java ByteBuffer as I use, but at best I could make it double the speed. There is a lot of inefficient structure in the faces implementation with large mutable data structures that are appended to. I did try to change some of these to predefined Arrays, but also with limited success.

So I'm more in favour of keeping a simple reader in scalismo and having a more advanced (also for texture meshes) in scalismo-faces.

But maybe someone else can see how we can somehow easily make a merge of the two?

Andreas-Forster commented 9 months ago

I would be open to merge the fast PLY IO and later check if we can speedup scalismo-faces. As long as they can read each others files for those parts that both support, this is the best option with the least overhead.

madsendennis commented 9 months ago

I updated the ply reader to follow the structure of the stl reader to be more coherent. Hopefully, it will be easier to read now that it is split across multiple files. I also added a more general way to read properties from the ply file. Now it reads all kinds of properties (also texture if it is there), but it only makes use of the items requested, i.e. only vertex and faces if requesting to read a TriangleMesh.