Skip to content

[codex] Support inherited keyframes and clarify mixer controller naming - #142

Merged
meekmachine merged 3 commits into
mainfrom
codex/136-inherited-keyframes
May 27, 2026
Merged

[codex] Support inherited keyframes and clarify mixer controller naming#142
meekmachine merged 3 commits into
mainfrom
codex/136-inherited-keyframes

Conversation

@meekmachine

@meekmachine meekmachine commented May 6, 2026

Copy link
Copy Markdown
Owner

Summary

  • Adds support for snippet curves whose first keyframe uses inherit: true, treating the first point as a live-value anchor instead of an authored reset.
  • Captures current morph, AU, viseme, bone rotation, and bone translation values through the Loom3 host so inherited starts work across direct morph tracks, AU-driven morphs, viseme jaw tracks, composite rotations, and translation bindings.
  • Rebuilds inherited dynamic clips at playback/replay time so Firestore-loaded snippets transition from the character's current pose, not stale clip-construction values.
  • Renames the mixed mixer owner from BakedAnimationController to AnimationController, updates Loom3 internals/tests/docs to stop implying procedural snippets are baked animations, and keeps deprecated aliases for deep-import compatibility.
  • Renames derived baked "runtime clips" to "mixer clips" so the code reflects Three.js AnimationMixer ownership instead of implying a parallel Loom3 animation runtime.

Re-evaluation Notes

Latest origin/main now includes the camera helper exports LoomLarge imports from published @lovelace_lol/loom3@1.0.47; this branch was rebased onto current origin/main on May 14, 2026 so linked LoomLarge installs no longer lose those exports.

CurvePoint.inherit was still only a type/documentation hint before this PR, and the mixer path sampled first keyframes as authored absolute values. This PR keeps the existing snippet schema and adds live target readers plus inherited-source metadata so playback resolves the first keyframe against current character state.

The controller rename is intentionally a naming/ownership correction, not a behavioral rewrite: Loom3 still uses Three.js AnimationMixer for playback. The old BakedAnimationController and BakedAnimationHost names remain as deprecated aliases to avoid breaking consumers that deep-imported them.

Linked Work

Validation

  • npm run typecheck
  • npm test -- --run src/engines/three/AnimationThree.inheritedKeyframes.test.ts src/engines/three/AnimationThree.playbackState.test.ts src/engines/three/bakedClipPartitioning.test.ts
  • npm test -- --run
  • npm run build
  • git diff --check

@meekmachine
meekmachine force-pushed the codex/136-inherited-keyframes branch from ad63126 to ebdf44d Compare May 14, 2026 22:20
@meekmachine
meekmachine marked this pull request as ready for review May 26, 2026 02:36

Copy link
Copy Markdown
Owner Author

Coordinator queue packet:

Status checked this run:

  • Non-draft, open PR targeting main.
  • Mergeability: MERGEABLE / CLEAN.
  • Checks: green via gh pr checks (Build, Typecheck, Test; old matrix placeholder skipped).
  • Review threads: none unresolved.
  • Scope: 5 files, +600/-64, touching inherited first-keyframe runtime behavior plus README/types/tests.

Coordinator classification: needs-review. It is mechanically mergeable, but because this changes Loom3 runtime semantics and docs, it should get an explicit Loom architecture/release-boundary review before being marked ready-for-human-merge. I did not merge the PR.

@meekmachine

Copy link
Copy Markdown
Owner Author

Gate Verdict: needs changes

Findings:

  • [P1] src/engines/three/AnimationThree.ts:2177 - Replaying an inherited ClipHandle resolves handle.finished immediately. The new inherited replay path calls cleanupClipMonitor(actionId) inside handle.play(), and cleanupClipMonitor unconditionally calls monitor.resolveFinished(). Because the returned finishedPromise is created once for the handle and is not replaced, callers awaiting handle.finished see replay start as completion instead of waiting for the replayed action to finish. Non-inherited clips keep the monitor alive when handle.play() restarts, so this is a regression specific to the dynamic inherited path.
  • [P2] src/engines/three/AnimationThree.ts:2433 - Inherited morph tracks read one host-level current morph value for every mesh track. addTrackForMesh() loops each target mesh, but getCurrentMorphValue(morphKey, meshNames) and getCurrentMorphIndexValue(morphIndex, meshNames) resolve to the first matching target in Loom3. If the same morph name/index exists on multiple routed meshes with different live values, later tracks are baked from the first mesh's current value and jump at t=0 instead of inheriting their own live value.

Fix Plan:

  • Split monitor cleanup/rebind so internal inherited replay can detach the old monitor without resolving the handle's finished promise, then add a regression that handle.finished remains pending after handle.play() and resolves only after the replay reaches completion.
  • For morph-name and morph-index inherited tracks, seed the first value from the mesh-local infl[morphIndex] being tracked, or pass a single-mesh read target through the host API. Add two-mesh regression coverage for both morph-name and morph-index tracks.

