Skip to content

[codex] Add PLY meshfile support - #384

Merged
pariterre merged 5 commits into
pyomeca:masterfrom
mickaelbegon:codex/support-ply-meshfiles
Jul 15, 2026
Merged

[codex] Add PLY meshfile support#384
pariterre merged 5 commits into
pyomeca:masterfrom
mickaelbegon:codex/support-ply-meshfiles

Conversation

@mickaelbegon

@mickaelbegon mickaelbegon commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds PLY meshfile support alongside the existing STL/OBJ/VTP/bioMesh readers.

  • Makes meshfile extension dispatch case-insensitive.
  • Reworks the ASCII PLY reader to parse the header explicitly, read vertex x/y/z properties, ignore extra properties, and read face vertex_indices / vertex_index lists.
  • Triangulates polygonal PLY faces into MeshFace triangles.
  • Adds a .PLY fixture and a MeshFile.FileIoPly test covering uppercase extension handling and quad-face triangulation.

Validation

  • git diff --check
  • Attempted cmake --build build-vitpose-conda, but the local build currently fails in libc++ headers before project code completes (__builtin_ctzg / __builtin_clzg missing). The same toolchain issue appears while compiling unrelated files such as Joints.cpp.
  • ctest -N in build-vitpose-conda reports Total Tests: 0.

This change is Reviewable

@EveCharbie

Copy link
Copy Markdown
Collaborator

@pariterre @mickaelbegon is this ready for review ?

@mickaelbegon

Copy link
Copy Markdown
Contributor Author

yes. I think so

@pariterre pariterre left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello :)

Triangulates polygonal PLY faces into MeshFace triangles.

What does that mean? MeshFace does not seem to be called in the code?

@pariterre reviewed 9 files and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on mickaelbegon).


src/ModelReader.cpp line 2282 at r1 (raw file):

             ++iProperty) {
          const PlyProperty &property(element.properties[iProperty]);
          if (property.isList) {

Please add a test for when it is not a list

@mickaelbegon

Copy link
Copy Markdown
Contributor Author

Thanks for the review :)

I added coverage for the non-list face property case in cube.PLY: the face element now has a scalar material property after the vertex_indices list, so the reader exercises the branch that skips scalar face properties.

About the wording in the PR body: by “triangulates polygonal PLY faces into MeshFace triangles”, I meant that a PLY face with more than 3 vertices is split into triangles and added through Mesh::addFace({ ... }). That overload constructs a MeshFace internally, so the stored mesh still uses the existing triangular MeshFace representation.

@mickaelbegon

Copy link
Copy Markdown
Contributor Author

Small follow-up: the coverage build also showed that the new test needs the complete MeshFace type when calling faces().size(), so I added the missing RigidBody/MeshFace.h include as well.

@EveCharbie

Copy link
Copy Markdown
Collaborator

"triangulates polygonal PLY faces into MeshFace triangles".
@pariterre for vtp files, we have always done this outside of biorbd. I think the reason is that we want to write new triangular mesh files instead of repeating this triangulation process each time we load the biorbd model because it is long.
@mickaelbegon If we decide that this job belongs to biorbd, could you please open an issue to try to generalize the triangulation you have added to other file formats ?

@EveCharbie

Copy link
Copy Markdown
Collaborator

@pariterre could you follow up on this when you have time ? (I am ready on the BioBuddy side, so making a release once it is merged would be nice)

pariterre
pariterre previously approved these changes Jul 15, 2026

@pariterre pariterre left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

@pariterre reviewed 2 files and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on mickaelbegon).

@pariterre
pariterre marked this pull request as ready for review July 15, 2026 13:54
@pariterre

Copy link
Copy Markdown
Member

@mickaelbegon GG :)

@EveCharbie do you need a release of Biorbd?

@EveCharbie

EveCharbie commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator

Thanks !
Yes please :)

@pariterre pariterre left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

@pariterre reviewed 1 file and all commit messages, and made 1 comment.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on mickaelbegon).

@pariterre
pariterre merged commit 0cc1ce1 into pyomeca:master Jul 15, 2026
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants