From 3147c8e063a72d999447aca8bb0528480bdcd043 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sat, 11 Jul 2026 03:33:32 -0400 Subject: [PATCH 01/11] fix(core): capture authored inline opacity at parse time and follow source 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. --- packages/core/src/colorGrading.ts | 12 +++ .../core/src/runtime/colorGrading.test.ts | 91 ++++++++++++++++++- packages/core/src/runtime/colorGrading.ts | 89 +++++++++++++++++- packages/core/src/runtime/entry.ts | 6 ++ packages/core/src/runtime/picker.ts | 2 +- 5 files changed, 195 insertions(+), 5 deletions(-) diff --git a/packages/core/src/colorGrading.ts b/packages/core/src/colorGrading.ts index d71ce1776..d5411e20a 100644 --- a/packages/core/src/colorGrading.ts +++ b/packages/core/src/colorGrading.ts @@ -1,5 +1,17 @@ export const HF_COLOR_GRADING_ATTR = "data-color-grading"; +// Runtime <-> studio contract attributes. The runtime grading engine writes +// them; studio editing/soft-reload code reads them. Single owner — never +// re-declare these literals elsewhere. +/** Set on a graded source while its pixels render on the grading canvas. */ +export const COLOR_GRADING_SOURCE_HIDDEN_ATTR = "data-hf-color-grading-source-hidden"; +/** + * The element's AUTHORED inline opacity, stamped at document parse time before + * any animation engine mutates it ("" = authored none; attribute absent = + * never captured). See installAuthoredOpacityCapture in the runtime. + */ +export const COLOR_GRADING_AUTHORED_OPACITY_ATTR = "data-hf-authored-opacity"; + export const HF_COLOR_GRADING_CANVAS_ID_PREFIX = "__hf_color_grading_"; export const HF_COLOR_GRADING_COLOR_SPACE = "rec709"; diff --git a/packages/core/src/runtime/colorGrading.test.ts b/packages/core/src/runtime/colorGrading.test.ts index e3a3ce49d..cf3e0e048 100644 --- a/packages/core/src/runtime/colorGrading.test.ts +++ b/packages/core/src/runtime/colorGrading.test.ts @@ -1,7 +1,11 @@ // fallow-ignore-file code-duplication import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { HF_COLOR_GRADING_ATTR, serializeHfColorGrading } from "../colorGrading"; -import { createColorGradingRuntime, type RuntimeColorGradingApi } from "./colorGrading"; +import { + createColorGradingRuntime, + installAuthoredOpacityCapture, + type RuntimeColorGradingApi, +} from "./colorGrading"; let lastUniform1f: ReturnType | null = null; let lastUniform3f: ReturnType | null = null; @@ -192,6 +196,60 @@ describe("createColorGradingRuntime", () => { runtime?.redraw(); } + it("restores the authored inline opacity captured before animation transients", () => { + const video = makeDrawableVideo(); + // Parse-time capture stamped the authored value; by hide time GSAP has + // already left a from()-tween transient (0) in the inline style. + video.setAttribute("data-hf-authored-opacity", "0.75"); + video.style.opacity = "0"; + startRuntimeWithVideo(video); + + expect(video.style.getPropertyPriority("opacity")).toBe("important"); + + runtime?.destroy(); + runtime = null; + + // Restore must use the authored 0.75, not the GSAP transient 0. + expect(video.style.getPropertyValue("opacity")).toBe("0.75"); + expect(video.style.getPropertyPriority("opacity")).toBe(""); + }); + + it("restores no inline opacity when the authored capture recorded none", () => { + const video = makeDrawableVideo(); + video.setAttribute("data-hf-authored-opacity", ""); + video.style.opacity = "0"; + startRuntimeWithVideo(video); + + runtime?.destroy(); + runtime = null; + + expect(video.style.getPropertyValue("opacity")).toBe(""); + }); + + it("re-syncs the graded canvas when the source's inline transform changes", async () => { + const { video } = startRuntimeWithVideo(); + const drawsBefore = texImage2DCalls.length; + + // Simulate a studio drag draft: only the inline transform moves. + video.style.transform = "translate(120px, 60px)"; + await new Promise((resolve) => requestAnimationFrame(() => resolve(null))); + + expect(texImage2DCalls.length).toBeGreaterThan(drawsBefore); + }); + + it("does not redraw-loop on its own hide writes (opacity/visibility only)", async () => { + const { video } = startRuntimeWithVideo(); + await new Promise((resolve) => requestAnimationFrame(() => resolve(null))); + const drawsBefore = texImage2DCalls.length; + + // drawEntry's own source-hide writes touch opacity — geometry unchanged. + video.style.opacity = "0.5"; + await new Promise((resolve) => requestAnimationFrame(() => resolve(null))); + await new Promise((resolve) => requestAnimationFrame(() => resolve(null))); + + expect(texImage2DCalls.length).toBe(drawsBefore); + }); + it("re-hides source media after timeline visibility sync", () => { const { video, canvas } = startRuntimeWithVideo(); @@ -555,3 +613,34 @@ describe("createColorGradingRuntime", () => { expect(video.style.getPropertyValue("opacity")).toBe("0"); }); }); + +describe("installAuthoredOpacityCapture", () => { + it("stamps graded elements at insertion and never overwrites the stamp", async () => { + installAuthoredOpacityCapture(); + const el = document.createElement("img"); + el.setAttribute(HF_COLOR_GRADING_ATTR, serializeHfColorGrading({ adjust: { exposure: 0.5 } })); + el.style.opacity = "0.98"; + document.body.appendChild(el); + await Promise.resolve(); + expect(el.getAttribute("data-hf-authored-opacity")).toBe("0.98"); + + // A re-insert after an animation engine mutated the element keeps the + // original capture (has-attribute guard). + el.style.opacity = "0"; + el.remove(); + document.body.appendChild(el); + await Promise.resolve(); + expect(el.getAttribute("data-hf-authored-opacity")).toBe("0.98"); + el.remove(); + }); + + it("stamps an empty value for graded elements without an authored inline opacity", async () => { + installAuthoredOpacityCapture(); + const el = document.createElement("img"); + el.setAttribute(HF_COLOR_GRADING_ATTR, serializeHfColorGrading({ adjust: { exposure: 0.5 } })); + document.body.appendChild(el); + await Promise.resolve(); + expect(el.getAttribute("data-hf-authored-opacity")).toBe(""); + el.remove(); + }); +}); diff --git a/packages/core/src/runtime/colorGrading.ts b/packages/core/src/runtime/colorGrading.ts index 5b32c6c65..75e85ebd8 100644 --- a/packages/core/src/runtime/colorGrading.ts +++ b/packages/core/src/runtime/colorGrading.ts @@ -6,6 +6,8 @@ import { normalizeHfColorGradingWithVariables, type HfColorGradingTarget, type NormalizedHfColorGrading, + COLOR_GRADING_SOURCE_HIDDEN_ATTR, + COLOR_GRADING_AUTHORED_OPACITY_ATTR, } from "../colorGrading"; import { DEFAULT_MAX_CUBE_LUT_SIZE, @@ -196,8 +198,51 @@ type LutCacheEntry = const LUT_CACHE = new Map(); const COLOR_GRADING_CANVAS_ATTR = "data-hf-color-grading-canvas"; -const COLOR_GRADING_SOURCE_HIDDEN_ATTR = "data-hf-color-grading-source-hidden"; const COLOR_GRADING_CANVAS_CLASS = "__hf_color_grading_canvas__"; + +/** + * Capture each color-graded element's AUTHORED inline opacity before any + * animation engine can mutate it. + * + * The grading engine hides its source elements with `opacity: 0 !important` + * and mirrors their pixels onto a canvas — so at runtime, a graded element's + * inline/computed opacity no longer represents authored state. Everything that + * later re-reads element state (GSAP from()-tween re-initialization after an + * invalidate or a studio soft reload, restoring the source when grading is + * removed, lint/selection tooling) needs the authored value, and by then it is + * unrecoverable from the DOM. Stamp it onto the element as + * `data-hf-authored-opacity` (empty string = no authored inline opacity). + * + * Must be installed at runtime-bundle evaluation, while the document is still + * parsing: the runtime ` +`; + const result = await lintHyperframeHtml(html); + const finding = result.findings.find((f) => f.code === "gsap_from_opacity_noop"); + expect(finding).toBeDefined(); + }); + + it("does NOT error for inline opacity: 0.98 + gsap.from({opacity:0}) — fractional is not zero", async () => { + const html = ` + +
+ +
+ +`; + const result = await lintHyperframeHtml(html); + const finding = result.findings.find((f) => f.code === "gsap_from_opacity_noop"); + expect(finding).toBeUndefined(); + }); + + it("still errors for inline opacity: 0 without a trailing semicolon", async () => { + const html = ` + +
+
Hello
+
+ +`; + const result = await lintHyperframeHtml(html); + const finding = result.findings.find((f) => f.code === "gsap_from_opacity_noop"); + expect(finding).toBeDefined(); + }); + it("does NOT error when gsap.from({opacity:0}) and CSS has no opacity:0", async () => { const html = ` diff --git a/packages/lint/src/rules/gsap.ts b/packages/lint/src/rules/gsap.ts index b54b7c873..ec2e4c9b8 100644 --- a/packages/lint/src/rules/gsap.ts +++ b/packages/lint/src/rules/gsap.ts @@ -1128,11 +1128,18 @@ export const gsapRules: LintRule[] = [ const findings: HyperframeLintFinding[] = []; const cssOpacityZeroSelectors = new Set(); + // Single owner of "this declaration list sets opacity to EXACTLY zero" — + // boundary-anchored so `opacity: 0.98` never matches. Works for both a CSS + // block body (brace already stripped by the block regex) and an inline + // style attribute: the declaration ends at `;` or at end of input, which + // also catches a final declaration without a trailing semicolon. + const opacityExactlyZero = /opacity\s*:\s*0(?:\.0+)?\s*(?:;|$)/; + for (const style of styles) { for (const [, selector, body] of style.content.matchAll( /([#.][a-zA-Z0-9_-]+)\s*\{([^}]+)\}/g, )) { - if (body && /opacity\s*:\s*0\s*[;}]/.test(body)) { + if (body && opacityExactlyZero.test(body)) { cssOpacityZeroSelectors.add((selector ?? "").trim()); } } @@ -1140,7 +1147,7 @@ export const gsapRules: LintRule[] = [ for (const tag of tags) { const inlineStyle = readAttr(tag.raw, "style"); - if (!inlineStyle || !/opacity\s*:\s*0/.test(inlineStyle)) continue; + if (!inlineStyle || !opacityExactlyZero.test(inlineStyle)) continue; const id = readAttr(tag.raw, "id"); const classes = readAttr(tag.raw, "class")?.split(/\s+/).filter(Boolean) ?? []; if (id) cssOpacityZeroSelectors.add(`#${id}`); diff --git a/packages/studio/src/components/editor/domEditingDom.ts b/packages/studio/src/components/editor/domEditingDom.ts index cfd347445..82b564978 100644 --- a/packages/studio/src/components/editor/domEditingDom.ts +++ b/packages/studio/src/components/editor/domEditingDom.ts @@ -28,7 +28,7 @@ export function isTextBearingTag(tagName: string): boolean { return ["div", "span", "p", "strong", "h1", "h2", "h3", "h4", "h5", "h6"].includes(tagName); } -const COLOR_GRADING_SOURCE_HIDDEN_ATTR = "data-hf-color-grading-source-hidden"; +import { COLOR_GRADING_SOURCE_HIDDEN_ATTR } from "@hyperframes/core/color-grading"; export function isElementVisibleThroughAncestors(el: HTMLElement): boolean { const win = el.ownerDocument.defaultView; From b7fddd548acf1d268294a7fe71adc9a946ccaa40 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sat, 11 Jul 2026 14:58:17 -0400 Subject: [PATCH 08/11] fix(studio): crop restore value is owned by the lifted element's gesture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../editor/DomEditCropHandles.test.tsx | 86 +++++++++++++++++++ .../components/editor/DomEditCropHandles.tsx | 37 ++++---- 2 files changed, 107 insertions(+), 16 deletions(-) create mode 100644 packages/studio/src/components/editor/DomEditCropHandles.test.tsx diff --git a/packages/studio/src/components/editor/DomEditCropHandles.test.tsx b/packages/studio/src/components/editor/DomEditCropHandles.test.tsx new file mode 100644 index 000000000..09c73decc --- /dev/null +++ b/packages/studio/src/components/editor/DomEditCropHandles.test.tsx @@ -0,0 +1,86 @@ +// @vitest-environment happy-dom +import React, { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { afterEach, describe, expect, it } from "vitest"; +import type { DomEditSelection } from "./domEditing"; +import type { OverlayRect } from "./domEditOverlayGeometry"; +import { DomEditCropHandles } from "./DomEditCropHandles"; + +(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +afterEach(() => { + document.body.innerHTML = ""; +}); + +const overlayRect: OverlayRect = { + left: 0, + top: 0, + width: 200, + height: 100, + editScaleX: 1, + editScaleY: 1, +}; + +function selectionFor(el: HTMLElement): DomEditSelection { + return { element: el, id: el.id, selector: `#${el.id}` } as unknown as DomEditSelection; +} + +function makeEl(id: string, clip: string): HTMLElement { + const el = document.createElement("div"); + el.id = id; + if (clip) el.style.setProperty("clip-path", clip); + document.body.append(el); + return el; +} + +function render(el: HTMLElement): { root: Root; rerender: (next: HTMLElement) => void } { + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + const draw = (target: HTMLElement) => + act(() => { + root.render( + undefined} + />, + ); + }); + draw(el); + return { root, rerender: draw }; +} + +// Regression: the deselect restore used 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 A used to get B's crop string (or lose its crop entirely). The +// restore value must be owned by A's own lift effect / crop gesture. +describe("DomEditCropHandles clip lift/restore", () => { + it("lifts on select and restores the inline clip verbatim on unmount", () => { + const a = makeEl("a", "inset(16px round 12px)"); + const { root } = render(a); + expect(a.style.getPropertyValue("clip-path")).toBe("none"); + act(() => root.unmount()); + expect(a.style.getPropertyValue("clip-path")).toBe("inset(16px round 12px)"); + }); + + it("restores A's own clip when switching directly to B", () => { + const a = makeEl("a", "inset(16px)"); + const b = makeEl("b", "inset(40px 8px 4px 2px)"); + const { root, rerender } = render(a); + rerender(b); + // A got ITS clip back, not B's (and not removed); B is now lifted. + expect(a.style.getPropertyValue("clip-path")).toBe("inset(16px)"); + expect(b.style.getPropertyValue("clip-path")).toBe("none"); + act(() => root.unmount()); + expect(b.style.getPropertyValue("clip-path")).toBe("inset(40px 8px 4px 2px)"); + }); + + it("never lifts an uneditable clip and leaves it untouched across select/deselect", () => { + const a = makeEl("a", "circle(50% at 50% 50%)"); + const { root } = render(a); + expect(a.style.getPropertyValue("clip-path")).toBe("circle(50% at 50% 50%)"); + act(() => root.unmount()); + expect(a.style.getPropertyValue("clip-path")).toBe("circle(50% at 50% 50%)"); + }); +}); diff --git a/packages/studio/src/components/editor/DomEditCropHandles.tsx b/packages/studio/src/components/editor/DomEditCropHandles.tsx index 746c774f9..7ccdaad3e 100644 --- a/packages/studio/src/components/editor/DomEditCropHandles.tsx +++ b/packages/studio/src/components/editor/DomEditCropHandles.tsx @@ -84,7 +84,7 @@ export function DomEditCropHandles({ const cropStateFor = (element: HTMLElement) => { const parsed = readElementCropInsets(element); const { radius, ...insets } = parsed ?? { top: 0, right: 0, bottom: 0, left: 0, radius: 0 }; - return { element, croppable: parsed !== null, insets: insets as ClipPathInsetSides, radius }; + return { element, croppable: parsed !== null, insets, radius }; }; const [state, setState] = useState(() => cropStateFor(selection.element)); @@ -102,32 +102,32 @@ export function DomEditCropHandles({ state.insets.bottom > 0 || state.insets.left > 0; - // Latest committed crop — re-applied to the element when the selection drops. - const committedRef = useRef(null); - committedRef.current = hasCrop ? buildInsetClipPathSides(state.insets, state.radius) : null; - // Lift the clip while the element is selected so the full content shows and the // cropped-away area can be dimmed; restore on deselect. Keyed on the element so // switching selections restores the previous one. Runs after render, so the // state re-sync above still reads the element's real committed clip. Restore // prefers the pre-lift inline value VERBATIM — the rebuilt inset only replaces // it after a crop gesture actually commits, so a mere select+deselect can - // never reformat (or drop) what the author wrote. + // never reformat (or drop) what the author wrote. Both refs are written only + // by THIS element's lift effect and crop gestures — never derived from render + // state, which by cleanup time already describes the NEXT selection (a direct + // A→B switch re-syncs state to B before A's cleanup runs). const liftedRef = useRef(false); const preLiftInlineClipRef = useRef(""); - const cropCommittedRef = useRef(false); + // null = no crop gesture committed this selection; "" = committed a crop + // removal; anything else = the exact committed clip-path value. + const committedClipRef = useRef(null); useEffect(() => { const el = selection.element; if (readElementCropInsets(el) === null) return; preLiftInlineClipRef.current = el.style.getPropertyValue("clip-path"); - cropCommittedRef.current = false; + committedClipRef.current = null; el.style.setProperty("clip-path", "none"); liftedRef.current = true; return () => { liftedRef.current = false; - const restore = cropCommittedRef.current - ? committedRef.current - : preLiftInlineClipRef.current || null; + const committed = committedClipRef.current; + const restore = committed !== null ? committed || null : preLiftInlineClipRef.current || null; if (restore) el.style.setProperty("clip-path", restore); else el.style.removeProperty("clip-path"); }; @@ -194,12 +194,17 @@ export function DomEditCropHandles({ const reLift = () => { if (liftedRef.current) el.style.setProperty("clip-path", "none"); }; - void Promise.resolve( - onStyleCommit?.("clip-path", buildInsetClipPathSides(state.insets, state.radius)), - ).then(() => { + const committedValue = buildInsetClipPathSides(state.insets, state.radius); + const cropped = + state.insets.top > 0 || + state.insets.right > 0 || + state.insets.bottom > 0 || + state.insets.left > 0; + void Promise.resolve(onStyleCommit?.("clip-path", committedValue)).then(() => { // Only a landed commit makes the rebuilt inset the restore value; a - // failed one keeps restoring the pre-lift clip. - cropCommittedRef.current = true; + // failed one keeps restoring the pre-lift clip. Store the value itself — + // by deselect time, render state describes the next selection. + committedClipRef.current = cropped ? committedValue : ""; reLift(); }, reLift); }; From 67cfae2587e15b0dd6a064d9fdbd4790831905cf Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sat, 11 Jul 2026 14:58:21 -0400 Subject: [PATCH 09/11] refactor: address review nits - 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) --- packages/core/src/runtime/picker.ts | 2 +- packages/studio/src/components/editor/domEditingDom.ts | 3 +-- packages/studio/src/hooks/gsapResizeIntercept.ts | 9 +++++---- packages/studio/src/utils/gsapSoftReload.ts | 6 ++++-- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/packages/core/src/runtime/picker.ts b/packages/core/src/runtime/picker.ts index e9ced66c2..52921fb82 100644 --- a/packages/core/src/runtime/picker.ts +++ b/packages/core/src/runtime/picker.ts @@ -1,4 +1,5 @@ import type { RuntimeJson, RuntimeOutboundMessage, RuntimePickerElementInfo } from "./types"; +import { COLOR_GRADING_SOURCE_HIDDEN_ATTR } from "../colorGrading"; import { swallow } from "./diagnostics"; type PickerModuleDeps = { @@ -17,7 +18,6 @@ const PICKER_BLOCK_SELECTOR = [ "[data-hyperframes-picker-block]", "[data-hyper-shader-loading]", ].join(","); -import { COLOR_GRADING_SOURCE_HIDDEN_ATTR } from "../colorGrading"; export type PickerModule = { enablePickMode: () => void; diff --git a/packages/studio/src/components/editor/domEditingDom.ts b/packages/studio/src/components/editor/domEditingDom.ts index 82b564978..b1f341ca7 100644 --- a/packages/studio/src/components/editor/domEditingDom.ts +++ b/packages/studio/src/components/editor/domEditingDom.ts @@ -3,6 +3,7 @@ * selector utilities, and composition source resolution. * No imports from other domEditing* modules — safe to import from anywhere. */ +import { COLOR_GRADING_SOURCE_HIDDEN_ATTR } from "@hyperframes/core/color-grading"; import { CURATED_STYLE_PROPERTIES } from "./domEditingTypes"; // ─── Type guard ─────────────────────────────────────────────────────────────── @@ -28,8 +29,6 @@ export function isTextBearingTag(tagName: string): boolean { return ["div", "span", "p", "strong", "h1", "h2", "h3", "h4", "h5", "h6"].includes(tagName); } -import { COLOR_GRADING_SOURCE_HIDDEN_ATTR } from "@hyperframes/core/color-grading"; - export function isElementVisibleThroughAncestors(el: HTMLElement): boolean { const win = el.ownerDocument.defaultView; if (!win) return true; diff --git a/packages/studio/src/hooks/gsapResizeIntercept.ts b/packages/studio/src/hooks/gsapResizeIntercept.ts index aee92be6f..9396384f2 100644 --- a/packages/studio/src/hooks/gsapResizeIntercept.ts +++ b/packages/studio/src/hooks/gsapResizeIntercept.ts @@ -16,19 +16,18 @@ import { commitStaticGsapSize, commitKeyframedSizeFromResize, computeCurrentPercentage, + findExistingPositionWrite, findSizeSetAnimation, materializeIfDynamic, } from "./gsapDragCommit"; import type { GsapDragCommitCallbacks } from "./gsapDragCommit"; -import { pickClosestToPlayhead } from "./gsapPositionDetection"; +import { pickClosestToPlayhead, readGsapPositionFromIframe } from "./gsapPositionDetection"; +import { commitWholePropertyOffset } from "./gsapWholePropertyOffsetCommit"; import { resolveTweenStart, resolveTweenDuration } from "../utils/globalTimeCompiler"; import { selectorFromSelection } from "./gsapShared"; import { roundTo3 } from "../utils/rounding"; import { resolveGroupTween, POSITION_CHANNELS } from "./gsapRuntimeBridge"; import { hasNonHoldTweenForElement } from "./gsapRuntimeKeyframes"; -import { readGsapPositionFromIframe } from "./gsapPositionDetection"; -import { findExistingPositionWrite } from "./gsapDragCommit"; -import { commitWholePropertyOffset } from "./gsapWholePropertyOffsetCommit"; const IDENTITY_ONE_PROPS = new Set(["opacity", "autoAlpha", "scale", "scaleX", "scaleY"]); @@ -114,6 +113,8 @@ export async function tryGsapResizeIntercept( let scaleDraftDropPoint: { x: number; y: number } | null = null; let nonUniformScale = false; if (resizeGroup === "scale") { + // Iframe-realm element — instanceof HTMLElement fails across realms; the + // selector targets composition elements, and every use below is duck-typed. const el = iframe?.contentDocument?.querySelector(selector ?? "") as HTMLElement | null; // The resize draft modifies el.style.width/height, so read the ORIGINAL // dimensions saved by the draft system before it ran. diff --git a/packages/studio/src/utils/gsapSoftReload.ts b/packages/studio/src/utils/gsapSoftReload.ts index a5f08eea3..f9f051fa8 100644 --- a/packages/studio/src/utils/gsapSoftReload.ts +++ b/packages/studio/src/utils/gsapSoftReload.ts @@ -319,8 +319,10 @@ export function applySoftReload( if (allTargets.length > 0 && win.gsap?.set) { const saved: Array<[HTMLElement, string]> = []; for (const el of allTargets) { - const s = (el as HTMLElement).style; - if (s?.cssText != null) saved.push([el as HTMLElement, s.cssText]); + // Iframe-realm node: instanceof HTMLElement fails across realms, and + // gsap targets() only yields elements here — style access is duck-typed. + const styled = el as HTMLElement; + if (styled.style?.cssText != null) saved.push([styled, styled.style.cssText]); } try { win.gsap.set(allTargets, { clearProps: "all" }); From 5d1cafff8225177ee1ae99da85a0ce0ce4e5babd Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sat, 11 Jul 2026 15:18:49 -0400 Subject: [PATCH 10/11] fix(studio): address review findings on graded-element editing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../core/src/runtime/colorGrading.test.ts | 22 +++ packages/core/src/runtime/colorGrading.ts | 15 +- packages/parsers/src/gsapWriterAcorn.ts | 8 +- .../src/components/editor/DomEditOverlay.tsx | 7 +- .../src/hooks/gsapResizeIntercept.test.ts | 129 ++++++++++++++++++ .../studio/src/hooks/gsapResizeIntercept.ts | 9 +- .../studio/src/hooks/gsapRuntimeReaders.ts | 10 +- .../src/hooks/useAnimatedPropertyCommit.ts | 37 +++-- .../src/hooks/useGsapScriptCommits.test.tsx | 97 ++++++------- .../studio/src/hooks/useGsapScriptCommits.ts | 10 +- packages/studio/src/utils/authoredOpacity.ts | 9 +- .../studio/src/utils/gsapSoftReload.test.ts | 88 +++++++++++- packages/studio/src/utils/gsapSoftReload.ts | 14 +- 13 files changed, 367 insertions(+), 88 deletions(-) create mode 100644 packages/studio/src/hooks/gsapResizeIntercept.test.ts diff --git a/packages/core/src/runtime/colorGrading.test.ts b/packages/core/src/runtime/colorGrading.test.ts index cf3e0e048..a02d6bdb3 100644 --- a/packages/core/src/runtime/colorGrading.test.ts +++ b/packages/core/src/runtime/colorGrading.test.ts @@ -643,4 +643,26 @@ describe("installAuthoredOpacityCapture", () => { expect(el.getAttribute("data-hf-authored-opacity")).toBe(""); el.remove(); }); + + it("stamps an already-inserted element the moment it GAINS grading at runtime", async () => { + installAuthoredOpacityCapture(); + const el = document.createElement("img"); + el.style.opacity = "0.9"; + document.body.appendChild(el); + await Promise.resolve(); + expect(el.hasAttribute("data-hf-authored-opacity")).toBe(false); + + // Studio applies a preset to a previously ungraded element — no re-insert. + el.setAttribute(HF_COLOR_GRADING_ATTR, serializeHfColorGrading({ adjust: { exposure: 0.5 } })); + await Promise.resolve(); + expect(el.getAttribute("data-hf-authored-opacity")).toBe("0.9"); + + // Later attribute rewrites (preset tweaks) never overwrite the stamp, + // even if a transient is live by then. + el.style.opacity = "0"; + el.setAttribute(HF_COLOR_GRADING_ATTR, serializeHfColorGrading({ adjust: { exposure: 0.9 } })); + await Promise.resolve(); + expect(el.getAttribute("data-hf-authored-opacity")).toBe("0.9"); + el.remove(); + }); }); diff --git a/packages/core/src/runtime/colorGrading.ts b/packages/core/src/runtime/colorGrading.ts index 75e85ebd8..8ec35806b 100644 --- a/packages/core/src/runtime/colorGrading.ts +++ b/packages/core/src/runtime/colorGrading.ts @@ -239,8 +239,21 @@ export function installAuthoredOpacityCapture(): void { new MutationObserver((mutations) => { for (const mutation of mutations) { for (const node of mutation.addedNodes) scan(node); + // An element can also GAIN grading at runtime (studio applies a preset to + // a previously ungraded element). Stamp at that moment — strictly earlier + // than the engine's hide, so the captured value can never be worse than + // the hide-time fallback, and after a soft reload's authored restore it + // IS the authored value. stamp() is idempotent: an existing stamp wins. + if (mutation.type === "attributes" && mutation.target instanceof Element) { + if (mutation.target.hasAttribute(HF_COLOR_GRADING_ATTR)) stamp(mutation.target); + } } - }).observe(root, { childList: true, subtree: true }); + }).observe(root, { + childList: true, + subtree: true, + attributes: true, + attributeFilter: [HF_COLOR_GRADING_ATTR], + }); } // Map insertion order gives us simple FIFO eviction for authoring sessions that cycle LUTs. diff --git a/packages/parsers/src/gsapWriterAcorn.ts b/packages/parsers/src/gsapWriterAcorn.ts index a0381e324..556eecd09 100644 --- a/packages/parsers/src/gsapWriterAcorn.ts +++ b/packages/parsers/src/gsapWriterAcorn.ts @@ -57,11 +57,15 @@ function buildTweenStatementCode(timelineVar: string, anim: Omit `${safeKey(k)}: ${valueToCode(v)}`); + const emitted = new Set(Object.keys(props)); if (anim.extras) { for (const [k, v] of Object.entries(anim.extras)) { // A key carried by both properties and extras (a set's parsed - // `immediateRender: true`) must emit once — properties win. - if (!(k in props)) entries.push(`${safeKey(k)}: ${valueToCode(v)}`); + // `immediateRender: true`) must emit once — properties win. Same + // dedupe shape as the recast twin (gsapParser.ts buildTweenStatementCode). + if (emitted.has(k)) continue; + emitted.add(k); + entries.push(`${safeKey(k)}: ${valueToCode(v)}`); } } const objCode = `{ ${entries.join(", ")} }`; diff --git a/packages/studio/src/components/editor/DomEditOverlay.tsx b/packages/studio/src/components/editor/DomEditOverlay.tsx index 5b444a7c7..c0adaef18 100644 --- a/packages/studio/src/components/editor/DomEditOverlay.tsx +++ b/packages/studio/src/components/editor/DomEditOverlay.tsx @@ -315,7 +315,12 @@ export const DomEditOverlay = memo(function DomEditOverlay({ if (!hoverSelectionRef.current && onMarqueeSelectRef.current && compRect.width > 0) { const iframe = iframeRef.current; const freshTarget = iframe - ? getPreviewTargetFromPointer(iframe, event.clientX, event.clientY, activeCompositionPath) + ? getPreviewTargetFromPointer( + iframe, + event.clientX, + event.clientY, + activeCompositionPathRef.current, + ) : null; if (freshTarget) return; const overlayEl = overlayRef.current; diff --git a/packages/studio/src/hooks/gsapResizeIntercept.test.ts b/packages/studio/src/hooks/gsapResizeIntercept.test.ts new file mode 100644 index 000000000..6a0d75088 --- /dev/null +++ b/packages/studio/src/hooks/gsapResizeIntercept.test.ts @@ -0,0 +1,129 @@ +// @vitest-environment happy-dom +import { afterEach, expect, it, vi } from "vitest"; +import type { GsapAnimation } from "@hyperframes/core/gsap-parser"; +import type { DomEditSelection } from "../components/editor/domEditingTypes"; +import { usePlayerStore } from "../player/store/playerStore"; +import { tryGsapResizeIntercept } from "./gsapResizeIntercept"; + +afterEach(() => { + vi.restoreAllMocks(); + usePlayerStore.setState({ currentTime: 0, activeKeyframePct: null }); +}); + +/** + * Scale-route resize: an element whose visual size is driven by a scale-group + * tween. The intercept must (a) route the commit through SCALE, never + * width/height, and (b) resolve convert-to-keyframes from-values through the + * group filter — an opacity-touching intro tween on the same element must not + * ride into the converted keyframes (the disappearance bake class). + */ +function makeGradedElement(): HTMLElement { + const el = document.createElement("img"); + el.id = "clip"; + el.setAttribute("data-hf-studio-original-width", "640"); + el.setAttribute("data-hf-studio-original-height", "360"); + // Grading contract: source hidden, canvas carries effective opacity. + el.setAttribute("data-hf-color-grading-source-hidden", ""); + const canvas = document.createElement("canvas"); + canvas.id = "__hf_color_grading_clip"; + canvas.style.opacity = "0.98"; + document.body.append(el, canvas); + return el; +} + +function fakeIframe(el: HTMLElement, gsapValues: Record) { + // The element's OPACITY intro tween lives on the timeline: unfiltered + // capture would pick `opacity` up via the other-tween sweep. + const opacityIntro = { targets: () => [el], vars: { opacity: 0, duration: 0.8 } }; + return { + contentWindow: { + __timelines: { main: { getChildren: () => [opacityIntro] } }, + gsap: { getProperty: (_el: Element, prop: string) => gsapValues[prop] ?? 0 }, + }, + contentDocument: document, + } as unknown as HTMLIFrameElement; +} + +function scaleFromTween(): GsapAnimation { + return { + id: "#clip-from-200-scale", + targetSelector: "#clip", + propertyGroup: "scale", + method: "from", + properties: { scale: 0.9 }, + position: 0.2, + resolvedStart: 0.2, + duration: 0.8, + } as unknown as GsapAnimation; +} + +function keyframedScaleFixture(): GsapAnimation { + return { + ...scaleFromTween(), + keyframes: { + keyframes: [ + { percentage: 0, properties: { scale: 0.9 } }, + { percentage: 100, properties: { scale: 1 } }, + ], + }, + } as unknown as GsapAnimation; +} + +/** Drive one resize through the intercept, returning every committed mutation. */ +async function runResize( + el: HTMLElement, + iframe: HTMLIFrameElement, + size: { width: number; height: number }, +): Promise>> { + const selection = { id: "clip", selector: "#clip", element: el } as unknown as DomEditSelection; + usePlayerStore.setState({ currentTime: 0.5 }); // inside the tween's range + const committed: Array> = []; + const commitMutation = vi.fn(async (_sel: unknown, mutation: Record) => { + committed.push(mutation); + }); + const handled = await tryGsapResizeIntercept( + selection, + size, + [scaleFromTween()], + iframe, + commitMutation as never, + async () => [keyframedScaleFixture()], + ); + expect(handled).toBe(true); + return committed; +} + +it("scale-route resize converts via the group filter and commits scale, not width/height", async () => { + const el = makeGradedElement(); + const iframe = fakeIframe(el, { scale: 1, scaleX: 1, scaleY: 1, opacity: 0, rotation: 0 }); + // uniform: 800/640 === 450/360 + const committed = await runResize(el, iframe, { width: 800, height: 450 }); + + const convert = committed.find((m) => m.type === "convert-to-keyframes"); + expect(convert).toBeDefined(); + const fromValues = convert!.resolvedFromValues as Record; + // Group filter: the opacity intro tween must NOT leak into the conversion. + expect(fromValues).not.toHaveProperty("opacity"); + expect(fromValues).toHaveProperty("scale"); + + // Every committed property is scale-group — the resize never writes + // width/height for a scale-driven element (the double-apply bug class). + const allProps = committed.flatMap((m) => [ + ...Object.keys((m.properties as Record) ?? {}), + ...Object.keys((m.resolvedFromValues as Record) ?? {}), + ]); + expect(allProps).not.toContain("width"); + expect(allProps).not.toContain("height"); + expect(allProps.some((p) => p === "scale" || p === "scaleX")).toBe(true); +}); + +it("non-uniform drag commits scaleX/scaleY longhands", async () => { + const el = makeGradedElement(); + const iframe = fakeIframe(el, { scale: 1, scaleX: 1, scaleY: 1, opacity: 0 }); + // scaleX 1.25 vs scaleY 1.0 → non-uniform + const committed = await runResize(el, iframe, { width: 800, height: 360 }); + + const serialized = JSON.stringify(committed); + expect(serialized).toContain("scaleX"); + expect(serialized).toContain("scaleY"); +}); diff --git a/packages/studio/src/hooks/gsapResizeIntercept.ts b/packages/studio/src/hooks/gsapResizeIntercept.ts index 9396384f2..e7cc47670 100644 --- a/packages/studio/src/hooks/gsapResizeIntercept.ts +++ b/packages/studio/src/hooks/gsapResizeIntercept.ts @@ -106,7 +106,12 @@ export async function tryGsapResizeIntercept( const coalesceKey = `gsap:resize:${anim.id}`; const selector = selectorFromSelection(selection); - const runtimeProps = selector ? readAllAnimatedProperties(iframe, selector, anim) : {}; + // Scope every capture to the resize group — same contract as the rotation + // intercept. Unfiltered, an opacity-touching intro tween on the element + // would ride into resize conversions/backfills (the Fix-2 bake class). + const runtimeProps = selector + ? readAllAnimatedProperties(iframe, selector, anim, resizeGroup) + : {}; let resizeProps: Record; let scaleDraftEl: HTMLElement | null = null; @@ -254,7 +259,7 @@ export async function tryGsapResizeIntercept( if (newId) anim = { ...anim, id: newId }; } else if (!anim.keyframes) { const resolvedFromValues = selector - ? readAllAnimatedProperties(iframe, selector, anim) + ? readAllAnimatedProperties(iframe, selector, anim, resizeGroup) : undefined; await commitMutation( selection, diff --git a/packages/studio/src/hooks/gsapRuntimeReaders.ts b/packages/studio/src/hooks/gsapRuntimeReaders.ts index 3c78ec608..48d2ed3ff 100644 --- a/packages/studio/src/hooks/gsapRuntimeReaders.ts +++ b/packages/studio/src/hooks/gsapRuntimeReaders.ts @@ -106,11 +106,9 @@ export function readAllAnimatedProperties( // point of property-group tweens is that a rotation commit never carries // opacity/rotationX/etc. captured from unrelated tweens on the element. const inGroup = (p: string) => !group || classifyPropertyGroup(p) === group; - for (const p of propKeys) { - if (!inGroup(p)) propKeys.delete(p); - } + const groupedPropKeys = new Set([...propKeys].filter(inGroup)); - for (const prop of propKeys) { + for (const prop of groupedPropKeys) { const val = readLiveGsapValue(gsap, el, prop); if (Number.isFinite(val)) { result[prop] = POSITION_PROPS.has(prop) ? Math.round(val) : roundTo3(val); @@ -142,7 +140,7 @@ export function readAllAnimatedProperties( } } } catch {} - for (const p of propKeys) otherTweenProps.delete(p); + for (const p of groupedPropKeys) otherTweenProps.delete(p); // Tier 1: Transform + visual properties with universal CSS defaults. // Safe to compare against hardcoded values — these are always 0 or 1 @@ -174,7 +172,7 @@ export function readAllAnimatedProperties( // Collect all properties that ANY tween on this element explicitly targets. // Only capture baseline values for these — GSAP reports non-default values // (scaleZ=0, brightness=0) for untouched properties, polluting keyframes. - const allTweenedProps = new Set([...propKeys, ...otherTweenProps]); + const allTweenedProps = new Set([...groupedPropKeys, ...otherTweenProps]); for (const [prop, defaultVal] of Object.entries(UNIVERSAL_BASELINE)) { if (prop in result) continue; if (!allTweenedProps.has(prop)) continue; diff --git a/packages/studio/src/hooks/useAnimatedPropertyCommit.ts b/packages/studio/src/hooks/useAnimatedPropertyCommit.ts index 9e5914f10..e1cd37b96 100644 --- a/packages/studio/src/hooks/useAnimatedPropertyCommit.ts +++ b/packages/studio/src/hooks/useAnimatedPropertyCommit.ts @@ -186,19 +186,40 @@ async function commitStaticSet( byGroup.set(group, batch); } const sets = animations.filter((a) => a.method === "set" && a.targetSelector === selector); + // Resolve every group's target BEFORE committing anything, and coalesce + // groups that land on the SAME set into one commit: the `sets` snapshot is + // captured once, so if two groups resolved to one legacy mixed set, a first + // commit could re-shape it server-side and leave the second chasing a stale + // id (404 on legacy pre-split files). + const byTargetSet = new Map(); + const newSetBatches: [string, number | string][][] = []; for (const [group, batch] of byGroup) { - const existingSet = - // A set already dedicated to this group wins; else a mixed set that - // already carries a property of this group (merging same-group values - // there beats spawning a second writer for the same channel). - sets.find((a) => a.propertyGroup === group) ?? - sets.find((a) => Object.keys(a.properties).some((k) => classifyPropertyGroup(k) === group)); + const existingSet = findGroupOwningSet(sets, group); if (existingSet) { - await commitSetProps(selection, existingSet, batch, selector, animations, commit); + byTargetSet.set(existingSet, [...(byTargetSet.get(existingSet) ?? []), ...batch]); } else { - await addGlobalStaticSet(selection, batch, selector, commit); + newSetBatches.push(batch); } } + for (const [targetSet, batch] of byTargetSet) { + await commitSetProps(selection, targetSet, batch, selector, animations, commit); + } + // Fresh adds don't reshape existing sets, so their ids can't go stale. + for (const batch of newSetBatches) { + await addGlobalStaticSet(selection, batch, selector, commit); + } +} + +/** + * The set that owns a property group: one already dedicated to the group wins; + * else a mixed set that already carries a property of the group (merging + * same-group values there beats spawning a second writer for the channel). + */ +function findGroupOwningSet(sets: GsapAnimation[], group: string): GsapAnimation | undefined { + return ( + sets.find((a) => a.propertyGroup === group) ?? + sets.find((a) => Object.keys(a.properties).some((k) => classifyPropertyGroup(k) === group)) + ); } /** diff --git a/packages/studio/src/hooks/useGsapScriptCommits.test.tsx b/packages/studio/src/hooks/useGsapScriptCommits.test.tsx index 8416a4a08..2e867a4f4 100644 --- a/packages/studio/src/hooks/useGsapScriptCommits.test.tsx +++ b/packages/studio/src/hooks/useGsapScriptCommits.test.tsx @@ -51,6 +51,14 @@ function syncDragPreview(res: MutationResult, reloadPreview: () => void) { applyPreviewSync(FAKE_IFRAME, res, dragOptions(), reloadPreview); } +function expectSoftReloadedWith(onAsyncFailure: unknown, authoredHtml: string | undefined) { + expect(applySoftReload).toHaveBeenCalledWith(FAKE_IFRAME, "SCRIPT", { + onAsyncFailure, + currentTimeOverride: 0, + authoredHtml, + }); +} + describe("applyPreviewSync", () => { beforeEach(() => { patchRuntimeTweenInPlace.mockReset(); @@ -81,13 +89,7 @@ describe("applyPreviewSync", () => { // reloadPreview is wired as onAsyncFailure (3rd arg) so a MotionPath-plugin // CDN load failure escalates to a full reload — but it is NOT called eagerly. - expect(applySoftReload).toHaveBeenCalledWith( - FAKE_IFRAME, - "SCRIPT", - reloadPreview, - 0, - undefined, - ); + expectSoftReloadedWith(reloadPreview, undefined); expect(reloadPreview).not.toHaveBeenCalled(); // A successful instant patch is the fast path; here it missed → fallback event. expect(trackStudioEvent).toHaveBeenCalledWith( @@ -105,13 +107,7 @@ describe("applyPreviewSync", () => { // U4: "verify-failed" is the TRANSIENT empty-timeline window — the live state // is correct, so we must NOT escalate to a full reload. - expect(applySoftReload).toHaveBeenCalledWith( - FAKE_IFRAME, - "SCRIPT", - reloadPreview, - 0, - undefined, - ); + expectSoftReloadedWith(reloadPreview, undefined); expect(reloadPreview).not.toHaveBeenCalled(); // Telemetry records the suppressed transient (escalated: false). expect(trackStudioEvent).toHaveBeenCalledWith( @@ -132,13 +128,7 @@ describe("applyPreviewSync", () => { syncDragPreview(result({ scriptText: "SCRIPT" }), reloadPreview); // Structural failure: the preview is genuinely stale/broken → full reload. - expect(applySoftReload).toHaveBeenCalledWith( - FAKE_IFRAME, - "SCRIPT", - reloadPreview, - 0, - undefined, - ); + expectSoftReloadedWith(reloadPreview, undefined); expect(reloadPreview).toHaveBeenCalledTimes(1); expect(trackStudioEvent).toHaveBeenCalledWith( "gsap_soft_reload_outcome", @@ -162,13 +152,7 @@ describe("applyPreviewSync", () => { ); expect(patchRuntimeTweenInPlace).not.toHaveBeenCalled(); - expect(applySoftReload).toHaveBeenCalledWith( - FAKE_IFRAME, - "SCRIPT", - reloadPreview, - 0, - undefined, - ); + expectSoftReloadedWith(reloadPreview, undefined); expect(reloadPreview).not.toHaveBeenCalled(); // "applied" emits no telemetry (only the failure paths do). expect(trackStudioEvent).not.toHaveBeenCalled(); @@ -186,13 +170,7 @@ describe("applyPreviewSync", () => { ); // onAsyncFailure is wired, but the transient result does not trigger it. - expect(applySoftReload).toHaveBeenCalledWith( - FAKE_IFRAME, - "SCRIPT", - reloadPreview, - 0, - undefined, - ); + expectSoftReloadedWith(reloadPreview, undefined); expect(reloadPreview).not.toHaveBeenCalled(); expect(trackStudioEvent).toHaveBeenCalledWith( "gsap_soft_reload_outcome", @@ -211,13 +189,7 @@ describe("applyPreviewSync", () => { reloadPreview, ); - expect(applySoftReload).toHaveBeenCalledWith( - FAKE_IFRAME, - "SCRIPT", - reloadPreview, - 0, - undefined, - ); + expectSoftReloadedWith(reloadPreview, undefined); expect(reloadPreview).toHaveBeenCalledTimes(1); expect(trackStudioEvent).toHaveBeenCalledWith( "gsap_soft_reload_outcome", @@ -330,6 +302,31 @@ describe("runCommit — instantPatch wiring", () => { expect(deps.reloadPreview).not.toHaveBeenCalled(); }); + it("no-op commit whose instant patch MISSES soft-reloads (never full-reloads)", async () => { + // Server contract: gsap-mutations returns scriptText on EVERY response, + // including changed:false — so the fallback re-runs the identical script + // ("applied") instead of escalating a genuine no-op to a full reload. + patchRuntimeTweenInPlace.mockReturnValue(false); + applySoftReload.mockReturnValue("applied"); + mockFetchResult({ changed: false }); + const deps = renderCommitHook(); + + await act(async () => { + await deps.api.commitMutation( + selection, + { type: "update-property", property: "y", value: 311 }, + { + label: "Move layer", + softReload: true, + instantPatch: { selector: "#a", change: { kind: "set", props: { x: 485, y: 311 } } }, + }, + ); + }); + + expectSoftReloadedWith(deps.reloadPreview, "AFTER"); + expect(deps.reloadPreview).not.toHaveBeenCalled(); + }); + beforeEach(() => { patchRuntimeTweenInPlace.mockReset(); applySoftReload.mockReset(); @@ -368,13 +365,7 @@ describe("runCommit — instantPatch wiring", () => { }); expect(fetch).toHaveBeenCalledTimes(1); - expect(applySoftReload).toHaveBeenCalledWith( - FAKE_IFRAME, - "SCRIPT", - deps.reloadPreview, - 0, - "AFTER", - ); + expectSoftReloadedWith(deps.reloadPreview, "AFTER"); expect(deps.reloadPreview).not.toHaveBeenCalled(); expect(deps.onCacheInvalidate).toHaveBeenCalledTimes(1); }); @@ -389,13 +380,7 @@ describe("runCommit — instantPatch wiring", () => { }); expect(patchRuntimeTweenInPlace).not.toHaveBeenCalled(); - expect(applySoftReload).toHaveBeenCalledWith( - FAKE_IFRAME, - "SCRIPT", - deps.reloadPreview, - 0, - "AFTER", - ); + expectSoftReloadedWith(deps.reloadPreview, "AFTER"); expect(deps.reloadPreview).not.toHaveBeenCalled(); }); }); diff --git a/packages/studio/src/hooks/useGsapScriptCommits.ts b/packages/studio/src/hooks/useGsapScriptCommits.ts index a28478acb..cb088e347 100644 --- a/packages/studio/src/hooks/useGsapScriptCommits.ts +++ b/packages/studio/src/hooks/useGsapScriptCommits.ts @@ -72,13 +72,11 @@ function softReloadOrEscalate( // not the iframe's raw `__player.getTime()` — see the comment in // applySoftReload for why the two can desync after a keyframe-node drag. const currentTime = usePlayerStore.getState().currentTime; - const result: SoftReloadResult = applySoftReload( - iframe, - scriptText, - reloadPreview, - currentTime, + const result: SoftReloadResult = applySoftReload(iframe, scriptText, { + onAsyncFailure: reloadPreview, + currentTimeOverride: currentTime, authoredHtml, - ); + }); if (result === "applied") return; trackStudioEvent("gsap_soft_reload_outcome", { origin, diff --git a/packages/studio/src/utils/authoredOpacity.ts b/packages/studio/src/utils/authoredOpacity.ts index 6a6b4998b..53b3ede30 100644 --- a/packages/studio/src/utils/authoredOpacity.ts +++ b/packages/studio/src/utils/authoredOpacity.ts @@ -21,7 +21,14 @@ export function readStampedAuthoredOpacity(element: AttributeReader): string | n return element.getAttribute(COLOR_GRADING_AUTHORED_OPACITY_ATTR); } -/** Write an authored inline opacity back: "" removes the property, a value sets it. */ +/** + * Write an authored inline opacity back: "" removes the property, a value sets + * it. Priority-lossy by design: the capture reads `style.opacity` (value only) + * and the write sets no priority, so an authored `opacity: X !important` + * round-trips as `opacity: X`. The only `!important` opacity in the pipeline + * is the color-grading runtime hide — a transient this contract exists to + * discard — and authored compositions don't `!important` their opacity. + */ export function applyAuthoredInlineOpacity(style: CSSStyleDeclaration, authored: string): void { if (authored === "") style.removeProperty("opacity"); else style.setProperty("opacity", authored); diff --git a/packages/studio/src/utils/gsapSoftReload.test.ts b/packages/studio/src/utils/gsapSoftReload.test.ts index 1576a8bc8..25e07edd9 100644 --- a/packages/studio/src/utils/gsapSoftReload.test.ts +++ b/packages/studio/src/utils/gsapSoftReload.test.ts @@ -107,7 +107,7 @@ describe("applySoftReload", () => { // async commit resolves. The rebuilt timeline must re-seek to the caller's // value, not the iframe's possibly-stale one. const { iframe, contentWindow } = buildMockIframe(); - const result = applySoftReload(iframe, SCRIPT_TEXT, undefined, 0); + const result = applySoftReload(iframe, SCRIPT_TEXT, { currentTimeOverride: 0 }); expect(result).toBe("applied"); expect(contentWindow.__player.seek).toHaveBeenCalledWith(0); }); @@ -244,7 +244,7 @@ describe("applySoftReload", () => { (iframe.contentDocument as unknown as { head: unknown }).head = head; const onAsyncFailure = vi.fn(); - const result = applySoftReload(iframe, MOTION_PATH_SCRIPT_TEXT, onAsyncFailure); + const result = applySoftReload(iframe, MOTION_PATH_SCRIPT_TEXT, { onAsyncFailure }); // Optimistically "applied" (script will run once the plugin loads) — and the // script has NOT executed yet, so the timeline isn't rebound synchronously. @@ -363,3 +363,87 @@ describe("ensureMotionPathPluginLoaded", () => { expect(appendedScripts).toHaveLength(2); }); }); + +// The authored-opacity restore: before the script re-runs (and its tweens +// re-capture bounds), every animated element's inline opacity must be put back +// to its AUTHORED value — from the after-write file HTML when provided, else +// from the parse-time stamp. Otherwise a runtime transient (the color-grading +// hide's 0, a mid-flight tween value) becomes a permanent tween bound. +describe("applySoftReload authored-opacity restore", () => { + function buildIframeWithTarget(el: HTMLElement, overrides: Record = {}) { + const scriptEl = document.createElement("script"); + scriptEl.textContent = + 'const tl = gsap.timeline({ paused: true }); tl.to("#box", { opacity: 0.5 });'; + const tl = { + kill: vi.fn(), + pause: vi.fn(), + getChildren: () => [{ targets: () => [el] }], + }; + const contentWindow = { + gsap: { timeline: vi.fn(), set: vi.fn() }, + __hfForceTimelineRebind: vi.fn(), + __timelines: { root: tl } as Record, + __player: { getTime: () => 2.0, seek: vi.fn() }, + __hfStudioManualEditsApply: vi.fn(), + ...overrides, + }; + const container = document.createElement("div"); + container.appendChild(scriptEl); + // Intercept only POST-SETUP appends: simulate the re-run script + // repopulating __timelines (as in buildMockIframe). + const realAppendChild = container.appendChild.bind(container); + container.appendChild = (node: T): T => { + const result = realAppendChild(node); + if (node instanceof HTMLScriptElement && node.textContent?.includes("gsap.timeline")) { + contentWindow.__timelines.root = { kill: vi.fn(), pause: vi.fn() }; + } + return result; + }; + const contentDocument = { + querySelectorAll: (sel: string) => (sel === "script:not([src])" ? [scriptEl] : []), + createElement: (tag: string) => document.createElement(tag), + body: container, + head: document.createElement("div"), + }; + return { iframe: { contentWindow, contentDocument } as unknown as HTMLIFrameElement }; + } + + /** Run one restore cycle over `el` and return the final inline opacity. */ + function restoreOpacity(el: HTMLElement, authoredHtml?: string): string { + const { iframe } = buildIframeWithTarget(el); + expect(applySoftReload(iframe, SCRIPT_TEXT, authoredHtml ? { authoredHtml } : {})).toBe( + "applied", + ); + return el.style.getPropertyValue("opacity"); + } + + it("restores opacity from the after-write HTML (matched by data-hf-id)", () => { + const el = document.createElement("img"); + el.setAttribute("data-hf-id", "hf-1"); + el.style.setProperty("opacity", "0", "important"); // the grading hide + + const opacity = restoreOpacity( + el, + '', + ); + + expect(opacity).toBe("0.98"); + expect(el.style.getPropertyPriority("opacity")).toBe(""); + }); + + it("falls back to the parse-time stamp when no after-write HTML is given", () => { + const el = document.createElement("img"); + el.setAttribute("data-hf-authored-opacity", "0.75"); + el.style.opacity = "0.123"; // mid-flight tween transient + + expect(restoreOpacity(el)).toBe("0.75"); + }); + + it("an empty stamp (authored none) removes the inline opacity", () => { + const el = document.createElement("img"); + el.setAttribute("data-hf-authored-opacity", ""); + el.style.opacity = "0"; + + expect(restoreOpacity(el)).toBe(""); + }); +}); diff --git a/packages/studio/src/utils/gsapSoftReload.ts b/packages/studio/src/utils/gsapSoftReload.ts index f9f051fa8..63012108c 100644 --- a/packages/studio/src/utils/gsapSoftReload.ts +++ b/packages/studio/src/utils/gsapSoftReload.ts @@ -174,13 +174,21 @@ export type SoftReloadResult = "applied" | "verify-failed" | "cannot-soft-reload * caller should perform a full reload to recover. It never fires on the * synchronous paths. */ +export interface SoftReloadOptions { + /** Escalation for async plugin-load failures (e.g. MotionPath CDN error). */ + onAsyncFailure?: () => void; + /** Seek target for the rebuilt timeline; defaults to the iframe player time. */ + currentTimeOverride?: number; + /** After-write file HTML — the primary source for authored-opacity restore. */ + authoredHtml?: string; +} + export function applySoftReload( iframe: HTMLIFrameElement | null, scriptText: string, - onAsyncFailure?: () => void, - currentTimeOverride?: number, - authoredHtml?: string, + options: SoftReloadOptions = {}, ): SoftReloadResult { + const { onAsyncFailure, currentTimeOverride, authoredHtml } = options; if (!iframe || !scriptText) return "cannot-soft-reload"; const win = iframe.contentWindow as IframeWindow | null; From 39f33bd3a3bcd2ea20980bca1e871bbc9c6592f3 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sat, 11 Jul 2026 15:34:41 -0400 Subject: [PATCH 11/11] fix(studio): draw the crop UI in the element's rotated frame MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../components/editor/DomEditCropHandles.tsx | 72 ++++++++++----- .../editor/domEditOverlayCrop.test.ts | 90 +++++++++++++++++++ .../components/editor/domEditOverlayCrop.ts | 88 ++++++++++++++++++ 3 files changed, 227 insertions(+), 23 deletions(-) diff --git a/packages/studio/src/components/editor/DomEditCropHandles.tsx b/packages/studio/src/components/editor/DomEditCropHandles.tsx index 7ccdaad3e..7ac3b485b 100644 --- a/packages/studio/src/components/editor/DomEditCropHandles.tsx +++ b/packages/studio/src/components/editor/DomEditCropHandles.tsx @@ -4,9 +4,11 @@ import type { OverlayRect } from "./domEditOverlayGeometry"; import { type CropEdge, cropRectFromInsets, + readElementCropFrame, readElementCropInsets, resolveCropInsetFromEdgeDrag, resolveCropInsetFromMoveDrag, + rotateDeltaIntoFrame, } from "./domEditOverlayCrop"; import { buildInsetClipPathSides, type ClipPathInsetSides } from "./clipPathHelpers"; @@ -17,6 +19,10 @@ interface CropGestureState { startY: number; startInsets: ClipPathInsetSides; didMove: boolean; + /** Element frame captured at gesture start: pointer deltas rotate into it. */ + angleDeg: number; + scaleX: number; + scaleY: number; } interface DomEditCropHandlesProps { @@ -133,11 +139,20 @@ export function DomEditCropHandles({ }; }, [selection.element]); - const scaleX = overlayRect.editScaleX > 0 ? overlayRect.editScaleX : 1; - const scaleY = overlayRect.editScaleY > 0 ? overlayRect.editScaleY : 1; - const width = overlayRect.width / scaleX; - const height = overlayRect.height / scaleY; - const cropRect = cropRectFromInsets(overlayRect, state.insets, scaleX, scaleY); + // The crop applies in the element's LOCAL frame (clip-path precedes the + // transform), so all crop UI is drawn inside a container rotated with the + // element — on a rotated element an axis-aligned dim visually "straightens" + // it by masking the rotated corners. + const frame = readElementCropFrame(selection.element, overlayRect); + const width = frame.width / frame.scaleX; // element CSS px + const height = frame.height / frame.scaleY; + // Crop rect in FRAME-LOCAL coordinates (origin = frame top-left). + const cropRect = cropRectFromInsets( + { left: 0, top: 0, width: frame.width, height: frame.height }, + state.insets, + frame.scaleX, + frame.scaleY, + ); const startCropGesture = (edge: CropEdge | "move", event: ReactPointerEvent) => { if (!onStyleCommit) return; @@ -151,6 +166,9 @@ export function DomEditCropHandles({ startY: event.clientY, startInsets: state.insets, didMove: false, + angleDeg: frame.angleDeg, + scaleX: frame.scaleX, + scaleY: frame.scaleY, }; // Clip is already lifted by the selection effect; just flag the drag so the // rule-of-thirds grid shows. @@ -162,12 +180,17 @@ export function DomEditCropHandles({ if (!gesture || gesture.pointerId !== event.pointerId) return; event.preventDefault(); event.stopPropagation(); + const local = rotateDeltaIntoFrame( + event.clientX - gesture.startX, + event.clientY - gesture.startY, + gesture.angleDeg, + ); const drag = { startInsets: gesture.startInsets, - deltaX: event.clientX - gesture.startX, - deltaY: event.clientY - gesture.startY, - scaleX, - scaleY, + deltaX: local.deltaX, + deltaY: local.deltaY, + scaleX: gesture.scaleX, + scaleY: gesture.scaleY, }; const nextInsets = gesture.edge === "move" @@ -225,24 +248,27 @@ export function DomEditCropHandles({ if (!state.croppable) return null; return ( - <> +
{/* Dim the cropped-away area whenever the element is cropped and selected, - so the hidden content is visible (ghosted) without dragging. */} + so the hidden content is visible (ghosted) without dragging. Clipped to + the element's own (rotated) box. */} {hasCrop && ( -
+
); })} - +
); } diff --git a/packages/studio/src/components/editor/domEditOverlayCrop.test.ts b/packages/studio/src/components/editor/domEditOverlayCrop.test.ts index a23abc0e7..9831f06ed 100644 --- a/packages/studio/src/components/editor/domEditOverlayCrop.test.ts +++ b/packages/studio/src/components/editor/domEditOverlayCrop.test.ts @@ -2,9 +2,11 @@ import { describe, expect, it } from "vitest"; import { cropRectFromInsets, hugRectForElement, + readElementCropFrame, readElementCropInsets, resolveCropInsetFromEdgeDrag, resolveCropInsetFromMoveDrag, + rotateDeltaIntoFrame, } from "./domEditOverlayCrop"; describe("resolveCropInsetFromEdgeDrag", () => { @@ -151,3 +153,91 @@ describe("readElementCropInsets tri-state", () => { expect(hugRectForElement(rect, fakeEl("circle(50%)"))).toEqual(rect); }); }); + +// Regression: crop UI drawn on the axis-aligned bounding box visually +// "straightens" a rotated element — the dim masks the rotated corners. The +// frame gives the element's own box + rotation so the UI rotates with it. +describe("readElementCropFrame", () => { + const overlayRect = { left: 100, top: 50, width: 220, height: 130, editScaleX: 1, editScaleY: 1 }; + + const fakeEl = (transform: string, offsetWidth = 200, offsetHeight = 100) => + ({ + offsetWidth, + offsetHeight, + ownerDocument: { defaultView: { getComputedStyle: () => ({ transform }) } }, + }) as unknown as HTMLElement; + + it("identity transform → the axis-aligned overlay rect", () => { + expect(readElementCropFrame(fakeEl("none"), overlayRect)).toEqual({ + angleDeg: 0, + left: 100, + top: 50, + width: 220, + height: 130, + scaleX: 1, + scaleY: 1, + }); + }); + + it("rotated element → its own box, centered on the AABB, with the angle", () => { + // rotate(30deg): matrix(cos, sin, -sin, cos, tx, ty) + const cos = Math.cos(Math.PI / 6); + const sin = Math.sin(Math.PI / 6); + const frame = readElementCropFrame( + fakeEl(`matrix(${cos}, ${sin}, ${-sin}, ${cos}, 10, 20)`), + overlayRect, + ); + expect(frame.angleDeg).toBeCloseTo(30, 3); + expect(frame.width).toBeCloseTo(200, 3); + expect(frame.height).toBeCloseTo(100, 3); + // centered on the AABB center (210, 115) + expect(frame.left + frame.width / 2).toBeCloseTo(210, 3); + expect(frame.top + frame.height / 2).toBeCloseTo(115, 3); + expect(frame.scaleX).toBeCloseTo(1, 3); + }); + + it("scaled element → scale factored into px-per-element-px", () => { + const frame = readElementCropFrame(fakeEl("matrix(1.5, 0, 0, 2, 0, 0)"), overlayRect); + expect(frame.angleDeg).toBe(0); + expect(frame.scaleX).toBeCloseTo(1.5, 3); + expect(frame.scaleY).toBeCloseTo(2, 3); + expect(frame.width).toBeCloseTo(300, 3); + expect(frame.height).toBeCloseTo(200, 3); + }); + + it("3D transform falls back to the axis-aligned frame", () => { + const frame = readElementCropFrame( + fakeEl("matrix3d(1,0,0,0,0,1,0,0,0,0,1,0,0,0,0,1)"), + overlayRect, + ); + expect(frame).toEqual({ + angleDeg: 0, + left: 100, + top: 50, + width: 220, + height: 130, + scaleX: 1, + scaleY: 1, + }); + }); +}); + +describe("rotateDeltaIntoFrame", () => { + it("passes deltas through at 0deg", () => { + expect(rotateDeltaIntoFrame(10, 5, 0)).toEqual({ deltaX: 10, deltaY: 5 }); + }); + + it("rotates a screen delta into a 90deg-rotated frame", () => { + // Element rotated +90°: dragging DOWN on screen moves along the element's +x. + const { deltaX, deltaY } = rotateDeltaIntoFrame(0, 10, 90); + expect(deltaX).toBeCloseTo(10, 6); + expect(deltaY).toBeCloseTo(0, 6); + }); + + it("round-trips a 30deg rotation", () => { + const local = rotateDeltaIntoFrame(7, -3, 30); + const back = rotateDeltaIntoFrame(local.deltaX, local.deltaY, -30); + expect(back.deltaX).toBeCloseTo(7, 6); + expect(back.deltaY).toBeCloseTo(-3, 6); + }); +}); diff --git a/packages/studio/src/components/editor/domEditOverlayCrop.ts b/packages/studio/src/components/editor/domEditOverlayCrop.ts index 4f0e7dd7f..cfff9daf4 100644 --- a/packages/studio/src/components/editor/domEditOverlayCrop.ts +++ b/packages/studio/src/components/editor/domEditOverlayCrop.ts @@ -122,3 +122,91 @@ export function hugRectForElement( return rect; return cropRectFromInsets(rect, insets, rect.editScaleX, rect.editScaleY); } + +/** + * The element's own (unrotated) box in overlay space, plus the rotation to + * apply when drawing crop UI over it. `clip-path` applies in the element's + * LOCAL frame — before its transform — so the crop dim/outline/handles must be + * drawn rotated with the element, not on its axis-aligned bounding box: an + * AABB-drawn dim visually "straightens" a rotated element by masking its + * corners (the crop window looks axis-aligned while the pixels are not). + * + * scaleX/scaleY are overlay px per element CSS px (element's own scale × the + * editor zoom), so element-space insets map straight onto the frame. Assumes + * the default 50%/50% transform-origin (the GSAP/studio convention). 3D or + * unparseable transforms fall back to the axis-aligned frame (angle 0, AABB + * box) — the pre-existing presentation. + */ +export interface CropFrame { + angleDeg: number; + left: number; + top: number; + width: number; + height: number; + scaleX: number; + scaleY: number; +} + +export function readElementCropFrame( + element: HTMLElement, + overlayRect: CropScreenRect & { editScaleX: number; editScaleY: number }, +): CropFrame { + const editX = overlayRect.editScaleX > 0 ? overlayRect.editScaleX : 1; + const editY = overlayRect.editScaleY > 0 ? overlayRect.editScaleY : 1; + const aabb: CropFrame = { + angleDeg: 0, + left: overlayRect.left, + top: overlayRect.top, + width: overlayRect.width, + height: overlayRect.height, + scaleX: editX, + scaleY: editY, + }; + let transform = ""; + try { + transform = element.ownerDocument.defaultView?.getComputedStyle(element).transform ?? ""; + } catch { + return aabb; + } + if (!transform || transform === "none") return aabb; + const m = /^matrix\(([^)]+)\)$/.exec(transform); + if (!m) return aabb; // matrix3d or unparseable → axis-aligned fallback + const [a, b, c, d] = m[1]!.split(",").map((v) => Number.parseFloat(v)); + if (![a, b, c, d].every(Number.isFinite)) return aabb; + const elScaleX = Math.hypot(a!, b!); + const det = a! * d! - b! * c!; + const elScaleY = elScaleX !== 0 ? det / elScaleX : 1; + if (elScaleX <= 0 || elScaleY <= 0) return aabb; + const angleDeg = (Math.atan2(b!, a!) * 180) / Math.PI; + const scaleX = elScaleX * editX; + const scaleY = elScaleY * editY; + const width = element.offsetWidth * scaleX; + const height = element.offsetHeight * scaleY; + if (!(width > 0) || !(height > 0)) return aabb; + // Rotation about the default center keeps the center invariant, so the + // local box is centered on the AABB center. + const cx = overlayRect.left + overlayRect.width / 2; + const cy = overlayRect.top + overlayRect.height / 2; + return { + angleDeg, + left: cx - width / 2, + top: cy - height / 2, + width, + height, + scaleX, + scaleY, + }; +} + +/** Rotate a screen-space pointer delta into the element's local frame. */ +export function rotateDeltaIntoFrame( + deltaX: number, + deltaY: number, + angleDeg: number, +): { deltaX: number; deltaY: number } { + if (angleDeg === 0) return { deltaX, deltaY }; + const rad = (-angleDeg * Math.PI) / 180; + const cos = Math.cos(rad); + const sin = Math.sin(rad); + return { deltaX: deltaX * cos - deltaY * sin, deltaY: deltaX * sin + deltaY * cos }; +}