fix(mocap): stop high-knee landmark aims from flipping Mixamo legs - #957
Merged
Conversation
Landmark getRotationTo on near-antiparallel thigh aims (high knee / brief tracking glitches) was folding legs 180° up the back while the MediaPipe overlay still looked correct. Clamp leg swing to 115° toward torso forward, and refuse PoseIK quats that would invert the thigh. Co-authored-by: Cursor <cursoragent@cursor.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe leg retargeting path now clamps large landmark swings, uses PoseIK for near-neutral landmarks, rejects inverted thigh quaternions, and falls back to valid aims or the base pose. Two regression tests cover high-knee and seated-neutral poses. ChangesMocap leg retargeting
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This is a localized mocap leg-retargeting change with regression coverage; no actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant MocapLandmarks
participant LegRetargeting
participant clampAimSwing
participant PoseIK
participant BasePose
MocapLandmarks->>LegRetargeting: provide leg landmark aims
LegRetargeting->>clampAimSwing: normalize and clamp aim swing
clampAimSwing-->>LegRetargeting: return clamped aim
LegRetargeting->>PoseIK: request articulation for near-neutral aim
PoseIK-->>LegRetargeting: return candidate thigh quaternion
LegRetargeting->>LegRetargeting: check thigh direction
LegRetargeting->>BasePose: use fallback when the quaternion inverts the thigh
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![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: c14e3bd05d
ℹ️ 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
+2594
to
+2595
| const bool dirNearNeutral = | ||
| haveLmAim && dref.dotProduct(dsLeg) > 0.9995f; |
There was a problem hiding this comment.
When calibration occurs while the subject is seated or already has a raised leg, the live direction naturally equals dref, so this condition selects the quaternion branch below. Because quatDeltaArtic is relative to the same neutral frame, its delta is identity and it returns the rig's bind/standing base rather than aligning the leg to the captured neutral direction; a stationary seated subject therefore snaps to standing and remains there. The near-neutral fallback needs to retain the absolute neutral landmark alignment when composing the quaternion delta.
Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment.
Valid catch — thanks. Fixed in 2981297: when landmarks are near the calibrated neutral, the PoseIK delta is now composed onto the landmark-aligned local (not bind base), so a seated/raised-leg calibration stays put when live ≈ dref. Added BodyRetargeterSeatedNeutralDoesNotSnapToStanding to lock it in.
When live leg aim matches the calibrated neutral, composing PoseIK's identity delta onto bind snapped seated/raised-leg calibrations back to standing. Compose onto the landmark-aligned aim instead, and cover it with a regression test. Co-authored-by: Cursor <cursoragent@cursor.com>
|
fernandotonon
added a commit
that referenced
this pull request
Aug 24, 2026
…n + bind-anchored roll (#958) * feat(anim): bind-anchored roll baseline (refRoll) — #954 reference-independence for the twist channel The retarget's twist channel zeroes at the extraction's reference frame, so re-extractions that pick different references shift arm bend-planes by the reference roll. Export the source's bind->reference roll per role (parent-relative — immune to the armature world offset that blocks bind comparisons) and add it to the twist channel so the roll baseline anchors to the SOURCE BIND regardless of reference choice. Empty refRoll -> legacy behavior (shipped libraries unaffected). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(anim): Arm height slider — pitch arm chains up/down about the torso lateral axis (#957) The existing Arm space slider swings about the torso FORWARD axis, so clips whose arms point forward (raised-arm walks — 2017 Quaternius packs) sit ON its rotation axis and don't respond ('-30 still too wide'). Arm height pitches about the LATERAL axis instead: negative lowers the arms toward hanging, positive raises. Same absolute + idempotent per-clip contract as arm-space (rename migration, clear-on-regenerate, live paused-clip refresh). Surfaces: Inspector 'Arm height' slider under Arm space; CLI --arm-elevation (generate + standalone --animation adjust). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * wip(#954): parent-chain space correction for deep bones — NOT CONVERGED, do not merge Blender ground truth (headless import of the Quaternius Woman) proves the importer raises Blender-FBX arm chains ~90deg: Blender plays the walk with arms HANGING (upper arm dominant -Z in its Z-up world) while our import holds them horizontal. Root cause: the animation-channel space correction uses the bone's OWN bind mismatch; per-bone node-vs-skin bind mismatches accumulate along ancestors, so deep bones need a PARENT-CHAIN correction. Three formulations tested (full-chain / root-rebased / hybrid): arms come down but the global body orientation scrambles (hip line reads +Y). Mixamo rigs identity/unaffected in all variants. Next session: dump per-bone node binds vs skin binds for the Woman's arm chain and solve C from data before recompiling. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(anim): #954 — apply the FBX animation space-change to ROOT bones only The #936 space-change C = ogreBindLocal·nodeBindLocal⁻¹ was applied to EVERY bone. By chain induction the correct keyframe for a bone with an Ogre parent is simply ogreBindLocal⁻¹·rawKey — applying C to deep bones double-corrects any bone whose skin bind differs from its node bind, which on Blender-FBX rigs (per-bone PreRotations on the arm chain) rotated whole ARM CHAINS ~90° into the air while legs stayed correct. The ROOT is the one bone whose Ogre parent chain differs from its node ancestors, and #936's re-rooting bakes those ancestors into the root bind (G = ancestors·N), making the own-bind form exact exactly there. Verified: Quaternius Woman walk now plays with hanging, swinging arms (matches Blender's import — the external ground truth); Quaternius lowpoly punch recovers its chest-high boxing guard (matches the July gold library); Mixamo (Rumba) raw-anatomy output is bit-identical. Fixes the regression that poisoned the motion-library re-extraction (#954, hosted library had been rolled back on 2026-08-21). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(anim): exclude the hip from the bind-anchored roll baseline (#954) Role 0's 'roll' about its vertical axis is whole-body FACING, which the retarget deliberately anchors to the TARGET's own bind. Re-basing it injected the source armature's yaw convention as a constant ~80-96 deg hip twist (user-reported: torso forward, hips/legs sideways on the Woman clips). Spine/limb roles keep the bind-anchored baseline. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(anim): clip-averaged FACING in the canonical extraction (#954) The canonical frame's facing came from the hip line at the single reference frame — a reference that catches the pelvis BLADED (running mid-stride, seated twist) yaws the whole clip ~90 deg, so the retarget renders the character sideways (user-reported on the Woman Run/Sit). Average the hip line over ALL frames (horizontal component, magnitude-weighted) and yaw-correct C so the mean lands on canonical +X — the same treatment the up-leveling already applies to tilt. Clips with square references are unchanged (mean == reference). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(anim): facing-aware reference detection + bladed-reference audit (#954) Two extraction fixes for whole-clip yaw: 1. The bind-vs-animation frame detection only compared torso UPRIGHTNESS — after the #954 import fix the up axes agree, so a bind whose FACING differs ~90 deg from the animations (Quaternius Woman) passed as a valid reference and yawed the whole canonical clip (Run/Sit rendered sideways while Walk was fine). Also compare the HIP LINE at bind vs the sampled animated frame (leveled, horizontal): > ~40 deg apart => the bind is not a usable reference, search animated frames instead. 2. Reference-frame audit: whichever path picked the reference, if its hip line deviates > ~40 deg from the CLIP-MEAN hip line (a bladed mid-stride / seated twist), re-pick across all frames with a combined uprightness x facing-squareness score (fixed the Punch reference). Plus clip-averaged FACING for C (mean hip line -> canonical +X), the yaw analog of the existing clip-averaged up-leveling. Verified on Rumba front views: Woman Run/Sit/Walk all face the camera; all 8 Woman clips now carry lateral (+-X-dominant) collar references. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Revert "feat(anim): Arm height slider — pitch arm chains up/down about the torso lateral axis (#957)" This reverts commit e08df62. * review(#958): parity harness consumes refRoll; drop the Arm height slider - --apply-canonical now parses the dump's refRoll and passes it through applyMotionClip so self-retarget parity exercises the same bind- anchored twist path as the library retarget (Codex P2). - The Arm height slider (+ CLI --arm-elevation, controller, core) is REVERTED per user decision — with the #954 importer + extraction fixes the clips it was built to rescue extract correctly, so it was UI clutter; git history keeps it if ever needed. Its removal also moots the arm-space/elevation composition-order concern (Codex P1). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
4 tasks
fernandotonon
added a commit
that referenced
this pull request
Aug 25, 2026
Restore the 115° thigh/shin swing cap from #957 so antiparallel aims still clamp, and keep plantarFlexAim on torso-forward when the shin is near-vertical so occluded-toe standing feet do not snap ±180° from cross-product noise. Co-authored-by: Cursor <cursoragent@cursor.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.
Summary
getRotationTo.BodyRetargeterHighKneeDoesNotFlipThighregression covering the extreme high-knee singularity.Test plan
UnitTests --gtest_filter='AnimationMergerTest.BodyRetargeter*'(3/3 pass)Made with Cursor
Summary by CodeRabbit
Bug Fixes
Tests