feat(pose-lib): D-CLI apply — qtmesh pose --library apply#606
Merged
Conversation
…ply <name> -o <out> Adds the apply side of `--library` mode (the list side shipped in #604). Loads a mesh, loads a .poselib sidecar, applies the named pose to the skeleton, exports the posed mesh. ## CLI shape qtmesh pose <mesh.fbx> --library apply --lib <library.poselib> --apply <name> -o <out.fbx> `filePath` (positional) is the MESH in apply mode, contrasting with list mode where it's the library file. The library path arrives via `--lib` so the positional convention stays consistent across all `pose` modes (positional = primary input). ## Implementation - Reuses `MeshImporterExporter::importer` + `exportCurrentPose` — same path the `--animation --time` mode takes, just with the pose source being `PoseLibrary::applyPose` instead of an `AnimationState`. - Strict input validation: - mesh file exists - library file exists - mesh has a skeleton (apply needs one) - pose name exists in the library (otherwise lists available names on stderr so the user can fix their command line) - Sentry breadcrumbs `cli.pose` + `file.import` for telemetry. Help text updated next to the existing `--library list` line. No new tests in this PR — the existing PoseLibrary + CLI tests cover the loadPoseLibrary / applyPose paths in isolation, and the new code is just glue between them. ## #521 status | Sub-slice | Status | |-|-| | D1 — Singleton data layer | shipped (#592) | | D-MCP — 7 tools | shipped (#593, #599, #604) | | D3 — Undo commands | shipped (#595) | | D4 — Mirror pose | shipped (#597) | | D-Project — .poselib sidecar | shipped (#602) | | D-CLI — `pose --library list` | shipped (#604) | | D-CLI — `pose --library apply` | **this PR** | | D2 — Inspector subgroup | follow-up | | D5 — Apply-with-mask | follow-up | | D6 — Time-blended apply | follow-up | | D-Thumbnail | follow-up | Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe ChangesPose Library Apply Feature
Sequence DiagramsequenceDiagram
participant User as CLI User
participant Pipeline as cmdPose
participant Importer as MeshImporterExporter
participant PoseLib as PoseLibrary
participant Ogre as Ogre Entity
User->>Pipeline: --library apply --apply name --lib file.poselib
Pipeline->>Importer: importMesh(mesh_path)
Importer->>Ogre: Create Mesh with Skeleton
Importer-->>Pipeline: Return Entity
Pipeline->>PoseLib: Load from sidecar file
Pipeline->>PoseLib: Get pose by name
PoseLib-->>Pipeline: Return Pose
Pipeline->>Ogre: Apply pose to skeleton
Pipeline->>Importer: exportCurrentPose(output_path)
Importer-->>User: Posed mesh saved
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/CLIPipeline.cpp`:
- Around line 2547-2549: Before calling
MeshImporterExporter::exportCurrentPose(entity, outFi.absoluteFilePath()), add a
Sentry breadcrumb for the export action: call SentryReporter::addBreadcrumb()
with category "file.export", a short message like "Exporting mesh" and include
contextual data such as outputPath (or outFi.absoluteFilePath()) and the entity
identifier if available; then proceed to call exportCurrentPose and keep the
existing result handling. This ensures the export write is tracked alongside the
import.
- Around line 2508-2523: The loop currently selects the first Ogre::Entity
regardless of whether it is skinned, causing failure if an unskinned helper
appears first; change the selection logic (in the block using
Manager::getSingleton()->getEntities() and the variable entity) to iterate all
movables, for each obj with getMovableType() == "Entity" cast to Ogre::Entity
and immediately check hasSkeleton(), and pick/assign the first entity that
returns true; if none found, keep the existing err() message referencing
filePath and return 1.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
+2508
to
+2523
| auto& movables = Manager::getSingleton()->getEntities(); | ||
| Ogre::Entity* entity = nullptr; | ||
| for (auto* obj : movables) { | ||
| if (obj && obj->getMovableType() == "Entity") { | ||
| entity = static_cast<Ogre::Entity*>(obj); | ||
| break; | ||
| } | ||
| } | ||
| if (!entity) { | ||
| err() << "Error: Failed to load mesh: " << filePath << Qt::endl; | ||
| return 1; | ||
| } | ||
| if (!entity->hasSkeleton()) { | ||
| err() << "Error: Mesh has no skeleton — cannot apply a pose." << Qt::endl; | ||
| return 1; | ||
| } |
There was a problem hiding this comment.
Pick the first skinned entity, not just the first Entity.
This loop stops at the first Entity and only then checks hasSkeleton(). On multi-entity imports, an unskinned helper mesh appearing first will make --library apply fail even though a valid rigged mesh was loaded later in the scene.
🐛 Proposed fix
auto& movables = Manager::getSingleton()->getEntities();
Ogre::Entity* entity = nullptr;
for (auto* obj : movables) {
- if (obj && obj->getMovableType() == "Entity") {
- entity = static_cast<Ogre::Entity*>(obj);
- break;
- }
+ if (!obj || obj->getMovableType() != "Entity")
+ continue;
+ auto* candidate = static_cast<Ogre::Entity*>(obj);
+ if (!entity)
+ entity = candidate;
+ if (candidate->hasSkeleton()) {
+ entity = candidate;
+ break;
+ }
}
if (!entity) {
err() << "Error: Failed to load mesh: " << filePath << Qt::endl;
return 1;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| auto& movables = Manager::getSingleton()->getEntities(); | |
| Ogre::Entity* entity = nullptr; | |
| for (auto* obj : movables) { | |
| if (obj && obj->getMovableType() == "Entity") { | |
| entity = static_cast<Ogre::Entity*>(obj); | |
| break; | |
| } | |
| } | |
| if (!entity) { | |
| err() << "Error: Failed to load mesh: " << filePath << Qt::endl; | |
| return 1; | |
| } | |
| if (!entity->hasSkeleton()) { | |
| err() << "Error: Mesh has no skeleton — cannot apply a pose." << Qt::endl; | |
| return 1; | |
| } | |
| auto& movables = Manager::getSingleton()->getEntities(); | |
| Ogre::Entity* entity = nullptr; | |
| for (auto* obj : movables) { | |
| if (!obj || obj->getMovableType() != "Entity") | |
| continue; | |
| auto* candidate = static_cast<Ogre::Entity*>(obj); | |
| if (!entity) | |
| entity = candidate; | |
| if (candidate->hasSkeleton()) { | |
| entity = candidate; | |
| break; | |
| } | |
| } | |
| if (!entity) { | |
| err() << "Error: Failed to load mesh: " << filePath << Qt::endl; | |
| return 1; | |
| } | |
| if (!entity->hasSkeleton()) { | |
| err() << "Error: Mesh has no skeleton — cannot apply a pose." << Qt::endl; | |
| return 1; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/CLIPipeline.cpp` around lines 2508 - 2523, The loop currently selects the
first Ogre::Entity regardless of whether it is skinned, causing failure if an
unskinned helper appears first; change the selection logic (in the block using
Manager::getSingleton()->getEntities() and the variable entity) to iterate all
movables, for each obj with getMovableType() == "Entity" cast to Ogre::Entity
and immediately check hasSkeleton(), and pick/assign the first entity that
returns true; if none found, keep the existing err() message referencing
filePath and return 1.
Comment on lines
+2547
to
+2549
| QFileInfo outFi(outputPath); | ||
| const int result = MeshImporterExporter::exportCurrentPose(entity, outFi.absoluteFilePath()); | ||
| if (result != 0) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add a file.export breadcrumb before exporting.
This new apply flow records the mesh import, but not the output write.
🧭 Proposed fix
QFileInfo outFi(outputPath);
+ SentryReporter::addBreadcrumb("file.export",
+ QString("Exporting posed mesh %1").arg(outFi.absoluteFilePath()));
const int result = MeshImporterExporter::exportCurrentPose(entity, outFi.absoluteFilePath());📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| QFileInfo outFi(outputPath); | |
| const int result = MeshImporterExporter::exportCurrentPose(entity, outFi.absoluteFilePath()); | |
| if (result != 0) { | |
| QFileInfo outFi(outputPath); | |
| SentryReporter::addBreadcrumb("file.export", | |
| QString("Exporting posed mesh %1").arg(outFi.absoluteFilePath())); | |
| const int result = MeshImporterExporter::exportCurrentPose(entity, outFi.absoluteFilePath()); | |
| if (result != 0) { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/CLIPipeline.cpp` around lines 2547 - 2549, Before calling
MeshImporterExporter::exportCurrentPose(entity, outFi.absoluteFilePath()), add a
Sentry breadcrumb for the export action: call SentryReporter::addBreadcrumb()
with category "file.export", a short message like "Exporting mesh" and include
contextual data such as outputPath (or outFi.absoluteFilePath()) and the entity
identifier if available; then proceed to call exportCurrentPose and keep the
existing result handling. This ensures the export write is tracked alongside the
import.
|
fernandotonon
added a commit
that referenced
this pull request
May 18, 2026
…dcrumb (#607) Two CodeRabbit Major findings on PR #606 (merged). ## Pick first SKINNED entity, not first entity The entity loop stopped at the first `Entity` and then checked `hasSkeleton()`. On multi-entity imports an unskinned helper mesh (collision proxy, prop) often appears first; the old code would reject the apply even though a valid rigged mesh loaded later in the scene. Fix: walk the whole entity list. Track the first-seen entity (any kind) AND the first skinned one separately. Two distinct error paths now: - no entity at all → "Failed to load mesh" - entities exist but none skinned → "no skinned entity — cannot apply a pose" ## Add file.export breadcrumb on the export write The apply flow recorded the mesh import but not the output write. Added a `file.export` breadcrumb naming the target path right before `exportCurrentPose`. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds the apply side of
--librarymode (the list side shipped in #604). Loads a mesh, loads a.poselibsidecar, applies the named pose to the skeleton, exports the posed mesh.CLI shape
filePath(positional) is the mesh in apply mode, contrasting with list mode where it's the library file. The library path arrives via--libso the positional convention stays consistent (positional = primary input).Implementation
MeshImporterExporter::importer+exportCurrentPose— same path the--animation --timemode uses, withPoseLibrary::applyPosedriving the bones instead of anAnimationState.cli.pose+file.import.No new tests — existing PoseLibrary + CLI tests cover
loadPoseLibrary/applyPosepaths; the new code is glue.#521 status
pose --library listpose --library apply🤖 Generated with Claude Code
Summary by CodeRabbit
--applyand--libcommand-line parameters.