Checks Run:

  • Reviewed PR metadata, changed files, full diff, and existing PR discussion; no existing comments/reviews were present.
  • Read Loom architect review references, plus Loom3 README.md, VISION_AND_PRD.md, RECENT_CHANGES.md, and surrounding AnimationThree.ts / Loom3.ts runtime code.
  • npm ci (also ran package prepare build)
  • npm run typecheck
  • npm test -- --run src/engines/three/AnimationThree.inheritedKeyframes.test.ts
  • npm test
  • npm run build
  • GitHub checks for Build, Typecheck, and Test are passing on the PR.
  • Rendered UI inspection not applicable; this is a Loom3 runtime/package change.

@meekmachine

Copy link
Copy Markdown
Owner Author

Architecture/release-boundary gate re-run against ebdf44d.

Classification: needs-review/follow-up.
Gate Verdict: needs changes.

Confirmed the existing runtime blockers in #142 (comment):

  • P1: inherited ClipHandle replay resolves handle.finished at replay start.
  • P2: multi-mesh inherited morph tracks seed from the first matching mesh value.

No additional architecture/release-boundary blockers found. PR is non-draft, MERGEABLE/CLEAN, Build/Typecheck/Test are green, and reviewThreads is empty.

Copy link
Copy Markdown
Owner Author

Addressed the two architecture-gate findings in 21a76da:

  • P1 replay monitor/promise: inherited ClipHandle.play() now detaches and rebinds the old monitor without resolving the handle's existing finished promise. Added a regression that finished stays pending through replay start and resolves only after completion.
  • P2 per-mesh inherited morph seeding: inherited morph-name and morph-index tracks now seed from the tracked mesh's own morphTargetInfluences value instead of the first host-level matching mesh. Added two-mesh regressions for both paths.

Verification:

  • npm test -- --run src/engines/three/AnimationThree.inheritedKeyframes.test.ts (10 passed)
  • npm run typecheck
  • git diff --check

No public API/docs or src/engines/three/Loom3.ts changes were needed; the fixes stayed inside AnimationThree.ts and targeted tests.

@meekmachine

Copy link
Copy Markdown
Owner Author

Gate Verdict: pass

Findings: none.

Fix Plan:

  • No follow-up required for the inherited keyframe runtime changes after the replay monitor and per-mesh morph seeding fixes already landed.

Checks Run:

  • Confirmed PR [codex] Support inherited keyframes and clarify mixer controller naming #142 is rebased/current against origin/main; git rebase origin/main reported up to date.
  • Re-read the Loom architecture/release-boundary rubric and reviewed the changed runtime/types/docs surface.
  • npm run typecheck
  • npm test -- --run src/engines/three/AnimationThree.inheritedKeyframes.test.ts (10 passed)
  • npm run build
  • GitHub checks are green: Build, Typecheck, Test.
  • Rendered UI inspection not applicable; this PR is Loom3 runtime/package behavior.

@meekmachine

Copy link
Copy Markdown
Owner Author

Gate Verdict: pass

Findings: none blocking.

Implementation notes:

  • The core behavior is implemented in the right layer for Loom3: CurvePoint.inherit is documented on the shared curve type, clips are rebuilt at playback/replay time, and inherited starts are resolved from live morph/bone state rather than stale clip-construction values.
  • Coverage is strong for the feature shape: direct morphs, per-mesh morph starts, morph indices, AU-driven morphs, viseme jaw, composite rotations, bone translations, replay, and pending finished promises are all covered.
  • Before publishing, I would consider three small cleanups: add an inherited-keyframe line to RECENT_CHANGES.md; either remove or actually use the new getCurrentMorphValue / getCurrentMorphIndexValue host hooks since current morph anchoring reads mesh influences directly; and normalize/sort curve keyframes in snippetToClip() or document the sorted-array invariant explicitly, because inherited detection and maxTime currently rely on array order.

Checks Run:

  • Inspected PR [codex] Support inherited keyframes and clarify mixer controller naming #142 diff and current branch codex/136-inherited-keyframes.
  • Read README.md, RECENT_CHANGES.md, src/core/types.ts, src/engines/three/AnimationThree.ts, src/engines/three/Loom3.ts, and src/engines/three/AnimationThree.inheritedKeyframes.test.ts.
  • npm test -- --run src/engines/three/AnimationThree.inheritedKeyframes.test.ts.
  • npm run typecheck.
  • npm run build.
  • npm test -- --run.

@meekmachine meekmachine changed the title [codex] Support inherited first keyframes in snippets [codex] Support inherited keyframes and clarify mixer controller naming May 27, 2026
@meekmachine
meekmachine merged commit a69f7ce into main May 27, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant