Skip to content

fix(studio): plain elements keep their place and size when keyframed - #5045

Merged
miguel-heygen merged 20 commits into
mainfrom
fix/studio-plain-element-keyframes
Oct 5, 2026
Merged

miguel-heygen merged 20 commits into
mainfrom
fix/studio-plain-element-keyframes

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

What

Turning a plain element into an animated one through auto-record and "Add keyframe" should leave it where it is, at the size you gave it, at every time.

  • Add keyframe on a moved plain element. A move on an element with no GSAP writes CSS translate. Add keyframe (toolbar button or K) wrote its first key as x: 0, y: 0, and GSAP takes the element's translate over once it animates x/y, so the box jumped back by its offset in the preview, after reload and in the render. The first key now starts from the offset the move left in translate (px, read with the same length evaluator a move uses, so calc(-50% - 120px) gives -120; a percentage stays with xPercent, as GSAP reads it).
  • A resize under auto-record keys size. On an element that already has a keyframed tween, a resize wrote plain CSS width/height, so resizing at one keyframe changed the box at every other time. The first resize now writes a size key at the playhead, in the same write as the anchor move; later resizes key into that tween. The first size key holds before itself, After Effects style: the Studio hold that already pins a first position key from t=0 now pins the size of a size key Studio made, and a lone key at t=0 (GSAP 3.15 never renders a lone "0%" keyframe by itself). The hold stays when that tween gains more keys. Rule: an edit never changes frames the user did not touch, so these holds are only added for a tween the edit touched (a split counts; moving a clip does not), and a tween the edit left alone keeps only the hold made from its own first key (an authored size tween keeps its own lead-in, even after a Studio key on the same element is deleted). Elements whose only keyframes fade them, or whose tweens are flat, keep plain CSS size, as before.
  • The resize hands its draft to GSAP. After a keyed resize the gesture's draft size stayed on the element and was re-applied after every seek, pinning the size to the last drag at all times. The keyed size routes now clear it and draw the committed size through GSAP in the same frame, as the scale route already did.
  • Keyframe tweens end on their last key. GSAP 3.15 renders a keyframed tween whose last key sits before 100% differently on its first render than once played: keys 0%: 440, 50%: 340 over 2 s at 2 s show 390 on a direct seek to 3 s and 340 after playing through 2 s. Add keyframe makes a single-key tween that runs to the clip end, so every later key landed inside it. The server now trims the keyless tail of each tween a keyframe edit wrote or changed (duration scaled, keys rescaled; the same render once played, no new keyframe). Tweens the edit did not touch keep their bytes (a chained .to().to() link is compared by its own arguments). No trim when anything in the script loops (repeat:, yoyo:), sets the timeline's length or speed, or is placed relative to another tween's end (no position, >, +=, tl.add, tl.call, addLabel), since a shorter tween would move it; static gsap.set calls do not count. No trim either when a trim could end the film early: anywhere in a film whose root composition has no data-duration (its length follows every timeline it plays), in a sub-composition whose host clip has neither data-duration nor data-end, or in a root no host mounts and that has no data-duration. A host that gives the length keeps the fix.
  • A resize's anchor uses its own drag stamp. A resize that moves its anchor (here, on an element with a flat GSAP tween) wrote that move after awaiting its size save, reading the drag-start attributes from the element. A drag pressed meanwhile had re-stamped them, so the anchor was applied on top of the new base (saved x −40 instead of 10). The resize now reads its stamp at release and every commit it makes uses that copy; the clean-up that clears the attributes skips a stamp whose gesture is over.
  • An undo shown in place is not replaced by an older reload. Undoing "Enable keyframes" on a file that had no GSAP script asks for a full preview reload; the next undo is shown in the live preview at once, before the server writes it. The reload still swapped in the file without that undo and put the undone move back on screen (284 px off, file correct), in two orders: a reload fetched before the undo was shown, and one fetched after it was shown but before its write landed. Showing or applying an undo in place now counts as a preview change, so an earlier reload is fetched again, and an undo shown before its write lands holds a reload the way a saving edit already does.

