Maniphest T101294

Implement a PLY importer/exporter in C++
Needs Triage, NormalDESIGN

Assigned To
Nathan Rozendaal (super_jo_nathan)
Authored By
Nathan Rozendaal (super_jo_nathan)
Sep 23 2022, 11:59 AM
Tags
  • Pipeline, Assets & I/O
  • Import/Export
Subscribers
Aras Pranckevicius (aras_p)
Arjan van Diest (McDoodly)
bas hendriks (bashendriks10)
Bastien Montagne (mont29)
Keir (KeirCB)
Lilith Houtjes (Stroopwafel)
Michael Prostka (MProstka)
4 More Subscribers
Tokens
"Love" token, awarded by bashendriks10."Love" token, awarded by softyoda."Love" token, awarded by Stroopwafel."Love" token, awarded by Tetone."Love" token, awarded by Dangry.

Description

The problem

There is an importer/exporter for PLY already implemented in Blender, but it is implemented in Python which means it is slow for bigger files. The importer is also missing support for point cloud PLY files with vertex colors. Since PLY is often used for laser scan data (which are big files that contain a point cloud and sometimes vertex colors) it would be a shame not to address these issues.

The idea

We are a group of software engineering students from the university of applied sciences Windesheim in the Netherlands. And we would like to rewrite the current Python based importer/exporter to C++, with a focus on performance, extendibility and implementing missing functionality.


Requirements

Importer
  • Vertex data
  • Face data
  • Edge data
  • Vertex colors
  • Vertex normals
  • UV data
Exporter
  • Vertex data
  • Face data
  • Edge data
  • Vertex colors
  • Vertex normals
  • UV data
  • Triangulation
Nonfunctional requirements
  • The C++ based importer/exporter should be 5x faster than the current Python based implementation.
  • The behavior and results of current python add-on should be matched as closely as possible. Divergences, if any, should be clearly justified and documented.

Priorities

Our main focus will be implementing a fully featured importer, since we feel that will benefit Blender users the most. But having the importer in C++, and the exporter in Python is not optimal So we will split the development team into two parts, so we can work on the importer and exporter in parallel.

Possible additions

The PLY file format supports user-defined properties; we could make use of this fact by exposing these properties to geometry nodes as attributes. This would allow us to generically support all user defined properties (which we personally think is very cool). But some research needs to be done to see if this is practical and able to be implemented.

Scope

Since this is a student project, we have a definite deadline (20th of January). And after that deadline we won’t be able to work on the importer/exporter as much, if at all. @Aras Pranckevicius (aras_p) is willing to maintain the code after the project ends (if we send him some cookies), so we will be counting on him.

Testing strategy

For this project we are planning on utilizing test driven development. Detailed unit tests will be written for a large part of the added code. We will also gather a large amount of PLY files from different sources, to keep track of which files get loaded correctly. Both fine grain and coarse grain tests will be written.

If there is functionality missing in this design, or if you have other feedback. Please let us know.

Revisions and Commits

Event Timeline

Nathan Rozendaal (super_jo_nathan) created this task.Sep 23 2022, 11:59 AM
Daniel Gryningstjerna (Dangry) awarded a token.Sep 25 2022, 9:49 PM
Aras Pranckevicius (aras_p) added a comment.Sep 26 2022, 9:15 AM

Looks good! Questions I have:

  1. "should be faster" -- by how much? e.g. I'd imagine if it's 20% faster, then it's not worth replacing the current (Python) one with the C++ one. If it's 2x faster, then it's an open question. If it's 10x faster, then it's a "yes, replace of course".
  2. "utilizing test driven development" -- to what extent and at what granularity? Would it be more like "unit testing" (testing individual pieces of the importer/exporter), or more like "functionality testing", perhaps based on exporting some .blend files and comparing result to "golden" .stl results? The latter is what OBJ importer/exporter mostly uses, btw.
  3. Do you plan to write the importer/exporter by first closely following the existing Python one, i.e. "first literally port it to C++", or do you plan to write the new one based on PLY spec only? The first approach has an advantage that you'd likely find various "under-specified bits" or gotchas that the existing Python code had to work around with. My experience with OBJ C++ rewrite is that because it was "written from scratch", it had to re-discover 10 or 20 various issues that were handled by Python code, but are not there in any official OBJ spec.
Nathan Rozendaal (super_jo_nathan) added a comment.Sep 26 2022, 11:56 AM

Thank you for your feedback! We have discussed your questions within our team.

  1. After looking at the initial implementation D3114: Move STL import/export to C++ (WIP) and the current implementation D14941: Native STL Importer of the C++ based STL importer/exporter. We think that a 5x speed increase can be expected at least, so we will set that as a requirement.
  2. We would like to implement both fine grain testing of individual pieces of code, and functionality testing of the importer/exporter as a whole. With functionality testing you can only see when the importer/exporter is broken or functional. But you can’t see where the fault is located if there is one. We hope to decrease time spent debugging for us and future developers by writing fine grain tests.
  3. Our plan is to read through the current Python based implementation first, to deeply understand what it is doing. Using that information and the information given by the specs we will design the C++ based implementation. So, to answer your question: a combination of both.
