diff --git a/src/AnimationMerger.cpp b/src/AnimationMerger.cpp index c03545a7..4155d659 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,8 +4284,172 @@ AnimationMerger::ApplyMotionResult AnimationMerger::applyMotionClip( if (!canonSeen[c]) { canonSeen[c] = 1; ++distinct; } } } - if (cmuLibraryHandedness) - compensateCanonicalHandedness(skel, boneToCanon); + // ── #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. + // 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)) + ->_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 + {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] = bonePos[i]; + 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); + } + // 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); + 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); + } + + // ---- (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; // left roles 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); + } + } + + // 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.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..471e1661 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,92 @@ 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"); + + // 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); + 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_TRUE(resNorm.sideSwapApplied) + << "Mixamo-convention naming takes the single composed swap"; + + // 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_FALSE(resMir.sideSwapApplied) + << "anatomically-named rig needs no permutation"; + + // 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_TRUE(resMirYaw.sideSwapApplied) + << "backward-facing flips trueLeft — the swap is needed again"; + + auto* sm = Manager::getSingleton()->getSceneMgr(); + sm->destroyEntity(norm); + sm->destroyEntity(mir); +} 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); +} 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"; +}