Bench

  • seqplainkeys (12 cases): move a plain element, turn on auto-record, Add keyframe, then move and resize at and between keyframes, then seek. Every step waits for its save and is scored against where it put the box; a seek also checks the size the size keys hold there (before the first key: that key's size). The moves stay inside the nested composition's frame, which clips the box in preview and render alike.
  • seqresizedrag-tween and seqresizeundodrag-tween, root and nested, settled per step: a resize on a flat tween saves its size and its anchor separately (one undo), which the unsettled stack model counted as two edits.
before after
Add keyframe step, box error 362 (px), 384 (percent), 168 (center) 0 in every case
Seek 3 s after the resizes, box / size error 120 / 100 0 / 0
Seek 1 s, before the first size key, size error 0 (reload 240.8 off with the size hold removed)
Direct seek vs played, position tween at 3 s 241.7 vs 47.754 47.754 both
Direct seek vs played, size tween at 3 s 390 vs 340 340 both
Resize then drag on a flat tween, saved anchor x −40 (wanted 10) 10
Undo back to the first move, rotated root case, 4× CPU throttle box 284 px off in 1 of 3 runs 0 in 6 of 6
Undo back to the first move, root cases, 2× throttle while recording 284 px off in 1 of 1 with only the first undo fix 0 in 8 of 8

Not in this PR

  • An edited tween can take over a size hold on its element that another tween made (for example an authored tween moved ahead of Studio's lone size key); frames before it change, as they already do on main.

  • Under the opt-in recast writer, moving a clip still counts as touching its tweens for holds; the default writer does not.

  • In a project whose root file is not index.html, a sub-composition is judged by its own root rather than its host when deciding whether to trim.

  • A size tween moved to start at 0 drops its hold (nothing to hold before it), and moving it later again does not bring the hold back.

  • A drag pressed while a resize's save is still pending: the resize's teardown clears the drag's own start stamp. Same family as the case below; fixed in the follow-up.

  • Add keyframe, then a move pressed before the new tween reaches the preview: the move is saved as plain CSS translate, which the new keys then override (40.8 px off). It routes the gesture on the preview's old state and is not this writer; it is the follow-up's second red case.

  • The keyframe writes behind the SDK cutover flags, and the split path, do not run the trim yet.

  • Resize, undo, then drag on a nested element (seqresizeundodrag-tween-px-r0-nested-z100): after the undo settles, the drag does not move the box. It fails here as on main and stays out of the bank; it is the red case of the follow-up PR.

Before

Move a plain element, turn on auto-record, Add keyframe, then resize at and between keyframes (repo fixture, edit bench case seqplainkeys-none-px-r0-root-z100). On Add keyframe the box jumps back by the offset of the move, and the resizes change its size at every time.

add-keyframe-before.webm

After

The same steps: the box stays where it was moved and keeps each resize at its keyframe.

add-keyframe-after.webm

@github-actions

github-actions Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Edit accuracy: accurate 2055 (base branch 2040), smooth 1576 of those

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

Newly passing (15)

  • seqplainkeys-none-px-r30-root-z100
  • seqresizedrag-tween-px-r0-nested-z100
  • seqplainkeys-none-px-r0-root-z100
  • seqplainkeys-none-px-r0-nested-z100
  • seqplainkeys-none-pct-r0-root-z100
  • seqplainkeys-none-pct-r0-nested-z100
  • seqplainkeys-none-center-r0-root-z100
  • seqplainkeys-none-px-r30-nested-z100
  • seqplainkeys-none-center-r0-nested-z100
  • seqplainkeys-none-pct-r30-root-z100
  • seqplainkeys-none-pct-r30-nested-z100
  • seqplainkeys-none-center-r30-root-z100
  • seqplainkeys-none-center-r30-nested-z100
  • seqresizeundodrag-tween-px-r0-root-z100
  • seqresizedrag-tween-px-r0-root-z100

Quarantined, measured but not gated (0)

@miguel-heygen
miguel-heygen force-pushed the fix/studio-plain-element-keyframes branch 4 times, most recently from 4d5a271 to 15cb22c Compare October 5, 2026 10:38
Comment thread packages/parsers/src/gsapWriterAcorn.ts Fixed
@miguel-heygen
miguel-heygen force-pushed the fix/studio-plain-element-keyframes branch 4 times, most recently from 7c60d90 to a486fda Compare October 5, 2026 12:35
@miguel-heygen
miguel-heygen force-pushed the fix/studio-plain-element-keyframes branch from a486fda to 4ad5721 Compare October 5, 2026 13:02
@miguel-heygen
miguel-heygen force-pushed the fix/studio-plain-element-keyframes branch from 4ad5721 to 2273dbc Compare October 5, 2026 13:33

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approving at ebad31f4.

What I checked

  • Tests: the changed unit tests pass three runs in a row (parsers 238, studio-server 113, studio 133), and settle.test.mjs passes under bun (4/4). All 20 edit-accuracy shards are green at this head.
  • Trim vs real GSAP 3.15: keys 0%: 0, 50%: 100 over 2 s, compared before and after the trim at 1.25 / 1.5 / 1.75 / 2 / 2.5 s, played through and on a direct seek. The values are identical, including under a timeline defaults: { ease: "power2.out" }, which GSAP does not apply to the keyframe run. A tween with an explicit ease: "power2.out" is left untrimmed, as intended.
  • Moves after a trimmed tween's end: a move after the last key now lands outside the tween, so I followed that path. It goes through the existing "outside the tween, add one there" branch (buildExtendedKeyframes), so trimming does not strand later keys.
  • Mutation testing: 16 of 17 guards caught by the unit tests. Caught: lone-key hold scope, untouched-tween hold, size in holds, the untouched skip in the trim, the timeline regex, placedAfter, loops, ease, unsized root, host data-end, draft clear, auto-record gate, frozen stamp, shown-restore saving, preview-change count, and lone-at-0 hold.

Non-blocking

  1. The one survivor is the headline fix. Replacing readTranslatePxLeavingPercent(...) in Add keyframe with { x: 0, y: 0 } passes every unit test; only the seqplainkeys bench catches it. A small useEnableKeyframes test (plain element with translate: 90px -40px → first key x: 90, y: -40) would pin it without a browser.
  2. Can it be simpler: the PR has two notions of "the edit touched this tween". holdScope compares parsed JSON signatures; trimTrailingKeyframeSpans compares call source text. They could share one, so holds and trims can't disagree on what was touched.
  3. Reuse: readTranslatePxLeavingPercent repeats most of readTranslatePx. It could be readTranslatePx with a zero reference box.
  4. lengthIsTimeline parses every file in the root's closure on each keyframe save. That's fine at today's project sizes.
  5. The CodeQL polynomial-regex alert on PLACED_BY_TIMELINE.test(script) looks like a false positive. A 2.2 MB adversarial input runs in 2 ms, and the whole trim on an 864 KB timeline({{-repeated script runs in 2 ms, so it is safe to dismiss.

— Rames

@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 5, 2026
Merged via the queue into main with commit 5c7f11a Oct 5, 2026
79 checks passed
@miguel-heygen
miguel-heygen deleted the fix/studio-plain-element-keyframes branch October 5, 2026 15:58
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.

3 participants