Repository navigation
fix(core): scrubbing back across a scene no longer leaves its content hidden - #5766
Conversation
Edit accuracy: accurate 271 (base branch 271), smooth 177 of thoseThe gate passes. Quarantined, measured but not gated (0) Unstable (1)
|
jrusso1020
left a comment
There was a problem hiding this comment.
Requesting changes at e38913f9 because the required Studio: edit accuracy gate fails. It failed on the first run (39011185991) and again on the rerun (39011237683). The gate counts a case as failing only when it fails 2 of 3 runs, so this isn't flakiness.
The accurate count is 255, down from 271 on main. Sixteen cases that pass on main fail here:
- move:
fromto-…-mid,keys-…-on,keys-…-mid,keys-…-after - nudge:
fromto-…-mid,keys-…-on,keys-…-mid,keys-…-after - resize:
fromto-…-mid,scale-…-mid,size-…-mid,keys-…-on,keys-…-mid,keys-…-after,keys-px-r0-nested-z100-on - rotate:
spin-…-mid
(… = px-r0-root-z100; the full list is in the sticky edit-accuracy comment.)
Every one of them is a px-r0 case, and all but one are root-level. That points at the seek path this PR changes, not at one edit kind. I haven't reproduced it locally, so these two suspects are unconfirmed:
seekGsapTimelinenow redraws whenever the seek lands behindfrom. The editor seeks repeatedly while it measures an edit at a fixed time. If one of those seeks counts as "backward", the redraw could repaint the element at a state other than the one the edit gate reads back.renderOntoInAuthoredOrdernow callstimeline.render(t, true, true)for t < 1 ms. WithsuppressEventsandforceboth set, the render can produce a different start state than stepping up from below did, for tweens whosefromvalues the editor has just rewritten.
The code itself reads correctly to me, so this is the only blocker. Please run the edit-accuracy suite locally on one of the keys-…-mid cases and diff the measured geometry against main. Then either fix the seek path or quarantine the cases with a stated reason. I'll re-review as soon as the gate is green.
— Rames
|
Thanks for the precise list. Both suspects were real, and the head now removes both code paths instead of tuning them.
The edit accuracy gate at |
jrusso1020
left a comment
There was a problem hiding this comment.
Approving at ebc5d810. This clears my change request from e38913f9: both seek-path changes I suspected for the edit-accuracy drop are gone.
- No whole-timeline redraw on a backward seek.
seekGsapTimelinenow re-applies only the steps placed exactly at the landing time (stepsAt, within float noise), in authored order. Each one is rendered at its start silently, then itsratio/_zTime/_actare restored, so GSAP's own bookkeeping for the next seek is unchanged. Landings between steps cost nothing extra, and the new testnever redraws a whole scene for a backward landingcovers that. - No forced
timeline.render(t, true, true)at 0.renderOntoInAuthoredOrdergoes back to stepping up from just above 0, then re-applies the steps at 0 in order. Therenderbinding is removed from every caller. - The new structure key comes from
gsapStructureOf, which returnsundefinedonce render capture starts. So the extra work is preview-only, and export seeks take the same path as before.
The tests compare against plain GSAP, which is the right oracle here: keyframes arrays and percent keyframes match plain GSAP at 0 across the root and scene seek sequences, and calls on the landing time fire exactly as often as plain GSAP across back-and-forth seeks. Required CI is green at this head, including Studio: edit accuracy gate.
No live run.
— Rames
What
Scrubbing the Studio preview past a scene, back before it, and then forward into it again left that scene's background on screen with its content hidden. It stayed that way until the scene's next step. The rendered export was always right. With this change the preview matches the export on every scrub path.
Why
To reproduce: take a sub-composition whose script reveals its content with two steps at its own time 0, such as
tl.set(field, { visibility: "hidden" }, 0)followed bytl.set(field, { visibility: "visible" }, 0)(common when a scene builds clones at load and shows them from its first frame). Seek past the scene, seek back to just before its start, then scrub forward into it. Before this change the field stays hidden until the scene's next visibility step. On the test fixture below that is about 1.2 s of an empty dark frame.Cause: on every seek the runtime also seeks each registered scene timeline to its own time (
seekStandaloneRegisteredTimelines). A scene that has not started yet is held at local time 0. When GSAP lands a timeline exactly on a time coming from a later time, it applies the zero-duration steps at that time in reverse authored order and marks them done. So "hidden, then visible" ends hidden, and the next forward move never applies them again. Plain GSAP 3.14.2 and 3.15.0 do this:The same happens at any later time a scene lands on exactly from above, e.g. two steps on a whole second. The root timeline already re-renders after every eventful seek, from just below the playhead, except at 0, where it steps down from above and so applies the steps at 0 in reverse too.
Related work
Refs #5614, which made
seekGsapTimelinethe path every runtime GSAP seek takes and added the never-rendered redraw this extends.How
One function owns the repair,
reapplyStepsInAuthoredOrder: it takes the zero-duration steps sitting exactly on the landing time from the cached step index (landingMarksOf's structure-keyed index from #5674), renders each in authored order, and restores GSAP's own bookkeeping on it (ratio,_zTime,_act), so callbacks fire exactly as often as in plain GSAP. Two callers:seekGsapTimeline, when a timeline lands backward (a scene parked at 0, or scrubbed back onto a step). Nothing else is redrawn, so a backward seek costs what it did on main, plus a binary search.rerenderGsapTimelineAtat 0, after its existing step down from above and its keyframe priming, which stay as they were.Both seek paths in
init.tsnow pass the same structure key (gsapStructureOf). Render capture keeps passing none, as before.Test plan
New cases in
packages/core/src/runtime/adapters/gsap.steps.test.ts, run with real GSAP:Against main's
gsap.tsandinit.ts, the four symptom tests fail (exit 1, 4 failed). On this branch: 74 passed, green on every run. Mutations: removing the re-application on backward landings turns 3 tests red, removing it at 0 turns 1 red, a full redraw on backward landings turns 1 red, and dropping the bookkeeping restore turns the callback test red (7 calls vs 5).packages/coretest:hyperframe-runtime-ci(typecheck, build, contract, behavior, seek, parity, security, coverage): exit 0, 83 files, 2104 passed.packages/producerfirstFrameStartingState.integration.test.ts(real render): exit 0, 2 passed. Studio edit accuracy gate in CI: 271 accurate, same as main.What I measured
Fixture: a 12 s, four-scene composition with generated images. The third scene clones a 3x3 field of windows full of pictures at load and reveals it with steps at 0, and the second ends in a zoom push with motion blur. Driven through the published Studio's own timeline with a real mouse drag (park at 8 s, drag from 4.5 s to 8 s), headless Chrome, GPU, device scale 2, recording the compositor's frames:
The same back-then-forward seek order on Linux (headless Chrome 152): main shows the empty scene from 6.1 s and no new frame through 7.0 s, while the fix shows the field at every step.
Cost in Chrome on the same fixture, main vs this branch, three alternating rounds. Per-seek main-thread time over forward, backward and forward sweeps at 0.1 s steps, then 5 s of playback:
On a 38,000-step timeline (the long-caption case from #5674), a backward root seek costs a median 12.07 ms on this branch vs 11.89 ms on main over 8 alternating rounds, and a backward scene seek costs 1.5-2.6 ms on both.
Before / After
Studio preview during the drag, same times. Before: the third scene shows only its background. After: its window field.
The frame that looks low-res during the second scene's exit is authored motion blur, and the export has it too. Studio preview with this change next to the rendered export:
What I did NOT exercise