feat(pose-lib): sub-slice D4 — mirror pose (_l ↔ _r heuristic)#597
Merged
Conversation
Fourth sub-slice of #521 (named-pose library). Adds the "mirror this asymmetric pose to the other side" operation that every DCC tool ships — turn a left-side facial expression into its right-side counterpart with one click. ## What ships ### `PoseLibrary::flipBoneName(name)` static helper Pure function (no Ogre dependency) that maps bone names across the three common rig conventions: - `_l` ↔ `_r` and `_L` ↔ `_R` (Mixamo, generic) - `.L` ↔ `.R` (Blender) - `Left*` ↔ `Right*` (Maya prefix), where the prefix must be followed by an uppercase letter, underscore, or end-of-string so "Lefty" doesn't flip to "Righty" Bones that don't match any rule (centre-line: Spine, Hips, Head) pass through unchanged. Exposed publicly so future apply-with-mask code can reuse the same name-mapping. ### `PoseLibrary::mirrorPose(entity, srcName, dstName)` Reads `srcName`, applies the X-symmetric reflection to every bone, saves as `dstName`. The TRS flip is: - `pos.x → -pos.x` - rotation: `(w,x,y,z) → (w,x,-y,-z)` (X-symmetric reflection of an orientation — conjugating by the reflection-X transform) - `scale.x → -scale.x` (negative scale preserves volume and produces correct orientation after the renderer applies the bone transform) For each source bone, the resulting TRS is written under the mirrored bone name returned by `flipBoneName`. Centre-line bones get the X-flipped TRS in place (they ARE on the axis of symmetry, so the X-flip is a self-mirror). The implementation rounds-trip-applies the source pose through the live skeleton to read its bone TRS (same idiom the commands use; PoseLibrary's public API doesn't expose pose snapshots directly). Live state is restored before and after. ## What's deferred - D4-Cmd / MCP / CLI surface for mirror — wait for the natural follow-up PR that wires the Inspector subgroup (D2). Right now the API is callable from C++ + tests; no QML/MCP/CLI yet. - Undo coverage — mirror is "save a new pose" semantically, so it benefits from SavePoseCommand around it. D2's Inspector wiring will push the right command. ## 6 new tests - 4 `FlipBoneName_*` standalone tests cover each suffix/prefix pattern plus the "Lefty" non-flip edge case. - `MirrorPoseProducesXFlippedSnapshot` — apply the mirrored pose to a clean skeleton and verify pos/rot are X-flipped. - `MirrorPoseRejectsMissingSourceAndEmptyDst` — error-path validation (missing src, empty dst, null entity). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 279c14f545
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+249
to
+253
| applyPose(entity, srcName); | ||
| for (unsigned short i = 0; i < skel->getNumBones(); ++i) { | ||
| Ogre::Bone* bone = skel->getBone(i); | ||
| if (!bone) continue; | ||
| BonePoseSnapshot bs; |
There was a problem hiding this comment.
This code rebuilds src by applying srcName and then recapturing every current skeleton bone, which makes the mirror result depend on whatever live transforms remain on bones that are not actually in the saved pose. In the same file, applyPose already documents partial-apply behavior for skeleton/LOD differences, so this path can silently pull unrelated live bone values into the mirrored pose after topology changes. To keep mirroring deterministic and faithful to srcName, build src from the stored pose snapshot itself (or iterate only the source pose entries) instead of recapturing the full live skeleton.
Useful? React with 👍 / 👎.
|
fernandotonon
added a commit
that referenced
this pull request
May 18, 2026
…ct (#598) Codex P2 on PR #597 (merged): The mirror code rebuilt the source pose by applying it to the live skeleton then re-capturing every bone. For saved poses that only covered a subset of the current skeleton (LOD swap, partial save), that pulled live state of un-mirrored bones into the result — non-deterministic and not faithful to the saved data. ## Fix — two halves ### Read source from m_byEntity directly Member function has access to the private store. Look up the `{entity, srcName}` snapshot directly and iterate it. Only bones that were actually saved feed the mirror — matches `applyPose`'s "partial apply" contract on the read side. ### Write mirrored result directly Previously: applied mirrored snapshot to live skeleton, called `savePose`, restored live state. That was a 3-step round-trip through the public API only because the public API was the only write surface I'd given the mirror code. Now that we're inside the class anyway, write mirrored straight into `m_byEntity` plus the parallel `order` list, and emit `posesChanged` ourselves. Removes the temporary skeleton mutation entirely. Behaviour preserved on the test-visible surface — same X-flip math, same `posesChanged` signal, same insertion-order semantics. The new path is just simpler and deterministic. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fernandotonon
added a commit
that referenced
this pull request
May 18, 2026
) Fifth sub-slice of #521 (named-pose library). Persists each entity's pose library to a `.poselib` sidecar JSON file so libraries survive across editor sessions and can be version-controlled alongside the asset they describe. Unblocks meaningful CLI surface (`qtmesh pose --library list/apply`) and any future "save project" workflow that wants pose data folded in. ## File format — `qtmesheditor.poselib.v1` ```json { "schema": "qtmesheditor.poselib.v1", "poses": [ { "name": "JawOpen", "bones": { "JawBone": { "t": [x,y,z], "r": [w,x,y,z], "s": [x,y,z] } } } ] } ``` Insertion order is preserved (the library walks `entIt->order` when writing). Bone names are the stable identifier — matches the in-memory representation, so the LOD/skeleton-swap resilience carries over to disk. ## What ships - `PoseLibrary::savePoseLibrary(entity, filePath)` — `QSaveFile` atomic write. Rejects empty libraries (don't write a `poses: []` file that loadPoseLibrary would reject), empty paths, null entities. - `PoseLibrary::loadPoseLibrary(entity, filePath)` — strict schema check, replaces existing per-entity library 1:1 (partial overlay would be confusing UX on name collision). Emits `posesChanged`. - `*ForSelection` Q_INVOKABLE wrappers for Inspector / MCP use. ## 4 tests - `SaveAndLoadLibraryRoundTripsViaSidecar` — write, forget, load, verify pose list + TRS values survive. - `SaveLibraryRejectsEmptyLibraryOrInvalidPath` — empty library (no poses saved), empty path, null entity → false. - `LoadLibraryRejectsMissingFileAndBadSchema` — three error paths: missing file, bad JSON, valid JSON with wrong schema string. - `LoadLibraryWipesExistingPosesFirst` — load replaces, doesn't merge: in-memory pose that's not in the file is dropped. ## #521 status | Sub-slice | Status | |-|-| | D1 — Singleton data layer | shipped (#592) | | D-MCP — MCP tools (5: list/save/apply/delete/mirror) | shipped (#593, #599) | | D3 — Undo commands | shipped (#595) | | D4 — Mirror pose | shipped (#597) | | D-Project — `.poselib` sidecar | **this PR** | | D2 — Inspector subgroup | follow-up | | D5 — Apply-with-mask | follow-up | | D6 — Time-blended apply | follow-up | | D-Thumbnail | follow-up | | D-CLI | now unblocked — follow-up | Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fernandotonon
added a commit
that referenced
this pull request
May 18, 2026
Closes the GUI / MCP / CLI triad for the pose library now that D-Project (#602) provides the .poselib sidecar format. ## What ships ### MCP tools (2 new) - `save_pose_library` — required `path`. Wraps `PoseLibrary::savePoseLibraryForSelection`. Returns ok+path on success; error on no selection / empty library / unwritable path. - `load_pose_library` — required `path`. Wraps `loadPoseLibraryForSelection`. Returns ok+path+count on success; error on no selection / missing file / malformed JSON or schema (in-memory library is preserved on parse failure thanks to the D-Project staging fix). ### CLI mode (new branch in `cmdPose`) `qtmesh pose <library.poselib> --library list [--json]` — reads the sidecar JSON directly (no mesh load) and prints the pose names. JSON shape mirrors the other CLI tools: `{ file, count, poses: [name…] }`. Doesn't go through `PoseLibrary` itself — there's no entity to key against in a standalone CLI invocation. Just parses the JSON. This intentionally bypasses the strict schema-replacement semantics that `loadPoseLibrary` enforces on a live entity; for read-only listing, schema-checking the file before walking is enough. `--library apply <name> -o out.fbx` is deliberately not in this PR — it needs the round-trip exporter to write pose-driven bone states back into the mesh. Documented in the help text as a follow-up. ## 3 new MCP tests - `SavePoseLibrary_MissingPathRejected` — empty args → error mentions 'path'. - `LoadPoseLibrary_MissingPathRejected` — same for load. - `LoadPoseLibrary_MissingFileRejected` — nonexistent file → error. ## Manual CLI verification - `qtmesh pose test.poselib --library list` → "Poses (N): …" - `qtmesh pose test.poselib --library list --json` → JSON shape. - `qtmesh pose /nonexistent.poselib --library list` → exit 1 with "File not found:". ## #521 status | Sub-slice | Status | |-|-| | D1 — Singleton data layer | shipped (#592) | | D-MCP — 5 + 2 = 7 tools | shipped (#593, #599, **#604**) | | D3 — Undo commands | shipped (#595) | | D4 — Mirror pose | shipped (#597) | | D-Project — .poselib sidecar | shipped (#602) | | D-CLI — `qtmesh pose --library list` | shipped (**this PR**) | | D2 — Inspector subgroup | follow-up | | D5 — Apply-with-mask | follow-up | | D6 — Time-blended apply | follow-up | | D-Thumbnail | follow-up | | D-CLI apply | follow-up (needs exporter round-trip) | Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fernandotonon
added a commit
that referenced
this pull request
May 18, 2026
…ply <name> -o <out> (#606) 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>
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.
Fourth sub-slice of #521 (named-pose library). Adds the "mirror this asymmetric pose to the other side" operation every DCC tool ships — turn a left-side facial expression into its right-side counterpart with one click.
What ships
PoseLibrary::flipBoneName(name)static helperPure function (no Ogre dependency) mapping bone names across three common rig conventions:
_l↔_rand_L↔_R(Mixamo / generic).L↔.R(Blender)Left*↔Right*(Maya prefix), where the prefix must be followed by uppercase / underscore / end-of-string so "Lefty" doesn't flip to "Righty"Bones that don't match any rule (centre-line: Spine, Hips, Head) pass through unchanged. Exposed publicly so future apply-with-mask code can reuse it.
PoseLibrary::mirrorPose(entity, srcName, dstName)Reads
srcName, applies the X-symmetric reflection to every bone, saves asdstName. TRS flip:pos.x → -pos.x(w, x, y, z) → (w, x, -y, -z)(X-symmetric reflection of an orientation)scale.x → -scale.x(negative scale preserves volume and produces correct orientation)For each source bone, the resulting TRS is written under the mirrored bone name from
flipBoneName. Centre-line bones get the X-flipped TRS in place (they ARE on the axis of symmetry).6 new tests
FlipBoneName_*standalone tests covering each suffix/prefix pattern + the "Lefty" non-flip edge case.MirrorPoseProducesXFlippedSnapshot— apply mirrored pose and verify pos/rot are X-flipped.MirrorPoseRejectsMissingSourceAndEmptyDst.#521 status
.poselibsidecar🤖 Generated with Claude Code