Skip to content

t2m: composed clips need a PER-FRAME refRoll baseline, not one constant #1033

Description

@fernandotonon

Follow-up from the review of #1032 (which fixed #1023).

The gap

AnimationMerger::applyMotionClip takes clipRefRoll as one per-role constant and adds it to every frame's twist:

for (int c = 1; c < Jc && c < 22; ++c) {
    const float base = clipRefRoll[c];
    for (int f = 0; f < frames; ++f)
        twistTheta[f][c] += base;
}

That is correct for a single library take, which has exactly one reference frame. It is wrong for a composed clip, whose frames come from several takes with different reference frames.

Measured on the shipped motion-library-v2.json (122 clips, all carrying refRoll):

pair disagreement
max between any two clips 3.94 rad (226°)
run vs punch 2.13 rad (122°)
walk vs wave 1.01 rad (58°)
walk vs sit 0.56 rad (32°)

"walk then punch" is a prompt MotionComposer really produces, so a 122° wrong baseline over the punch segment is reachable today.

Current state (after #1032)

MotionComposer::compose now emits no refRoll for a multi-take stitch — empty selects the pre-#954 legacy path. That removes the known-wrong constant but also gives up the #954 improvement on every composed clip. Single-take compositions (one action, possibly repeated) keep their baseline, since they have one reference frame.

This mirrors the existing gate on fingers.

What the real fix needs

  1. Carry a per-frame roll baseline out of the composer — each Piece already knows its source take, so the baseline can be emitted alongside quats and follow the same cut/blend/trim as every other per-frame channel (rootY is the precedent).
  2. Extend applyMotionClip to accept it: either a new std::vector<std::vector<float>> parameter, or promote clipRefRoll to per-frame and keep a one-row form for the single-take case.
  3. Blend the baseline across each seam. A step change at a junction would produce exactly the visible roll snap the crossfade exists to avoid — the baseline must ramp with the pose blend.
  4. Thread it through all three surfaces (CLI, AnimationControlController, MCPServer), which t2m: GUI 'Generate from text' does not pass refRoll to applyMotionClip (CLI does) #1023 has just aligned.

Why it wasn't done in #1032

That PR is a one-line-per-surface argument fix with a measured before/after. A new retarget parameter plus seam-blended roll is a different change with its own regression surface — and roll is precisely the channel that produced the twisted-arm regressions in #954/#838. It deserves its own before/after on both a Mixamo and a UniRig-style rig.

Verification bar

  • "walk then punch" on a Mixamo rig and a UniRig-style rig (Goblin_rigged.glb), before/after render.
  • No roll discontinuity at the seam frame (Composition::seamFrames marks them).
  • A single-take composition must stay bit-identical to today's output.
  • Pin the source clip with variant_index when measuring — "punch" resolves non-deterministically among three takes, and an unpinned comparison measures clip choice rather than the change.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions