From 8540c9823ff1f1863abdb84342a67e404d8d5abb Mon Sep 17 00:00:00 2001 From: Fernando Date: Tue, 1 Sep 2026 17:51:52 -0400 Subject: [PATCH 1/4] =?UTF-8?q?fix(anim):=20detect=20mirror-named=20rigs?= =?UTF-8?q?=20=E2=80=94=20templates=20no=20longer=20apply=20L/R=20swapped?= =?UTF-8?q?=20on=20UniRig=20skeletons?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/AnimationMerger.cpp | 78 ++++++++++++++++++++++++++++++ src/AnimationMerger.h | 2 + src/AnimationMerger_test.cpp | 94 ++++++++++++++++++++++++++++++++++++ 3 files changed, 174 insertions(+) diff --git a/src/AnimationMerger.cpp b/src/AnimationMerger.cpp index c03545a7..1a955bca 100644 --- a/src/AnimationMerger.cpp +++ b/src/AnimationMerger.cpp @@ -4268,6 +4268,84 @@ AnimationMerger::ApplyMotionResult AnimationMerger::applyMotionClip( if (!canonSeen[c]) { canonSeen[c] = 1; ++distinct; } } } + // ── #969 name-vs-anatomy side check ──────────────────────────────── + // Rigs whose bone NAMES mirror their anatomy (UniRig/AutoRig outputs, + // Blender -X-scale exports whose chirality differs from the Mixamo-import + // fleet norm the motion library is calibrated against) retarget every + // clip with L/R swapped: the left-leg track lands on the anatomical + // right. Detect it name-independently from bind GEOMETRY: the character's + // TRUE left is up × forward (up = +Y; forward = ±Z from the caller's + // mesh-derived facing — the same yaw180 detectBackwardFacing() feeds us). + // Sum dot(namedLeft − namedRight, trueLeft) over the paired roles + // (weighted by separation, so a near-centred pair can't flip the vote). + // The expected sign is calibrated on the known-good Mixamo case + // (kExpectedSideSign below); the opposite sign ⇒ mirror every L/R role + // (and V2 finger side) so names match anatomy for the whole retarget. + { + const Ogre::Vector3 up(0, 1, 0); + const Ogre::Vector3 fwd(0, 0, yaw180 ? -1.0f : 1.0f); + const Ogre::Vector3 trueLeft = up.crossProduct(fwd); + // (left role, right role) pairs, most reliable first. + static const int kPairs[][2] = { + {19, 15}, // hips + {10, 6}, // collars + {11, 7}, // upper arms + {13, 9}, // hands + {21, 17}, // feet + }; + Ogre::Vector3 rolePos[22]; + bool roleHas[22] = {}; + for (int i = 0; i < nBones; ++i) { + const int c = boneToCanon[i]; + if (c < 0 || c >= 22 || roleHas[c]) continue; + rolePos[c] = skel->getBone(static_cast(i)) + ->_getDerivedPosition(); + roleHas[c] = true; + } + double side = 0.0; + for (const auto& pr : kPairs) { + if (!roleHas[pr[0]] || !roleHas[pr[1]]) continue; + side += (rolePos[pr[0]] - rolePos[pr[1]]).dotProduct(trueLeft); + } + // Calibrated on Mixamo Rumba as loaded by OUR importer (templates + // apply correctly there today): it measures side = -2.36, i.e. in + // Ogre's frame the fleet-norm rigs put named-left at MINUS up×fwd — + // the same "all imports mirror alike" #951 measured from the toes. + // A POSITIVE score is the odd one out (UniRig/AutoRig outputs) and + // triggers the swap. + constexpr double kExpectedSideSign = -1.0; + if (qEnvironmentVariableIsSet("QTMESH_T2M_SIDE_DEBUG")) + fprintf(stderr, "[t2m] side score %.4f (expected sign %+.0f)\n", + side, kExpectedSideSign); + const QByteArray forceSwap = qgetenv("QTMESH_T2M_SIDE_SWAP"); + const bool swap = !forceSwap.isEmpty() + ? (forceSwap != "0") + : (side != 0.0 && (side * kExpectedSideSign) < 0.0); + if (swap) { + auto mirrorRole = [canonN](int c) { + // V1 body pairs: 6..9 ↔ 10..13, 14..17 ↔ 18..21. + if (c >= 6 && c <= 9) return c + 4; + if (c >= 10 && c <= 13) return c - 4; + if (c >= 14 && c <= 17) return c + 4; + if (c >= 18 && c <= 21) return c - 4; + // V2 finger slots: 22 + (side*5 + finger)*3 + seg — flip side. + if (c >= 22 && c < canonN) { + const int f = c - 22; + return 22 + (f < 15 ? f + 15 : f - 15); + } + return c; + }; + for (int i = 0; i < nBones; ++i) + if (boneToCanon[i] >= 0) + boneToCanon[i] = mirrorRole(boneToCanon[i]); + res.sideSwapApplied = true; + fprintf(stderr, + "[t2m] bone naming is mirrored vs anatomy (side score " + "%.3f) — swapped L/R canonical roles to match geometry\n", + side); + } + } + if (cmuLibraryHandedness) compensateCanonicalHandedness(skel, boneToCanon); diff --git a/src/AnimationMerger.h b/src/AnimationMerger.h index 38d98559..37d1d2b2 100644 --- a/src/AnimationMerger.h +++ b/src/AnimationMerger.h @@ -197,6 +197,8 @@ class AnimationMerger { float length = 0.0f; // seconds bool refined = false; // RMIB refine pass ran (smoothed the motion) bool usedModel = false; // RMIB model (vs spline) used in the refine pass + bool sideSwapApplied = false; // #969: bone names mirrored vs anatomy — + // L/R canonical roles were swapped }; /// Apply a TEMPLATE MOTION CLIP (#411) onto a skeleton as a new animation. diff --git a/src/AnimationMerger_test.cpp b/src/AnimationMerger_test.cpp index 6129764c..23abb635 100644 --- a/src/AnimationMerger_test.cpp +++ b/src/AnimationMerger_test.cpp @@ -23,9 +23,17 @@ class AnimationMergerTest : public ::testing::Test { ASSERT_TRUE(tryInitOgre()) << "Ogre init failed (Xvfb/GL required in CI)"; createStandardOgreMaterials(); + // #969: the synthetic rigs in this suite historically place named- + // LEFT bones at +X, which the new name-vs-anatomy side check reads + // as mirror-named (the fleet norm is named-left at -X) and would + // silently swap the L/R canonical roles under the side-specific + // assertions below. Pin the check off for the suite; the dedicated + // ApplyMotionClipDetectsMirroredSideNaming test re-enables it. + qputenv("QTMESH_T2M_SIDE_SWAP", "0"); } void TearDown() override { + qunsetenv("QTMESH_T2M_SIDE_SWAP"); if (app) app->processEvents(); } @@ -1886,3 +1894,89 @@ TEST_F(AnimationMergerTest, VerticalDescentLowersRootDescentOnly) sm->destroyEntity(ent); } + +TEST_F(AnimationMergerTest, ApplyMotionClipDetectsMirroredSideNaming) +{ + // #969: rigs whose bone NAMES mirror their anatomy (UniRig outputs) must + // have their L/R canonical roles swapped; fleet-norm rigs must not. + // The check is geometric: named-left vs named-right positions against + // trueLeft = up × forward. Build the same full humanoid twice, with the + // side names on opposite lateral signs. + auto build = [&](const char* prefix, float leftX) { + auto skelRes = Ogre::SkeletonManager::getSingleton().create( + std::string(prefix) + "_skel", + Ogre::ResourceGroupManager::DEFAULT_RESOURCE_GROUP_NAME); + unsigned short h = 0; + auto bone = [&](const std::string& n, const Ogre::Vector3& p, + Ogre::Bone* par) { + auto* b = skelRes->createBone(n, h++); + b->setPosition(p); + if (par) par->addChild(b); + return b; + }; + auto* hips = bone("Hips", {0, 1.0f, 0}, nullptr); + auto* spine = bone("Spine", {0, 0.2f, 0}, hips); + auto* chest = bone("Spine1", {0, 0.2f, 0}, spine); + auto* neck = bone("Neck", {0, 0.2f, 0}, chest); + bone("Head", {0, 0.15f, 0}, neck); + const float lx = leftX, rx = -leftX; + auto* rsh = bone("RightShoulder", {rx * 0.05f, 0.1f, 0}, chest); + auto* rarm = bone("RightArm", {rx * 0.15f, 0, 0}, rsh); + auto* rfa = bone("RightForeArm", {rx * 0.25f, 0, 0}, rarm); + bone("RightHand", {rx * 0.2f, 0, 0}, rfa); + auto* lsh = bone("LeftShoulder", {lx * 0.05f, 0.1f, 0}, chest); + auto* larm = bone("LeftArm", {lx * 0.15f, 0, 0}, lsh); + auto* lfa = bone("LeftForeArm", {lx * 0.25f, 0, 0}, larm); + bone("LeftHand", {lx * 0.2f, 0, 0}, lfa); + auto* rleg = bone("RightUpLeg", {rx * 0.1f, -0.25f, 0}, hips); + auto* rknee = bone("RightLeg", {0, -0.25f, 0}, rleg); + bone("RightFoot", {0, -0.5f, 0}, rknee); + auto* lleg = bone("LeftUpLeg", {lx * 0.1f, -0.25f, 0}, hips); + auto* lknee = bone("LeftLeg", {0, -0.25f, 0}, lleg); + bone("LeftFoot", {0, -0.5f, 0}, lknee); + skelRes->setBindingPose(); + auto mesh = createInMemoryMesh(std::string(prefix) + "_mesh", skelRes); + return Manager::getSingleton()->getSceneMgr()->createEntity( + std::string(prefix) + "_ent", mesh); + }; + + // The fixture pins the side check off for the legacy suite — this test + // is ABOUT the check, so re-enable real detection. + qunsetenv("QTMESH_T2M_SIDE_SWAP"); + + // Fleet norm (Mixamo-in-Ogre): named-left sits at MINUS up×fwd — with + // forward +Z (yaw180=false) that is negative X. No swap expected. + Ogre::Entity* norm = build("sidenorm", -1.0f); + ASSERT_NE(norm, nullptr); + const auto quats = identityClip(3); + const auto resNorm = AnimationMerger::applyMotionClip( + norm->getSkeleton(), "sideclip", quats, 30, /*worldFrame=*/true, + srcRestWorld(), false, 8, false, canonRestDirs()); + ASSERT_TRUE(resNorm.ok) << resNorm.error.toStdString(); + EXPECT_FALSE(resNorm.sideSwapApplied) + << "fleet-norm naming must not be side-swapped"; + + // UniRig-style: named-left on the OPPOSITE lateral sign — swap expected. + Ogre::Entity* mir = build("sidemir", +1.0f); + ASSERT_NE(mir, nullptr); + const auto resMir = AnimationMerger::applyMotionClip( + mir->getSkeleton(), "sideclip", quats, 30, /*worldFrame=*/true, + srcRestWorld(), false, 8, false, canonRestDirs()); + ASSERT_TRUE(resMir.ok) << resMir.error.toStdString(); + EXPECT_TRUE(resMir.sideSwapApplied) + << "mirror-named rig must have its L/R roles swapped"; + + // yaw180 flips forward and therefore trueLeft: the SAME mirror-named rig + // evaluated as backward-facing must NOT swap (its names match anatomy + // when the character faces -Z). + const auto resMirYaw = AnimationMerger::applyMotionClip( + mir->getSkeleton(), "sideclip2", quats, 30, /*worldFrame=*/true, + srcRestWorld(), false, 8, /*yaw180=*/true, canonRestDirs()); + ASSERT_TRUE(resMirYaw.ok) << resMirYaw.error.toStdString(); + EXPECT_FALSE(resMirYaw.sideSwapApplied) + << "backward-facing flips trueLeft — mirror naming becomes correct"; + + auto* sm = Manager::getSingleton()->getSceneMgr(); + sm->destroyEntity(norm); + sm->destroyEntity(mir); +} From 2a5b3773cb4fef826acac1d6c15c1e923c319d97 Mon Sep 17 00:00:00 2001 From: Fernando Date: Tue, 1 Sep 2026 18:09:52 -0400 Subject: [PATCH 2/4] fix(anim): UniRig A-pose arms no longer retarget as legs; robust facing on mis-named rigs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/AnimationMerger.cpp | 133 ++++++++++++++++++++++++++++------- src/UniRigPredictor.cpp | 22 ++++-- src/UniRigPredictor_test.cpp | 52 ++++++++++++++ 3 files changed, 177 insertions(+), 30 deletions(-) diff --git a/src/AnimationMerger.cpp b/src/AnimationMerger.cpp index 1a955bca..4303401b 100644 --- a/src/AnimationMerger.cpp +++ b/src/AnimationMerger.cpp @@ -2053,14 +2053,30 @@ bool AnimationMerger::detectBackwardFacing(Ogre::Entity* entity) Ogre::SkeletonInstance* skel = entity->getSkeleton(); if (!skel) return false; - // Ankle reference: bones resolving to the canonical foot roles (17/21). + // Ankle reference: bones resolving to the canonical foot roles (17/21) — + // but only those that are actually LOW. Mis-labeled rigs (#969: UniRig + // A-pose arms named "*Foot_1") put foot-named bones at chest height; + // averaging those drags the "foot band" into the torso and the centroid + // test reads the backplate/cloak instead of the toes (false backward → + // every generated clip contorts). Gate each candidate to the lowest + // quarter of the skeleton's height. + float loB = 0, hiB = 0; + for (unsigned short i = 0; i < skel->getNumBones(); ++i) { + const float y = skel->getBone(i)->_getDerivedPosition().y; + if (i == 0) { loB = hiB = y; } + loB = std::min(loB, y); hiB = std::max(hiB, y); + } + const float skelH = std::max(1e-6f, hiB - loB); Ogre::Vector3 ankleSum = Ogre::Vector3::ZERO; int ankles = 0; for (unsigned short i = 0; i < skel->getNumBones(); ++i) { Ogre::Bone* b = skel->getBone(i); const int c = MotionInbetween::canonicalIndexForBone( QString::fromStdString(b->getName())); - if (c == 17 || c == 21) { ankleSum += b->_getDerivedPosition(); ++ankles; } + if (c != 17 && c != 21) continue; + const Ogre::Vector3 p = b->_getDerivedPosition(); + if (p.y > loB + 0.25f * skelH) continue; // "foot" at chest height + ankleSum += p; ++ankles; } if (ankles == 0) return false; const Ogre::Vector3 ankle = ankleSum / static_cast(ankles); @@ -4268,24 +4284,69 @@ AnimationMerger::ApplyMotionResult AnimationMerger::applyMotionClip( if (!canonSeen[c]) { canonSeen[c] = 1; ++distinct; } } } - // ── #969 name-vs-anatomy side check ──────────────────────────────── - // Rigs whose bone NAMES mirror their anatomy (UniRig/AutoRig outputs, - // Blender -X-scale exports whose chirality differs from the Mixamo-import - // fleet norm the motion library is calibrated against) retarget every - // clip with L/R swapped: the left-leg track lands on the anatomical - // right. Detect it name-independently from bind GEOMETRY: the character's - // TRUE left is up × forward (up = +Y; forward = ±Z from the caller's - // mesh-derived facing — the same yaw180 detectBackwardFacing() feeds us). - // Sum dot(namedLeft − namedRight, trueLeft) over the paired roles - // (weighted by separation, so a near-centred pair can't flip the vote). - // The expected sign is calibrated on the known-good Mixamo case - // (kExpectedSideSign below); the opposite sign ⇒ mirror every L/R role - // (and V2 finger side) so names match anatomy for the whole retarget. + // ── #969 name-vs-anatomy consistency (side swap + arm/leg guard) ── + // Two real-world failure modes on generated (UniRig/AutoRig) rigs: + // (a) bone names put "Left*" on the anatomical right (the side swap); + // (b) whole ARM chains carry LEG names ("LeftUpLeg_1" at chest height — + // the pre-fix UniRig labeler classified A-pose arms, which descend, + // as legs), so leg tracks drive the arms. + // Both are resolved GEOMETRICALLY from the bind pose: up = +Y, forward = + // ±Z from the caller's mesh-derived yaw180 facing, trueLeft = up × fwd. { const Ogre::Vector3 up(0, 1, 0); const Ogre::Vector3 fwd(0, 0, yaw180 ? -1.0f : 1.0f); const Ogre::Vector3 trueLeft = up.crossProduct(fwd); - // (left role, right role) pairs, most reliable first. + + std::vector bonePos(static_cast(nBones)); + for (int i = 0; i < nBones; ++i) + bonePos[i] = skel->getBone(static_cast(i)) + ->_getDerivedPosition(); + + // Hip reference + body height (for the arm/leg altitude bands). + int hipIdx = -1; + float loY = 0, hiY = 0; + for (int i = 0; i < nBones; ++i) { + if (boneToCanon[i] == 0 && hipIdx < 0) hipIdx = i; + loY = i ? std::min(loY, bonePos[i].y) : bonePos[i].y; + hiY = i ? std::max(hiY, bonePos[i].y) : bonePos[i].y; + } + const float bodyH = std::max(1e-6f, hiY - loY); + + // ---- (b) arm/leg altitude guard -------------------------------- + // A LEG-role bone bound well ABOVE the hips is a mis-named arm + // segment: strip it (remembering its segment for the rescue below). + // An ARM-role bone well BELOW the hips is stripped outright. + // seg: 0 = upper limb root, 1 = middle, 2 = tip; -1 = buttock/collar. + struct Rescue { int bone; int seg; }; + std::vector rescue; + if (hipIdx >= 0) { + const float hipY = bonePos[hipIdx].y; + for (int i = 0; i < nBones; ++i) { + const int c = boneToCanon[i]; + if (c < 0) continue; + const bool legRole = (c >= 14 && c <= 21); + const bool armRole = (c >= 6 && c <= 13); + if (legRole && bonePos[i].y > hipY + 0.10f * bodyH) { + int seg = -1; + if (c == 15 || c == 19) seg = 0; + else if (c == 16 || c == 20) seg = 1; + else if (c == 17 || c == 21) seg = 2; + rescue.push_back({i, seg}); + boneToCanon[i] = -1; + } else if (armRole && bonePos[i].y < hipY - 0.10f * bodyH) { + boneToCanon[i] = -1; // "arm" below the hips — drop + } + } + } + + // ---- (a) side check on the CLEANED mapping --------------------- + // Sum dot(namedLeft − namedRight, trueLeft) over the paired roles + // (weighted by separation, so a near-centred pair can't flip the + // vote). The expected sign is calibrated on the known-good Mixamo + // case as loaded by OUR importer: it measures side = -2.36, i.e. the + // fleet-norm rigs put named-left at MINUS up×fwd (the same "all + // imports mirror alike" #951 measured from the toes). A POSITIVE + // score is the odd one out (UniRig outputs) and triggers the swap. static const int kPairs[][2] = { {19, 15}, // hips {10, 6}, // collars @@ -4298,8 +4359,7 @@ AnimationMerger::ApplyMotionResult AnimationMerger::applyMotionClip( for (int i = 0; i < nBones; ++i) { const int c = boneToCanon[i]; if (c < 0 || c >= 22 || roleHas[c]) continue; - rolePos[c] = skel->getBone(static_cast(i)) - ->_getDerivedPosition(); + rolePos[c] = bonePos[i]; roleHas[c] = true; } double side = 0.0; @@ -4307,12 +4367,6 @@ AnimationMerger::ApplyMotionResult AnimationMerger::applyMotionClip( if (!roleHas[pr[0]] || !roleHas[pr[1]]) continue; side += (rolePos[pr[0]] - rolePos[pr[1]]).dotProduct(trueLeft); } - // Calibrated on Mixamo Rumba as loaded by OUR importer (templates - // apply correctly there today): it measures side = -2.36, i.e. in - // Ogre's frame the fleet-norm rigs put named-left at MINUS up×fwd — - // the same "all imports mirror alike" #951 measured from the toes. - // A POSITIVE score is the odd one out (UniRig/AutoRig outputs) and - // triggers the swap. constexpr double kExpectedSideSign = -1.0; if (qEnvironmentVariableIsSet("QTMESH_T2M_SIDE_DEBUG")) fprintf(stderr, "[t2m] side score %.4f (expected sign %+.0f)\n", @@ -4344,6 +4398,37 @@ AnimationMerger::ApplyMotionResult AnimationMerger::applyMotionClip( "%.3f) — swapped L/R canonical roles to match geometry\n", side); } + + // ---- (b, continued) rescue mis-named arm chains ---------------- + // The stripped chest-height "leg" chains ARE the arms — remap each + // segment onto the matching arm role, side chosen geometrically in + // the POST-swap convention (fleet norm: left roles live at MINUS + // trueLeft). Only fills a side whose arm roles are entirely vacant, + // so a rig with real (correctly named) arms is never stomped. + if (!rescue.empty() && hipIdx >= 0) { + bool armTaken[2] = {false, false}; // 0 = right(7..9), 1 = left(11..13) + for (int i = 0; i < nBones; ++i) { + const int c = boneToCanon[i]; + if (c >= 7 && c <= 9) armTaken[0] = true; + if (c >= 11 && c <= 13) armTaken[1] = true; + } + static const int kArmSeg[2][3] = {{7, 8, 9}, {11, 12, 13}}; + int rescued = 0; + for (const Rescue& rc : rescue) { + if (rc.seg < 0) continue; + const float lat = (bonePos[rc.bone] - bonePos[hipIdx]) + .dotProduct(trueLeft); + const int sideIdx = lat < 0.0f ? 1 : 0; // fleet norm: left at -trueLeft + if (armTaken[sideIdx]) continue; + boneToCanon[rc.bone] = kArmSeg[sideIdx][rc.seg]; + ++rescued; + } + if (rescued > 0) + fprintf(stderr, + "[t2m] %d leg-named bone(s) bound at chest height — " + "remapped onto the vacant arm roles (mis-labeled " + "A-pose arms)\n", rescued); + } } if (cmuLibraryHandedness) diff --git a/src/UniRigPredictor.cpp b/src/UniRigPredictor.cpp index e3a0e6a1..cc5db09d 100644 --- a/src/UniRigPredictor.cpp +++ b/src/UniRigPredictor.cpp @@ -459,15 +459,25 @@ void UniRigPredictor::labelJointsAnatomically(std::vector& joints, int up if (claimed[k]) continue; std::vector chain = walkChain(k); if (chain.empty()) continue; - // ARM vs LEG by the CHAIN'S DIRECTION, not its attach height: a leg - // DESCENDS (its tip is well below its root in up); an arm extends - // sideways/level. (The hips are mid-height, so attach-height alone - // mislabels hip-rooted legs as arms — observed on real UniRig rigs.) + // ARM vs LEG: attach HEIGHT relative to the ROOT decides when it + // can — arms hang off the upper spine (chest/shoulders, well above + // the hips), legs off the root itself. Chain DIRECTION alone + // (the previous rule: "arms extend sideways") mislabels A-POSE + // arms, which descend almost as steeply as legs — observed on a + // real UniRig rig where BOTH arm chains were named *UpLeg_1 and + // the leg tracks drove the arms. Direction remains the + // tie-breaker only inside the ambiguous attach band. const double dropFrac = (up(chain.front()) - up(chain.back())) / bodyH; const double sideReach = std::abs(side(chain.back()) - side(chain.front())); const double upDrop = up(chain.front()) - up(chain.back()); - // Leg if it mostly goes DOWN; arm if it mostly goes SIDEWAYS. - const bool isArm = (sideReach >= upDrop) && (dropFrac < 0.25); + const double attachFrac = (up(a) - up(root)) / bodyH; // a = attach joint + bool isArm; + if (attachFrac > 0.15) + isArm = true; // attached well above the hips → arm (A-pose safe) + else if (attachFrac < 0.05) + isArm = false; // attached at/below the hips → leg + else + isArm = (sideReach >= upDrop) && (dropFrac < 0.25); const bool left = (side(chain.front()) >= 0.0); const QString pre = left ? QStringLiteral("Left") : QStringLiteral("Right"); const QStringList armN = { pre + "Arm", pre + "ForeArm", pre + "Hand" }; diff --git a/src/UniRigPredictor_test.cpp b/src/UniRigPredictor_test.cpp index 53100805..79f9ff4e 100644 --- a/src/UniRigPredictor_test.cpp +++ b/src/UniRigPredictor_test.cpp @@ -473,3 +473,55 @@ TEST(UniRigPredictor, EnsureModelBlockingHonoursNoDownloadGuard) SUCCEED(); (void)p; } + +TEST(UniRigPredictor, LabelJointsClassifiesAPoseArmsByAttachHeight) +{ + // #969: A-pose arms DESCEND almost as steeply as legs, so the old + // direction-only rule ("arms extend sideways") named both arm chains + // *UpLeg_1 on a real UniRig rig and the leg tracks drove the arms. + // Attach height must win: chains hanging off the upper spine are arms + // even when they drop; chains off the root are legs even when they splay. + using J = UniRigPredictor::Joint; + std::vector j; + auto add = [&](double x, double y, double z, int parent) { + J jt; jt.pos = {x, y, z}; jt.parent = parent; jt.part = -1; + jt.name = QStringLiteral("joint_%1").arg(j.size()); + j.push_back(jt); + return static_cast(j.size()) - 1; + }; + const int hips = add(0, 1.00, 0, -1); + const int sp0 = add(0, 1.15, 0, hips); + const int sp1 = add(0, 1.30, 0, sp0); + const int chest = add(0, 1.45, 0, sp1); + const int neck = add(0, 1.55, 0, chest); + add(0, 1.70, 0, neck); // head + // A-POSE arms: attach at the chest, drop steeply (dy ≈ -0.45 over the + // chain vs lateral reach ≈ 0.35 — the old rule called this a leg). + const int lsh = add(+0.15, 1.42, 0, chest); + const int lel = add(+0.30, 1.15, 0, lsh); + add(+0.42, 0.90, 0, lel); + const int rsh = add(-0.15, 1.42, 0, chest); + const int rel = add(-0.30, 1.15, 0, rsh); + add(-0.42, 0.90, 0, rel); + // Legs: attach at the root, straight down. + const int lhip = add(+0.12, 0.95, 0, hips); + const int lkne = add(+0.12, 0.50, 0, lhip); + add(+0.12, 0.05, 0, lkne); + const int rhip = add(-0.12, 0.95, 0, hips); + const int rkne = add(-0.12, 0.50, 0, rhip); + add(-0.12, 0.05, 0, rkne); + + UniRigPredictor::labelJointsAnatomically(j, /*upAxis=*/1); + + int armChains = 0, legChains = 0, legNamedHigh = 0; + for (const auto& jt : j) { + if (jt.name.contains(QLatin1String("Arm")) + && !jt.name.contains(QLatin1String("ForeArm"))) ++armChains; + if (jt.name.contains(QLatin1String("UpLeg"))) ++legChains; + if (jt.name.contains(QLatin1String("UpLeg")) && jt.pos[1] > 1.2) + ++legNamedHigh; + } + EXPECT_EQ(armChains, 2) << "both A-pose chains must be named as arms"; + EXPECT_EQ(legChains, 2) << "exactly the two root chains are legs"; + EXPECT_EQ(legNamedHigh, 0) << "no leg name may land at chest height"; +} From b9c1aef9aec5196e8bf6f1a1bbf7f0223a87b238 Mon Sep 17 00:00:00 2001 From: Fernando Date: Tue, 1 Sep 2026 18:14:08 -0400 Subject: [PATCH 3/4] =?UTF-8?q?fix(anim):=20compose=20the=20L/R=20permutat?= =?UTF-8?q?ion=20=E2=80=94=20one=20decision,=20bind-pose=20measured=20(rev?= =?UTF-8?q?iew)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/AnimationMerger.cpp | 27 ++++++++++++++++++++++----- src/AnimationMerger_test.cpp | 27 +++++++++++++++------------ 2 files changed, 37 insertions(+), 17 deletions(-) diff --git a/src/AnimationMerger.cpp b/src/AnimationMerger.cpp index 4303401b..4155d659 100644 --- a/src/AnimationMerger.cpp +++ b/src/AnimationMerger.cpp @@ -4292,11 +4292,22 @@ AnimationMerger::ApplyMotionResult AnimationMerger::applyMotionClip( // as legs), so leg tracks drive the arms. // Both are resolved GEOMETRICALLY from the bind pose: up = +Y, forward = // ±Z from the caller's mesh-derived yaw180 facing, trueLeft = up × fwd. - { + // Template/library clips only (cmuLibraryHandedness) — this block + // REPLACES compensateCanonicalHandedness for that path: one composed + // permutation instead of two independent ones that could cancel (the + // old compensator was facing-blind raw world-X and read the FIRST bone + // per role, so mis-named chest-height "legs" fed it garbage). The mocap + // path (cmuLibraryHandedness=false) keeps its historical mapping. + if (cmuLibraryHandedness) { const Ogre::Vector3 up(0, 1, 0); const Ogre::Vector3 fwd(0, 0, yaw180 ? -1.0f : 1.0f); const Ogre::Vector3 trueLeft = up.crossProduct(fwd); + // BIND pose positions — the current pose may be mid-animation + // (crossed limbs would flip the vote). Same reset the old + // compensator used on this exact path. + skel->reset(true); + skel->_updateTransforms(); std::vector bonePos(static_cast(nBones)); for (int i = 0; i < nBones; ++i) bonePos[i] = skel->getBone(static_cast(i)) @@ -4367,7 +4378,12 @@ AnimationMerger::ApplyMotionResult AnimationMerger::applyMotionClip( if (!roleHas[pr[0]] || !roleHas[pr[1]]) continue; side += (rolePos[pr[0]] - rolePos[pr[1]]).dotProduct(trueLeft); } - constexpr double kExpectedSideSign = -1.0; + // One-permutation convention: canonical LEFT roles must end on the + // bones at +trueLeft (anatomical left) — that is the state the old + // compensator produced on the validated Mixamo case (named-left at + // -trueLeft, one swap). So: swap when the named pairs sit at + // -trueLeft; an anatomically-named rig (UniRig) needs none. + constexpr double kExpectedSideSign = +1.0; if (qEnvironmentVariableIsSet("QTMESH_T2M_SIDE_DEBUG")) fprintf(stderr, "[t2m] side score %.4f (expected sign %+.0f)\n", side, kExpectedSideSign); @@ -4418,7 +4434,7 @@ AnimationMerger::ApplyMotionResult AnimationMerger::applyMotionClip( if (rc.seg < 0) continue; const float lat = (bonePos[rc.bone] - bonePos[hipIdx]) .dotProduct(trueLeft); - const int sideIdx = lat < 0.0f ? 1 : 0; // fleet norm: left at -trueLeft + const int sideIdx = lat >= 0.0f ? 1 : 0; // left roles at +trueLeft if (armTaken[sideIdx]) continue; boneToCanon[rc.bone] = kArmSeg[sideIdx][rc.seg]; ++rescued; @@ -4431,8 +4447,9 @@ AnimationMerger::ApplyMotionResult AnimationMerger::applyMotionClip( } } - if (cmuLibraryHandedness) - compensateCanonicalHandedness(skel, boneToCanon); + // NB compensateCanonicalHandedness is intentionally NOT called here any + // more for the library path — the #969 block above composes the same + // decision (facing-aware, on the cleaned mapping, V2 fingers included). res.canonicalJoints = distinct; // Bones-per-role: rigs segment chains differently (Mixamo has Spine AND diff --git a/src/AnimationMerger_test.cpp b/src/AnimationMerger_test.cpp index 23abb635..471e1661 100644 --- a/src/AnimationMerger_test.cpp +++ b/src/AnimationMerger_test.cpp @@ -1944,8 +1944,11 @@ TEST_F(AnimationMergerTest, ApplyMotionClipDetectsMirroredSideNaming) // is ABOUT the check, so re-enable real detection. qunsetenv("QTMESH_T2M_SIDE_SWAP"); - // Fleet norm (Mixamo-in-Ogre): named-left sits at MINUS up×fwd — with - // forward +Z (yaw180=false) that is negative X. No swap expected. + // ONE composed permutation (replaces the old compensateCanonicalHandedness + // on the library path): canonical-LEFT roles must end on the bones at + // +trueLeft (anatomical left). A Mixamo-convention rig — named-left at + // MINUS up×fwd (negative X when facing +Z) — therefore gets exactly ONE + // swap (what the old compensator did on the validated case). Ogre::Entity* norm = build("sidenorm", -1.0f); ASSERT_NE(norm, nullptr); const auto quats = identityClip(3); @@ -1953,28 +1956,28 @@ TEST_F(AnimationMergerTest, ApplyMotionClipDetectsMirroredSideNaming) norm->getSkeleton(), "sideclip", quats, 30, /*worldFrame=*/true, srcRestWorld(), false, 8, false, canonRestDirs()); ASSERT_TRUE(resNorm.ok) << resNorm.error.toStdString(); - EXPECT_FALSE(resNorm.sideSwapApplied) - << "fleet-norm naming must not be side-swapped"; + EXPECT_TRUE(resNorm.sideSwapApplied) + << "Mixamo-convention naming takes the single composed swap"; - // UniRig-style: named-left on the OPPOSITE lateral sign — swap expected. + // Anatomically-named rig (UniRig): named-left already at +trueLeft — the + // roles land correctly with NO permutation. Ogre::Entity* mir = build("sidemir", +1.0f); ASSERT_NE(mir, nullptr); const auto resMir = AnimationMerger::applyMotionClip( mir->getSkeleton(), "sideclip", quats, 30, /*worldFrame=*/true, srcRestWorld(), false, 8, false, canonRestDirs()); ASSERT_TRUE(resMir.ok) << resMir.error.toStdString(); - EXPECT_TRUE(resMir.sideSwapApplied) - << "mirror-named rig must have its L/R roles swapped"; + EXPECT_FALSE(resMir.sideSwapApplied) + << "anatomically-named rig needs no permutation"; - // yaw180 flips forward and therefore trueLeft: the SAME mirror-named rig - // evaluated as backward-facing must NOT swap (its names match anatomy - // when the character faces -Z). + // yaw180 flips forward and therefore trueLeft: the SAME rig evaluated as + // backward-facing has its named-left at -trueLeft — swap expected. const auto resMirYaw = AnimationMerger::applyMotionClip( mir->getSkeleton(), "sideclip2", quats, 30, /*worldFrame=*/true, srcRestWorld(), false, 8, /*yaw180=*/true, canonRestDirs()); ASSERT_TRUE(resMirYaw.ok) << resMirYaw.error.toStdString(); - EXPECT_FALSE(resMirYaw.sideSwapApplied) - << "backward-facing flips trueLeft — mirror naming becomes correct"; + EXPECT_TRUE(resMirYaw.sideSwapApplied) + << "backward-facing flips trueLeft — the swap is needed again"; auto* sm = Manager::getSingleton()->getSceneMgr(); sm->destroyEntity(norm); From 9e3146f7ac068f081ed57832cf50c302a70cb667 Mon Sep 17 00:00:00 2001 From: Fernando Date: Tue, 1 Sep 2026 18:36:53 -0400 Subject: [PATCH 4/4] =?UTF-8?q?fix(anim):=20posture-aware=20take=20samplin?= =?UTF-8?q?g=20=E2=80=94=20the=20'backwards'=20run?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit '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 --- src/MotionLibrary.cpp | 39 +++++++++++++++++++++++++++++++++++--- src/MotionLibrary.h | 21 ++++++++++++++++++++ src/MotionLibrary_test.cpp | 31 ++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 3 deletions(-) diff --git a/src/MotionLibrary.cpp b/src/MotionLibrary.cpp index 74949544..86c92166 100644 --- a/src/MotionLibrary.cpp +++ b/src/MotionLibrary.cpp @@ -236,6 +236,7 @@ bool MotionLibrary::parse(const QByteArray& json) } clip.quality = static_cast( std::clamp(co.value("quality").toDouble(1.0), 0.0, 1.0)); + clip.uprightness = meanChestLean(clip.quats); if (clip.frames > 0 && !clip.action.isEmpty()) m_clips.push_back(std::move(clip)); } @@ -253,6 +254,37 @@ std::vector MotionLibrary::actions() const return out; } +float MotionLibrary::meanChestLean( + const std::vector>>& quats) +{ + // Chest = canonical joint 2. up' = q * (0,1,0) * q⁻¹; report mean up'.z. + double sum = 0.0; + int n = 0; + for (const auto& frame : quats) { + if (frame.size() <= 2) + continue; + const auto& q = frame[2]; + const float x = q[0], y = q[1], z = q[2], w = q[3]; + // z-component of the rotated +Y axis: 2(yz + wx)... careful with + // convention: R(q)·(0,1,0) = (2(xy − wz), 1 − 2(x² + z²), 2(yz + wx)). + sum += 2.0f * (y * z + w * x); + ++n; + } + return n ? static_cast(sum / n) : 0.0f; +} + +double MotionLibrary::takeWeight(const QString& action, float quality, + float uprightness) +{ + double w = static_cast(quality) * quality; // the #855 rule + const QString a = action.toLower(); + const bool locomotion = (a == QLatin1String("walk")) + || (a == QLatin1String("run")); + if (locomotion && uprightness < -0.10f) + w *= 0.02; // backpedal-look take: only picked when nothing else + return w; +} + int MotionLibrary::matchPrompt(const QString& prompt, QString* matchedAction) const { const QString p = prompt.toLower(); @@ -279,9 +311,10 @@ int MotionLibrary::matchPrompt(const QString& prompt, QString* matchedAction) co QList weights; weights.reserve(hits.size()); for (int i : hits) { - const double q = m_clips[static_cast(i)].quality; - weights.append(q * q); - total += q * q; + const auto& c = m_clips[static_cast(i)]; + const double w = takeWeight(c.action, c.quality, c.uprightness); + weights.append(w); + total += w; } if (total <= 1e-9) return hits.at(QRandomGenerator::global()->bounded(hits.size())); diff --git a/src/MotionLibrary.h b/src/MotionLibrary.h index fd555328..5a64bfb4 100644 --- a/src/MotionLibrary.h +++ b/src/MotionLibrary.h @@ -56,6 +56,12 @@ class MotionLibrary { // leg length and lowers the root bone's Y (descent-only) so crouch/ // pickup/working actually sink. Empty → flat root (locomotion clips). std::vector rootY; + // Mean chest forward-lean over the clip: chest joint's up-vector + // z-component averaged per frame (canonical frame faces +Z, so + // 0 = upright, positive = leaning into the motion, NEGATIVE = the + // torso tips BACKWARD — a run that reads as backpedaling). Computed + // at parse time; feeds the posture-aware take sampling. + float uprightness = 0.0f; /// Optional (#838 finger animation): per-frame LOCAL finger curl, /// size frames × 30 (2 sides × 5 fingers × 3 segments, see /// AnimationMerger::fingerSlot). Empty when the source rig has no @@ -116,6 +122,21 @@ class MotionLibrary { // (optional) reports which action was chosen. int matchPrompt(const QString& prompt, QString* matchedAction = nullptr) const; + /// Mean chest forward-lean of a canonical clip (see Clip::uprightness). + /// Pure — exposed for unit tests. + static float meanChestLean( + const std::vector>>& quats); + + /// Sampling weight for one take of `action`: quality² (the #855 rule) + /// times a posture penalty — locomotion takes whose torso tips backward + /// (uprightness < -0.10) are nearly never picked while upright takes of + /// the same action exist (the user-facing symptom was "the run plays + /// backwards": 2 of 3 library run takes lean back ≥ 13°, walk takes are + /// all upright). Non-locomotion actions are exempt (sit/crawl/death + /// legitimately tip). Pure — exposed for unit tests. + static double takeWeight(const QString& action, float quality, + float uprightness); + // ---- Download / cache (mirrors the ONNX-model pattern) ----------------- // Absolute path the library is cached at (AppData/ai_models/motion/...). static QString libraryPath(); diff --git a/src/MotionLibrary_test.cpp b/src/MotionLibrary_test.cpp index 25bdf7ba..528e32d2 100644 --- a/src/MotionLibrary_test.cpp +++ b/src/MotionLibrary_test.cpp @@ -249,3 +249,34 @@ TEST(MotionLibrary, ActionsListed) ASSERT_EQ(a.size(), 2u); EXPECT_EQ(a[0].toStdString(), "walk"); } + +TEST(MotionLibrary, MeanChestLeanSignConvention) +{ + // Identity chest → upright (0). A chest pitched BACKWARD (top of the + // torso tips toward -Z) must read negative; pitched forward positive. + using F = std::vector>>; + F upright(3, std::vector>(22, {0, 0, 0, 1})); + EXPECT_NEAR(MotionLibrary::meanChestLean(upright), 0.0f, 1e-5f); + + // -20° about +X tips +Y toward -Z (backward); +20° tips it toward +Z. + const float a = 20.0f * static_cast(M_PI) / 180.0f; + F back = upright, fwd = upright; + for (auto& fr : back) fr[2] = {std::sin(-a / 2), 0, 0, std::cos(-a / 2)}; + for (auto& fr : fwd) fr[2] = {std::sin(+a / 2), 0, 0, std::cos(+a / 2)}; + EXPECT_LT(MotionLibrary::meanChestLean(back), -0.3f); + EXPECT_GT(MotionLibrary::meanChestLean(fwd), +0.3f); +} + +TEST(MotionLibrary, TakeWeightPenalizesBackleaningLocomotion) +{ + // #969 follow-up ("the run plays backwards"): 2 of 3 library run takes + // tip the torso back >= 13 deg. Locomotion takes with backward lean get a + // near-zero sampling weight while upright takes keep the #855 quality^2. + EXPECT_NEAR(MotionLibrary::takeWeight("run", 1.0f, 0.0f), 1.0, 1e-9); + EXPECT_NEAR(MotionLibrary::takeWeight("run", 0.5f, 0.05f), 0.25, 1e-9); + EXPECT_LT(MotionLibrary::takeWeight("run", 1.0f, -0.22f), 0.05); + EXPECT_LT(MotionLibrary::takeWeight("walk", 1.0f, -0.34f), 0.05); + // Non-locomotion actions legitimately tip (sit/crawl/death) — no penalty. + EXPECT_NEAR(MotionLibrary::takeWeight("sit", 1.0f, -0.6f), 1.0, 1e-9); + EXPECT_NEAR(MotionLibrary::takeWeight("death", 0.8f, -0.9f), 0.64, 1e-6); +}