fix(#411): overhaul text-to-motion — library v4, retarget fixes, retrained model - #792
Merged
Conversation
… t2m model Both motion sources produced unusable results: TEMPLATE LIBRARY (default path): clips were the FIRST 4 seconds of each CMU trial — usually idle lead-in, so "wave" never waved and "kick" shuffled. The "idle" source trial (69_01) actually walks, and "dance" (05_02) was ballet whose pirouettes fold under the locked-root retarget. build-motion-library.py v4: - ACTIVE-WINDOW selection: max-motion-energy window per clip (min for idle), start snapped to a calm, near-neutral frame — the retarget composes every frame as a delta against clip frame 0, so windows must open on a settled standing-like pose - corrected + expanded curation: salsa dance, real wait-for-bus idle, plus sit / throw / boxing (13 actions, from 10); synonyms extended in both matchers (sitting/toss/pitch/box/fight/spar) - joint-name ALIAS table so both the Daz-friendly and MotionBuilder-friendly CMU BVH conversions parse (the canonical set previously matched only Daz naming) GENERATIVE MODEL (--model path): the v3 model folded the character in half at ~3.5x real angular velocity. Root causes: delta-cumsum decoding accumulated per-frame errors; training on raw-120fps 0.33s windows (a third of a walk cycle) while the app plays 30fps; local-frame output consumed by the weaker v1 retarget path; and inference at z=0, which a low-beta CVAE never visits. prep-t2m-v4.py + train-t2m-onnx-v4.py (replacing prep-t2m-clean.py usage): - 30fps WORLD-frame FK windows with neutral starts, labels from the public CMU index (single-action trials + curated overrides), mirror augmentation - absolute-pose transformer decoder (self+cross attention, no cumsum) - per-sample velocity/acceleration matching in BOTH 6D and true rotation (geodesic) space — 6D L1 under-measures jitter - derived-local supervision (parent-inverse-times-child — the exact quantity applyMotionClip renders; world-only losses let spine-chain errors stack into a visible fold) - z=0 latent dropout + aggregate-posterior moment matching so the app's seed=zeros inference condition is supervised and in-distribution - 1-2-1 temporal smoothing baked into the exported ONNX graph C++: MotionGenerator reads "frame"/"fps" from the vocab json and surfaces Result::worldFrame; the CLI/MCP/GUI call sites pass it to applyMotionClip so v4 model clips ride the same world-frame retarget as v3 template clips (v3 vocab jsons without the key keep the local path — back-compat). Per-action angular velocity at z=0 now tracks real CMU motion (walk 3.2°/f vs 3.2 real; idle 0.7 vs 0.85; was 11-22°/f across the board). The template library remains the default and quality bar; the model stays opt-in. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Ogre AVERAGES all enabled animation states. The generate paths (GUI + MCP) added the new clip without touching enable states, so it blended with the import's auto-enabled animation (and any previously generated clips) — the average of disagreeing motions renders as a character shaking around a static mid-pose. The old near-idle library clips masked this; the v4 active-window clips exposed it. After a successful applyMotionClip, all other animation states are disabled and the generated clip is enabled + looped at t=0 — "generate walk" now visibly walks. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Auto-rigged skeletons (UniRig / template) have no prior animation, so applyMotionClip's raw path applies clip motion axes without the harvested- standing-pose change-of-basis that absorbs facing on Mixamo-style rigs. The CMU clips' anatomical forward is +Z (verified from the conversion's T-pose toe offsets), so a mesh that faces -Z steps BACKWARD — a moonwalk. AnimationMerger::detectBackwardFacing(entity) detects the mesh's facing from its foot region (toe mass extends forward of the ankle joints; conservative on inconclusive geometry), and applyMotionClip gains a yaw180 parameter that views every articulation delta in a 180°-yawed frame — applied only on the raw path (rigs WITH a standing pose are already covered by the Mc change-of-basis). All three generate surfaces (CLI/GUI/MCP) pass it. QTMESH_T2M_YAW180=0|1 overrides the heuristic for exotic meshes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…path After the first --generate succeeds, the skeleton HAS an animation — so the next --generate harvested that generated clip as the rig's "standing pose" and switched from the correct raw path to the standing-pose change-of-basis path, whose Mc is documented-bogus on rigs with non-identity bone rests (UniRig / template auto-rig): the second and later generates rendered as a splayed, shaking pose while the first looked fine. Two guards: (1) generated_* clips are never harvested as the standing pose — only rig-authored animations describe it; (2) the Mc change-of-basis is additionally gated on identity bone rests, the Mixamo-style assumption it was designed for. Verified: walk → dance chained generates on the same file now both render upright. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The exclusive-enable in generateMotion changed Ogre animation states without emitting PropertiesPanelController::animationStateChanged, so the Inspector's Animations checkboxes displayed STALE enabled flags — a user toggling from that stale state crossed the enables back into a blended, shaking pose. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Rigs segment body chains differently from the 22-role canonical skeleton: Mixamo has Spine AND Spine1 inside the canonical "abdomen" span (both map to that role), and some rigs multi-segment the arms. applyMotionClip applied the FULL role delta to every mapped bone, so such chains bent N times over — a walking character leaned/twisted with arms in odd positions. Count the bones mapped to each canonical role and give each the N-th fractional rotation (identity-slerp), so the chain's total articulation matches the clip regardless of segment count. Verified in the live viewport: walk now renders an upright, natural stride on a Mixamo rig. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… wiggle
The model path fed seed=zeros ("the mean clip"). A CVAE's conditional mean
averages the action's phase-misaligned training windows, leaving small,
directionally-incoherent residue — measured: local axis coherence +0.13 vs
+0.45 for real clips — which renders as a character shaking in place.
Sample the seed at ~0.6σ instead: the decoded clip's local motion statistics
match the real template clips (step 2.73 vs 2.77 deg/frame, coherence +0.50
vs +0.45), and each generate produces a fresh take. Verified in the live
viewport: the model checkbox now yields a coherent walk.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A single sampled seed is a lottery — tail seeds decode to extreme poses (body folded backward, arms flung across the face). Draw 8 seeds per generate (the model runs in ~ms), score every candidate clip on the DERIVED-LOCAL statistics the retarget actually renders — energy near real-clip magnitude, temporal axis coherence, bounded articulation from the starting pose — and keep the winner. Verified across repeated generates in the live viewport: consistently upright, coherent takes with per-generate variety preserved. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…target Deltas and the standing-pose frame map (Mc) were taken against clip frame 0. Template windows open on a settled pose so that was fine — but MODEL-generated clips have noisy per-joint frame-0 orientations, and referencing them handed every bone a slightly wrong constant offset on rigs that use the standing-pose path (Mixamo): arms held in weird positions while the same clips looked fine on auto-rigged skeletons (raw path, no Mc). Pick the clip's minimum-motion frame as the reference for both the articulation deltas and Mc. Template clips resolve to ~frame 0 as before; model clips get their cleanest frame. Verified on a Mixamo rig in the live viewport: model-path takes now hold natural arm positions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…iety No licensable pretrained text-to-motion model exists that the editor could bundle (the field's small models are all HumanML3D/AMASS = non-commercial; the new billion-parameter options need 24+ GB VRAM), so the practical route to a USABLE product is more of what already works: real mocap templates. - CURATED grows from 13 to 47 clips across 15 actions (adds sweep + wash; several takes per action, all through the active-window selection) - MotionLibrary::matchPrompt picks at RANDOM among same-action clips, so repeat generates give a different take each time — variety with real-mocap quality, which is what the generative path was chasing - library ~4 MB (was 1.1 MB); the experimental model path is unchanged Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 19 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR ships a v4/v3 text-to-motion feature: new offline scripts preprocess CMU BVH data and train/export an ONNX generative model ( ChangesV4 Text-to-Motion Feature
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant Caller as CLIPipeline/MCPServer/GUI
participant MotionLibrary
participant MotionGenerator
participant AnimationMerger
Caller->>MotionLibrary: matchPrompt(prompt)
MotionLibrary-->>Caller: matchedAction, clip candidate
alt useModel enabled
Caller->>MotionGenerator: generate(action, vocab)
MotionGenerator->>MotionGenerator: sample kCandidates latents, run ONNX inference
MotionGenerator->>MotionGenerator: score candidates, select best, set worldFrame
MotionGenerator-->>Caller: Result(clip, worldFrame)
end
Caller->>AnimationMerger: detectBackwardFacing(entity)
AnimationMerger-->>Caller: yaw180
Caller->>AnimationMerger: applyMotionClip(clip, worldFrame, yaw180, ...)
AnimationMerger->>AnimationMerger: select refFrame, resolve rests, retarget per canonical role
AnimationMerger-->>Caller: ApplyMotionResult
Caller->>Caller: disable all animation states, enable generated state
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
6 tasks
![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: fd76301929
ℹ️ 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".
| return m.reshape(q.shape[:-1] + (3, 3)) | ||
|
|
||
| def mat_to_6d(m): | ||
| return m[..., :, :2].reshape(m.shape[:-2] + (6,)) |
There was a problem hiding this comment.
When this training script builds the v4 ONNX model, m[..., :, :2].reshape(...) interleaves the first two columns row-wise ([m00,m01,m10,...]) while sixd_to_mat() expects [col0, col1]. For an identity rotation this produces [1,0,0,1,0,0], making the two 3D vectors parallel and the reconstructed matrix degenerate, so the reconstruction/loss/export path trains on corrupted rotations. Transpose the 3x2 slice before flattening.
Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment.
Confirmed and fixed — verified empirically: the old m[..., :, :2].reshape row-major flatten produced [m00,m01,m10,...] (identity → [1,0,0,1,0,0], parallel vectors, roundtrip error 1.0). Added .transpose(-1,-2) before the flatten so it's [col0(3); col1(3)] — roundtrip is now exact (identity/yaw err 0.0, random 1e-7). Good catch; this trained the model on degenerate rotation targets and likely explains much of the model path's poor quality. Retraining + re-uploading the corrected model now.
Owner
Author
There was a problem hiding this comment.
Model retrained on the corrected representation and re-uploaded to HF. Per-action angular velocities now track real CMU motion (walk 2.3 vs 3.2, run 4.8 vs 5.1, sit 1.9 vs 1.9, idle 0.65 vs 0.85 deg/frame) and render as coherent, upright takes with natural arm placement — a clear improvement. Pushed in d679267.
Comment on lines
+309
to
+313
| std::vector<std::vector<Q4>> loc = w; | ||
| for (int f = 0; f < T; ++f) | ||
| for (int j = 0; j < J && j < 22; ++j) | ||
| if (kParent[j] >= 0) | ||
| loc[f][j] = qMul(qConj(w[f][kParent[j]]), w[f][j]); |
There was a problem hiding this comment.
When an existing/custom vocab has no frame: "world" key (the path this function maps to worldFrame=false), the ONNX output is already parent-relative, but this scoring path still derives parent^-1 * child as if w contained world rotations. That makes the best-of-N selector rank local-model candidates using double-composed rotations while the chosen clip is later retargeted as local, so cached v3/local models can consistently pick bad samples; skip this derivation unless worldFrame is true.
Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment.
Fixed — the derived-locals step (parent^-1 * child) is now gated on worldFrame; local-frame (v3/no-"frame" key) models are scored directly on their already-local output instead of double-composing.
Two review findings from PR #792 (both confirmed): - P1 (Codex): mat_to_6d flattened the 3x2 column slice ROW-major, giving [m00,m01,m10,m11,m20,m21] — identity became [1,0,0,1,0,0], two parallel vectors, a degenerate reconstruction (verified roundtrip error 1.0). The model was training on corrupted rotation targets, a major cause of the poor model-path quality. Transpose the slice to [col0(3); col1(3)] before flattening — roundtrip now exact. Model retrained on the fix: per-action angular velocities track real CMU motion (walk 2.3 vs 3.2, sit 1.9 vs 1.9, idle 0.65 vs 0.85 deg/frame) and render as coherent upright takes. - P2 (Codex): the best-of-N candidate scorer derived parent^-1*child unconditionally, double-composing local-frame (v3 / no "frame":"world") model outputs. Gate the derivation on worldFrame so local models are scored on their already-local output. - SonarCloud: annotate the latent-noise mt19937 as // NOSONAR — non-crypto, matching the MeshSegmenter / UniRigPredictor convention. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/MCPServer.cpp (1)
3537-3538: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse
ai.tool_callfor the MCP generation breadcrumb.
toolGenerateMotionis an MCP AI operation, but its breadcrumb category isai.assist.text_to_motion, outside the established set. Categorize this flow withai.tool_callso MCP tool activity remains consistently searchable.Suggested breadcrumb adjustment
- SentryReporter::addBreadcrumb(QStringLiteral("ai.assist.text_to_motion"), + SentryReporter::addBreadcrumb(QStringLiteral("ai.tool_call"), QStringLiteral("MCP generate_motion"));As per coding guidelines, track all user-facing actions and significant operations with
SentryReporter::addBreadcrumb(category, message)using the established categories (ui.action,ai.tool_call,file.import,file.export).Also applies to: 3614-3634
🤖 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 3537 - 3538, The MCP motion generation breadcrumb is using the wrong Sentry category; update the breadcrumb calls in toolGenerateMotion and the related MCP generation flow to use ai.tool_call instead of ai.assist.text_to_motion. Keep the existing message text in SentryReporter::addBreadcrumb(category, message), and make sure the same category is applied consistently across the referenced MCP generation paths so these AI tool operations remain searchable under the established category set.Source: Coding guidelines
src/AnimationControlController.cpp (1)
1679-1680: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse the established Sentry breadcrumb categories.
This GUI generate/apply flow is a user-facing AI operation, but it is currently categorized as
ai.assist.text_to_motion, which is outside the allowed category set. Useui.actionfor the GUI action and/orai.tool_callaround the generated-motion apply step.Suggested breadcrumb adjustment
- SentryReporter::addBreadcrumb(QStringLiteral("ai.assist.text_to_motion"), + SentryReporter::addBreadcrumb(QStringLiteral("ui.action"), QStringLiteral("GUI generate_motion")); ... + SentryReporter::addBreadcrumb(QStringLiteral("ai.tool_call"), + QStringLiteral("GUI apply generated motion clip")); const auto res = AnimationMerger::applyMotionClip(skel.get(), animName, quats, fps,As per coding guidelines, track all user-facing actions and significant operations with
SentryReporter::addBreadcrumb(category, message)using the established categories (ui.action,ai.tool_call,file.import,file.export).Also applies to: 1736-1764
🤖 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 1679 - 1680, The breadcrumb in the GUI generate/apply flow uses an unsupported category, so update the SentryReporter::addBreadcrumb calls in AnimationControlController to use the established breadcrumb categories instead of ai.assist.text_to_motion. For the user-facing generate action, switch to ui.action, and for the generated-motion apply step in the related generate/apply path, use ai.tool_call where appropriate; keep the existing message text but align the category values with the approved set.Source: Coding guidelines
src/CLIPipeline.cpp (1)
1860-1861: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse a standard breadcrumb category for CLI generation.
The CLI text-to-motion flow is still logged under
ai.assist.text_to_motion; the new apply/retarget step should be queryable through the established categories, e.g.ui.actionfor the CLI command and/orai.tool_callfor motion generation/application.As per coding guidelines, track all user-facing actions and significant operations with
SentryReporter::addBreadcrumb(category, message)using the established categories (ui.action,ai.tool_call,file.import,file.export).Also applies to: 1950-1954
🤖 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 1860 - 1861, Update the breadcrumb category used in the CLI text-to-motion and apply/retarget flow so it follows the established Sentry categories instead of ai.assist.text_to_motion. In CLIPipeline’s CLI generation path, adjust the SentryReporter::addBreadcrumb calls to use ui.action for the user-triggered CLI command and ai.tool_call for the motion generation/application step, keeping the breadcrumb message descriptive and queryable. Make the same category change in the other breadcrumb block referenced by this flow so all related user-facing actions are tracked consistently.Source: Coding guidelines
🧹 Nitpick comments (1)
src/AnimationMerger.h (1)
164-183: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUpdate the public retargeting contract docs.
Line 183 adds
yaw180, but theapplyMotionClipdocs do not describe it, and Lines 166-167 still say world-frame retargeting uses frame 0 while the implementation now chooses the calmest reference frame.Suggested doc update
- /// The retarget takes a clean world delta vs frame 0, + /// The retarget takes a clean world delta vs the selected calmest reference frame, /// `dWorld(f) = clip(f)·clip(0)⁻¹`, and transports it into the target ... + /// `yaw180` applies a 180-degree yaw correction for raw-path rigs detected + /// as facing −Z; callers can derive it from detectBackwardFacing(entity).🤖 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.h` around lines 164 - 183, Update the public contract docs on applyMotionClip to describe the new yaw180 parameter and clarify the worldFrame behavior. In the applyMotionClip declaration comments, add a brief note for yaw180 and revise the worldFrame description so it no longer claims the retarget always uses frame 0; instead document that the implementation selects the calmest reference frame for world-frame clips. Keep the wording aligned with the existing applyMotionClip, worldFrame, and yaw180 symbols so the API docs match the current behavior.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@CLAUDE.md`:
- Line 300: The MotionLibrary / text-to-motion documentation is inconsistent
about the v4 shipped library size. Update the count in this section so it
matches the shipped asset summary and the model notes, and keep the action
list/count in sync with the MotionLibrary v4 description and the MotionLibrary /
MotionGenerator references that describe the library contents.
In `@scripts/build-motion-library.py`:
- Around line 163-168: The snap search in the start-selection logic misses the
upper candidate because the slice used in the `np.argmin` window excludes the
last valid start. Update the `build-motion-library.py` start-snap code around
the `a = max(lo, s - 15); b = min(nF - L, s + 15)` block so the window includes
`b` when scanning `vel` before assigning `s`, keeping the `nF - L` candidate
eligible in the calmest-frame selection.
In `@scripts/prep-t2m-v4.py`:
- Line 183: The world lookup in the function containing the j-index check
triggers Ruff E701 because it uses a one-line if statement. Update that
conditional in the relevant loop/helper so the return is placed on its own
indented block, keeping the logic the same while making it Ruff-compliant.
- Line 266: The argparse default for the `--out` option in `prep-t2m-v4.py` is
hard-coded to `/tmp/t2m_v4.npz`, which is non-portable and triggers Ruff. Update
the `ap.add_argument("--out", ...)` definition to use a relative/default path
that works across environments, or make `--out` required and handle the output
path explicitly in the script’s main flow.
- Around line 94-106: Keep the v4 prep vocabulary in sync with the motion
library’s full action set by adding the missing `sweep` and `wash` entries to
both `VOCAB` and `KEYWORDS` in `prep-t2m-v4.py`. Update the `VOCAB` list so it
includes the two new actions, and extend the `KEYWORDS` mapping so descriptions
containing those terms resolve to the correct actions, matching the set curated
by `build-motion-library.py`.
In `@scripts/train-t2m-onnx-v4.py`:
- Line 248: Ruff is flagging one-line branch statements in the training script,
so split the inline if/continue patterns into multi-line blocks to satisfy E701.
Update the conditional branches in the relevant logic around the current len(m_)
check, and also the similar branches near the other reported locations, keeping
the behavior unchanged while making the control flow explicit.
- Around line 125-132: The mat_to_quat helper in train-t2m-onnx-v4.py uses a
trace-only formula that becomes unstable near 180° rotations and can produce
invalid quaternions in t2m.onnx. Replace this conversion with a branch-stable
matrix-to-quaternion implementation that selects the dominant diagonal element
(or equivalent numerically stable branches), and keep the output normalized;
update the logic inside mat_to_quat so it no longer depends on dividing by 4*w
when w is near zero.
In `@src/MotionGenerator.cpp`:
- Around line 285-295: Add a single ONNX inference breadcrumb for the best-of-N
path in MotionGenerator::generate before entering the candidate loop, using
SentryReporter::addBreadcrumb with the ai.tool_call category and a message that
describes the inference operation. Do not emit the breadcrumb inside the
per-candidate loop; keep it once per overall inference call so the breadcrumb
tracks the significant AI operation without duplication.
In `@src/MotionLibrary.cpp`:
- Around line 38-40: The alias/action lookup in MotionLibrary is using
contains(...) for short terms like seat, box, pitch, and sit, which causes false
positives on unrelated prompts. Update the matching logic in the selection path
to prefer token-boundary or whole-token matches before choosing a clip, and
apply the same fix in the related branches around the action dictionaries and
clip-selection checks. Use the existing alias maps and matching code in
MotionLibrary to centralize this boundary-aware matching so short aliases do not
trigger on substrings like Seattle, sandbox, or visit.
In `@THIRD_PARTY_AI_MODELS.md`:
- Around line 127-156: The template-library summary has a mismatched action
count: the opening “Data” bullet still says 13 actions while the hosted library
and clip total indicate 15 actions. Update the count in that bullet in
THIRD_PARTY_AI_MODELS.md so it matches the shipped `motion/motion-library.json`
description and stays consistent with the rest of the section, including the
`qtmesh-motion-library-v3`/v4 wording and the listed actions.
---
Outside diff comments:
In `@src/AnimationControlController.cpp`:
- Around line 1679-1680: The breadcrumb in the GUI generate/apply flow uses an
unsupported category, so update the SentryReporter::addBreadcrumb calls in
AnimationControlController to use the established breadcrumb categories instead
of ai.assist.text_to_motion. For the user-facing generate action, switch to
ui.action, and for the generated-motion apply step in the related generate/apply
path, use ai.tool_call where appropriate; keep the existing message text but
align the category values with the approved set.
In `@src/CLIPipeline.cpp`:
- Around line 1860-1861: Update the breadcrumb category used in the CLI
text-to-motion and apply/retarget flow so it follows the established Sentry
categories instead of ai.assist.text_to_motion. In CLIPipeline’s CLI generation
path, adjust the SentryReporter::addBreadcrumb calls to use ui.action for the
user-triggered CLI command and ai.tool_call for the motion
generation/application step, keeping the breadcrumb message descriptive and
queryable. Make the same category change in the other breadcrumb block
referenced by this flow so all related user-facing actions are tracked
consistently.
In `@src/MCPServer.cpp`:
- Around line 3537-3538: The MCP motion generation breadcrumb is using the wrong
Sentry category; update the breadcrumb calls in toolGenerateMotion and the
related MCP generation flow to use ai.tool_call instead of
ai.assist.text_to_motion. Keep the existing message text in
SentryReporter::addBreadcrumb(category, message), and make sure the same
category is applied consistently across the referenced MCP generation paths so
these AI tool operations remain searchable under the established category set.
---
Nitpick comments:
In `@src/AnimationMerger.h`:
- Around line 164-183: Update the public contract docs on applyMotionClip to
describe the new yaw180 parameter and clarify the worldFrame behavior. In the
applyMotionClip declaration comments, add a brief note for yaw180 and revise the
worldFrame description so it no longer claims the retarget always uses frame 0;
instead document that the implementation selects the calmest reference frame for
world-frame clips. Keep the wording aligned with the existing applyMotionClip,
worldFrame, and yaw180 symbols so the API docs match the current behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6b2d8692-8769-4abd-8644-958f76499a58
📒 Files selected for processing (13)
CLAUDE.mdTHIRD_PARTY_AI_MODELS.mdscripts/build-motion-library.pyscripts/prep-t2m-v4.pyscripts/train-t2m-onnx-v4.pysrc/AnimationControlController.cppsrc/AnimationMerger.cppsrc/AnimationMerger.hsrc/CLIPipeline.cppsrc/MCPServer.cppsrc/MotionGenerator.cppsrc/MotionGenerator.hsrc/MotionLibrary.cpp
| - **ExportOptimizer** (`src/ExportOptimizer.h/cpp`, issue #399): Pipeline that runs `meshopt_optimizeVertexCache` → `meshopt_optimizeOverdraw` (threshold 1.05) → `meshopt_optimizeVertexFetchRemap` on every submesh of an entity. Surfaced through the **Inspector validation flow** — the "Optimize Geometry (cache + overdraw + fetch)" button in `PropertiesPanel.qml` runs it via `MeshValidator::optimizeVertexCache`. NOT hooked into `MeshImporterExporter::exporter` by default (an earlier draft did this and crashed on macOS during a normal export — silent buffer mutation during export is dangerous; explicit user invocation via the validation button is safer). Vertex-fetch is skipped when the submesh uses `useSharedVertices` since remapping shared verts would scramble other submeshes' indices. `qtmesh info --json` includes `submeshAcmr[]` per submesh so downstream tooling can decide whether to recommend re-optimization. Sentry breadcrumb category `ai.assist.optimize_export`. | ||
| - **MotionInbetween** (`src/MotionInbetween.h/cpp`, issue #409): AI animation in-betweening — fills the gap between two sparse keyframes with smooth, plausible intermediate poses. The issue proposes **Robust Motion In-betweening** (Harvey et al., Ubisoft, SIGGRAPH 2020), a small transition transformer; like #404/#408 the ML path runs on ONNX Runtime (`#ifdef ENABLE_ONNX`) and is the **third ONNX consumer**. **The spline fallback is first-class** (per the issue's acceptance criteria): `interpolateSpline` (cubic-Hermite with Catmull-Rom tangents for translation/scale, shortest-arc `slerpQuat` for rotation) is Ogre-free, always compiled, and used automatically whenever the binary lacks ONNX, the model is missing/un-downloadable, the skeleton is incompatible with the model, or the run fails — `Result::usedModel` + `fallbackReason` tell the caller which path ran. The core works on flat per-frame pose arrays (channels = bones × 10 DoF: `[tx,ty,tz, qx,qy,qz,qw, sx,sy,sz]`) with a `Channel` layout (`Scalar`/`QuatStart`/`QuatCont`) so it's unit-tested without Ogre/GL. `MotionInbetween::ensureModelBlocking()` downloads `rmib.onnx` on first use to `AppData/ai_models/inbetween/` (override `QTMESH_INBETWEEN_MODEL_BASE_URL` / `QSettings ai/inbetweenModelBaseUrl`; offline guard `QTMESH_INBETWEEN_NO_DOWNLOAD`) — the #408 self-contained pattern, with the `#ifndef ENABLE_ONNX return {}` guard. `AnimationMerger::inbetweenAnimation(skel, animName, t0, t1, gapFrames, modelPath, forceFallback)` is the Ogre adapter: it packs every bracketing node track's start/end pose into ONE predict() call (so the model sees the full skeleton), scatters the predicted per-frame poses back as keyframes at uniform interior times, and returns an `InbetweenResult` (keyframesInserted / tracksAffected / usedModel / fallbackReason). Surfaced via **CLI `qtmesh anim <file> --in-between --gap-frames N [--start-time S] [--end-time S] [--no-model] [--animation NAME] [-o out]`** (`CLIPipeline::cmdAnim`), the **MCP `motion_in_between` tool** (`MCPServer::toolMotionInBetween`, args `{gap_frames, entity_name?, animation_name?, start_time?, end_time?, no_model?}`, registered heavy), and the **dope sheet "AI in-between … Fill gap" control** (`qml/AnimationDopeSheet.qml` → `AnimationControlController::inbetweenWindow`, shown when the selection spans a time window; emits `inbetweenStatus`). Sentry breadcrumb category `ai.assist.in_between`. **Canonical skeleton + retargeting:** the model is trained on a FIXED 22-joint CMU core-body skeleton (C=220), so `AnimationMerger::inbetweenAnimation` maps the entity's track bones onto those 22 roles via `MotionInbetween::canonicalIndexForBone()` (handles Mixamo `mixamorig:*`, generic `L_Shoulder`, and CMU names; rejects finger/toe/face bones; note Mixamo "Shoulder"=clavicle→collar while "Arm"=upper-arm→the CMU shoulder role). When a strong majority (≥¾) of the 22 roles resolve it packs the canonical pose, runs the model, and scatters predictions back to the matched tracks; unmatched/non-bracketed tracks (and rigs that don't resolve enough roles, and non-ONNX builds) use the per-track spline. **Model: ours, trained from scratch on CMU MoCap** (`scripts/export-rmib-onnx.py`, one-time offline dev tool — NOT shipped) — CMU is permissively licensed (commercial-OK), unlike the field-standard LAFAN1 (CC-BY-NC-ND, rejected). Validated: rotation error < half of slerp on held-out CMU motion. **Hosting:** `rmib.onnx` (~13 MB) is live in the [`fernandotonon/QtMeshEditor-models`](https://huggingface.co/fernandotonon/QtMeshEditor-models) HF repo under `inbetween/`, downloads on first use (see `THIRD_PARTY_AI_MODELS.md`). | ||
| - **MotionLibrary / text-to-motion** (`src/MotionLibrary.h/cpp`, issue #411, experimental): generate a skeletal animation from a text prompt. **The #411 spike (see `docs/TEXT_TO_MOTION_SPIKE_411.md`) proved a from-scratch GENERATIVE model (MDM-style) collapses to a static pose without multi-day ML effort, and all off-the-shelf models (MDM/T2M-GPT/MotionGPT) train on AMASS-derived HumanML3D/KIT-ML = non-commercial (the LAFAN1/ShapeNet wall again).** So the SHIPPED feature is a **template-clip MVP**: a curated library of permissive **CMU MoCap** clips (commercial-OK, same source as #409 RMIB), matched to the prompt by action keyword + synonyms (`MotionLibrary::matchPrompt`), then retargeted onto the user's rig via `AnimationMerger::applyMotionClip` → `MotionInbetween::canonicalIndexForBone` (the SAME 22-joint canonical mapping as #409). `MotionLibrary` is Ogre-free + unit-tested (`MotionLibrary_test.cpp`): parses `qtmesh-motion-library-v1`/`v2` JSON (per-frame, per-joint canonical quats; v2 adds an optional 22-entry `cmuRestWorld` block) + keyword matching. Library downloads on first use to `AppData/ai_models/motion/` (override `QTMESH_MOTION_LIBRARY_BASE_URL` / `QSettings ai/motionLibraryBaseUrl`; offline guard `QTMESH_MOTION_NO_DOWNLOAD`) — built offline by `scripts/build-motion-library.py` (10 actions: walk/run/jump/dance/march/kick/punch/wave/climb/idle, ~0.9 MB), hosted on the [`fernandotonon/QtMeshEditor-models`](https://huggingface.co/fernandotonon/QtMeshEditor-models) HF repo under `motion/`. **Retarget (`applyMotionClip`) — the part that makes it look right:** the CMU clip stores each joint's LOCAL (parent-relative) rotation with rest ≈ identity (the rest DIRECTION is in the BVH bone offsets, captured as the v2 `cmuRestWorld` per-joint world-rest `Wcmu`). The exact per-bone formula is `local(f) = parentWorld⁻¹ · (Wcmu · clip(f) · Wcmu⁻¹) · parentWorld · bind`, where `bind` = the rig's STANDING pose harvested from frame-0 of its existing animation (Mixamo bone rest is identity — the standing pose lives in the anim, NOT in `getInitial{Orientation,Position}` which is inflated and would stretch the mesh), and the root (hip) is locked to the standing pose (CMU bakes whole-body facing into the root). The `Wcmu·clip·Wcmu⁻¹` conjugation is the CMU↔target change-of-basis that cancels the per-bone ROLL twist between rigs with different bone axes (Mixamo arms point down their length / sideways; UniRig is axis-aligned). v1 libraries (no `cmuRestWorld`) fall back to `Wcmu`=identity (parent-world transport only — direction-correct, residual roll). Writes rotation-only keyframes (translation/scale stay at the standing pose, preserving rig proportions) and requires ≥½ of the 22 canonical roles to resolve (else fails — not a humanoid rig). Render-verified on the Rumba (Mixamo) rig via the isometric loop: walk = upright stride with arms hanging+swinging; wave = upright + natural. Surfaced via **CLI `qtmesh anim <file> --generate "<prompt>" [--duration N] [-o out]`** (`CLIPipeline::cmdAnimGenerate`), the **MCP `generate_motion` tool** (`MCPServer::toolGenerateMotion`, args `{prompt, entity_name?, duration?, output_path?}`, registered heavy), and the **Animation panel "Generate from text" control** (`qml/AnimationControlPanel.qml` → `AnimationControlController::generateMotion`, emits `generateMotionStatus`). Sentry breadcrumb `ai.assist.text_to_motion`. NOT an ONNX consumer (the dev `scripts/export-t2m-onnx.py` generative prototype exists but is NOT shipped — kept for a future generative upgrade). **Quality limit:** template clips, not generation — limited to the bundled action vocabulary; non-T-pose source rigs retarget best. | ||
| - **MotionLibrary / text-to-motion** (`src/MotionLibrary.h/cpp`, issue #411, experimental): generate a skeletal animation from a text prompt. **The #411 spike (see `docs/TEXT_TO_MOTION_SPIKE_411.md`) proved a from-scratch GENERATIVE model (MDM-style) collapses to a static pose without multi-day ML effort, and all off-the-shelf models (MDM/T2M-GPT/MotionGPT) train on AMASS-derived HumanML3D/KIT-ML = non-commercial (the LAFAN1/ShapeNet wall again).** So the SHIPPED feature is a **template-clip MVP**: a curated library of permissive **CMU MoCap** clips (commercial-OK, same source as #409 RMIB), matched to the prompt by action keyword + synonyms (`MotionLibrary::matchPrompt`), then retargeted onto the user's rig via `AnimationMerger::applyMotionClip` → `MotionInbetween::canonicalIndexForBone` (the SAME 22-joint canonical mapping as #409). `MotionLibrary` is Ogre-free + unit-tested (`MotionLibrary_test.cpp`): parses `qtmesh-motion-library-v1`/`v2` JSON (per-frame, per-joint canonical quats; v2 adds an optional 22-entry `cmuRestWorld` block) + keyword matching. Library downloads on first use to `AppData/ai_models/motion/` (override `QTMESH_MOTION_LIBRARY_BASE_URL` / `QSettings ai/motionLibraryBaseUrl`; offline guard `QTMESH_MOTION_NO_DOWNLOAD`) — built offline by `scripts/build-motion-library.py` (10 actions: walk/run/jump/dance/march/kick/punch/wave/climb/idle, ~0.9 MB), hosted on the [`fernandotonon/QtMeshEditor-models`](https://huggingface.co/fernandotonon/QtMeshEditor-models) HF repo under `motion/`. **Retarget (`applyMotionClip`) — the part that makes it look right:** the CMU clip stores each joint's LOCAL (parent-relative) rotation with rest ≈ identity (the rest DIRECTION is in the BVH bone offsets, captured as the v2 `cmuRestWorld` per-joint world-rest `Wcmu`). The exact per-bone formula is `local(f) = parentWorld⁻¹ · (Wcmu · clip(f) · Wcmu⁻¹) · parentWorld · bind`, where `bind` = the rig's STANDING pose harvested from frame-0 of its existing animation (Mixamo bone rest is identity — the standing pose lives in the anim, NOT in `getInitial{Orientation,Position}` which is inflated and would stretch the mesh), and the root (hip) is locked to the standing pose (CMU bakes whole-body facing into the root). The `Wcmu·clip·Wcmu⁻¹` conjugation is the CMU↔target change-of-basis that cancels the per-bone ROLL twist between rigs with different bone axes (Mixamo arms point down their length / sideways; UniRig is axis-aligned). v1 libraries (no `cmuRestWorld`) fall back to `Wcmu`=identity (parent-world transport only — direction-correct, residual roll). Writes rotation-only keyframes (translation/scale stay at the standing pose, preserving rig proportions) and requires ≥½ of the 22 canonical roles to resolve (else fails — not a humanoid rig). Render-verified on the Rumba (Mixamo) rig via the isometric loop: walk = upright stride with arms hanging+swinging; wave = upright + natural. Surfaced via **CLI `qtmesh anim <file> --generate "<prompt>" [--duration N] [-o out]`** (`CLIPipeline::cmdAnimGenerate`), the **MCP `generate_motion` tool** (`MCPServer::toolGenerateMotion`, args `{prompt, entity_name?, duration?, output_path?}`, registered heavy), and the **Animation panel "Generate from text" control** (`qml/AnimationControlPanel.qml` → `AnimationControlController::generateMotion`, emits `generateMotionStatus`). Sentry breadcrumb `ai.assist.text_to_motion`. **v4 library** (July 2026): clips are the trial's ACTIVE window (max motion energy, start snapped to a calm near-neutral frame — the retarget deltas against clip frame 0), replacing first-4s slices that mostly captured idle lead-ins; 13 actions (adds sit/throw/boxing; 'idle' now a real wait trial — the old 69_01 source walked; 'dance' is salsa — ballet pirouettes fold under the locked root). **Generative path (opt-in `--model` / `model:true` / GUI checkbox): `MotionGenerator` + `motion/t2m.onnx`** — a CVAE transformer trained from scratch on the same CMU source (`scripts/prep-t2m-v4.py` + `scripts/train-t2m-onnx-v4.py`, offline): 30fps WORLD-frame windows w/ neutral starts (the v3 model trained on raw-120fps 0.33s local-frame windows and folded/flailed at 3.5x real velocity), absolute-pose decoder (no error-accumulating delta-cumsum), per-sample + rotation-space (geodesic) velocity matching, derived-local supervision (parent^-1*child — the exact quantity applyMotionClip renders; world-only losses let spine-chain errors stack into a visible fold), and z=0 latent-dropout supervision (the app infers with seed=zeros; an unsupervised z=0 is out-of-distribution for a low-beta CVAE). The vocab json declares `frame:world` + `fps`, read by `MotionGenerator::generate` → `Result::worldFrame` → applyMotionClip, so model clips ride the same world retarget as v3 template clips. Template library stays the default + automatic fallback. **Quality limit:** the model's z=0 output is smooth/upright but gentler than real clips (conditional-mean effect; the medoid-exemplar alternative is crisper numerically but renders twisted — `--z0-target` flag documents both); the template path is the quality bar. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Align the v4 clip count with the shipped library.
This section still says 13 actions, but the shipped asset summary and third-party model notes describe 47 clips / 15 actions. Please update the count here so the docs stay consistent.
♻️ Proposed fix
- ...; 13 actions ...
+ ...; 15 actions ...📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - **MotionLibrary / text-to-motion** (`src/MotionLibrary.h/cpp`, issue #411, experimental): generate a skeletal animation from a text prompt. **The #411 spike (see `docs/TEXT_TO_MOTION_SPIKE_411.md`) proved a from-scratch GENERATIVE model (MDM-style) collapses to a static pose without multi-day ML effort, and all off-the-shelf models (MDM/T2M-GPT/MotionGPT) train on AMASS-derived HumanML3D/KIT-ML = non-commercial (the LAFAN1/ShapeNet wall again).** So the SHIPPED feature is a **template-clip MVP**: a curated library of permissive **CMU MoCap** clips (commercial-OK, same source as #409 RMIB), matched to the prompt by action keyword + synonyms (`MotionLibrary::matchPrompt`), then retargeted onto the user's rig via `AnimationMerger::applyMotionClip` → `MotionInbetween::canonicalIndexForBone` (the SAME 22-joint canonical mapping as #409). `MotionLibrary` is Ogre-free + unit-tested (`MotionLibrary_test.cpp`): parses `qtmesh-motion-library-v1`/`v2` JSON (per-frame, per-joint canonical quats; v2 adds an optional 22-entry `cmuRestWorld` block) + keyword matching. Library downloads on first use to `AppData/ai_models/motion/` (override `QTMESH_MOTION_LIBRARY_BASE_URL` / `QSettings ai/motionLibraryBaseUrl`; offline guard `QTMESH_MOTION_NO_DOWNLOAD`) — built offline by `scripts/build-motion-library.py` (10 actions: walk/run/jump/dance/march/kick/punch/wave/climb/idle, ~0.9 MB), hosted on the [`fernandotonon/QtMeshEditor-models`](https://huggingface.co/fernandotonon/QtMeshEditor-models) HF repo under `motion/`. **Retarget (`applyMotionClip`) — the part that makes it look right:** the CMU clip stores each joint's LOCAL (parent-relative) rotation with rest ≈ identity (the rest DIRECTION is in the BVH bone offsets, captured as the v2 `cmuRestWorld` per-joint world-rest `Wcmu`). The exact per-bone formula is `local(f) = parentWorld⁻¹ · (Wcmu · clip(f) · Wcmu⁻¹) · parentWorld · bind`, where `bind` = the rig's STANDING pose harvested from frame-0 of its existing animation (Mixamo bone rest is identity — the standing pose lives in the anim, NOT in `getInitial{Orientation,Position}` which is inflated and would stretch the mesh), and the root (hip) is locked to the standing pose (CMU bakes whole-body facing into the root). The `Wcmu·clip·Wcmu⁻¹` conjugation is the CMU↔target change-of-basis that cancels the per-bone ROLL twist between rigs with different bone axes (Mixamo arms point down their length / sideways; UniRig is axis-aligned). v1 libraries (no `cmuRestWorld`) fall back to `Wcmu`=identity (parent-world transport only — direction-correct, residual roll). Writes rotation-only keyframes (translation/scale stay at the standing pose, preserving rig proportions) and requires ≥½ of the 22 canonical roles to resolve (else fails — not a humanoid rig). Render-verified on the Rumba (Mixamo) rig via the isometric loop: walk = upright stride with arms hanging+swinging; wave = upright + natural. Surfaced via **CLI `qtmesh anim <file> --generate "<prompt>" [--duration N] [-o out]`** (`CLIPipeline::cmdAnimGenerate`), the **MCP `generate_motion` tool** (`MCPServer::toolGenerateMotion`, args `{prompt, entity_name?, duration?, output_path?}`, registered heavy), and the **Animation panel "Generate from text" control** (`qml/AnimationControlPanel.qml` → `AnimationControlController::generateMotion`, emits `generateMotionStatus`). Sentry breadcrumb `ai.assist.text_to_motion`. **v4 library** (July 2026): clips are the trial's ACTIVE window (max motion energy, start snapped to a calm near-neutral frame — the retarget deltas against clip frame 0), replacing first-4s slices that mostly captured idle lead-ins; 13 actions (adds sit/throw/boxing; 'idle' now a real wait trial — the old 69_01 source walked; 'dance' is salsa — ballet pirouettes fold under the locked root). **Generative path (opt-in `--model` / `model:true` / GUI checkbox): `MotionGenerator` + `motion/t2m.onnx`** — a CVAE transformer trained from scratch on the same CMU source (`scripts/prep-t2m-v4.py` + `scripts/train-t2m-onnx-v4.py`, offline): 30fps WORLD-frame windows w/ neutral starts (the v3 model trained on raw-120fps 0.33s local-frame windows and folded/flailed at 3.5x real velocity), absolute-pose decoder (no error-accumulating delta-cumsum), per-sample + rotation-space (geodesic) velocity matching, derived-local supervision (parent^-1*child — the exact quantity applyMotionClip renders; world-only losses let spine-chain errors stack into a visible fold), and z=0 latent-dropout supervision (the app infers with seed=zeros; an unsupervised z=0 is out-of-distribution for a low-beta CVAE). The vocab json declares `frame:world` + `fps`, read by `MotionGenerator::generate` → `Result::worldFrame` → applyMotionClip, so model clips ride the same world retarget as v3 template clips. Template library stays the default + automatic fallback. **Quality limit:** the model's z=0 output is smooth/upright but gentler than real clips (conditional-mean effect; the medoid-exemplar alternative is crisper numerically but renders twisted — `--z0-target` flag documents both); the template path is the quality bar. | |
| - **MotionLibrary / text-to-motion** (`src/MotionLibrary.h/cpp`, issue `#411`, experimental): generate a skeletal animation from a text prompt. **The `#411` spike (see `docs/TEXT_TO_MOTION_SPIKE_411.md`) proved a from-scratch GENERATIVE model (MDM-style) collapses to a static pose without multi-day ML effort, and all off-the-shelf models (MDM/T2M-GPT/MotionGPT) train on AMASS-derived HumanML3D/KIT-ML = non-commercial (the LAFAN1/ShapeNet wall again).** So the SHIPPED feature is a **template-clip MVP**: a curated library of permissive **CMU MoCap** clips (commercial-OK, same source as `#409` RMIB), matched to the prompt by action keyword + synonyms (`MotionLibrary::matchPrompt`), then retargeted onto the user's rig via `AnimationMerger::applyMotionClip` → `MotionInbetween::canonicalIndexForBone` (the SAME 22-joint canonical mapping as `#409`). `MotionLibrary` is Ogre-free + unit-tested (`MotionLibrary_test.cpp`): parses `qtmesh-motion-library-v1`/`v2` JSON (per-frame, per-joint canonical quats; v2 adds an optional 22-entry `cmuRestWorld` block) + keyword matching. Library downloads on first use to `AppData/ai_models/motion/` (override `QTMESH_MOTION_LIBRARY_BASE_URL` / `QSettings ai/motionLibraryBaseUrl`; offline guard `QTMESH_MOTION_NO_DOWNLOAD`) — built offline by `scripts/build-motion-library.py` (10 actions: walk/run/jump/dance/march/kick/punch/wave/climb/idle, ~0.9 MB), hosted on the [`fernandotonon/QtMeshEditor-models`](https://huggingface.co/fernandotonon/QtMeshEditor-models) HF repo under `motion/`. **Retarget (`applyMotionClip`) — the part that makes it look right:** the CMU clip stores each joint's LOCAL (parent-relative) rotation with rest ≈ identity (the rest DIRECTION is in the BVH bone offsets, captured as the v2 `cmuRestWorld` per-joint world-rest `Wcmu`). The exact per-bone formula is `local(f) = parentWorld⁻¹ · (Wcmu · clip(f) · Wcmu⁻¹) · parentWorld · bind`, where `bind` = the rig's STANDING pose harvested from frame-0 of its existing animation (Mixamo bone rest is identity — the standing pose lives in the anim, NOT in `getInitial{Orientation,Position}` which is inflated and would stretch the mesh), and the root (hip) is locked to the standing pose (CMU bakes whole-body facing into the root). The `Wcmu·clip·Wcmu⁻¹` conjugation is the CMU↔target change-of-basis that cancels the per-bone ROLL twist between rigs with different bone axes (Mixamo arms point down their length / sideways; UniRig is axis-aligned). v1 libraries (no `cmuRestWorld`) fall back to `Wcmu`=identity (parent-world transport only — direction-correct, residual roll). Writes rotation-only keyframes (translation/scale stay at the standing pose, preserving rig proportions) and requires ≥½ of the 22 canonical roles to resolve (else fails — not a humanoid rig). Render-verified on the Rumba (Mixamo) rig via the isometric loop: walk = upright stride with arms hanging+swinging; wave = upright + natural. Surfaced via **CLI `qtmesh anim <file> --generate "<prompt>" [--duration N] [-o out]`** (`CLIPipeline::cmdAnimGenerate`), the **MCP `generate_motion` tool** (`MCPServer::toolGenerateMotion`, args `{prompt, entity_name?, duration?, output_path?}`, registered heavy), and the **Animation panel "Generate from text" control** (`qml/AnimationControlPanel.qml` → `AnimationControlController::generateMotion`, emits `generateMotionStatus`). Sentry breadcrumb `ai.assist.text_to_motion`. **v4 library** (July 2026): clips are the trial's ACTIVE window (max motion energy, start snapped to a calm near-neutral frame — the retarget deltas against clip frame 0), replacing first-4s slices that mostly captured idle lead-ins; 15 actions (adds sit/throw/boxing; 'idle' now a real wait trial — the old 69_01 source walked; 'dance' is salsa — ballet pirouettes fold under the locked root). **Generative path (opt-in `--model` / `model:true` / GUI checkbox): `MotionGenerator` + `motion/t2m.onnx`** — a CVAE transformer trained from scratch on the same CMU source (`scripts/prep-t2m-v4.py` + `scripts/train-t2m-onnx-v4.py`, offline): 30fps WORLD-frame windows w/ neutral starts (the v3 model trained on raw-120fps 0.33s local-frame windows and folded/flailed at 3.5x real velocity), absolute-pose decoder (no error-accumulating delta-cumsum), per-sample + rotation-space (geodesic) velocity matching, derived-local supervision (parent^-1*child — the exact quantity applyMotionClip renders; world-only losses let spine-chain errors stack into a visible fold), and z=0 latent-dropout supervision (the app infers with seed=zeros; an unsupervised z=0 is out-of-distribution for a low-beta CVAE). The vocab json declares `frame:world` + `fps`, read by `MotionGenerator::generate` → `Result::worldFrame` → applyMotionClip, so model clips ride the same world retarget as v3 template clips. Template library stays the default + automatic fallback. **Quality limit:** the model's z=0 output is smooth/upright but gentler than real clips (conditional-mean effect; the medoid-exemplar alternative is crisper numerically but renders twisted — `--z0-target` flag documents both); the template path is the quality bar. |
🤖 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 300, The MotionLibrary / text-to-motion documentation is
inconsistent about the v4 shipped library size. Update the count in this section
so it matches the shipped asset summary and the model notes, and keep the action
list/count in sync with the MotionLibrary v4 description and the MotionLibrary /
MotionGenerator references that describe the library contents.
Comment on lines
+163
to
+168
| # snap the start to the calmest frame in a ±0.5 s neighbourhood so the | ||
| # clip opens on a settled pose (the delta reference) | ||
| a = max(lo, s - 15); b = min(nF - L, s + 15) | ||
| if b > a: | ||
| s = int(a + np.argmin(vel[a:b])) | ||
| return s |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Include the upper snap candidate.
Line 165 sets b to the last valid start, but vel[a:b] excludes b; when the best window is at nF - L, the snap search cannot keep that selected start and may shift the clip earlier.
Proposed fix
- a = max(lo, s - 15); b = min(nF - L, s + 15)
+ a = max(lo, s - 15); b = min(nF - L, s + 15) + 1
if b > a:
s = int(a + np.argmin(vel[a:b]))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # snap the start to the calmest frame in a ±0.5 s neighbourhood so the | |
| # clip opens on a settled pose (the delta reference) | |
| a = max(lo, s - 15); b = min(nF - L, s + 15) | |
| if b > a: | |
| s = int(a + np.argmin(vel[a:b])) | |
| return s | |
| # snap the start to the calmest frame in a ±0.5 s neighbourhood so the | |
| # clip opens on a settled pose (the delta reference) | |
| a = max(lo, s - 15); b = min(nF - L, s + 15) + 1 | |
| if b > a: | |
| s = int(a + np.argmin(vel[a:b])) | |
| return s |
🤖 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` around lines 163 - 168, The snap search in
the start-selection logic misses the upper candidate because the slice used in
the `np.argmin` window excludes the last valid start. Update the
`build-motion-library.py` start-snap code around the `a = max(lo, s - 15); b =
min(nF - L, s + 15)` block so the window includes `b` when scanning `vel` before
assigning `s`, keeping the `nF - L` candidate eligible in the calmest-frame
selection.
Comment on lines
+94
to
+106
| # v4 vocab — mirrors the template library's action set so both motion sources | ||
| # cover the same prompts. | ||
| VOCAB = ["walk", "run", "jump", "dance", "march", "kick", "punch", "wave", | ||
| "climb", "sit", "throw", "boxing", "idle"] | ||
|
|
||
| # description-keyword → action (single hit required), applied to the index. | ||
| KEYWORDS = { | ||
| "walk": "walk", "run": "run", "jog": "run", "jump": "jump", | ||
| "dance": "dance", "salsa": "dance", "lambada": "dance", | ||
| "march": "march", "kick": "kick", "punch": "punch", "boxing": "boxing", | ||
| "wave": "wave", "climb": "climb", "sit": "sit", "throw": "throw", | ||
| "wait": "idle", | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep the v4 training vocab aligned with the template library.
VOCAB/KEYWORDS omit sweep and wash, but scripts/build-motion-library.py now curates those actions, so the v4 cache/model cannot learn or emit the full 15-action set described by this PR.
Proposed fix
VOCAB = ["walk", "run", "jump", "dance", "march", "kick", "punch", "wave",
- "climb", "sit", "throw", "boxing", "idle"]
+ "climb", "sit", "throw", "boxing", "sweep", "wash", "idle"]
@@
"march": "march", "kick": "kick", "punch": "punch", "boxing": "boxing",
- "wave": "wave", "climb": "climb", "sit": "sit", "throw": "throw",
+ "wave": "wave", "climb": "climb", "sit": "sit", "throw": "throw",
+ "sweep": "sweep", "wash": "wash",
"wait": "idle",
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # v4 vocab — mirrors the template library's action set so both motion sources | |
| # cover the same prompts. | |
| VOCAB = ["walk", "run", "jump", "dance", "march", "kick", "punch", "wave", | |
| "climb", "sit", "throw", "boxing", "idle"] | |
| # description-keyword → action (single hit required), applied to the index. | |
| KEYWORDS = { | |
| "walk": "walk", "run": "run", "jog": "run", "jump": "jump", | |
| "dance": "dance", "salsa": "dance", "lambada": "dance", | |
| "march": "march", "kick": "kick", "punch": "punch", "boxing": "boxing", | |
| "wave": "wave", "climb": "climb", "sit": "sit", "throw": "throw", | |
| "wait": "idle", | |
| } | |
| # v4 vocab — mirrors the template library's action set so both motion sources | |
| # cover the same prompts. | |
| VOCAB = ["walk", "run", "jump", "dance", "march", "kick", "punch", "wave", | |
| "climb", "sit", "throw", "boxing", "sweep", "wash", "idle"] | |
| # description-keyword → action (single hit required), applied to the index. | |
| KEYWORDS = { | |
| "walk": "walk", "run": "run", "jog": "run", "jump": "jump", | |
| "dance": "dance", "salsa": "dance", "lambada": "dance", | |
| "march": "march", "kick": "kick", "punch": "punch", "boxing": "boxing", | |
| "wave": "wave", "climb": "climb", "sit": "sit", "throw": "throw", | |
| "sweep": "sweep", "wash": "wash", | |
| "wait": "idle", | |
| } |
🤖 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-v4.py` around lines 94 - 106, Keep the v4 prep vocabulary in
sync with the motion library’s full action set by adding the missing `sweep` and
`wash` entries to both `VOCAB` and `KEYWORDS` in `prep-t2m-v4.py`. Update the
`VOCAB` list so it includes the two new actions, and extend the `KEYWORDS`
mapping so descriptions containing those terms resolve to the correct actions,
matching the set curated by `build-motion-library.py`.
| return o | ||
| world = {} | ||
| def world_of(j): | ||
| if j in world: return world[j] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Fix the Ruff E701 lint error.
Line 183 uses a one-line if body; Ruff reports this as an error.
Proposed fix
- if j in world: return world[j]
+ if j in world:
+ return world[j]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if j in world: return world[j] | |
| if j in world: | |
| return world[j] |
🧰 Tools
🪛 Ruff (0.15.20)
[error] 183-183: Multiple statements on one line (colon)
(E701)
🤖 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-v4.py` at line 183, The world lookup in the function
containing the j-index check triggers Ruff E701 because it uses a one-line if
statement. Update that conditional in the relevant loop/helper so the return is
placed on its own indented block, keeping the logic the same while making it
Ruff-compliant.
Source: Linters/SAST tools
| ap = argparse.ArgumentParser() | ||
| ap.add_argument("--bvh", required=True) | ||
| ap.add_argument("--index", required=True) | ||
| ap.add_argument("--out", default="/tmp/t2m_v4.npz") |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Avoid a hard-coded /tmp output default.
Ruff flags this default, and it is also non-portable. Prefer a relative default or require --out.
Proposed fix
- ap.add_argument("--out", default="/tmp/t2m_v4.npz")
+ ap.add_argument("--out", default="t2m_v4.npz")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ap.add_argument("--out", default="/tmp/t2m_v4.npz") | |
| ap.add_argument("--out", default="t2m_v4.npz") |
🧰 Tools
🪛 Ruff (0.15.20)
[error] 266-266: Probable insecure usage of temporary file or directory: "/tmp/t2m_v4.npz"
(S108)
🤖 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-v4.py` at line 266, The argparse default for the `--out`
option in `prep-t2m-v4.py` is hard-coded to `/tmp/t2m_v4.npz`, which is
non-portable and triggers Ruff. Update the `ap.add_argument("--out", ...)`
definition to use a relative/default path that works across environments, or
make `--out` required and handle the output path explicitly in the script’s main
flow.
Source: Linters/SAST tools
Comment on lines
+125
to
+132
| def mat_to_quat(m): | ||
| t = m[..., 0, 0] + m[..., 1, 1] + m[..., 2, 2] | ||
| w = torch.sqrt(torch.clamp(1 + t, min=1e-8)) / 2 | ||
| w4 = (4 * w).clamp_min(1e-8) | ||
| x = (m[..., 2, 1] - m[..., 1, 2]) / w4 | ||
| y = (m[..., 0, 2] - m[..., 2, 0]) / w4 | ||
| z = (m[..., 1, 0] - m[..., 0, 1]) / w4 | ||
| return torch.stack([x, y, z, w], -1) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use a branch-stable matrix-to-quaternion conversion.
The current trace-only path clamps w near zero for ~180° rotations, then divides by 4*w; that can export wrong or non-unit quaternions into t2m.onnx.
Proposed fix
def mat_to_quat(m):
- t = m[..., 0, 0] + m[..., 1, 1] + m[..., 2, 2]
- w = torch.sqrt(torch.clamp(1 + t, min=1e-8)) / 2
- w4 = (4 * w).clamp_min(1e-8)
- x = (m[..., 2, 1] - m[..., 1, 2]) / w4
- y = (m[..., 0, 2] - m[..., 2, 0]) / w4
- z = (m[..., 1, 0] - m[..., 0, 1]) / w4
- return torch.stack([x, y, z, w], -1)
+ m00, m11, m22 = m[..., 0, 0], m[..., 1, 1], m[..., 2, 2]
+ qw = torch.sqrt(torch.clamp(1.0 + m00 + m11 + m22, min=0.0)) * 0.5
+ qx = torch.sqrt(torch.clamp(1.0 + m00 - m11 - m22, min=0.0)) * 0.5
+ qy = torch.sqrt(torch.clamp(1.0 - m00 + m11 - m22, min=0.0)) * 0.5
+ qz = torch.sqrt(torch.clamp(1.0 - m00 - m11 + m22, min=0.0)) * 0.5
+ qx = torch.where((m[..., 2, 1] - m[..., 1, 2]) < 0, -qx, qx)
+ qy = torch.where((m[..., 0, 2] - m[..., 2, 0]) < 0, -qy, qy)
+ qz = torch.where((m[..., 1, 0] - m[..., 0, 1]) < 0, -qz, qz)
+ return F.normalize(torch.stack([qx, qy, qz, qw], -1), dim=-1, eps=1e-8)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def mat_to_quat(m): | |
| t = m[..., 0, 0] + m[..., 1, 1] + m[..., 2, 2] | |
| w = torch.sqrt(torch.clamp(1 + t, min=1e-8)) / 2 | |
| w4 = (4 * w).clamp_min(1e-8) | |
| x = (m[..., 2, 1] - m[..., 1, 2]) / w4 | |
| y = (m[..., 0, 2] - m[..., 2, 0]) / w4 | |
| z = (m[..., 1, 0] - m[..., 0, 1]) / w4 | |
| return torch.stack([x, y, z, w], -1) | |
| def mat_to_quat(m): | |
| m00, m11, m22 = m[..., 0, 0], m[..., 1, 1], m[..., 2, 2] | |
| qw = torch.sqrt(torch.clamp(1.0 + m00 + m11 + m22, min=0.0)) * 0.5 | |
| qx = torch.sqrt(torch.clamp(1.0 + m00 - m11 - m22, min=0.0)) * 0.5 | |
| qy = torch.sqrt(torch.clamp(1.0 - m00 + m11 - m22, min=0.0)) * 0.5 | |
| qz = torch.sqrt(torch.clamp(1.0 - m00 - m11 + m22, min=0.0)) * 0.5 | |
| qx = torch.where((m[..., 2, 1] - m[..., 1, 2]) < 0, -qx, qx) | |
| qy = torch.where((m[..., 0, 2] - m[..., 2, 0]) < 0, -qy, qy) | |
| qz = torch.where((m[..., 1, 0] - m[..., 0, 1]) < 0, -qz, qz) | |
| return F.normalize(torch.stack([qx, qy, qz, qw], -1), dim=-1, eps=1e-8) |
🤖 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-v4.py` around lines 125 - 132, The mat_to_quat helper
in train-t2m-onnx-v4.py uses a trace-only formula that becomes unstable near
180° rotations and can produce invalid quaternions in t2m.onnx. Replace this
conversion with a branch-stable matrix-to-quaternion implementation that selects
the dominant diagonal element (or equivalent numerically stable branches), and
keep the output normalized; update the logic inside mat_to_quat so it no longer
depends on dividing by 4*w when w is near zero.
| medoid6 = torch.zeros(V, T, D6) | ||
| for aI in range(V): | ||
| m_ = (act_idx == aI).nonzero(as_tuple=True)[0] | ||
| if not len(m_): continue |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Split Ruff E701 one-line branches.
These lines are reported as Ruff errors and can fail linting unless E701 is explicitly ignored.
Proposed fix
- if not len(m_): continue
+ if not len(m_):
+ continue
...
- if word in VOCAB: t[0, VOCAB.index(word)] = 1.0
+ if word in VOCAB:
+ t[0, VOCAB.index(word)] = 1.0
...
- if not m_.any(): continue
+ if not m_.any():
+ continueAlso applies to: 330-330, 340-340
🧰 Tools
🪛 Ruff (0.15.20)
[error] 248-248: Multiple statements on one line (colon)
(E701)
🤖 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-v4.py` at line 248, Ruff is flagging one-line branch
statements in the training script, so split the inline if/continue patterns into
multi-line blocks to satisfy E701. Update the conditional branches in the
relevant logic around the current len(m_) check, and also the similar branches
near the other reported locations, keeping the behavior unchanged while making
the control flow explicit.
Source: Linters/SAST tools
Comment on lines
+285
to
+295
| for (int cand = 0; cand < kCandidates; ++cand) { | ||
| std::vector<float> seed(static_cast<size_t>(Z), 0.0f); | ||
| for (float& v : seed) v = gauss(rng); | ||
| std::vector<Ort::Value> inputs; | ||
| inputs.push_back(Ort::Value::CreateTensor<float>( | ||
| memInfo, tokens.data(), tokens.size(), tShape.data(), tShape.size())); | ||
| inputs.push_back(Ort::Value::CreateTensor<float>( | ||
| memInfo, seed.data(), seed.size(), sShape.data(), sShape.size())); | ||
| auto out = session.Run(Ort::RunOptions{nullptr}, inNames.data(), | ||
| inputs.data(), inputs.size(), | ||
| outNames.data(), 1); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add one breadcrumb for ONNX inference.
The best-of-N candidate loop is a significant AI operation; emit a single ai.tool_call breadcrumb before the loop, not once per candidate. As per coding guidelines, “Track all user-facing actions and significant operations with SentryReporter::addBreadcrumb(category, message) using the established categories.”
Proposed fix
std::vector<std::vector<Q4>> best; // [T][J]
float bestScore = -1e30f;
+ SentryReporter::addBreadcrumb(
+ QStringLiteral("ai.tool_call"),
+ QStringLiteral("Text-to-motion ONNX best-of-%1 inference for %2")
+ .arg(kCandidates).arg(matched));
for (int cand = 0; cand < kCandidates; ++cand) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| for (int cand = 0; cand < kCandidates; ++cand) { | |
| std::vector<float> seed(static_cast<size_t>(Z), 0.0f); | |
| for (float& v : seed) v = gauss(rng); | |
| std::vector<Ort::Value> inputs; | |
| inputs.push_back(Ort::Value::CreateTensor<float>( | |
| memInfo, tokens.data(), tokens.size(), tShape.data(), tShape.size())); | |
| inputs.push_back(Ort::Value::CreateTensor<float>( | |
| memInfo, seed.data(), seed.size(), sShape.data(), sShape.size())); | |
| auto out = session.Run(Ort::RunOptions{nullptr}, inNames.data(), | |
| inputs.data(), inputs.size(), | |
| outNames.data(), 1); | |
| for (int cand = 0; cand < kCandidates; ++cand) { | |
| std::vector<float> seed(static_cast<size_t>(Z), 0.0f); | |
| for (float& v : seed) v = gauss(rng); | |
| std::vector<Ort::Value> inputs; | |
| inputs.push_back(Ort::Value::CreateTensor<float>( | |
| memInfo, tokens.data(), tokens.size(), tShape.data(), tShape.size())); | |
| inputs.push_back(Ort::Value::CreateTensor<float>( | |
| memInfo, seed.data(), seed.size(), sShape.data(), sShape.size())); | |
| auto out = session.Run(Ort::RunOptions{nullptr}, inNames.data(), | |
| inputs.data(), inputs.size(), | |
| outNames.data(), 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/MotionGenerator.cpp` around lines 285 - 295, Add a single ONNX inference
breadcrumb for the best-of-N path in MotionGenerator::generate before entering
the candidate loop, using SentryReporter::addBreadcrumb with the ai.tool_call
category and a message that describes the inference operation. Do not emit the
breadcrumb inside the per-candidate loop; keep it once per overall inference
call so the breadcrumb tracks the significant AI operation without duplication.
Source: Coding guidelines
Comment on lines
+38
to
+40
| {"sitting", "sit"}, {"seat", "sit"}, | ||
| {"throwing", "throw"}, {"toss", "throw"}, {"pitch", "throw"}, | ||
| {"box", "boxing"}, {"fight", "boxing"}, {"spar", "boxing"}, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Avoid substring-only matching for short aliases.
seat, box, pitch, and short action names like sit can match unrelated prompts such as “Seattle”, “sandbox”, or “visit” because both branches use contains(...). Match aliases/actions on token boundaries before selecting a clip.
Also applies to: 162-166, 170-175
🤖 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 38 - 40, The alias/action lookup in
MotionLibrary is using contains(...) for short terms like seat, box, pitch, and
sit, which causes false positives on unrelated prompts. Update the matching
logic in the selection path to prefer token-boundary or whole-token matches
before choosing a clip, and apply the same fix in the related branches around
the action dictionaries and clip-selection checks. Use the existing alias maps
and matching code in MotionLibrary to centralize this boundary-aware matching so
short aliases do not trigger on substrings like Seattle, sandbox, or visit.
Comment on lines
+127
to
+156
| - **Data:** a curated set of clips from the **CMU Graphics Lab Motion Capture | ||
| Database** (mocap.cs.cmu.edu) — **permissively licensed, commercial-OK** (the same | ||
| source as the #409 RMIB model). Built offline by `scripts/build-motion-library.py` | ||
| into a `qtmesh-motion-library-v1` JSON (per-frame, per-joint canonical-quaternion | ||
| poses on the 22-joint CMU core-body skeleton) — no model weights, no AMASS. | ||
| - **Hosting:** `motion/motion-library.json` (~0.9 MB, 10 actions) in the | ||
| into a `qtmesh-motion-library-v3` JSON (per-frame, per-joint canonical WORLD-frame | ||
| quaternions on the 22-joint CMU core-body skeleton) — no model weights, no AMASS. | ||
| The v4 library build selects each clip's **active window** (max motion energy, | ||
| snapped to a calm near-neutral start frame — the retarget deltas against clip | ||
| frame 0), replacing the first-4-seconds slices that mostly captured idle | ||
| lead-ins, and covers **13 actions** (walk, run, jump, dance, march, kick, | ||
| punch, wave, climb, sit, throw, boxing, idle, sweep, wash) with SEVERAL | ||
| takes per action — the matcher picks among them at random so repeat | ||
| generates vary while every result is real mocap. | ||
| - **Hosting:** `motion/motion-library.json` (~4 MB, 47 clips / 15 actions) in the | ||
| [`fernandotonon/QtMeshEditor-models`](https://huggingface.co/fernandotonon/QtMeshEditor-models) | ||
| HF repo, downloaded on first use to `AppData/ai_models/motion/` (override | ||
| `QTMESH_MOTION_LIBRARY_BASE_URL` / `QSettings ai/motionLibraryBaseUrl`; offline | ||
| guard `QTMESH_MOTION_NO_DOWNLOAD`). | ||
| - **Retargeting:** `AnimationMerger::applyMotionClip` maps the canonical clip onto | ||
| the user's rig via `MotionInbetween::canonicalIndexForBone` (shared with #409). | ||
| - **Generative path:** postponed; the dev prototype `scripts/export-t2m-onnx.py` | ||
| is kept for a future upgrade but is NOT shipped (no model file). | ||
| - **Generative path (opt-in, experimental) — trained by us.** `motion/t2m.onnx` | ||
| (+ `t2m-vocab.json`), a CVAE transformer trained **from scratch on the same | ||
| CMU source** by `scripts/prep-t2m-v4.py` + `scripts/train-t2m-onnx-v4.py` | ||
| (offline dev tools, not shipped): 30 fps world-frame windows with neutral | ||
| starts, absolute-pose decoder, per-sample + rotation-space velocity matching, | ||
| derived-local supervision (what the retarget renders), and z=0 latent | ||
| supervision (the app's inference condition). The vocab json declares | ||
| `"frame":"world"` so model clips ride the same retarget path as the template | ||
| library. Selected via `--model` (CLI) / `model:true` (MCP) / the GUI | ||
| checkbox; the template library remains the default and the automatic | ||
| fallback. Same CMU licensing basis as above. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the template-library action count.
The bullet lists 15 actions, but the lead-in still says 13. Please update the count so this section matches the shipped 47-clip / 15-action library.
♻️ Proposed fix
- covers **13 actions** ...
+ covers **15 actions** ...📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - **Data:** a curated set of clips from the **CMU Graphics Lab Motion Capture | |
| Database** (mocap.cs.cmu.edu) — **permissively licensed, commercial-OK** (the same | |
| source as the #409 RMIB model). Built offline by `scripts/build-motion-library.py` | |
| into a `qtmesh-motion-library-v1` JSON (per-frame, per-joint canonical-quaternion | |
| poses on the 22-joint CMU core-body skeleton) — no model weights, no AMASS. | |
| - **Hosting:** `motion/motion-library.json` (~0.9 MB, 10 actions) in the | |
| into a `qtmesh-motion-library-v3` JSON (per-frame, per-joint canonical WORLD-frame | |
| quaternions on the 22-joint CMU core-body skeleton) — no model weights, no AMASS. | |
| The v4 library build selects each clip's **active window** (max motion energy, | |
| snapped to a calm near-neutral start frame — the retarget deltas against clip | |
| frame 0), replacing the first-4-seconds slices that mostly captured idle | |
| lead-ins, and covers **13 actions** (walk, run, jump, dance, march, kick, | |
| punch, wave, climb, sit, throw, boxing, idle, sweep, wash) with SEVERAL | |
| takes per action — the matcher picks among them at random so repeat | |
| generates vary while every result is real mocap. | |
| - **Hosting:** `motion/motion-library.json` (~4 MB, 47 clips / 15 actions) in the | |
| [`fernandotonon/QtMeshEditor-models`](https://huggingface.co/fernandotonon/QtMeshEditor-models) | |
| HF repo, downloaded on first use to `AppData/ai_models/motion/` (override | |
| `QTMESH_MOTION_LIBRARY_BASE_URL` / `QSettings ai/motionLibraryBaseUrl`; offline | |
| guard `QTMESH_MOTION_NO_DOWNLOAD`). | |
| - **Retargeting:** `AnimationMerger::applyMotionClip` maps the canonical clip onto | |
| the user's rig via `MotionInbetween::canonicalIndexForBone` (shared with #409). | |
| - **Generative path:** postponed; the dev prototype `scripts/export-t2m-onnx.py` | |
| is kept for a future upgrade but is NOT shipped (no model file). | |
| - **Generative path (opt-in, experimental) — trained by us.** `motion/t2m.onnx` | |
| (+ `t2m-vocab.json`), a CVAE transformer trained **from scratch on the same | |
| CMU source** by `scripts/prep-t2m-v4.py` + `scripts/train-t2m-onnx-v4.py` | |
| (offline dev tools, not shipped): 30 fps world-frame windows with neutral | |
| starts, absolute-pose decoder, per-sample + rotation-space velocity matching, | |
| derived-local supervision (what the retarget renders), and z=0 latent | |
| supervision (the app's inference condition). The vocab json declares | |
| `"frame":"world"` so model clips ride the same retarget path as the template | |
| library. Selected via `--model` (CLI) / `model:true` (MCP) / the GUI | |
| checkbox; the template library remains the default and the automatic | |
| fallback. Same CMU licensing basis as above. | |
| - **Data:** a curated set of clips from the **CMU Graphics Lab Motion Capture | |
| Database** (mocap.cs.cmu.edu) — **permissively licensed, commercial-OK** (the same | |
| source as the `#409` RMIB model). Built offline by `scripts/build-motion-library.py` | |
| into a `qtmesh-motion-library-v3` JSON (per-frame, per-joint canonical WORLD-frame | |
| quaternions on the 22-joint CMU core-body skeleton) — no model weights, no AMASS. | |
| The v4 library build selects each clip's **active window** (max motion energy, | |
| snapped to a calm near-neutral start frame — the retarget deltas against clip | |
| frame 0), replacing the first-4-seconds slices that mostly captured idle | |
| lead-ins, and covers **15 actions** (walk, run, jump, dance, march, kick, | |
| punch, wave, climb, sit, throw, boxing, idle, sweep, wash) with SEVERAL | |
| takes per action — the matcher picks among them at random so repeat | |
| generates vary while every result is real mocap. |
🤖 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 `@THIRD_PARTY_AI_MODELS.md` around lines 127 - 156, The template-library
summary has a mismatched action count: the opening “Data” bullet still says 13
actions while the hosted library and clip total indicate 15 actions. Update the
count in that bullet in THIRD_PARTY_AI_MODELS.md so it matches the shipped
`motion/motion-library.json` description and stays consistent with the rest of
the section, including the `qtmesh-motion-library-v3`/v4 wording and the listed
actions.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
fernandotonon
marked this pull request as draft
July 2, 2026 22:26
fernandotonon
marked this pull request as ready for review
July 2, 2026 22:26
|
6 tasks
This was referenced Jul 11, 2026
Merged
This was referenced Jul 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Text-to-motion (#411) produced unusable results on both paths — this PR fixes the full pipeline. The template path is now solid (real-mocap quality, 47 clips / 15 actions, per-generate variety); the trained-model path is much improved (coherent, upright, correct speeds) but stays honest-experimental behind its opt-in flag. New library + model are already live on the HF models repo.
Template library (default path)
matchPromptpicks randomly among same-action takes, so repeat generates vary while every result is real mocap.Retarget fixes (both paths, all rigs)
QTMESH_T2M_YAW180overrides.generated_*clips are never harvested and the change-of-basis is gated on identity rests.Trained model path (opt-in, experimental)
Verification
Every fix was verified in the live viewport via the MCP-driven app (screen captures), plus CLI renders per action. Docs updated (
THIRD_PARTY_AI_MODELS.md,CLAUDE.md). Training scripts (prep-t2m-v4.py,train-t2m-onnx-v4.py) are offline dev tools, not shipped.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes