Skip to content

fix(core): a set written before its tween no longer erases the tween's end - #7397

Merged
miguel-heygen merged 3 commits into
mainfrom
fix/set-authored-order
Nov 8, 2026
Merged

miguel-heygen merged 3 commits into
mainfrom
fix/set-authored-order

Conversation

@miguel-heygen

Copy link
Copy Markdown
Collaborator

What

A set written before the tween it sits inside no longer erases that tween's end value in a render or in preview. A film whose cards fade in with a stagger after a positionless set(cards, {opacity: 0}) keeps them visible until its own fade-out, as it does on 0.9.18 and earlier.

Why

Since 0.9.19 (#7283, extended by #7318), a set whose time falls inside an earlier-starting tween on the same property is replayed after that tween ends, so the set's value wins. That is right when the film wrote the set after the tween: an override. It is wrong when the film wrote the set first and the tween after: the set is the tween's initial state, and the tween's end is what the author sees.

Both films end up with the same timeline positions. GSAP sorts children by start time and keeps no record of the order they were written in, so the two can't be told apart from the timeline alone. Reduced to four cards:

tl.to("#scene", { opacity: 1, duration: 0.8 }, 5.5);   // timeline now ends at 6.3
tl.set(cards, { opacity: 0, scale: 0.95 });             // no position: appended at 6.3
tl.fromTo(cards, { opacity: 0, scale: 0.95 }, { opacity: 1, scale: 1, duration: 0.8, stagger: 0.15 }, 6.0);
tl.to(cards, { opacity: 0, scale: 0.8, duration: 1 }, 8.5);

Rendered at 30 fps with one worker, card pixel at 7.3, 7.6 and 8.2 s: 0.9.14, 0.9.17 and 0.9.18 show the cards (255). 0.9.19 to 0.9.22 show none (background). The cards vanish at frame 218, the moment the stagger ends.

Related work

Refs #7283 and #7318, whose panel case stays as it was: there the set is written after the tween.

How

When GSAP loads, the runtime wraps Timeline.to, from, fromTo and set so each child they add gets a write-order number (gsapAuthoredOrder.ts, installed beside the force3D setup in installFlatGsapTransforms). overlapsOnOneTarget marks the later write as a set that interrupts only when it was not written before the tween it overlaps. A film built before the runtime loaded has no numbers and behaves as before.

Test plan

  • Unit tests added: gsap.order.test.ts, "a set written before the tween it sits inside", through the render seek (every frame equals GSAP stepping in order) and through the runtime's seek and re-render (every frame from the end of the fade-in), with 1, 2, 3, 4 and 6 workers; init.authoredOrder.test.ts, the load hook numbers a timeline's children in write order.
  • fails without the fix: with the old gsap.ts both new gsap.order cases fail (frame 218, opacity 0 where GSAP shows 1); with stampAuthoredOrder removed from the load hook the new init test fails; the other 13 gsap.order cases, including fix(core): a set that interrupts a tween holds in a real render at any worker count #7318's panel cases, pass on both sides.
  • All of packages/core/src/runtime: 128 files, 2,975 tests pass.
  • Manual testing: real CLI renders of the reduced film from this branch at 1 and 3 workers: card pixel 255 at 7.3, 7.6 and 8.2 s and 140 at 8.8 s, the same values as 0.9.18.
  • Documentation updated (not applicable)
  • Comments follow CONTRIBUTING.md "Comments": comment ratchet clean.

Not covered: inside the overlap (one frame, 6.33 s in the reduced film) GSAP's own in-order stepping shows the set's value where the runtime's re-render shows the tween's; that differs before and after this change and the runtime's value is the tween's, so the runtime-seek test compares from the end of the fade-in.

@github-actions

github-actions Bot commented Nov 8, 2026 •

Copy link
Copy Markdown
Contributor

Edit accuracy: accurate 267 (base branch 267), smooth 109 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (8)

@heygengenesis heygengenesis Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving at 00d4d081: I found no blocker.

What I verified

  • The tests fail without the fix. At this head, gsap.order.test.ts and init.authoredOrder.test.ts pass, 16/16.
    • With main's gsap.ts, exactly the two new cases fail: "shows the tween's end after the fade-in through the render seek…" and "…through the runtime seek…". The other 13 pass, including #7318's panel cases.
    • With main's init.ts, the init test fails. That matches the fails without the fix: line.
  • Nothing else in the runtime breaks. All of packages/core/src/runtime passes: 128 files, 2,975 tests. tsc --noEmit for core and oxlint on the five files are clean.
  • Plain GSAP agrees with the new behaviour. I wrote a standalone probe that steps the reduced cards film frame by frame. The card is at opacity 1 at 7.5 s whether the set is written before or after the fromTo. So for the set-first case, this change brings the runtime back to what GSAP shows.
  • Stagger is handled. A staggered tween builds its own inner timeline, and the inner tl.to calls go through the patched prototype too. authoredOrderOf keeps the outermost stamp below owner, so a staggered child takes its parent tween's write order, not the inner one. Inner stamps are taken after any earlier set, so the comparison holds either way.
  • The guards are sound. _hfAuthored === undefined stops a call that added no child from re-stamping the previous _recent. When either side has no stamp, the code falls back to the old behaviour. This covers a film built before the runtime loaded, tl.add(gsap.to(…)), and a bundled GSAP.

Non-blocking notes

  • My probe shows GSAP's own frame-by-frame stepping also lets the tween's end win in #7318's panel case, where the set is written after the fade. That gives 0 after 1 s, while the runtime asserts 1. So the set-after override is still a deliberate difference from plain GSAP. This PR doesn't widen it, and it reads as intended per #7283, but it's worth knowing write order is not GSAP's own discriminator.
  • runtime/inspection.ts also wraps Timeline.prototype.{to,from,fromTo,set}. Its restorer only unwraps when it is still on top. If inspection hooks before GSAP loads, the stamp wrapper lands above it, so inspection's wrapper stays underneath after dispose. That is harmless, but worth knowing if either patch changes.

@miguel-heygen

Copy link
Copy Markdown
Collaborator Author

Thanks for the review. Both notes are known and I'm leaving the code as is:

  • Write order vs plain GSAP: agreed. The set-after override is the existing fix(core): a render lands where preview does when a set interrupts a tween #7283/fix(core): a set that interrupts a tween holds in a real render at any worker count #7318 rule, so this PR doesn't widen it: it only stops that rule from firing when the film wrote the set first, where the set is the tween's initial state. Write order is the one thing that separates the two films, since both end up with the same timeline positions.
  • inspection.ts wrappers: the stamp wrapper calls whatever method it wrapped, so the two compose in either order. In the worst case, if inspection hooks first, its wrapper stays under the stamp after dispose(). That wrapper only records, so it's harmless, and I'm not changing the stamp to restore itself because the stamp should last the page's life.

@heygengenesis heygengenesis Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-approving at 5e1d7e1f.

What changed since my approval at 00d4d081: only gsapAuthoredOrder.ts. stampAuthoredOrder now takes object | undefined and narrows internally to core.Timeline.prototype. The runtime logic and the tests are unchanged.

What I verified

  • The error I missed is fixed. My earlier pass only ran tsc --noEmit. Core's typecheck script also runs typecheck:runtime (tsconfig.runtime.json).
    • At 00d4d081, that config reports TS2345 at init.ts:468: Window["gsap"] has no properties in common with the old parameter type.
    • At 5e1d7e1f, it is clean.
  • Tests still pass. gsap.order.test.ts and init.authoredOrder.test.ts pass, 16/16. The fails without the fix: line is unchanged and still accurate.
  • CI is green on this head. CI, regression, Windows render verification, CodeQL, preview-regression and the Player checks all succeeded.

The non-blocking notes from my earlier review still apply.

@miguel-heygen
miguel-heygen added this pull request to the merge queue Nov 8, 2026
Merged via the queue into main with commit 75fe18c Nov 8, 2026
98 checks passed
@miguel-heygen
miguel-heygen deleted the fix/set-authored-order branch November 8, 2026 06:20
SanHsien added a commit to SanHsien/hyperframes that referenced this pull request Nov 9, 2026
Port the adopted upstream fixes from f5bde95 to the current upstream tip:
heygen-com#7375 multipart cloud upload, heygen-com#7382 minParallelChunks, heygen-com#7349 HLG header SEI,
heygen-com#7369 skip unmounted compositions in lint/check, heygen-com#7381 filesystem-path media
in the cloud archive, heygen-com#7383 Safari cut-point pre-roll revert, heygen-com#7373 clock
steering between audio time updates, heygen-com#7384 injector test isolation, heygen-com#7378
preview seek retention, heygen-com#7390 virtual timer schedule per worker, heygen-com#7394 CDN
script failure naming, heygen-com#7395 menu row radius, heygen-com#7119 decode timeout caller
test, heygen-com#7397 authored tween order, heygen-com#7402 capture page clip loading, heygen-com#7412 muted
video audio track at cuts, heygen-com#6150 template root variable test, heygen-com#7442 held tail
seek once.

Deliberate local differences: heygen-com#7067's audio-start hold is not carried into
clock.ts, mountedSources comes from heygen-com#6827 as a helper only, cloud/_gen/types.ts
omits the image-request types the fork does not generate, and the lint
symlink test skips through symlinkOrSkip where the account cannot create links.

Co-Authored-By: Claude Sonnet <noreply@anthropic.com>
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