Repository navigation
fix(core): renders keep a fromTo's from-only values from the frame it starts on - #5125
Conversation
terencecho
left a comment
There was a problem hiding this comment.
[P2, should fix] Keyframed fromTo loses its from-only transform origin after an overlapping tween — packages/core/src/runtime/adapters/gsap.ts:119-129
At the start of a keyframed fromTo, primedAtItsStart() takes the keyframe branch and returns without rendering that tween's _startAt. A controlled seek reproduces a wrong rendered pivot: put three fromTo tweens on one element at 0, 0.1, and 0.2 seconds; set their from-only transformOrigin to 0% 0%, 100% 100%, and 0% 0% respectively, and give the third tween two y keyframes. Seek sequentially through [0, 2/30, .1, 4/30, .2, 7/30]. At .2 and 7/30, direct GSAP and the exact PR base adapter render 0% 0%, while this head renders the previous 100% 100%. The incorrect pivot persists after the landing frame, affecting the visual motion.
I reproduced this in an isolated test at head 22a52eb87548640cdc3be16d68ad4882290b3191 and verified the base adapter fixture byte-for-byte against base 5fad52f21d0cb4742245d0b13c012d53c952d7ef. The intended #5122 frame-3 fix and focused tests pass, but this overlapping keyframed/from-only case needs coverage and correction before approval. No repository changes were made in this review.
— Review by tai (pr-review)
Edit accuracy: accurate 2055 (base branch 2055), smooth 1590 of thoseThe gate passes. Quarantined, measured but not gated (0) Unstable (1)
|
|
Thanks, reproduced and fixed (cfbbd59, plus a follow-on commit for staggers). Your case at 22a52eb, seeking [0, 2/30, .1, 4/30, .2, 7/30]:
The base is right at 0.2 only because the 0.1 origin had already been lost there (the #5122 bug); it is wrong at 0.1 and 4/30. The keyframe branch now re-applies the tween's start values after its keyframe prime, the same as any other tween starting on the seek time. Your case is a test: Checking the same path turned up one more case: each target of a stagger keeps its start values in the stagger's inner timeline, which the priming step never visited, so a later target lost its from-only values in renders (on the base too). The priming now descends into that timeline, and a test compares every target with GSAP's own playback on each of 13 frames. The PR body lists what is still left, both also on the base. |
terencecho
left a comment
There was a problem hiding this comment.
Approved at exact head 1e9f4d174c5a7d7701436037cd2f9570abc6e981 against base 5fad52f21d0cb4742245d0b13c012d53c952d7ef. This follow-up fixes my prior keyframed-overlap finding: the three-tween fromTo probe now matches direct GSAP at and after the keyframed start. Both added keyframe/stagger tests fail against the old head and pass here; focused adapter tests passed 262/262 and the runtime-seek checks passed. Independent forward-render stagger probes found no new regression. All 11 required CI contexts passed at this head.
A repeated-stagger reverse-seek origin mismatch was reproduced on the exact PR base and both heads, so it is an inherited scrub-path limitation, not a newly introduced blocker for this forward-render fix. This approval supersedes my earlier changes-requested review. No merge or deployment performed by tai.
— Review by tai (pr-review)
What
A rendered
fromTo()withimmediateRender: falsekeeps the properties that only its from-vars set, such astransformOrigin, from the frame the tween starts on. That matches Studio playback and 0.8.112 render output again.Fixes #5122
Why
Since #4911, every seek is re-rendered silently from just below the target time, so same-time
set()steps apply in authored order. When a tween starts exactly on the target frame, that step crosses back over its start. GSAP then reverts the values the tween applied at its start (itsstartAt). The forward pass re-applies only the properties in the to-vars, so a property set only in the from-vars, liketransformOrigin, stays at its CSS default. From that frame on, the tween animates about the wrong origin.The re-render already redoes a keyframed tween that starts on the target time, between the two passes. This change also re-applies any tween's start values there, keyframed or not, including each target of a stagger (those live in the stagger's own inner timeline). A
set()authored later at the same time still wins, because the final forward pass applies the timeline in authored order after it.Test plan
gsap.steps.test.tsseeks the issue's tween frame by frame, as a render does, and expectstransformOrigin: 0% 0%from its first frame. It fails on main (the origin readsundefinedfrom frame 3) and passes with the fix.set()authored later on the tween's start still wins over the restored from-values. This passes on both main and the branch, and pins the ordering fix(core): render GSAP set steps on the frame they land on #4911 protects.test:hyperframe-runtime-seek.The branch matches the reporter's 0.8.112 bounds exactly. The yellow box, which sets no origin, is unchanged.
Not in this PR
set()authored before the fromTo that writes, at the fromTo's start time, a property only the fromTo's from-vars set, still wins over that from-value. With aset()it lasts the start frame only; with a tween ending exactly there it can last the whole fromTo. Main behaves the same, so this is not a regression; it needs the re-render's ordering reworked and is left as a follow-up.immediateRender: falseand a from-only property. Played frame by frame, GSAP never applies the later target's from-only value; jumping straight to a later frame, it does. Renders now show the value (what the author wrote); main matched frame-by-frame playback.to()withstartAt: {...}, and a fromTo starting on the seek time inside a nested timeline that started earlier.Why a separate small PR
This fixes a render regression a user hit in production (#5122). The only other open change in this area is an unrelated Studio fix in another package, so this ships on its own rather than waiting on it.
Review
An independent adversarial review compared main, each head of this PR and GSAP's own forward seek over 32 scenarios, in render order and scrub order, on GSAP 3.14.2 and 3.15.0. The first version missed keyframed fromTo tweens (found in code review) and staggers (found by that review); both are fixed above with tests. A last pass over 40 scenarios (adding repeating, yoyo,
from(), zero-gap, eased and nested staggers) found the rest matching GSAP; its remaining notes are the bullets under "Not in this PR".Before / After
Frame 3 of the issue's composition: main on the left, this branch on the right.