fix(anim): retarget follow-ups — facing-blind side rule, arm/leg rescue, posture-aware takes; bump 3.36.1 - #971
Conversation
…wapped on UniRig skeletons Motion-library templates (and every applyMotionClip consumer) mapped bones to canonical L/R roles purely by NAME. UniRig-predicted skeletons name their sides with the opposite convention from imported rigs (the #951 fleet norm), so every clip landed mirrored: the left-leg track drove the anatomical right leg (user-reported). applyMotionClip now runs a name-independent side check after role resolution: the character's TRUE left is up × forward (forward from the caller's mesh-derived yaw180 facing), and the signed sum of dot(namedLeft − namedRight, trueLeft) over the paired roles (hips/collars/upper-arms/hands/feet) is compared against the sign calibrated on the known-good Mixamo case (measured -2.36; the fleet norm). The opposite sign means the names mirror the anatomy — every L/R body role (and V2 finger side) is swapped so names match geometry for the whole retarget. ApplyMotionResult::sideSwapApplied reports it; QTMESH_T2M_SIDE_SWAP=1/0 forces/disables, QTMESH_T2M_SIDE_DEBUG=1 prints the score. Verified live: Mixamo Rumba -2.36 → no swap (unchanged); AutoRig template -1.76 → no swap; UniRig 46-bone rig +0.95 → swap, and the generated punch now turns the same way as the Mixamo reference (forced-off A/B reproduces the old mirrored/contorted result). The legacy AnimationMerger test fixture pins the check off (its synthetic rigs are +X-left-named with side-specific assertions); a dedicated test covers norm/mirrored/yaw180 detection. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ng on mis-named rigs The user's UniRig-rigged orc showed 'arms/legs swapped' with template clips — literally: the UniRig geometric labeler classified both A-POSE arm chains (which descend almost as steeply as legs) as legs, naming them LeftUpLeg_1/ LeftLeg_1/LeftFoot_1, so the retarget's leg roles drove the arms and the arm roles went unresolved. Three fixes: - UniRigPredictor::labelJointsAnatomically: arm-vs-leg is now decided by ATTACH HEIGHT relative to the root (chains off the upper spine are arms even when they drop; chains off the root are legs), with the old direction rule kept only as the tie-breaker in the ambiguous band. Unit-tested with an A-pose fixture the old rule mislabels. - applyMotionClip: the #969 consistency block now also runs an altitude guard for rigs that were ALREADY rigged with bad names: leg-role bones bound well above the hips are stripped and — when the arm roles are vacant — rescued onto them (side chosen geometrically in the post-swap convention); 'arm'-role bones below the hips are dropped. The side-swap score is computed on the cleaned mapping. - detectBackwardFacing: the ankle reference now only trusts foot-role bones in the lowest quarter of the skeleton — chest-height '*Foot_1' bones dragged the foot band into the torso on mis-named rigs. Verified on the reporting asset (orc_low_rigged.mesh, UniRig): walk/punch now render an upright, correctly-sided gait with the rescued arms swinging; Mixamo and template-AutoRig behavior unchanged (side scores -2.36/-1.76, no swap, no rescue). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ured (review) Review (Codex P1 + CodeRabbit Major): the new side swap and the legacy compensateCanonicalHandedness were two INDEPENDENT permutations that could cancel on a forward-facing mirror-named rig (body swapped back, V2 fingers left swapped), and the side score read the CURRENT pose (mid-animation crossed limbs could flip the vote). Composed into one decision: the #969 block now replaces the old compensator on the library path (cmuLibraryHandedness) entirely — facing-aware, computed on the CLEANED mapping, from BIND positions (same skel reset the old compensator used), V2 finger sides included. Convention anchored on the validated case: canonical-LEFT roles end on the bones at +trueLeft (anatomical left) — exactly the state the old compensator produced on Mixamo, which therefore takes the same single swap as before (score -2.36), while anatomically-named UniRig rigs now take none. The mocap path (cmuLibraryHandedness=false) is untouched. Rescue side selection updated to the same convention; regression test updated to assert the composed behavior on both conventions and under yaw180. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
'The low poly run plays backwards' (user report, and reproduced on Mixamo — so library DATA, not the rig): measured chest lean over the library's run takes is -0.23 / -0.01 / -0.34 — two of three tip the torso backward >= 13°, which with a locked root reads as backpedaling. Walk takes are all upright (±0.04), matching 'walk looks fine'. MotionLibrary now computes Clip::uprightness (mean chest up-vector z, pure meanChestLean helper) at parse time, and pickAmong samples takes by takeWeight(): the #855 quality² times a posture penalty — locomotion takes (walk/run) with uprightness < -0.10 get 2% weight, so the upright take is effectively always chosen while alternatives exist, with a graceful degradation when an action has none. Non-locomotion actions are exempt (sit/crawl/death legitimately tip). Both helpers are pure + unit-tested; verified live: 3/3 orc runs now pick the upright take and render an upright run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… variant); chore: bump 3.36.1
User-reported regression: with the facing-aware trueLeft side rule, many
template animations came out L/R inverted on the -Z-facing UniRig orc ('like
the t2m training problems'). The library was extracted and validated against
the OLD compensator's rule, which is facing-blind: canonical-LEFT roles end
on the bones at world +X, period — facing is the clip-level yaw180's job and
must never leak into the side mapping. The composed block keeps everything
else (cleaned mapping, pair-summed measurement, V2 fingers, altitude rescue)
but decides on world X. Mixamo unchanged (-2.36 → one swap); the orc now
swaps (-0.44 vs +X) and its clips match the reference convention.
Version 3.36.1 for the follow-up bugfix release; doc refs synced.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (6)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe project version and CI references were updated to 3.36.1. Animation side detection now uses world +X and revised rescue rules. Local-frame motion clips now receive neutral uprightness. ChangesRelease and motion updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This PR corrects animation retargeting, skeleton-role classification, and motion-take selection while updating release references; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description provides a clear summary of the retargeting fixes, posture-aware sampling, version bump, and test results. The template's Technical Details, Features, and Bugfixes headings are not used explicitly, but the required information is covered in the summary. The PS1 section is not applicable. Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 4 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches 💡 1📝 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 |
![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: 806d72795a
ℹ️ 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".
| } else if (armRole && bonePos[i].y < hipY - 0.10f * bodyH) { | ||
| boneToCanon[i] = -1; // "arm" below the hips — drop |
There was a problem hiding this comment.
When a valid A-pose or relaxed bind pose has the wrist or elbow more than 10% of body height below the hips, this branch unmaps that correctly named arm bone solely because of its altitude. Unlike the chest-height leg path, these stripped arm roles are not rescued later, so library animations omit their tracks and leave lower arms or hands fixed in the bind pose. The guard should classify the chain using its attachment/topology rather than dropping each arm segment based on its individual height.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed by dropping the arm-below-hips strip entirely: a relaxed/hanging bind pose legitimately puts wrists below the hips and stripped arm roles had no rescue — the hands would freeze. The observed real-world failure is leg-named arms, so only the leg-altitude guard (with its rescue) remains (83bfb2e)
| } | ||
| clip.quality = static_cast<float>( | ||
| std::clamp(co.value("quality").toDouble(1.0), 0.0, 1.0)); | ||
| clip.uprightness = meanChestLean(clip.quats); |
There was a problem hiding this comment.
For supported schema-v1/v2 libraries, clip.quats contains parent-relative rotations, but meanChestLean treats joint 2 as a world-space chest orientation. On a legacy library with multiple walk or run takes, torso lean encoded in the hip/abdomen ancestors is therefore ignored, while an innocent local chest counter-rotation can be penalized as backward lean, changing take selection incorrectly. Compose the ancestor rotations for local-frame schemas, or apply this weighting only when m_worldFrame is true.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed: uprightness is computed only when the library is world-frame (schema v3+); legacy local-frame clips keep a neutral 0 so the penalty can never act on a parent-relative chest value (83bfb2e)
…te world-frame only - The symmetric arm-below-hips strip is dropped: a relaxed/hanging bind pose legitimately puts wrists below the hips, and stripped arm roles had no rescue — hands would freeze in bind. Leg-named-arms is the observed failure mode; the leg-altitude guard + rescue stays. - Clip::uprightness stays neutral (0) for legacy local-frame libraries — meanChestLean reads joint 2 as a world orientation, which only schema v3+ world-frame clips carry; a local chest counter-rotation could otherwise be mispenalized while real lean in the ancestors went unseen. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…-detection # Conflicts: # src/AnimationMerger.cpp # src/AnimationMerger_test.cpp # src/MotionLibrary.cpp
|
1 participant
Summary
Follow-ups to #969 (which was merged mid-iteration — master currently carries the first-cut facing-aware side rule that inverts many template animations on backward-facing rigs, user-reported):
trueLeftvariant from fix(anim): detect mirror-named rigs — templates no longer apply L/R swapped on UniRig skeletons #969 is reverted. One composed permutation replacescompensateCanonicalHandednesson the library path (cleaned mapping, pair-summed, V2 fingers included, bind-pose measured) — the two-independent-permutations cancellation flagged by review is gone.detectBackwardFacingonly trusts foot bones in the lowest quarter of the skeleton.Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation