feat(engine): per-subtree time remapping - #64
Merged
Conversation
time_scale/time_offset on containers (flex/grid/card/div/positioned) remap time for their whole subtree: t_local = (t - offset) * scale, composed in cascade as an affine (scale, shift) transform threaded through the builder recursion. The animation ctx stays GLOBAL down the recursion — each node derives its local time from the composed remap (a pre-remapped ctx double-applies; caught in review). Animations, timeline states, audio-reactive bindings and motion-blur ghosts follow the local time; start_at/end_at windows convert back to global after local stagger shifting; BuiltScene.time_params feeds the dispatcher so internal animations and PaintCtx.time advance at local time (scene_duration stays global, documented). validate rejects time_scale <= 0; builder clamps defensively. Skill rule added.
LeadcodeDev
added a commit
that referenced
this pull request
Aug 11, 2026
) Closes #164. PR #152 fixed a real bug — `scene.freeze_at` was read by no code path in the `world` view — by adding a fifth hand-written copy of the clamp "for parity with the other four". The cause it left untouched was that no single place computes a scene's render time, so every new render path has to remember to redo it. Writing the parameterized test first, before any fix, paid for itself twice. Four of the five paths agreed; the fifth did not, and the reason was not the cosmetic `.min()`-versus-`if` drift already noted on the issue: `render_world_frame_scaled` painted the active scene's animated background from the raw world clock, never clamped. PR #152's fifth copy was itself incomplete, and nothing had caught it. `RenderContext::time` is now a `SceneTime` rather than an `f64`. Its single field is private to a submodule and its only two constructors both apply the clamp, so it cannot be built anywhere else — verified by trying: error[E0423]: cannot initialize a tuple struct which contains private fields Since `render_with_new_pipeline` — the only function that paints a scene's component tree — requires a `&RenderContext`, a sixth render path is held to the clamp by the compiler rather than by discipline. That does not stop someone writing `frame_index as f64 / fps as f64` from scratch, but it removes the failure mode actually observed here: copying an existing path for parity. Every path left to copy now goes through `SceneTime`. Two findings worth recording rather than burying: - `time_scale`/`time_offset` were already available on `card`, `flex`, `grid`, `container` and `positioned` — PR #64 generalized them long before this chantier, with the affine composition, its tests and its rule documentation. Nothing here needed to change. - `freeze_at` does not need to enter that composition. Because `time_scale` is validated strictly positive, every composed remap is monotonic, so clamping the global time before composing is equivalent to clamping after, at any nesting depth. A test pins the brief's own example — a `card` at `time_scale: 2` over a `flex` at `time_offset: -1` — rather than leaving that as an argument. Not covered: a world crossfade with two scenes visible keeps the raw world clock for the background. Freezing it means deciding which scene wins during the transition, which is a design question, not an oversight; the comment says so where it applies.
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
1 participant
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.
Closes #53.
Goal: slow-motion/replay of a group without touching each child —
time_scale/time_offseton containers remap time for their whole subtree.t_local = (t_global − time_offset) × time_scale, composed in cascade as an affine(scale, shift)transform. The animation ctx stays global through the recursion; each node derives its local time from the composed remap — the pre-remapped-ctx variant double-applies the transform (a sub-agent introduced exactly that bug; caught and fixed during the workstream).start_at/end_atwindows are shifted by stagger in local time, then converted back to global ((t_local − shift)/scale) for the paint-pass check.BuiltScene.time_params(per-node, stagger_delays pattern) feeds the dispatcher: internal animations (counter, draw_in, typewriter) andPaintCtx.timeadvance at local time;scene_durationstays global (documented in code + skill).validaterejectstime_scale ≤ 0; the builder clamps defensively. HTML:<rm-flex time_scale="0.5">already works via the generic rm-* mapping (snake_case attrs; documented in the new skill rule).Tests: 7 new — five pixel tests (slowed fade, delayed start, window under scale, 0.5×0.5 cascade, internal draw_in through the dispatcher path) + two validator tests. 339 total, all green. Process caveat recorded: tests were written alongside the implementation (parallel sub-agents), not observed red against the final code — they did catch the double-remap bug.
Verify:
cargo test --workspace→ 339 passed ✓ ·cargo fmt --check✓ · clippy gated ≤1 ✓