Skip to content

suite du fbx pour corriger les erreurs cinématiques - #119

Open
mickaelbegon wants to merge 25 commits into
pyomeca:mainfrom
mickaelbegon:codex/add-fbx-segment-meshes
Open

suite du fbx pour corriger les erreurs cinématiques#119
mickaelbegon wants to merge 25 commits into
pyomeca:mainfrom
mickaelbegon:codex/add-fbx-segment-meshes

Conversation

@mickaelbegon

Copy link
Copy Markdown
Contributor

All Submissions:

  • Have you followed the guidelines in our Contributing document [docs/contribution.md]?
  • Have you checked to ensure there aren't other open [Pull Requests] for the same update/change?
  • Have you opened/linked the issue related to your pull request?
  • Have you used the tag [WIP] for on-going changes, and removed it when the pull request was ready?
  • When ready to merge, have you sent a comment pinging @EveCharbie in it?

New Feature Submissions:

  1. Does your submission pass the tests (if not please explain why this is intended)?
  2. Did you write a proper documentation (docstrings and ReadMe)
  3. Have you linted your code locally prior to submission (using the command: black . -l120")?

Changes to Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new examples for your core changes, as applicable?
  • Have you written new tests for your core changes, as applicable?

Comment thread biobuddy/components/real/biomechanical_model_real.py Outdated
@mickaelbegon
mickaelbegon force-pushed the codex/add-fbx-segment-meshes branch from 3368d63 to b46e6b5 Compare June 3, 2026 22:08
@codecov-commenter

codecov-commenter commented Jun 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.70579% with 53 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.26%. Comparing base (c25892f) to head (b46e6b5).
⚠️ Report is 173 commits behind head on main.

Files with missing lines Patch % Lines
biobuddy/model_parser/fbx/fbx_model_parser.py 91.83% 44 Missing ⚠️
biobuddy/model_parser/bvh/bvh_model_parser.py 88.37% 5 Missing ⚠️
...obuddy/components/real/biomechanical_model_real.py 90.69% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #119      +/-   ##
==========================================
+ Coverage   91.12%   91.26%   +0.14%     
==========================================
  Files          98      109      +11     
  Lines        8234    10099    +1865     
==========================================
+ Hits         7503     9217    +1714     
- Misses        731      882     +151     
Flag Coverage Δ
unittests 91.26% <91.70%> (+0.14%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@EveCharbie

Copy link
Copy Markdown
Collaborator

@codex .ply mesh files are not readable by biorbd. There seems to be an easy way to translate .ply to .stl files using trimesh. Could you please try to export the mesh files segmented by segments to a .stl format ? If the solution is more than 30 lines of code, please make a plan first.

@EveCharbie

Copy link
Copy Markdown
Collaborator

@mickaelbegon

  1. I don't get the package export. Could you please explain this feature ?
  2. I guess is some cases we would prefer not to re export the mesh files each time we read a .fbx file. Would it be possible to check if the files already exist and skip ? This means that there is no overwrite, which might sometimes be desirable. The other solution would be to do like for the vtp files and to parse/translate them separately from the model parsing/translation (files are in the folder mesh_parser).
  3. I would prefer if we splitted the model parsing from the kinematics parsing. So we would have model = BiomechanicalModelReal().from_fbx and kinematics = Kinematics().from_fbx. The Kinematics class would be what you called ParsedAnimation I guess, if would just have to be renamed and moved to the biobuddy/utils folder.

@pariterre

Copy link
Copy Markdown
Member

There seems to be an easy way to translate .ply to .stl

Biobrd could also support ply? I don't think it is a hard fix...

@EveCharbie

Copy link
Copy Markdown
Collaborator

There seems to be an easy way to translate .ply to .stl

Biobrd could also support ply? I don't think it is a hard fix...

That would be great ! Thanks @pariterre :)

@EveCharbie

Copy link
Copy Markdown
Collaborator

@mickaelbegon .ply are now available on the latest biorbd release :)
However, my 3 questions from June 8th still need adressing before I can merge this.

@mickaelbegon

  1. I don't get the package export. Could you please explain this feature ?
  2. I guess is some cases we would prefer not to re export the mesh files each time we read a .fbx file. Would it be possible to check if the files already exist and skip ? This means that there is no overwrite, which might sometimes be desirable. The other solution would be to do like for the vtp files and to parse/translate them separately from the model parsing/translation (files are in the folder mesh_parser).
  3. I would prefer if we splitted the model parsing from the kinematics parsing. So we would have model = BiomechanicalModelReal().from_fbx and kinematics = Kinematics().from_fbx. The Kinematics class would be what you called ParsedAnimation I guess, if would just have to be renamed and moved to the biobuddy/utils folder.

@mickaelbegon

mickaelbegon commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

I addressed the remaining FBX review items in commits f0b5be6 and 7883048.

  1. package_from_fbx is a convenience export for a portable artifact: one .bioMod, per-segment PLY meshes, an optional q/time/DoF archive plus diagnostics, and a copy of the source FBX. It is not part of model parsing. I left its scope unchanged in this pass; now that biorbd reads PLY, it no longer needs STL conversion.

  2. Mesh splitting no longer overwrites generated files by default. BiomechanicalModelReal().from_fbx(..., split_meshes_per_segment=True) reuses an existing segment mesh and attaches it to the model; overwrite_meshes=True explicitly regenerates it. The new test covers preservation and forced replacement.

  3. Kinematics is now separated from model loading: BiomechanicalModelReal().from_fbx(...) and .from_bvh(...) build models, while Kinematics().from_fbx(...) and .from_bvh(...) return q, time and DoF names. The PR-specific ParsedAnimation and model-level q_from_* shortcuts were removed.

I also fixed the 12 current CI failures by isolating parser/writer tests with the tmp_path fixture; the targeted FBX/BVH test set is now 34 passed, 1 skipped.

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.

4 participants