fix(studio): graded elements survive manual editing (disappear/resize/rotate/crop/panel)#2230
Conversation
…ource geometry The color-grading engine hides its source element with inline 'opacity: 0 !important', so any code that later reads or re-captures the element's opacity sees the hide instead of the authored value. Stamp the authored inline opacity on every [data-color-grading] element at document parse time (MutationObserver installed at runtime-bundle eval, before any composition script runs) and prefer the stamp when hiding/restoring. Also re-sync the grading canvas when the source's inline geometry mutates (rAF-throttled style observer): a studio drag moves the source via its transform, which fires no media event, so the visible canvas froze in place until the next seek.
Editing commits made elements vanish or dim permanently: invalidating the whole timeline (or re-running the composition script on soft reload) made GSAP re-capture tween bounds while runtime transients were live — the grading hide's opacity 0, or a mid-flight tween value — so from()/to() bounds got poisoned and the element rendered invisible from then on. - patch only the edited tween in place, never timeline.invalidate() - soft reload restores every animated element's authored inline opacity (after-write HTML first, parse-time stamp as fallback) before the script re-runs and re-captures - a paired x/y commit whose second half is a no-op (changed=false) still applies its instant patch, so panel edits reflect without deselecting
- resize on a scale-driven element commits per-axis scale (scaleX/scaleY for non-uniform drags) with keyframe normalization to the longhands, and clears the width/height draft so size can't double-apply; the intercept moves to gsapResizeIntercept.ts - the drop frame applies the corrected position synchronously in the same microtask chain as the soft reload (no network-window jump), and the draft pins the anchor through accumulated moves on scaled elements - gesture size/position math divides by the element's own content scale - convert-to-keyframes resolves current values through the property-group filter for ALL capture passes (opacity/rotationX from unrelated tweens no longer leak into a rotation commit), and a grading-hidden source's opacity is read from its canvas, not the inline hide - canvas pointer-down confirms the hover target with a synchronous hit-test before starting a marquee (stale-hover race lost selections)
The always-on crop lifted EVERY selected element's clip-path and restored only what it could parse as a px inset — selecting an element with a circle/polygon/percentage clip visually un-clipped it, and deselecting deleted the authored clip outright. readElementCropInsets is now tri-state (zeros = no clip, null = a clip the tool can't represent): uneditable clips get no lift and no handles, and the lift restores the pre-lift inline value verbatim unless a crop gesture actually committed.
commitStaticSet merged every property into the FIRST set found for the
selector: a panel W edit on an element whose only set was positional
produced tl.set("#el",{x,y,width}) — a mixed-group set the split
machinery exists to prevent — labeled "Set 3D transform" in undo.
Commits now batch per property group into a set that owns that group
(exact group match, then a mixed set already carrying the group, then a
fresh off-timeline gsap.set), with undo labels derived from the group
(Move layer / Resize layer / Rotate layer / Set 3D transform).
A parsed timeline set carries immediateRender in extras; the recast statement builder ALSO pushed the flag unconditionally, so every split/re-add of a set wrote 'immediateRender: true, immediateRender: true' into the file, doubling on each pass. Both writer twins (recast + acorn) now emit each vars key exactly once, properties winning over extras, and reconcileEditableProps skips newProps keys it already preserved.
gsap_from_opacity_noop matched 'opacity: 0' as a prefix — an authored 'opacity: 0.98' triggered the rule. One boundary-anchored predicate now owns the exactly-zero test for both block and inline declarations, including a block's last declaration without a trailing semicolon. domEditingDom now imports the grading contract attribute from core instead of re-declaring the literal.
james-russo-rames-d-jusso
left a comment
james-russo-rames-d-jusso
left a comment
There was a problem hiding this comment.
Reviewed at fe52966.
Read every fix end-to-end against the pre-PR diagnosis. The story lands: the seven commits map 1:1 to the seven root causes, and each one is genuinely at the layer where the bug lives — not a symptom patch. Particularly strong:
- Fix 1's blast-radius analysis. The docstring at
packages/studio/src/hooks/gsapRuntimePatch.ts:306-311names exactly whytimeline.invalidate()on a graded composition permanently poisonedfrom(opacity)tweens on untouched elements. Scopingtween.invalidate?.()to the resolved tween + gatingrestoreAuthoredOpacityForCaptureonchangeTouchesOpacity(change)is surgical, not blanket. - The parse-time authored-opacity stamp in
packages/core/src/runtime/colorGrading.ts:224-244is the right place — it runs while the document is still parsing, before the composition script or grading hide can mutate the inline value, and the has-attribute guard makes late re-inserts a no-op. The""vsnulltri-state is honored everywhere I traced (studio soft reload, runtimehideSourceElement,restoreSourceElement, the fallback into the graded canvas's own computed opacity). - Crop tri-state.
readElementCropInsets → nullvszerosfinally kills the class of "select+deselect silently destroyed my authoredcircle(50%)clip".preLiftInlineClipRef.currentrestore-verbatim +cropCommittedRefgating means an uneditable clip is never rewritten, and even an editable clip isn't reformatted until a crop gesture actually commits. - Property-group static-set routing. One commit per group, batched (
commitSetPropswrites all props in ONE mutation because a per-prop loop would shift the group-derived id mid-way and 404 the next update) is exactly right, andstaticSetLabelderiving the undo label from the group means panel W edits stop showing up as "Set 3D transform" in undo history. - Writer parity. Both twins get the same dedupe (recast at
gsapParser.ts:1387+, acorn atgsapWriterAcorn.ts:52+), and there's a regression test on each side — future writes to one path won't quietly drift out of sync with the other. - Lint anchor.
opacity\s*:\s*0(?:\.0+)?\s*(?:;|$)correctly rejects0.98while still catching the "last declaration, no trailing semicolon" case authors write by hand.
CI is clean (Windows lane still in progress, but the fast lanes — including regression shards, preview-parity, and player-perf — are all green). LGTM from my side, leaving as COMMENTED for Miguel + James to stamp.
Concerns
Nothing blocking. Two small notes:
packages/studio/src/utils/authoredOpacity.ts:26-28—applyAuthoredInlineOpacitywrites withstyle.setProperty("opacity", authored)(no priority), andinstallAuthoredOpacityCapturereads viael.style.opacity(priority-blind). So an authoredopacity: 0.5 !importantround-trips asopacity: 0.5— the!importantis silently dropped. Vanishingly rare in HF templates (the grading engine's own!importantis a runtime value, not authored, and everything else is regular declarations), but worth a docstring note so a future reader knows the round-trip is priority-lossy by design.packages/studio/src/hooks/gsapRuntimeReaders.ts:109-111—for (const p of propKeys) if (!inGroup(p)) propKeys.delete(p). Mutating a Set duringfor...ofiteration is defined per spec (each entry is processed at most once, never crashes), but it reads as a footgun. A one-linerpropKeys = new Set([...propKeys].filter(inGroup))— or copying to an array before iterating — is defensively clearer and cheap.
Questions
gsapSoftReload.ts:319-352: thecssTextsave/restore/strip-transform/apply-authored-opacity dance is subtle. If the savedcssTextalready carriedopacity: 0 !important(the grading hide, since the save happens while the runtime is live), and the authored value is"",applyAuthoredInlineOpacitycallsremoveProperty("opacity")— perfect, priority and value gone together. But for a non-empty authored value,setProperty("opacity", authored)sets it without!important. Is that intentional priority downgrade fine, or is there a path where the CSS cascade might then re-hide the element via a#el { opacity: 0 !important }block in the stylesheet? I think it's fine (HF authored stylesheets don't!important-hide graded sources by convention), but worth confirming.
What I didn't verify
- The 45-check scripted browser E2E from the PR body — I didn't re-run it. Trusted the "all passing" claim and CI's Windows/regression coverage as the substitute.
- Encoded render output. Miguel called this out as not covered; the runtime redraw hooks the E2E exercised are the same ones the render path uses, so I'm comfortable that graded elements will render correctly at capture time as long as the runtime is behaving.
- Undo/redo sequences and multi-select group gestures on graded elements — also called out as not covered.
The deselect restore read a ref recomputed from RENDER state — on a direct A→B selection switch, state re-syncs to B before A's effect cleanup runs, so after a committed crop gesture A was restored with B's crop string (or lost its crop when B had none). The committed value is now written at gesture-commit time (tri-state: none committed / crop removal / the exact committed string), so cleanup never touches render state. Adds component tests for lift/restore ordering, including the direct A→B switch and the uneditable-clip stand-down.
- merge gsapResizeIntercept's duplicate module imports - move the core-constant imports to the file headers (picker, domEditingDom) - justify the cross-realm HTMLElement casts (iframe-realm nodes fail instanceof; access is duck-typed)
vanceingalls
left a comment
vanceingalls
left a comment
There was a problem hiding this comment.
Verdict: 🟡 LGTM-conditional at 67cfae25
Reviewed at 67cfae25 (head advanced from fe529667 via b7fddd5 — crop A→B restore fix — and 67cfae2 — import/comment cleanup). Miguel's PR body enumerates 9 fixes; each maps cleanly to the failure mode it diagnoses and lands at the causative layer, not the symptom. Two under-scoped items of my own that Rames-D didn't cover (Fix 2 partial + Fix 6 stale-snapshot ordering hazard on legacy files). Neither blocks a stamp; both are worth a scoped follow-up. Peer-lens with Rames' three surface-level concerns (priority-lossy !important round-trip, propKeys.delete mutation, stylesheet-hide question) — I concur with all three, non-blocking.
Fix-by-fix disposition
Fix 1 — Elements vanish/dim after edits — ✅ addressed. Parse-time authored-opacity stamp + observer at packages/core/src/runtime/colorGrading.ts:224-244. patchRuntimeTweenInPlace invalidates only the edited tween and restores authored opacity on opacity-touching changes (packages/studio/src/hooks/gsapRuntimePatch.ts:306-311). Soft reload's readAuthoredOpacity prefers fresh HTML (via data-hf-id), falls back to runtime stamp, then removes !important 0. The "" vs null tri-state is honored across hideSourceElement / restoreSourceElement / applyAuthoredInlineOpacity / the soft-reload restore loop.
Fix 2 — Rotate/resize wrote opacity: 0 into keyframes — gsapRuntimeBridge.ts:334-336 passes "rotation" filter to readAllAnimatedProperties, and inGroup(p) at gsapRuntimeReaders.ts gates both propKeys and otherTweenProps when a group is supplied). However, resize intercept still calls readAllAnimatedProperties(iframe, selector, anim) with no group filter at two sites in packages/studio/src/hooks/gsapResizeIntercept.ts (the runtimeProps call and the resolvedFromValues call). The runtimeProps case is harmless (backfillDefaults only propagates Object.keys(resizeProps)). But resolvedFromValues is consumed by resolveConversionProps in gsapSerialize.ts:483-511, and for a from(scale) / to(scale) intro tween being resize-converted, extra opacity properties merge into toProps. insertKeyframesProp(varsArg, fromProps, toProps, ...) in gsapParser.ts:2570-2575 then writes them into the converted keyframes. Net effect: a resize on a scale-driven element whose intro from() touches opacity converts to a keyframed scale tween carrying opacity in toProps — the exact class of bake this PR eliminates for rotation. Narrow window (requires resize routing through convert-to-keyframes AND the same element having an opacity-touching intro), and none of the added tests cover this path — that's why I don't strong-block. Follow-up wants a "scale" (or "size") filter passed to those two readAllAnimatedProperties calls.
Fix 3 — Graded canvas frozen during drags — ✅ addressed. rAF-throttled MutationObserver on [style] at colorGrading.ts:1480-1508, signature over transform/translate/rotate/scale/left/top/width/height, ignores opacity/visibility so drawEntry's own writes don't loop. Behavioral tests direct. Cleanup wired into entry.cleanup.push → disconnected on destroyEntry.
Fix 4 — Resize on scale-driven elements — ✅ addressed for the enumerated scenarios. contentScaleX/Y = rect.width / (rect.editScaleX * actualWidth) fixes the drag-vs-cursor divergence. resizeAnchor accumulates pin drift via setElementGsapPosition. nonUniformScale > 0.01 triggers longhand-commit + scale → scaleX/scaleY keyframe normalization. finalizeScaleResizeCommit measures residual between dropRect and post-commit BCR, corrects synchronously — only for static (non-live-position-tween) elements. Anchor pin restored on movedDistance < BLOCKED_MOVE_THRESHOLD_PX, on .catch branch, on cancel. Minor: for the SIZE (width/height) route on an element with a non-GSAP-driven live scale (e.g. transform: scale(1.5) from CSS), the pin accumulates during move but there's no post-commit anchor overwrite (only scale route calls finalizeScaleResizeCommit). Brief jitter until next soft reload — rare, self-healing.
Fix 5 — Circle/polygon/percentage clip destroyed by select+deselect — ✅ addressed, hardened by b7fddd5. Tri-state (null = uneditable, zeros = croppable-uncropped, values = inset). readElementCropInsets → null for non-inset shapes; hasCrop/committedRef route unchanged for editable clips; preLiftInlineClipRef captures pre-lift verbatim; the early return at the top of the lift effect ensures uneditable clips are never rewritten. b7fddd5 additionally fixes a race in the deselect restore: previous committedRef.current = hasCrop ? build(state.insets, radius) : null recomputed on every render, so on a direct A→B selection switch the ref carried B's clip by the time A's cleanup ran, and A got restored with B's clip. New code writes committedClipRef only from the commit callback with the exact committed value — null = nothing committed → restore pre-lift; "" = crop removal committed → removeProperty; string = the committed inset. Cleanup never reads render state. New tests at DomEditCropHandles.test.tsx lock the A→B ordering + uneditable-clip stand-down + committed-value restore paths.
Fix 6 — Panel W edit contaminated the position set — ✅ addressed for the causative path; commitStaticSet at useAnimatedPropertyCommit.ts group-batches (byGroup: Map<group, entries>); each group finds its own set by propertyGroup === group first. Undo label derives from the group. Ordering hazard: the sets snapshot is captured once before the group loop; each iteration is a serialized await commit(...). For a pre-existing mixed set (position + size in one node — the legacy shape this PR stops creating but doesn't heal), the first group's commitSetProps call may cause the server to split the set; the second iteration then targets a stale existingSet.id. Miguel's PR body acknowledges legacy files carry baked damage this PR doesn't heal, so this fits that framing — worst case a 404 toast on legacy files, no data loss. Follow-up wants either a refetch between iterations or per-group serialize + refresh.
Fix 7 — immediateRender: true, immediateRender: true — ✅ addressed. emitted: Set seeds from Object.keys(props); the conditional immediateRender push checks emitted + extras; extras loop checks emitted. Acorn twin parallel (gsapWriterAcorn.ts:52+ and :265+). Semantics: properties win over extras. Regression tests on each writer path so future drift is caught.
Fix 8 — Paired x/y commits with no-op second half — ✅ addressed for the happy path; server-contract Q. On changed === false, applyPreviewSync fires when !options.skipReload && options.instantPatch. Idempotent when file and runtime already agree. Server-contract concern: applyPreviewSync tries patchRuntimeTweenInPlace, then falls through to softReloadOrEscalate(iframe, result.scriptText, …). If the server returns {changed: false} without scriptText (unverified from the diff) AND the instant patch happens to fail, a genuinely-no-op commit would trigger a full preview reload. Tests only exercise instant-patch-succeeds. Worth confirming server contract.
Fix 9 — Lint gsap_from_opacity_noop boundary anchor — ✅ addressed. opacityExactlyZero = /opacity\s*:\s*0(?:\.0+)?\s*(?:;|$)/. Walked against 0, 0.0, 0.00, 0.98, 0.000001, 0. — all classify correctly. The $ catches "last declaration without a semicolon" (block bodies have } stripped by outer regex).
Cross-fix cluster patterns
Two recurring shapes across the 9 fixes:
- Runtime-transient vs authored-value confusion (Fix 1, Fix 2 partial, soft-reload restore in Fix 1). Umbrella = parse-time authored-opacity stamp +
data-hf-authored-opacity+ grading-canvas opacity detour. This umbrella covers rotation but not resize'sresolvedFromValues— see Fix 2 disposition. Folding a group filter into the tworeadAllAnimatedPropertiescalls ingsapResizeIntercept.tscloses it. - Selection-side-effect that mutates authored state (Fix 5, Fix 6, and the unlisted marquee fresh-hit-test at
DomEditOverlay.tsx:309-320). All three land on the same insight: a decorative UI action (select, marquee-start, lift-for-crop) shouldn't reformat or destroy authored data unless the user actually committed something.
Additional findings
- Unenumerated bonus fix in
DomEditOverlay.tsx:309-320— synchronous hit-test guard before starting a marquee. Not in Miguel's PR body. Behaviorally sound (usesgetPreviewTargetFromPointer). Minor consistency nit: line 318 uses the propactiveCompositionPathdirectly, while surrounding code usesactiveCompositionPathRef.current. Handler isn'tuseCallback-wrapped so it captures the render's value — functionally equivalent, pattern-inconsistent. Non-blocking. installAuthoredOpacityCapturehas no teardown — intentional per the docstring ("stays alive so late insertions are stamped"). No leak since it lives with the runtime bundle.
Peer-lens with Rames-D
Concurring with his three concerns; not duplicating:
applyAuthoredInlineOpacity!importantround-trip — priority-lossy by design; worth a docstring per his suggestion.propKeys.deleteduringfor...of— defined per spec but reads as a footgun; thefilterrewrite is cheap.cssTextrestore stylesheet-hide question — I concur it's safe by HF convention (authored stylesheets don't!important-hide graded sources), and worth confirming with Miguel.
Adversarial pass
Initial draft had Fix-2-partial as a blocker. Downgraded to from()/to() intro on opacity AND (b) resize routes through convert-to-keyframes rather than outside-range or already-keyframed. Narrow, and the enumerated repro is rotation-driven (fixed).
Fix 6 stale-snapshot initially read as a blocker. Downgraded to
Peer-lens check confirms my two picks don't overlap with Rames' three. Miguel's band-aid bar not tripped — the rAF-throttled style observer is a legitimate mechanism, the pin-anchor is an accumulator-with-convergence, and none of the fixes read as "silence the symptom." Ship it after Fix 2's group-filter follow-up if Miguel confirms the read.
R1 by Via
Review follow-ups (both reviewers, all findings): - resize captures scope to the resize group: convert-to-keyframes resolvedFromValues and the whole-offset backfill pass the group filter, so an opacity-touching intro tween can't ride into a converted scale tween (the rotation fix's contract, now uniform across intercepts) - commitStaticSet resolves every group's target set BEFORE committing and coalesces groups landing on the same legacy mixed set into one commit — the second commit can no longer chase a stale group-derived id - installAuthoredOpacityCapture also stamps an element the moment it GAINS data-color-grading at runtime (attributeFilter), not just at insertion - both writer twins now share the same emitted-set dedupe shape - applySoftReload's positional tail becomes a SoftReloadOptions object - readAllAnimatedProperties builds the group-filtered key set immutably instead of deleting from the set mid-iteration - applyAuthoredInlineOpacity documents the priority-lossy round-trip - the marquee hit-test reads activeCompositionPathRef like its neighbors New tests: resize intercept (scale route + group filter + non-uniform longhands), after-write-HTML / stamp / empty-stamp opacity restore, the no-op-commit-with-missed-instant-patch soft-reload contract, and the runtime-gained-grading stamp.
|
Addressed everything from both reviews in
Also in the same push (from a self-review pass): both writer twins now share the identical Suites after all of it: studio 1572, parsers 808, core 1190, lint 355; typecheck + lint + audit gates green. |
vanceingalls
left a comment
vanceingalls
left a comment
There was a problem hiding this comment.
R2 — 🟢 LGTM at 5d1cafff
Verified my R1 items + Rames-D's three concerns cleanly addressed against 67cfae25...5d1cafff (base-rebase against main + address-review commits folded in).
Per-finding disposition at 5d1cafff
✅ Fix 2 partial — resize group filter added. packages/studio/src/hooks/gsapResizeIntercept.ts:113 and :262 now both call readAllAnimatedProperties(iframe, selector, anim, resizeGroup). Exactly the shape my R1 suggested — the resolvedFromValues path (which feeds resolveConversionProps → insertKeyframesProp) now filters by resize group, so a resize on a scale-driven element with an opacity-touching intro from() no longer routes opacity into the converted keyframes. Umbrella (runtime-transient vs authored) now covers all three edit-intercept paths (rotation + resize-runtime + resize-conversion). Upgrade my R1
✅ Rames-D — propKeys.delete during for...of — filter idiom. packages/studio/src/hooks/gsapRuntimeReaders.ts:109 — const groupedPropKeys = new Set([...propKeys].filter(inGroup));. Mutation-during-iteration footgun eliminated; downstream reads use the filtered set. Exactly the rewrite Rames suggested.
✅ Rames-D — applyAuthoredInlineOpacity priority-lossy round-trip — docstring landed. packages/studio/src/utils/authoredOpacity.ts:27-30 — the docstring now explicitly names the invariant: "authored opacity: X !important round-trips as opacity: X. The only !important opacity in the pipeline is [runtime-transient] discard — and authored compositions don't !important their opacity." Future readers get the priority contract without archaeology.
packages/studio/src/hooks/useAnimatedPropertyCommit.ts:188 — const sets = animations.filter(...) still captured once before the group loop at L196. Not refetched between iterations. This is the legacy-file 404 edge on pre-existing mixed sets — Miguel's PR body explicitly frames it as "stops new damage, does not heal existing files," so consistent with intent. Follow-up would either refetch between iterations or per-group serialize + refresh; not gated on this PR.
CI + convergence
CI green across the fast lanes at 5d1cafff. No new prior-reviewer state since Rames-D's R1 (fe52966) and my R1 (67cfae2) — my R2 is the first review at the new head. Miguel + Rames-D + I converged on the same three surface concerns; all three landed. The one open item (Fix 6 legacy healer) is out of scope by author decision.
Verdict
R1 verdict 🟡 LGTM-conditional → R2 verdict 🟢 LGTM at 5d1cafff. Ship it — the legacy-set healer is worth its own scoped PR.
R2 by Via
james-russo-rames-d-jusso
left a comment
james-russo-rames-d-jusso
left a comment
There was a problem hiding this comment.
Reviewed at 5d1caff — delta from R1 (fe52966).
R1 fully addressed, and Miguel went materially further:
- Both R1 nits landed cleanly.
applyAuthoredInlineOpacitygains an explicit docstring on the priority-lossy round-trip and why it's safe by design (the only!importantopacity in the pipeline is the transient grading hide, which the contract exists to discard).readAllAnimatedPropertiesswaps the mutate-during-iteration for an immutablegroupedPropKeys = new Set([...propKeys].filter(inGroup))— and correctly threads it through both theotherTweenProps.deleteandallTweenedPropsunions so no downstream semantics shift. - R1 clarifying Q on the soft-reload cssText path is answered by that same authored-opacity docstring — the
cssTextsave/restore path itself preserves priority (round-trips as string); the only priority-lossy step is the authored-opacity apply, which is intentional.
Beyond R1:
commitStaticSetstale-id-chase fix.findGroupOwningSetis extracted, and every group's target is resolved BEFORE any commit runs. Groups landing on the same legacy mixed set coalesce into one commit, so the second commit can no longer chase an id that the first commit's write reshaped server-side. Fresh-add batches are separated because their ids can't go stale. This closes a real footgun in the pre-split legacy shape.gsapResizeInterceptgroup-scoped captures. Both the initialreadAllAnimatedPropertiesand the convert-to-keyframesresolvedFromValuespassresizeGroup, so an opacity-touching intro tween on the element can't bake into a converted scale tween. Uniform with the rotation intercept's contract — this is the Fix-2 class made consistent.installAuthoredOpacityCapturecatches runtime-gained grading. Addingattributes: true+attributeFilter: [HF_COLOR_GRADING_ATTR]stamps at the moment a studio preset attaches, strictly earlier than the engine hide. Idempotent stamp means late re-inserts remain a no-op. Load-bearing for the studio grading flow — glad this is here.- Writer parity.
gsapWriterAcorn.ts:buildTweenStatementCodenow uses the sameemittedSet shape as the recast twin, with a comment naming the twin — kills the drift risk I was watching for. - Marquee hit-test path fix.
getPreviewTargetFromPointernow readsactiveCompositionPathRef.currentinstead of the closed-overactiveCompositionPath, matching the neighbors' ref-read discipline. - b7fddd5 — self-caught bug. The deselect restore was reading a ref recomputed from render state, so a direct A→B selection switch restored A with B's crop (or lost A's crop entirely when B had none). Moving the committed value to gesture-commit time with a proper tri-state (
null= no commit /""= commit removed the crop / value = commit installed the crop) is exactly the ordering discipline that path needed. Tests cover the direct A→B switch and the uneditable-clip stand-down. Great catch.
Tests. +364 LOC across colorGrading.test.ts (runtime-gained stamp), DomEditCropHandles.test.tsx (lift/restore, A→B, uneditable), gsapResizeIntercept.test.ts (scale route + group filter + non-uniform longhands), gsapSoftReload.test.ts (after-write-HTML / stamp / empty-stamp restore + no-op-commit-with-missed-instant-patch), and a rewrite of useGsapScriptCommits.test.tsx for the new options-object call shape. Each test maps 1:1 to a behavior added or fixed in this round.
CI. All required lanes green at HEAD; one regression-shard still running when I looked — not a required gate for merge given the rest of the matrix.
LGTM from my side. Pinging <@U0ARJFN5S6Q> for the stamp.
Selecting a rotated (cropped) element appeared to straighten it: the crop dim and dashed window were drawn on the axis-aligned bounding box, so the bright window was a straight rectangle and the element's rotated corners were masked to near-black — while the DOM transform was untouched. clip-path applies in the element's LOCAL frame, before its transform, so the crop visualization now renders inside a container rotated with the element: readElementCropFrame decomposes the computed 2D matrix into angle + per-axis scale (element scale finally factored into the px mapping too) and 3D/unparseable transforms keep the axis-aligned presentation. Pointer deltas rotate into the element frame before the inset resolvers, so edge/pan drags track the rotated handles correctly.
3 participants
What
Fixes the family of studio manual-editing bugs on compositions with color-graded elements: elements disappearing or dimming permanently after edits, panel edits not reflecting until deselect, resize/rotate gestures corrupting the composition file, selection visually un-clipping elements (and deleting non-inset clips on deselect), and duplicate keys accumulating in written GSAP calls.
Concretely fixed:
opacity: 0 !important, or a mid-flight tween value — so GSAP re-captured poisonedfrom()/to()bounds and the element rendered invisible from then on. Now only the edited tween is patched in place, and the soft reload restores every animated element's authored inline opacity (from the after-write HTML, falling back to a parse-time stamp) before the script re-runs.opacity: 0(and foreign properties) into keyframes on disk. The convert-to-keyframes resolver read "current values" viagsap.getProperty— which returns the grading hide, not the animated value — and its baseline capture passes ignored the property-group filter, so a rotation commit picked upopacity/rotationXfrom unrelated tweens. A grading-hidden source's opacity is now read from its canvas (the effective value), and the group filter governs every capture pass.clip-pathon selection and restored only what it could parse as a px inset. The crop reader is now tri-state; for clips the tool cannot represent it stands down entirely (no lift, no handles), and the deselect restore is the pre-lift inline value verbatim unless a crop gesture actually committed.tl.set("#el",{x,y,width}), labeled "Set 3D transform" in undo): the static-set commit picked the first set for the selector group-blind. Commits now batch per property group into a group-owning set, with group-derived undo labels.immediateRender: true, immediateRender: trueaccumulated in written sets: the statement builder pushed the flag unconditionally and also appended parsed extras carrying it. Both writers (recast and acorn) now emit each vars key exactly once.clip-pathactually applies), with pointer deltas rotated into that frame; element scale is factored into the px mapping; 3D transforms keep the axis-aligned fallback.gsap_from_opacity_noopmatchedopacity: 0.98via an unanchored prefix; the exactly-zero predicate is now boundary-anchored (block + inline, including a block's last declaration without a semicolon).Why
Reported against real graded compositions: manual edits in the studio made elements disappear, and the corruption persisted in the composition file (baked
opacity: 0keyframes, duplicated flags, mixed-group sets), so projects degraded with every editing session.Note: files edited on affected builds already carry that baked damage; this PR stops new damage but does not heal existing files.
How
@hyperframes/core/color-grading; the runtime stampsdata-hf-authored-opacityon[data-color-grading]elements at document parse time (observer installed at bundle eval, before composition scripts run), so authored opacity survives any later runtime mutation.gsapResizeIntercept.ts); shared group-tween resolution stays ingsapRuntimeBridge.readElementCropInsetsdistinguishes "no clip" (zeros) from "clip this tool can't edit" (null); everything crop-related derives from that one classification.Test plan
/previeworacle.Not covered: encoded render output (the render path uses the same runtime redraw hooks the E2E exercised, but frames were not diffed), undo/redo sequences, and multi-select group gestures on graded elements.