Paul Larson (GeorgiaPacific) added a subscriber: Paul Larson (GeorgiaPacific).Sep 28 2022, 8:01 AM
Nathan Rozendaal (super_jo_nathan) updated the task description.Sep 28 2022, 11:12 AM
Arjan van Diest (McDoodly) added a subscriber: Arjan van Diest (McDoodly).Sep 28 2022, 11:41 AM
Michael Prostka (MProstka) added a subscriber: Michael Prostka (MProstka).Sep 30 2022, 12:36 PM
Bastien Montagne (mont29) moved this task from Backlog to Long-term - Nice To Have on the Pipeline, Assets & I/O board.Oct 6 2022, 10:30 AM
Aras Pranckevicius (aras_p) added a comment.Oct 6 2022, 5:11 PM

Possible thing to also keep in mind: this abandoned Python patch, that's meant for improving PLY import support for some software that produces "slightly odd" file syntax. https://developer.blender.org/D14718

Bastien Montagne (mont29) added a subscriber: Bastien Montagne (mont29).Oct 7 2022, 11:13 AM

Project looks good to me, I would just add to the requirements something like "Match as closely as possible the behavior and results of current python add-on. Divergences, if any, should be clearly justified and documented."

Working on the importer first makes sense, but I would strongly recommend also considering the exporter as mandatory for this project. Would really rather not have a situation where we have a fully finished working importer in C++, and exporter still in python add-on.

stefano severi (Tetone) awarded a token.Oct 8 2022, 1:58 PM
Keir (KeirCB) added a subscriber: Keir (KeirCB).Oct 9 2022, 10:05 PM

Very excited for this,
In the possible additions section you discuss other attributes.
In point clouds generated with photogrammetry you generate the normals of the points. This is incredibly useful for mesh generation and is for me an integral attribute, I hope this can make it into the final release.

Lilith Houtjes (Stroopwafel) awarded a token.Oct 10 2022, 10:09 AM
Lilith Houtjes (Stroopwafel) added a subscriber: Lilith Houtjes (Stroopwafel).
Nathan Rozendaal (super_jo_nathan) added a comment.Oct 10 2022, 1:59 PM
In T101294#1428606, @Aras Pranckevicius (aras_p) wrote:

Possible thing to also keep in mind: this abandoned Python patch, that's meant for improving PLY import support for some software that produces "slightly odd" file syntax. https://developer.blender.org/D14718

@Michael Prostka (MProstka) contacted us and gave some helpful pointers about what the current importer struggles with. So we are definitely keeping this in mind!

Nathan Rozendaal (super_jo_nathan) added a comment.Oct 10 2022, 2:02 PM
In T101294#1429084, @Bastien Montagne (mont29) wrote:

Project looks good to me, I would just add to the requirements something like "Match as closely as possible the behavior and results of current python add-on. Divergences, if any, should be clearly justified and documented."

Thanks for the feedback! Where should these divergences be documented?

Working on the importer first makes sense, but I would strongly recommend also considering the exporter as mandatory for this project. Would really rather not have a situation where we have a fully finished working importer in C++, and exporter still in python add-on.

We will set the exporter as part of the minimum viable product.

Nathan Rozendaal (super_jo_nathan) added a comment.Oct 10 2022, 2:06 PM
In T101294#1429823, @Keir (KeirCB) wrote:

Very excited for this,
In the possible additions section you discuss other attributes.
In point clouds generated with photogrammetry you generate the normals of the points. This is incredibly useful for mesh generation and is for me an integral attribute, I hope this can make it into the final release.

Thanks for the feedback! We will try our best to support this. But we are sadly not able to guarantee this functionality.

Bastien Montagne (mont29) added a comment.Oct 10 2022, 2:42 PM
In T101294#1430140, @Nathan Rozendaal (super_jo_nathan) wrote:
In T101294#1429084, @Bastien Montagne (mont29) wrote:

Project looks good to me, I would just add to the requirements something like "Match as closely as possible the behavior and results of current python add-on. Divergences, if any, should be clearly justified and documented."

Thanks for the feedback! Where should these divergences be documented?

In the design task first, then in the release notes once pushed to master.

Nathan Rozendaal (super_jo_nathan) updated the task description.Nov 16 2022, 10:33 AM
Nathan Rozendaal (super_jo_nathan) updated the task description.Nov 16 2022, 10:35 AM
T.R.O. Nunes (3Rton) added a subscriber: T.R.O. Nunes (3Rton).Nov 21 2022, 11:16 PM
Nathan Rozendaal (super_jo_nathan) mentioned this in D16792: New C++ based ply importer/exporter.Dec 16 2022, 2:51 PM
yoann (softyoda) awarded a token.Dec 19 2022, 2:15 PM
yoann (softyoda) added a subscriber: yoann (softyoda).
Nathan Rozendaal (super_jo_nathan) updated the task description.Dec 20 2022, 1:02 PM
Yoran Huzen (Kwonunn) added a subscriber: Yoran Huzen (Kwonunn).Dec 20 2022, 1:03 PM
bas hendriks (bashendriks10) awarded a token.Dec 20 2022, 1:03 PM
bas hendriks (bashendriks10) added a subscriber: bas hendriks (bashendriks10).
Nathan Rozendaal (super_jo_nathan) added a revision: D16792: New C++ based ply importer/exporter.Jan 9 2023, 11:50 AM
Nathan Rozendaal (super_jo_nathan) mentioned this in D17067: New C++ based PLY importer/exporter.Fri, Jan 20, 5:01 PM
Nathan Rozendaal (super_jo_nathan) added a revision: D17067: New C++ based PLY importer/exporter.Fri, Jan 20, 5:06 PM