feat(#411): text-to-motion model (experimental) + UniRig naming/retarget + anim UI - #783
Conversation
…t + anim UI Text-to-motion (#411) end-to-end, plus the rigging/retarget fixes it surfaced. Model (experimental, opt-in): - MotionGenerator: ONNX consumer running a from-scratch t2m model (trained on clean dynamic single-action CMU + Quaternius CC0 windows; AMASS/HumanML3D excluded). Emits a canonical clip → same applyMotionClip retarget as templates. - Surfaced via `qtmesh anim --generate --model`, MCP generate_motion {model}, and a GUI "Use trained model (experimental)" checkbox. Falls back to the template-clip library automatically when the model is unavailable or the action isn't in its vocab. ENABLE_ONNX-guarded; downloads on first use (HF). - Training/data scripts (offline, not shipped): prep-t2m-clean.py (drops idle + multi-label windows — the key data-quality fix), prep-quaternius-t2m.py, train-t2m-onnx{,-v2,-v3}.py. Quality is action-dependent (locomotion coherent, gestures drift); template retarget stays the shipped default. Retarget + UniRig (fixes the "0/22 joints", mirrored, wrong-side results): - UniRigPredictor::labelJointsAnatomically — names UniRig's positional joints (root/joint_N) anatomically by rest-pose geometry (spread-based axis detection + chain-direction arm/leg classification); honours its body/hand part tokens when present. Unique names (Ogre createBone rejects dups). - applyMotionClip: handedness compensation swaps canonical L/R when the rig's left bone is on −X (CMU left = +X) so labels are anatomical AND motion isn't mirrored. Roll-correction disabled for no-standing-pose rigs (UniRig). - AutoRig::rigPriorPartLabels inherits a bone's part from its nearest named ancestor → segment "Select by Part" uses the exact rig prior on UniRig skins (resolve 0% → 100%). Animation UI: - Generate-from-text moved into the Animations group; shows for any skeleton (not only animated meshes); list auto-refreshes after generate (no reselect). - Per-animation delete (trash icon + inline confirm) via PropertiesPanelController::deleteAnimation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds an experimental MotionLibrary-backed text-to-motion flow with optional ONNX generation, skeleton retargeting, CLI/MCP/QML wiring, offline motion-library/training scripts, documentation updates, and related import, labeling, and version-reference changes. ChangesText-to-Motion Feature
Offline Motion Data Prep and Training Scripts
Unrelated Fixes and Version Bump
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: b070a37414
ℹ️ 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".
| guard.setSingleShot(true); | ||
| QObject::connect(&guard, &QTimer::timeout, &loop, [&]{ ok = false; loop.quit(); }); | ||
| guard.start(180000); | ||
| dl->startDownload(label, base + fileName, dest); |
There was a problem hiding this comment.
When the experimental model is first used and the files are not already cached, this call passes label as the URL and the real URL as the destination, while ModelDownloader::startDownload expects (url, destinationPath, modelName) like the other call sites. The completion/error signals are then emitted under the wrong model name, so the local event loop waits for the 180s guard and the model path never becomes available; this makes --model / model:true / the GUI checkbox effectively unable to download the model on first use.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in 163db2f. startDownload is (url, destinationPath, modelName); the call now passes (base+fileName, dest, label) to match the other consumers (MotionInbetween etc.), so the completion/error signals fire under the right name and first-use download resolves instead of timing out on the 180s guard.
| // not (AnimationMerger::applyMotionClip) — so correct labels do NOT mirror | ||
| // the motion. Decoupling label-side from retarget-side is the fix for "labels | ||
| // flipped vs animation mirrored" being in tension. | ||
| auto side = [&](int i) { return -joints[i].pos[SIDE]; }; |
There was a problem hiding this comment.
For Y-up T-pose/UniRig skeletons where the character's left side is on +X (the new unit test builds exactly that case, and CMU canonical left is also treated as +X in the retargeter), negating the side coordinate makes +X limbs evaluate as Right* when the limb code later checks side(...) >= 0. That reverses the anatomical bone names, so name-based canonical mapping and Select-by-Part labels are wrong even though the retarget swap may hide part of it during playback.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 163db2f. The labeler intentionally names the character's LEFT on the −X side (glTF/Ogre Y-up, faces-+Z convention) and the retarget does its own handedness compensation against the CMU clip (CMU left=+X) so names and motion are decoupled — this is what fixed the real-rig mislabel + mirrored-motion. The unit test had a stale +X=left assumption; I flipped its synthetic geometry so −X limbs assert as Left (canon 13) and +X as Right (canon 9), matching actual behavior.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/CLIPipeline.cpp (1)
2140-2154: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd the missing
--generateusage line.The error now lists
--generate, but the detailed usage block does not show the required prompt or optional--duration/--modelflags.📝 Proposed fix
err() << " qtmesh anim <file> --simplify [--preset {conservative|balanced|aggressive}] [--tolerance T] [--rotation-tolerance-deg D] [-o <output>] [--animation <name>]" << Qt::endl; err() << " (--tolerance T sets translation+scale tolerance in world units)" << Qt::endl; err() << " qtmesh anim <file> --analyze [--json] [--preset ...] [--tolerance T] [--rotation-tolerance-deg D]" << Qt::endl; err() << " qtmesh anim <file> --in-between --gap-frames N [--start-time S] [--end-time S] [--no-model] [-o <output>] [--animation <name>]" << Qt::endl; + err() << " qtmesh anim <file> --generate <prompt> [--duration seconds] [--model] [--json] [-o <output>]" << Qt::endl; return 2; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/CLIPipeline.cpp` around lines 2140 - 2154, The usage block in CLIPipeline’s animation command error handling is missing the `--generate` help line even though it is listed in the error summary. Update the usage text near the main `if (!listMode && !renameMode ... )` check to include a `qtmesh anim <file> --generate` line, and make sure it documents the required prompt plus the optional `--duration` and `--model` flags so the help output matches the supported option set.
🟠 Major comments (20)
src/AnimationMerger.cpp-919-919 (1)
919-919: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftHonor
cmuRestWorldor fail fast for local clips.The public API documents
cmuRestWorldas the CMU↔target basis for v1/v2 local clips, but the implementation ignores it and treats local clips asclipQ(f, c)only. Callers passing rest-basis data get silently wrong retargeting.Also applies to: 1048-1050
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/AnimationMerger.cpp` at line 919, The local-clip retargeting path in AnimationMerger is ignoring the cmuRestWorld basis even though it is part of the public API contract, so update the logic that consumes clipQ/clipT to actually use cmuRestWorld when processing v1/v2 local clips, or explicitly fail fast if that basis is not supported. Make the fix in the relevant AnimationMerger retargeting routine(s) that read the clip data so callers passing rest-basis transforms do not get silently incorrect results.src/AnimationMerger.cpp-1130-1139 (1)
1130-1139: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFill each decimated gap instead of the whole clip.
After
decimateAnimation(), the animation has interior keys inside(0, length), andinbetweenAnimation()explicitly rejects non-gap windows with interior keyframes. This refine path can therefore leave the clip decimated/sparse whileapplyMotionClip()still returnsok = true.Proposed fix
// Decimate every track to keep every refineStride-th key (+ the last), // then in-between-fill the whole clip so RMIB regenerates the interior. decimateAnimation(skel, animName, refineStride); - const auto fill = inbetweenAnimation( - skel, animName, 0.0f, length, refineStride - 1, modelPath, - /*forceFallback=*/modelPath.isEmpty()); - res.refined = fill.ok; - res.usedModel = fill.ok && fill.usedModel; + bool anyFilled = false; + bool anyUsedModel = false; + for (int start = 0; start < frames - 1; start += refineStride) { + const int end = std::min(start + refineStride, frames - 1); + const int gapFrames = end - start - 1; + if (gapFrames <= 0) + continue; + + const auto fill = inbetweenAnimation( + skel, animName, start * dt, end * dt, gapFrames, modelPath, + /*forceFallback=*/modelPath.isEmpty()); + anyFilled = anyFilled || fill.ok; + anyUsedModel = anyUsedModel || (fill.ok && fill.usedModel); + } + res.refined = anyFilled; + res.usedModel = anyUsedModel; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/AnimationMerger.cpp` around lines 1130 - 1139, The refine path in AnimationMerger::applyMotionClip currently calls inbetweenAnimation() over the full clip after decimateAnimation(), but that helper rejects windows containing interior keys, so the clip can remain sparse while still reporting success. Update the refine branch to iterate over each decimated gap and call inbetweenAnimation() only on gap windows between kept keys (instead of [0, length]), then aggregate the results into res.refined and res.usedModel so the whole clip is actually refilled.src/MotionLibrary.cpp-90-119 (1)
90-119: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winValidate the declared joint names and order.
This loader ignores the top-level
jointsarray entirely, even though the format comments require the 22 canonical CMU joints in a fixed order. A reordered or mislabeled library will still load and then retarget the wrong bones for every generated animation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/MotionLibrary.cpp` around lines 90 - 119, The MotionLibrary loader currently ignores the top-level joints declaration, so validate the declared joint names and fixed order before accepting clips. Update MotionLibrary::load to read and compare the root-level "joints" array against the 22 canonical CMU joints, and reject the file with m_error if the names or order do not match. Keep the existing clip/frame parsing in place, but gate it on the joint schema check so reordered or mislabeled libraries fail early.src/MotionLibrary.cpp-77-86 (1)
77-86: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReject malformed quaternion entries instead of silently defaulting them.
Both
cmuRestWorldand per-jointquatsreadq.at(0..3)without first checking that the JSON value is actually a 4-element array. Truncated or non-array entries are therefore accepted as zero/identity-ish quaternions, which breaks the library schema and feeds corrupted poses into retargeting.Also applies to: 107-113
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/MotionLibrary.cpp` around lines 77 - 86, In MotionLibrary’s JSON parsing, the cmuRestWorld loader (and the per-joint quats path mentioned in the comment) currently assumes every entry is a 4-element array and reads q.at(0..3) directly, which can silently accept malformed values. Update the parsing logic in the relevant MotionLibrary methods to validate that each quaternion value is actually an array of length 4 before constructing the quaternion, and reject/skip the entry or fail parsing when it is not. Use the existing cmuRestWorld and quats handling blocks as the place to add the guard so truncated or non-array entries are never defaulted into m_cmuRestWorld or joint poses.src/MotionGenerator.cpp-176-183 (1)
176-183: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winEnforce the exact 22-joint /
[1,T,C]model contract before returningok=true.Right now any vocab JSON with
C == J * 10is accepted, even ifJ != 22, and the output tensor only needs to have at leastT * Celements. A stale or mismatched model/vocab pair can therefore produce aMotionLibrary::Clipthat violates the downstream retarget contract or is unpacked from the wrong layout.Suggested contract checks
- if (V == 0 || T <= 0 || C != J * 10) { + if (V == 0 || T <= 0 || J != 22 || C != J * 10) { r.error = QStringLiteral("t2m vocab json malformed"); return r; } @@ - if (total < static_cast<int64_t>(T) * C) { - r.error = QStringLiteral("t2m output smaller than expected"); return r; + if (shape.size() != 3 || shape[0] != 1 || shape[1] != T || shape[2] != C || + total != static_cast<int64_t>(T) * C) { + r.error = QStringLiteral("t2m output shape mismatch"); return r; }Also applies to: 249-253
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/MotionGenerator.cpp` around lines 176 - 183, The vocab/model validation in MotionGenerator should enforce the exact downstream contract instead of only checking C == J * 10. Update the checks in the motion loading path around the existing vj parsing and the tensor-shape validation to require J == 22, C == 220, T > 0, and an output tensor shape exactly matching [1,T,C] rather than merely having enough elements. Use the existing MotionGenerator::loadMotion-style validation and r.error handling so stale or mismatched vocab/model pairs fail before ok=true is returned.src/MotionLibrary.cpp-145-159 (1)
145-159: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMatch whole tokens here, not raw substrings.
contains()makes prompts likewhite shirtmatchhit→punch, andgrandstandmatchstand→idle. Becausesrc/MotionGenerator.cppduplicates the same matcher shape, these false positives affect both template and model flows. Tokenize the prompt or use word-boundary matching and share the helper.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/MotionLibrary.cpp` around lines 145 - 159, The current matching in the clip lookup path uses raw contains() checks, so it can falsely match substrings instead of whole words. Update the matcher used by this logic around m_clips, kSynonyms, and findAction to tokenize the prompt or use word-boundary matching, and apply the same helper in MotionGenerator.cpp so both template and model flows share identical whole-token matching behavior.src/MotionGenerator.cpp-105-117 (1)
105-117: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSwap the
startDownload()arguments here.ModelDownloader::startDownload()expects(url, destinationPath, modelName), but this call passes(label, base + fileName, dest). That makes the request URL the human-readable label and stores the destination path as the model name, so thename == labelcompletion check will never match.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/MotionGenerator.cpp` around lines 105 - 117, The call in MotionGenerator::startDownload uses the arguments in the wrong order, so the URL, destination path, and model name are being mixed up. Update the call site in MotionGenerator.cpp to pass the download URL first, then the destination path, then the model name, matching ModelDownloader::startDownload(url, destinationPath, modelName) so the downloadCompleted/downloadError callbacks can match on the correct name.src/MCPServer.cpp-3378-3424 (1)
3378-3424: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winValidate MCP
durationbefore allocating retimed frames.A client can send a huge or non-finite
duration; line 3419 turns it into a vector size. Validate the JSON type and cap the accepted range, and reflect that bound in the tool schema description.🛡️ Proposed fix
+#include <cmath> + - const double duration = args.value("duration").toDouble(0.0); + const QJsonValue durationArg = args.value("duration"); + if (!durationArg.isUndefined() && !durationArg.isDouble()) + return makeErrorResult("Error: 'duration' must be a number."); + const double duration = durationArg.toDouble(0.0); + constexpr double kMaxGenerateDurationSeconds = 60.0; + if (!std::isfinite(duration) || duration < 0.0 || duration > kMaxGenerateDurationSeconds) + return makeErrorResult(QString("Error: 'duration' must be between 0 and %1 seconds.") + .arg(kMaxGenerateDurationSeconds, 0, 'f', 0));- props["duration"] = QJsonObject{{"type", "number"}, {"description", "Optional clip length in seconds (retimes the template). Default: the clip's native length."}}; + props["duration"] = QJsonObject{{"type", "number"}, {"description", "Optional clip length in seconds, 0..60 (retimes the template). Default: the clip's native length."}};Also applies to: 6623-6623
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/MCPServer.cpp` around lines 3378 - 3424, Validate the MCP `duration` input in the request-handling path before it reaches the retiming logic in `MCPServer.cpp` so a huge or non-finite value cannot become the `want` vector size in the template clip branch. Update the argument parsing around `args.value("duration")` to reject non-numeric/invalid values, clamp the accepted range to a safe maximum, and return a clear error via `makeErrorResult` when out of bounds. Also update the tool schema/description for this parameter to document the allowed range so callers know the limit.src/CLIPipeline.cpp-1922-1924 (1)
1922-1924: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winValidate
--durationbefore using it as a frame-count multiplier.
QString::toFloat()silently returns0on bad input, and very large values flow intostd::vector retimed(want). Reject non-numeric, negative, non-finite, and unreasonable durations before dispatching.🛡️ Proposed fix
+#include <cmath> + if (arg == "--duration" && i + 1 < argc) { - generateDuration = QString(argv[++i]).toFloat(); + bool ok = false; + generateDuration = QString::fromLocal8Bit(argv[++i]).toFloat(&ok); + constexpr float kMaxGenerateDurationSeconds = 60.0f; + if (!ok || !std::isfinite(generateDuration) || generateDuration < 0.0f + || generateDuration > kMaxGenerateDurationSeconds) { + err() << "Error: --duration must be a number between 0 and " + << kMaxGenerateDurationSeconds << " seconds." << Qt::endl; + return 2; + } continue; }Also applies to: 2079-2081
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/CLIPipeline.cpp` around lines 1922 - 1924, Validate the parsed --duration value before it is used in CLIPipeline’s retiming logic, especially in the block that computes want and constructs retimed. In the duration parsing path, reject non-numeric, negative, non-finite, and unreasonably large values instead of relying on QString::toFloat() defaults, and make sure the same validation applies to both call sites that feed the frame-count multiplier logic. If the value is invalid, fail early with a clear error before dispatching to the code that uses duration in std::max and std::vector allocation.src/AnimationControlController.cpp-1655-1728 (1)
1655-1728: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winValidate
durationbefore retiming allocates frames.Line 1722 converts QML-controlled
durationinto a frame count and allocatesretimed;NaN, infinity, or a huge value can overflow the cast or attempt an enormous allocation on the UI path.🛡️ Proposed fix
+#include <cmath> + QVariantMap AnimationControlController::generateMotion(const QString& prompt, double duration, bool useModel) { QVariantMap out; out["ok"] = false; auto fail = [&](const QString& e) { out["error"] = e; emit generateMotionStatus(e, true); return out; }; if (prompt.trimmed().isEmpty()) return fail(QStringLiteral("Enter a motion prompt (e.g. \"walking\").")); + constexpr double kMaxGenerateDurationSeconds = 60.0; + if (!std::isfinite(duration) || duration < 0.0 || duration > kMaxGenerateDurationSeconds) + return fail(QStringLiteral("Duration must be between 0 and %1 seconds, or 0 for native length.") + .arg(kMaxGenerateDurationSeconds, 0, 'f', 0));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/AnimationControlController.cpp` around lines 1655 - 1728, Validate the duration input in AnimationControlController::generateMotion before computing the retimed frame count. Reject non-finite values and clamp overly large durations to a safe maximum before the std::max/int cast and retimed allocation, and return a failure through the existing fail lambda if the value is invalid. Use the generateMotion, fail, and retimed paths to place the guard right before the duration-to-frames conversion.scripts/train-t2m-onnx.py-114-122 (1)
114-122: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse a branch-stable matrix-to-quaternion conversion.
This formula collapses rotations with trace near
-1becausewapproaches zero and the vector part is divided by4*w. Exported motion can contain non-unit or near-zero quaternions for 180° rotations, which breaks retargeting.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/train-t2m-onnx.py` around lines 114 - 122, The mat_to_quat helper currently relies on a trace-based formula that becomes unstable near 180° rotations because w approaches zero and x/y/z are divided by 4*w. Update the conversion in mat_to_quat to use a branch-stable matrix-to-quaternion algorithm (with per-case handling based on the largest diagonal term or equivalent stable logic) so rotations with trace near -1 still produce valid unit quaternions for export.scripts/prep-t2m-clean.py-73-73 (1)
73-73: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse a platform-independent default cache path.
Line 73 defaults to
/tmp/t2m_clean.npz, which fails on Windows environments without/tmp. Prefertempfile.gettempdir()or require--out.Portable default
import argparse, csv, glob, os, re +import tempfile ... - ap.add_argument("--out",default="/tmp/t2m_clean.npz") + ap.add_argument("--out", default=os.path.join(tempfile.gettempdir(), "t2m_clean.npz"))As per retrieved learnings, “All code must compile and run on Windows, Linux (Ubuntu), and macOS.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/prep-t2m-clean.py` at line 73, The default output path in the argument parser is hardcoded to a Unix-only temporary location, which breaks portability. Update the --out default in the main argparse setup to use a platform-independent temporary directory via tempfile.gettempdir() (or make --out mandatory if that better fits the script), so prep-t2m-clean.py works consistently across Windows, Linux, and macOS.Sources: Learnings, Linters/SAST tools
scripts/train-t2m-onnx.py-59-59 (1)
59-59: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse a platform-independent default cache path.
Line 59 defaults to
/tmp/t2m_preprocessed.npz, which is not portable to Windows. Usetempfile.gettempdir()or require--cache.Portable default
import json import os +import tempfile ... - ap.add_argument("--cache", default="/tmp/t2m_preprocessed.npz") + ap.add_argument("--cache", default=os.path.join(tempfile.gettempdir(), "t2m_preprocessed.npz"))As per retrieved learnings, “All code must compile and run on Windows, Linux (Ubuntu), and macOS.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/train-t2m-onnx.py` at line 59, The default cache path in the argument setup is platform-specific and breaks portability on Windows. Update the `ap.add_argument("--cache", ...)` logic in `train-t2m-onnx.py` to use a temp-directory-based default such as `tempfile.gettempdir()` (or make `--cache` required), so the script works consistently across Windows, Linux, and macOS.Sources: Learnings, Linters/SAST tools
scripts/export-t2m-onnx.py-171-181 (1)
171-181: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKey the preprocessing cache by inputs, or validate cache metadata.
The comment says the cache is keyed by BVH dir and vocab, but Line 173 always uses
t2m_preprocessed.npzbeside--out. Re-running with different--bvh,--annotations, orVOCABsilently trains on stale data.Metadata guard sketch
+ meta = { + "bvh": os.path.abspath(a.bvh), + "annotations": os.path.abspath(a.annotations), + "vocab": VOCAB, + } cache = os.path.join(os.path.dirname(a.out) or ".", "t2m_preprocessed.npz") if os.path.exists(cache): d = np.load(cache) - mo, tk = d["mo"], d["tk"] + if "meta" not in d or json.loads(str(d["meta"])) != meta: + raise SystemExit(f"stale preprocessing cache: remove {cache} or choose a new --out") + mo, tk = d["mo"], d["tk"] ... - np.savez(cache, mo=mo, tk=tk) + np.savez(cache, mo=mo, tk=tk, meta=json.dumps(meta))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/export-t2m-onnx.py` around lines 171 - 181, The preprocessing cache in the export script is using a fixed filename in the output directory, so it can be reused across different BVH, annotation, or vocab inputs. Update the caching logic around the `cache`/`np.load`/`np.savez` flow to either derive the cache key from the active inputs used by `load_annotations()` and `preprocess()`, or persist and validate cache metadata before loading. Keep the `t2m_preprocessed.npz` path only if it is guarded by input checks so `preprocess()` never consumes stale cached `mo` and `tk` data.qml/PropertiesPanel.qml-5718-5738 (1)
5718-5738: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMake the trained-model toggle keyboard/screen-reader reachable.
This custom checkbox never takes tab focus and exposes no
Accessiblerole/state, so keyboard users cannot opt into model generation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@qml/PropertiesPanel.qml` around lines 5718 - 5738, The custom trained-model checkbox in PropertiesPanel.qml is not keyboard or screen-reader accessible because the Rectangle-based control lacks focus handling and Accessible metadata. Update the useModelChk control so it can receive tab focus and be toggled via keyboard, and add the appropriate Accessible role/name/state to the checkbox element and its current checked state. Keep the existing MouseArea behavior, but make sure the same checked binding is exposed through the accessible properties and keyboard interaction for the “Use trained model (experimental)” option.qml/PropertiesPanel.qml-651-653 (1)
651-653: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftDon't gate the Animations section on the primary selection only.
animationData()renders per-entity groups for the whole selection, buthasSkeletonSelection()is documented as checking only the first resolved selection. With a mixed selection, one unrigged primary entity now hides the entire section even when another selected entity has a skeleton and animations.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@qml/PropertiesPanel.qml` around lines 651 - 653, The Animations section visibility is incorrectly tied to PropertiesPanelController.hasSkeletonSelection(), which only reflects the first resolved selection and can hide the section for mixed selections. Update the sectionVisible condition in PropertiesPanel.qml so it considers whether any selected entity has animation data rather than only the primary selection, using the existing animationData() / per-entity selection logic in this panel. Keep the mode check through root.modeToolSectionVisible(EditorModeController.AnimationMode, ...) but replace the skeleton-only gate with a selection-wide animation availability check.qml/PropertiesPanel.qml-6142-6184 (1)
6142-6184: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThe delete confirmation affordance is mouse-only.
The trash / ✓ / ✗ controls do not participate in tab order and are not exposed as buttons, so keyboard users cannot arm, confirm, or cancel animation deletion.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@qml/PropertiesPanel.qml` around lines 6142 - 6184, The trash/delete confirmation UI in PropertiesPanel.qml is mouse-only and cannot be reached or activated from the keyboard. Update the trashBtn, trashMouse, and the confirm/cancel controls inside the Row to expose proper keyboard-accessible button behavior and tab focus, so users can arm deletion with the trash control and confirm or cancel via keyboard as well as mouse. Ensure the confirm and cancel actions remain wired to PropertiesPanelController.deleteAnimation and the confirming state transitions, but make the interactive elements participate in tab order and expose accessible button semantics.qml/PropertiesPanel.qml-5694-5709 (1)
5694-5709: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winLet the busy state render before calling
generateMotion().
generateMotion()is explicitly documented as blocking on the first-use library download. Becauserun()invokes it synchronously on the main thread and clearsgenBtnBusyright after, the panel never repaints into its busy state; the first run just looks frozen.Proposed fix
function run() { if (genBtnBusy || !genPromptIn.text.trim()) return genBtnBusy = true genStatus.text = useModelChk.checked ? "Generating (experimental model)…" : "Generating… (first use downloads the motion library)" genStatus.isError = false - AnimationControlController.generateMotion(genPromptIn.text, 0.0, - useModelChk.checked) - genBtnBusy = false - // generateMotion adds an AnimationState synchronously, but - // it lives on AnimationControlController — the Inspector - // list (PropertiesPanelController.animationData) won't know - // until something re-queries. Refresh it directly so the - // new clip appears without reselecting the entity. - refreshAnimData() + const prompt = genPromptIn.text + const useModel = useModelChk.checked + Qt.callLater(function() { + try { + AnimationControlController.generateMotion(prompt, 0.0, useModel) + } finally { + genBtnBusy = false + // generateMotion adds an AnimationState synchronously, but + // it lives on AnimationControlController — the Inspector + // list (PropertiesPanelController.animationData) won't know + // until something re-queries. Refresh it directly so the + // new clip appears without reselecting the entity. + refreshAnimData() + } + }) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@qml/PropertiesPanel.qml` around lines 5694 - 5709, The busy state in run() is being set and then immediately blocked by the synchronous AnimationControlController.generateMotion() call, so the UI never repaints before the first-use download stalls it. In PropertiesPanel.qml, update run() to let the event loop process the genBtnBusy/genStatus changes before invoking generateMotion(), and only clear genBtnBusy after the generation work finishes; use the existing run() function and AnimationControlController.generateMotion() call as the fix point.src/UniRigPredictor.cpp-339-347 (1)
339-347: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThe left/right sign is inverted relative to the declared contract.
src/UniRigPredictor.hsays left is+X, and the new test builds left limbs on+X, butside = -joints[i].pos[SIDE]makes+SIDEresolve as right. Once axis detection is corrected, this still flips every limb name unless the sign or the documented convention is updated in the same PR.Suggested fix
- auto side = [&](int i) { return -joints[i].pos[SIDE]; }; + auto side = [&](int i) { return joints[i].pos[SIDE]; };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/UniRigPredictor.cpp` around lines 339 - 347, The left/right mapping in UniRigPredictor is inverted against the contract declared in UniRigPredictor.h and the new axis-based tests. Update the side-name logic in UniRigPredictor::predict (the local side lambda) so the sign matches the documented convention for +X = left, and keep the naming rule consistent with any axis-detection changes in the same PR. Ensure the LEFT/RIGHT labels produced from joints[i].pos[SIDE] align with the header contract and do not flip limb names when the detected axis is corrected.src/UniRigPredictor.cpp-319-337 (1)
319-337: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winHonor
upAxisinstead of redetecting it from spread.This ignores the public
upAxiscontract on normal rigs. A tall +Y-up skeleton can easily have a larger Y span than arm span, soSIDE = largest span/U = second-largestpicks the wrong axes and mislabels the whole skeleton even when the caller passed the correct up axis.Suggested fix
- int order[3] = {0, 1, 2}; - std::sort(order, order + 3, [&](int a, int b){ return spanV[a] > spanV[b]; }); - const int SIDE = order[0]; // widest — arms span - int U = order[1]; // second — head↔foot - // Degenerate guard: if the 2nd/3rd spans are near-equal and tiny, fall back - // to the passed-in up so a flat/odd rig doesn't pick noise. - if (spanV[order[1]] < 1e-4 && upAxis >= 0 && upAxis <= 2) U = upAxis; + int order[3] = {0, 1, 2}; + std::sort(order, order + 3, [&](int a, int b){ return spanV[a] > spanV[b]; }); + const int U = (upAxis >= 0 && upAxis <= 2) ? upAxis : order[1]; + const int SIDE = (order[0] == U) ? order[1] : order[0];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/UniRigPredictor.cpp` around lines 319 - 337, The axis-selection logic in UniRigPredictor should honor the caller-provided upAxis instead of inferring it from spread. Update the axis assignment block in the predictor so SIDE and FWD are derived relative to the known up axis, rather than using std::sort on spanV to pick the largest/second-largest spans. Keep the existing spread-based fallback only for degenerate cases where upAxis is invalid or the rig is too ambiguous, and make sure the main path in the UniRigPredictor axis detection code uses upAxis as the contract.
🟡 Minor comments (7)
src/MotionGenerator.cpp-35-43 (1)
35-43: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep the model synonym map in one place.
This table has already drifted from
src/MotionLibrary.cpp(hello,greet,spin,step,rest,jumping, etc. are missing here), so--modelcan reject or reinterpret prompts that the default template path accepts. Sharing one matcher would also fix the substring bug in both paths together.Also applies to: 137-151
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/MotionGenerator.cpp` around lines 35 - 43, The synonym mapping in MotionGenerator is duplicated and has drifted from the one used in MotionLibrary, causing different prompt handling between the default template path and --model. Refactor the lookup logic around the kSynonyms table and the matcher used by MotionGenerator so both paths share a single source of truth for synonym normalization, and update the matching behavior to eliminate the substring-based misinterpretation in both places. Make sure the shared matcher covers the missing entries referenced by MotionLibrary and is used consistently wherever motion words are resolved.src/CLIPipeline.cpp-1938-1941 (1)
1938-1941: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve the established no-skeleton CLI error text.
This new
cmdAnimpath returns the right exit code, but it bypasses the existing no-skeleton contract by printing a different message. Keep the exactError: No skeleton found.line and add the text-to-motion guidance separately if needed. Based on learnings, “In src/CLIPipeline.cpp, ensure CLIPipeline::cmdAnim returns 1 (error) when no skeleton is found for the input model, and emits the message "Error: No skeleton found."” <retrieved_learnings>💚 Proposed fix
if (!entity) { - err() << "Error: " << filePath << " has no skinned mesh — text-to-motion " - "needs a rigged humanoid skeleton to retarget onto." << Qt::endl; + err() << "Error: No skeleton found." << Qt::endl; + err() << "Text-to-motion needs a rigged humanoid skeleton to retarget onto." + << Qt::endl; return 1; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/CLIPipeline.cpp` around lines 1938 - 1941, The cmdAnim no-skeleton handling in CLIPipeline::cmdAnim is using a new message instead of the established CLI contract; change the error path so it still returns 1 and emits the exact "Error: No skeleton found." line, then add any extra text-to-motion guidance as a separate message if needed. Update the existing entity-null check in CLIPipeline.cpp to preserve the legacy no-skeleton text while keeping the new retargeting context around it.Source: Learnings
src/CLIPipeline.cpp-2075-2078 (1)
2075-2078: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMake
--modelorder-independent.Currently
--modelis only recognized after--generate;qtmesh anim file --model --generate walkingsilently falls back to templates.💚 Proposed fix
- if (arg == "--model" && generateMode) { generateUseModel = true; continue; } + if (arg == "--model") { generateUseModel = true; continue; }filePath = positional[0]; + if (generateUseModel && !generateMode) { + err() << "Error: --model is only valid with --generate." << Qt::endl; + return 2; + } + // `#411`: text-to-motion (template-clip MVP). Self-contained — load → match a🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/CLIPipeline.cpp` around lines 2075 - 2078, Make the anim subcommand’s --model flag order-independent in CLIPipeline parsing. Right now the check around generateMode only enables generateUseModel after --generate has already been seen, so a later --generate still falls back to templates. Update the argument handling in the CLIPipeline::run parsing loop so --model sets a pending/latched state regardless of order, and apply it when --generate is eventually parsed; use the existing generateMode and generateUseModel symbols to keep the behavior consistent.scripts/prep-quaternius-t2m.py-2-2 (1)
2-2: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAlign the compact one-line style with Ruff.
Static analysis reports multiple
E701errors, but Line 2 only suppressesE702,E741. Expand the colon one-liners or explicitly suppressE701for this offline script.Also applies to: 54-60, 89-109, 118-138, 147-147
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/prep-quaternius-t2m.py` at line 2, The script still triggers Ruff E701 on several one-line compound statements, but the current module-level noqa only covers E702 and E741. Update the affected one-liners in prep-quaternius-t2m.py by expanding them into multiline statements, or if the compact style must remain, add E701 to the existing Ruff suppression near the top of the file. Use the existing script blocks around the reported sections to locate and adjust the one-line conditionals and loops consistently.Source: Linters/SAST tools
scripts/prep-t2m-clean.py-2-2 (1)
2-2: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAlign the compact one-line style with Ruff.
The file-level
noqadoes not includeE701, but this file has many colon one-liners reported as Ruff errors. Either expand those branches or explicitly includeE701if this compact script style is intentional.Also applies to: 50-50, 58-58, 85-88, 90-100, 111-119
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/prep-t2m-clean.py` at line 2, The top-level Ruff suppression in prep-t2m-clean.py is missing E701, so the script’s colon one-liners still trigger lint errors. Either refactor the compact branches and loops into multi-line blocks throughout the affected logic, or, if the one-line style is intentional, update the file-level noqa to include E701 so the style is explicitly allowed. Use the existing script entrypoints and control-flow blocks in prep-t2m-clean.py as the places to adjust.Source: Linters/SAST tools
scripts/build-motion-library.py-2-2 (1)
2-2: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix or explicitly suppress the remaining Ruff errors.
Line 2 only ignores
E702,E741, but static analysis still reportsE701,E703, andS110in this file, so lint-gated CI can fail even though the script runs.Example cleanup
- if j in world: return world[j] + if j in world: + return world[j] ... - if np.linalg.norm(axis) < 1e-6: axis = np.cross(a, [0, 1, 0]) + if np.linalg.norm(axis) < 1e-6: + axis = np.cross(a, [0, 1, 0]) ... - axis = np.cross(a, b); s = np.sqrt((1 + d) * 2); + axis = np.cross(a, b) + s = np.sqrt((1 + d) * 2)Also applies to: 186-186, 226-229, 255-256
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/build-motion-library.py` at line 2, The Ruff suppression at the top of build-motion-library.py is incomplete, so the script can still fail lint gates on E701, E703, and S110. Either clean up the remaining single-line statements and any broad-exception usage in the affected top-level blocks, or expand the explicit noqa coverage where those patterns remain. Use the existing script sections around the cleanup/exception-handling logic to locate the remaining violations and ensure all reported Ruff errors are addressed.Source: Linters/SAST tools
CLAUDE.md-295-295 (1)
295-295: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the GUI file reference here.
This points contributors to
qml/AnimationControlPanel.qml, but the generate-from-text UI added in this PR lives inqml/PropertiesPanel.qml.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CLAUDE.md` at line 295, Update the MotionLibrary / text-to-motion documentation entry so the GUI reference points to qml/PropertiesPanel.qml instead of qml/AnimationControlPanel.qml. Keep the rest of the feature description intact, and make sure the “Generate from text” control is described using the actual UI location introduced in this PR, alongside the existing CLIPipeline::cmdAnimGenerate, MCPServer::toolGenerateMotion, and generateMotionStatus references.
🧹 Nitpick comments (3)
src/PropertiesPanelController.cpp (1)
905-944: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a breadcrumb around animation deletion.
This is a new irreversible user action, but the delete path does not record it, which makes post-crash reconstruction much harder. Based on learnings, "Track all user-facing actions and significant operations with
SentryReporter::addBreadcrumb(...)using the prescribed categories."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/PropertiesPanelController.cpp` around lines 905 - 944, The deleteAnimation flow in PropertiesPanelController is an irreversible user action but currently leaves no trace for crash reconstruction. Add a SentryReporter::addBreadcrumb(...) call at the start of PropertiesPanelController::deleteAnimation, using the prescribed user-action/significant-operation category and including the entityName and animName context so the deletion can be correlated later.Source: Learnings
src/UniRigPredictor_test.cpp (1)
384-442: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a second case for the new part/template path.
This test only covers the geometric fallback: every synthetic joint keeps the default
part, so the new body/hand-driven naming path and thedetokenize()part propagation can still regress unnoticed. Please add one case that sets body/hand parts explicitly, or driveslabelJointsAnatomically()through a token stream that emits those part markers.As per coding guidelines,
src/**/*_test.cpp: Add Google Test unit tests for new functionality insrc/using the_test.cppsuffix.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/UniRigPredictor_test.cpp` around lines 384 - 442, The current UniRigPredictor test only exercises the geometric fallback in labelJointsAnatomically and can miss regressions in the new part/template naming path. Add a second Google Test case in this test file that explicitly sets body/hand parts on the synthetic joints, or feeds labelJointsAnatomically a token stream that propagates those markers through detokenize(), so the new naming branch is covered. Use the existing UniRigPredictor::labelJointsAnatomically and MotionInbetween::canonicalIndexForBone symbols as the main reference points, and keep the uniqueness check/resolution assertions so the new case verifies both naming and canonical mapping.Source: Coding guidelines
src/AutoRig.cpp (1)
644-679: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a focused AutoRig test for ancestor-part inheritance.
This changes the rig-prior resolve path and the fallback gate, but there’s no focused regression test here for an unnamed helper bone inheriting
LeftArm/RightLegfrom its nearest named ancestor. A smallsrc/AutoRig_test.cppcase would lock the new behavior down before it silently falls back to model labeling again.As per coding guidelines,
src/**/*_test.cpp: Add Google Test unit tests for new functionality insrc/using the_test.cppsuffix.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/AutoRig.cpp` around lines 644 - 679, Add a focused Google Test in AutoRig_test.cpp for the ancestor-part inheritance path introduced in AutoRig::rig-prior resolve logic. Create a regression case where an unnamed/helper bone (for example a twist or joint_37-style bone) has a nearest named ancestor mapped by MeshSegmenter::partForBoneName, and verify partOfBone resolves the child to that ancestor’s part instead of Unknown. Also assert the vertex labeling/resolution path updates resolved/labels correctly so the fallback gate is exercised only when no named ancestor exists.Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f888ae2d-2c0b-4712-a772-cb2cabcb0d8a
📒 Files selected for processing (35)
CLAUDE.mdCMakeLists.txtREADME.mdTHIRD_PARTY_AI_MODELS.mdqml/PropertiesPanel.qmlscripts/build-motion-library.pyscripts/export-t2m-onnx.pyscripts/prep-quaternius-t2m.pyscripts/prep-t2m-clean.pyscripts/train-t2m-onnx-v2.pyscripts/train-t2m-onnx-v3.pyscripts/train-t2m-onnx.pysrc/AnimationControlController.cppsrc/AnimationControlController.hsrc/AnimationMerger.cppsrc/AnimationMerger.hsrc/Assimp/Importer.cppsrc/AutoRig.cppsrc/CLIPipeline.cppsrc/CLIPipeline.hsrc/CMakeLists.txtsrc/MCPServer.cppsrc/MCPServer.hsrc/MotionGenerator.cppsrc/MotionGenerator.hsrc/MotionLibrary.cppsrc/MotionLibrary.hsrc/MotionLibrary_test.cppsrc/PropertiesPanelController.cppsrc/PropertiesPanelController.hsrc/UniRigPredictor.cppsrc/UniRigPredictor.hsrc/UniRigPredictor_test.cpptests/CMakeLists.txtwebsite/src/hooks/useQtmeshActionRef.js
…ard, L/R test - MotionGenerator: startDownload args were (label,url,dest); corrected to the expected (url,dest,modelName) — first-use model download was silently waiting on the 180s guard and never resolving (Codex P2). - applyMotionClip: validate every clip frame carries all canonical joints before indexing, so a malformed/generated clip can't crash the retarget (CodeRabbit). - UniRigPredictor_test: the labeler names −X as the character's LEFT (glTF/Ogre +Z-facing convention); flip the synthetic rig's limb sides to match so the test reflects actual behavior instead of a +X=left assumption (Codex P2). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
UniRigPredictor.LabelsAnatomicallyResolveCanonicalJoints failed on a duplicate bone name: joints the labeler doesn't classify keep their incoming name, and the test seeded a shared placeholder — colliding names Ogre::Skeleton::createBone rejects. Two fixes: - labelJointsAnatomically now runs a FINAL UNIQUENESS PASS that forces any remaining collision to a unique joint_<i> (robust regardless of input names). - the test seeds unique joint_N names like the real predictor (jointName). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…(PR #783) unit-tests-linux still failed: the labeler's "widest span = SIDE" heuristic picked the wrong axes on a finger-less T-pose (head↔foot Y-extent 1.85 exceeded the arm span 1.30 → SIDE=Y, UP=X), so spine became a "limb" and arm/leg tips went unlabelled (resolved 10/17, hands = -1). Replace with an anatomical signal that holds with or without fingers: SIDE = the axis about which joints are most MIRROR-SYMMETRIC (limbs come in L/R pairs → mean ≈ span mid), UP = the larger- spread of the remaining two, FWD = the last. Verified: the synthetic test rig now resolves the expected joints (test green locally) and the real cat/asd UniRig dumps still detect Y/Z up correctly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
#411) MCPServerProtocolTest.ProcessMessageToolsListDispatchesToHandler hung in CI until the 20-min per-suite wall-clock cap SIGKILLed it (signal 9), leaving 26 tests unrun and failing unit-tests-linux (4884/4910). Deterministic on the branch, green on master — a threshold effect. Root cause: MCPServer::sendResponse does a single blocking write() of the full response to the output fd, and these tests read only AFTER processMessage() returns. tools/list now serialises ~89 verbose tool schemas (#411 generate_motion plus the UV/HDR tools merged from master) well past the 64 KiB Linux pipe capacity, so the write blocks mid-payload with nothing draining it — a classic single-threaded pipe deadlock. Production is unaffected: the real MCP client drains stdout continuously. - Enlarge the test pipe to 1 MiB (F_SETPIPE_SZ on Linux, _pipe size on Windows) so any single response fits one buffered write. - Set every AI-model *_NO_DOWNLOAD guard in test_main.cpp so no suite can ever block on a first-use model download (generate_motion's MotionGenerator / MotionLibrary ensure*Blocking() spin a QEventLoop on a real download otherwise). Respects a caller-provided value for intentional integration runs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
1 participant
Closes the #411 text-to-motion work, plus the rigging/retarget fixes it surfaced along the way. The shipped default stays the deterministic template-clip retarget; the trained model is an opt-in.
Text-to-motion model (experimental, opt-in)
MotionGenerator— ONNX consumer running a small from-scratch model that emits a 22-joint canonical clip, retargeted via the existingapplyMotionClippath (templateMotionLibraryis the automatic fallback).qtmesh anim --generate "…" --model, MCPgenerate_motion {model:true}, GUI "Use trained model (experimental)" checkbox.ENABLE_ONNX-guarded; model downloads on first use.prep-t2m-clean.py,prep-quaternius-t2m.py,train-t2m-onnx{,-v2,-v3}.py.QtMeshEditor-t2mandQtMeshEditor-models/motion/.UniRig naming + retarget (fixes 0/22-joints, mirrored, wrong-side)
UniRigPredictor::labelJointsAnatomically— names UniRig's positional joints (root/joint_N) anatomically from rest-pose geometry (spread-based axis detection + chain-direction arm/leg classification; honours body/hand part tokens; guarantees unique names).applyMotionCliphandedness compensation — swaps canonical L/R when the rig's left bone is on −X (CMU left = +X), so labels are anatomical and motion isn't mirrored. Roll-correction disabled for no-standing-pose rigs (UniRig).AutoRig::rigPriorPartLabelsinherits a bone's part from its nearest named ancestor → segmentation "Select by Part" uses the exact rig prior on UniRig skins (resolve 0% → 100%).Animation UI
PropertiesPanelController::deleteAnimation).Tests / docs
MotionLibrary_test,UniRigPredictor_test); UnitTests target builds green locally. CMU/Quaternius licensing documented inTHIRD_PARTY_AI_MODELS.md. Version 3.15.0 (doc-sync verified).🤖 Generated with Claude Code
Summary by CodeRabbit