diff --git a/packages/studio/src/components/editor/manualOffsetDrag.ts b/packages/studio/src/components/editor/manualOffsetDrag.ts index 731c41813f..bb4b7df792 100644 --- a/packages/studio/src/components/editor/manualOffsetDrag.ts +++ b/packages/studio/src/components/editor/manualOffsetDrag.ts @@ -524,6 +524,7 @@ export function restoreManualOffsetDragMembers(members: ManualOffsetDragMember[] } } +/** Teardown after a COMMITTED drag. */ export function endManualOffsetDragMembers(members: ManualOffsetDragMember[]): void { for (const member of members) { endStudioManualEditGesture(member.element, member.gestureToken); @@ -552,6 +553,7 @@ export function endManualOffsetDragMembers(members: ManualOffsetDragMember[]): v } } +/** Release the timelines this gesture paused, re-rendering at the playhead. */ export function resumeGsapTimelines(element: HTMLElement): void { const ids = element.getAttribute("data-hf-drag-paused-timelines"); element.removeAttribute("data-hf-drag-paused-timelines"); diff --git a/packages/studio/src/hooks/gsapRuntimePatch.test.ts b/packages/studio/src/hooks/gsapRuntimePatch.test.ts index 1a17f7e063..e34d293a41 100644 --- a/packages/studio/src/hooks/gsapRuntimePatch.test.ts +++ b/packages/studio/src/hooks/gsapRuntimePatch.test.ts @@ -523,3 +523,46 @@ describe("patchRuntimeTweenInPlace — composition isolation", () => { expect(otherTween.invalidate).not.toHaveBeenCalled(); }); }); + +describe("patchRuntimeTweenInPlace — deferSeek", () => { + /** + * A group drag commits one member at a time. Each in-place patch used to seek, + * and a seek re-renders the WHOLE timeline — so every member still queued behind + * the current one got repainted from its un-patched tween, back to where it sat + * before the drag, and stayed there until its own patch landed. That is the jump. + */ + it("does not seek while a group commit is still writing its other members", () => { + const a = { id: "a" }; + const rendered = { a: 0, b: 0 }; + const tweenA = makeTween({ vars: { x: 0 }, targetIds: ["a"], duration: 0 }, a); + const tweenB = makeTween({ vars: { x: 0 }, targetIds: ["b"], duration: 0 }, a); + const { iframe, seek } = fakeIframe(a, [tweenA, tweenB], { + onSeek: () => { + rendered.a = tweenA.vars.x as number; + rendered.b = tweenB.vars.x as number; + }, + }); + + const first = patchRuntimeTweenInPlace( + iframe, + "#a", + { kind: "set", props: { x: 500 } }, + undefined, + true, + ); + + expect(first).toBe(true); + expect(tweenA.vars.x).toBe(500); + // No repaint yet: "b" keeps the transform the gesture left on it instead of + // being rendered from its own tween, which still holds the pre-drag value. + expect(seek).not.toHaveBeenCalled(); + expect(rendered).toEqual({ a: 0, b: 0 }); + + tweenB.vars.x = 600; + const last = patchRuntimeTweenInPlace(iframe, "#a", { kind: "set", props: { x: 500 } }); + + expect(last).toBe(true); + expect(seek).toHaveBeenCalledTimes(1); + expect(rendered).toEqual({ a: 500, b: 600 }); + }); +}); diff --git a/packages/studio/src/hooks/gsapRuntimePatch.ts b/packages/studio/src/hooks/gsapRuntimePatch.ts index de2c6480e4..9d32650387 100644 --- a/packages/studio/src/hooks/gsapRuntimePatch.ts +++ b/packages/studio/src/hooks/gsapRuntimePatch.ts @@ -277,12 +277,16 @@ function applyChange(tween: RuntimeTween, change: RuntimeTweenChange): boolean { /** * Edit one tween in `window.__timelines` in place + re-seek to the current playhead. * Returns `true` on a confident patch, `false` otherwise (caller soft-reloads). + * + * `deferSeek` skips the re-render, for a caller patching several tweens in a row + * that will render once after the last one. */ export function patchRuntimeTweenInPlace( iframe: HTMLIFrameElement | null, selector: string, change: RuntimeTweenChange, compositionId?: string, + deferSeek = false, ): boolean { if (!iframe) return false; // A base `gsap.set` has no timeline tween to resolve — apply the value straight @@ -312,7 +316,13 @@ export function patchRuntimeTweenInPlace( if (change.kind !== "keyframe-rebuild") { tween.invalidate?.(); } - seekToCurrent(iframe, timeline); + // A seek re-renders the WHOLE timeline, not just the tween we patched. Under a + // multi-element commit that is a visible jump: the members still queued behind + // this one get repainted from their un-patched tweens, back to where they were + // before the gesture, and stay there until their own patch lands. Deferring + // leaves them showing the gesture's own transform, and the caller's last patch + // seeks once for the whole group. + if (!deferSeek) seekToCurrent(iframe, timeline); return true; } catch { return false; diff --git a/packages/studio/src/hooks/gsapScriptCommitTypes.ts b/packages/studio/src/hooks/gsapScriptCommitTypes.ts index 929e79b8ec..ed9f4a6ae4 100644 --- a/packages/studio/src/hooks/gsapScriptCommitTypes.ts +++ b/packages/studio/src/hooks/gsapScriptCommitTypes.ts @@ -22,6 +22,16 @@ export interface CommitMutationOptions { coalesceMs?: number; softReload?: boolean; skipReload?: boolean; + /** + * Write the source but leave the preview alone; the caller renders once when it + * is done. For a multi-write action like a group drag, rendering after each + * write shows a source where the members not yet written still hold their old + * values, so they snap back until their own write lands. This also defers the + * in-place runtime patch's seek, which re-renders the whole timeline and repaints + * the queued members the same way. Unlike `skipReload` this changes nothing about + * error handling — a failed write still throws. + */ + deferPreviewSync?: boolean; beforeReload?: () => void; /** * Serialize this commit against others sharing the same key. Used to chain @@ -39,6 +49,14 @@ export interface CommitMutationOptions { * existing soft/full reload path. Structural edits omit this and reload as before. */ instantPatch?: { selector: string; change: RuntimeTweenChange }; + /** + * The same fast path for a batched commit: one patch per element the batch + * wrote, applied in order. All of them must land for the reload to be skipped + * — one that can't be applied leaves the preview half-patched, so the whole + * batch falls back to the reload. Only the last patch re-renders (see + * `deferSeek`), so a ten-element batch repaints once. + */ + instantPatches?: Array<{ selector: string; change: RuntimeTweenChange }>; } export interface CommitMutationCall { diff --git a/packages/studio/src/hooks/keyframeCacheAstLoad.test.ts b/packages/studio/src/hooks/keyframeCacheAstLoad.test.ts new file mode 100644 index 0000000000..d9a1081df1 --- /dev/null +++ b/packages/studio/src/hooks/keyframeCacheAstLoad.test.ts @@ -0,0 +1,78 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { fetchParsedAnimations } from "./keyframeCacheAstLoad"; + +/** + * Parsing a composition is a whole-file read + parse on the server, and a + * multi-element action asks for the same file once per element. Callers that + * overlap in time share one request; a caller that comes after the last one + * settled does not, so a parse issued after a write is never served a + * pre-write answer. + */ +describe("fetchParsedAnimations — in-flight sharing", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + function stubFetch(): { calls: () => number; settle: () => void } { + let calls = 0; + const pending: Array<() => void> = []; + vi.stubGlobal("fetch", () => { + calls++; + return new Promise((resolve) => { + pending.push(() => + resolve({ + ok: true, + json: () => Promise.resolve({ animations: [{ id: "a", targetSelector: "#a" }] }), + } as Response), + ); + }); + }); + return { + calls: () => calls, + settle: () => { + for (const release of pending.splice(0, pending.length)) release(); + }, + }; + } + + it("serves overlapping reads of one file from a single request", async () => { + const fetchStub = stubFetch(); + + const pending = [ + fetchParsedAnimations("p", "index.html"), + fetchParsedAnimations("p", "index.html"), + fetchParsedAnimations("p", "index.html"), + ]; + fetchStub.settle(); + const results = await Promise.all(pending); + + expect(fetchStub.calls()).toBe(1); + expect(results.map((parsed) => parsed?.animations.length)).toEqual([1, 1, 1]); + }); + + it("does not share across files", async () => { + const fetchStub = stubFetch(); + + const pending = [ + fetchParsedAnimations("p", "index.html"), + fetchParsedAnimations("p", "other.html"), + ]; + fetchStub.settle(); + await Promise.all(pending); + + expect(fetchStub.calls()).toBe(2); + }); + + it("re-requests once the previous read has settled", async () => { + const fetchStub = stubFetch(); + + const first = fetchParsedAnimations("p", "index.html"); + fetchStub.settle(); + await first; + const second = fetchParsedAnimations("p", "index.html"); + fetchStub.settle(); + await second; + + expect(fetchStub.calls()).toBe(2); + }); +}); diff --git a/packages/studio/src/hooks/keyframeCacheAstLoad.ts b/packages/studio/src/hooks/keyframeCacheAstLoad.ts index 71e38bbb61..e4abc42525 100644 --- a/packages/studio/src/hooks/keyframeCacheAstLoad.ts +++ b/packages/studio/src/hooks/keyframeCacheAstLoad.ts @@ -42,7 +42,32 @@ function hasAnimations(value: unknown): value is ParsedGsapAnimations { ); } -export async function fetchParsedAnimations( +/** + * Requests for the same file that overlap in time, keyed `projectId|sourceFile`. + * + * Every parse re-reads and re-parses the whole composition server-side, and a + * multi-element action asks for the same file once per element. Sharing the + * in-flight promise makes that one request. Only OVERLAPPING calls share: the + * entry is dropped the moment it settles, so a call made after a write still + * gets a fresh parse. + */ +const inFlightParses = new Map>(); + +export function fetchParsedAnimations( + projectId: string, + sourceFile: string, +): Promise { + const key = `${projectId}|${sourceFile}`; + const inFlight = inFlightParses.get(key); + if (inFlight) return inFlight; + const request = requestParsedAnimations(projectId, sourceFile).finally(() => { + inFlightParses.delete(key); + }); + inFlightParses.set(key, request); + return request; +} + +async function requestParsedAnimations( projectId: string, sourceFile: string, ): Promise { diff --git a/packages/studio/src/hooks/useGsapAwareEditing.ts b/packages/studio/src/hooks/useGsapAwareEditing.ts index 0e84c15826..d23ae3b020 100644 --- a/packages/studio/src/hooks/useGsapAwareEditing.ts +++ b/packages/studio/src/hooks/useGsapAwareEditing.ts @@ -24,7 +24,11 @@ import { useGsapSaveFailureTelemetry, useSafeGsapCommitMutation, } from "./useSafeGsapCommitMutation"; -import type { CommitMutation } from "./gsapScriptCommitTypes"; +import type { + CommitMutation, + CommitMutationCall, + CommitMutationOptions, +} from "./gsapScriptCommitTypes"; import { setElementGsapPosition } from "../utils/elementGsap"; import { logResize, logResizeSettle } from "../utils/resizeDebug"; import type { DomEditGroupPathOffsetCommit } from "../components/editor/DomEditOverlay"; @@ -155,37 +159,73 @@ export function useGsapAwareEditing({ // it survives the N sequential server round-trips) onto each commit — // otherwise each member records its own entry and it takes N presses to undo. const coalesceKey = `group-drag:${++groupDragCommitCounter}`; - const coalescedCommit: typeof gsapCommitMutation = (selection, mutation, options) => - gsapCommitMutation(selection, mutation, { - ...options, - coalesceKey, - coalesceMs: Number.POSITIVE_INFINITY, + // Members are written one at a time, and a write that re-renders the preview + // re-runs the whole script — which still holds the OLD position of every + // member not yet written. Those members snap back to where they started and + // stay there until their own write lands, which is the single element seen + // jumping mid-commit while the rest of the group sat still. The drafted + // positions are already on screen, so holding the render until the last + // member has been written costs nothing and never shows a half-moved group. + let renderOnCommit = false; + const withGroupOptions = (options: CommitMutationOptions): CommitMutationOptions => ({ + ...options, + coalesceKey, + coalesceMs: Number.POSITIVE_INFINITY, + deferPreviewSync: !renderOnCommit, + }); + // Every member writes the same file. Queue their mutations and send them as + // ONE request instead of one round trip per member: the server reads, parses + // and writes the composition once, and the preview patches once. + const queued: CommitMutationCall[] = []; + const flushQueued = async () => { + if (queued.length === 0) return; + const calls = queued.splice(0, queued.length); + if (!gsapCommitMutation.batch) { + for (const call of calls) { + await gsapCommitMutation(call.selection, call.mutation, call.options); + } + return; + } + await gsapCommitMutation.batch(calls, { + ...(calls.at(-1)?.options ?? { label: "Move animated layer (group)" }), + label: "Move animated layer (group)", }); + }; + const coalescedCommit: typeof gsapCommitMutation = (selection, mutation, options) => { + queued.push({ selection, mutation, options: withGroupOptions(options) }); + return Promise.resolve(); + }; const preflightAnimations = new Map(); // Editability is user-atomic: prove every member can be written before // the first source mutation. Network failures after this point retain the // existing multi-request semantics, but a blocked member can never leave // earlier siblings partially moved. - for (const { selection } of updates) { - try { - const animations = await makeFetchFallback(selection, { failOnFetchError: true })(); - preflightAnimations.set(selection, animations); - const outcome = await tryGsapDragIntercept( - selection, - { x: 0, y: 0 }, - animations, - previewIframeRef.current, - coalescedCommit, - undefined, - { preflightOnly: true }, - ); - assertGsapEditPersisted(outcome); - } catch (error) { - trackGsapInteractionFailure(error, selection, "drag", "Move animated layer (group)"); - throw error; - } - } - for (const { selection, next } of updates) { + // Every member reads the same file, and a preflight writes nothing — so run + // them together. The parse layer shares one in-flight request per file, which + // turns N sequential round trips into one. + await Promise.all( + updates.map(async ({ selection }) => { + try { + const animations = await makeFetchFallback(selection, { failOnFetchError: true })(); + preflightAnimations.set(selection, animations); + const outcome = await tryGsapDragIntercept( + selection, + { x: 0, y: 0 }, + animations, + previewIframeRef.current, + coalescedCommit, + undefined, + { preflightOnly: true }, + ); + assertGsapEditPersisted(outcome); + } catch (error) { + trackGsapInteractionFailure(error, selection, "drag", "Move animated layer (group)"); + throw error; + } + }), + ); + for (const [index, { selection, next }] of updates.entries()) { + renderOnCommit = index === updates.length - 1; try { const outcome = await tryGsapDragIntercept( selection, @@ -193,7 +233,13 @@ export function useGsapAwareEditing({ preflightAnimations.get(selection) ?? [], previewIframeRef.current, coalescedCommit, - makeFetchFallback(selection), + // The intercept re-reads the file to resolve a stale or shared tween. + // Anything already queued has to be on disk before that read, or it + // resolves against a file missing writes it is about to build on. + async () => { + await flushQueued(); + return makeFetchFallback(selection)(); + }, { preflightPassed: true }, ); assertGsapEditPersisted(outcome); @@ -202,6 +248,15 @@ export function useGsapAwareEditing({ throw error; } } + try { + await flushQueued(); + } catch (error) { + const selection = updates.at(-1)?.selection; + if (selection) { + trackGsapInteractionFailure(error, selection, "drag", "Move animated layer (group)"); + } + throw error; + } }, [gsapCommitMutation, previewIframeRef, makeFetchFallback, trackGsapInteractionFailure], ); diff --git a/packages/studio/src/hooks/useGsapScriptCommits.test.tsx b/packages/studio/src/hooks/useGsapScriptCommits.test.tsx index 6236d032ab..76ca2129da 100644 --- a/packages/studio/src/hooks/useGsapScriptCommits.test.tsx +++ b/packages/studio/src/hooks/useGsapScriptCommits.test.tsx @@ -73,14 +73,77 @@ describe("applyPreviewSync", () => { syncDragPreview(result(), reloadPreview); - expect(patchRuntimeTweenInPlace).toHaveBeenCalledWith(FAKE_IFRAME, "#a", { - kind: "set", - props: { x: 10 }, - }); + expect(patchRuntimeTweenInPlace).toHaveBeenCalledWith( + FAKE_IFRAME, + "#a", + { + kind: "set", + props: { x: 10 }, + }, + undefined, + false, + ); + expect(applySoftReload).not.toHaveBeenCalled(); + expect(reloadPreview).not.toHaveBeenCalled(); + }); + + it("instantPatches: patches every element the batch wrote, rendering once at the end", () => { + patchRuntimeTweenInPlace.mockReturnValue(true); + const reloadPreview = vi.fn(); + + applyPreviewSync( + FAKE_IFRAME, + result(), + { + label: "Move animated layer (group)", + softReload: true, + instantPatches: [ + { selector: "#a", change: { kind: "set" as const, props: { x: 1 } } }, + { selector: "#b", change: { kind: "set" as const, props: { x: 2 } } }, + { selector: "#c", change: { kind: "set" as const, props: { x: 3 } } }, + ], + }, + reloadPreview, + ); + + // Only the last patch re-renders — the earlier two defer their seek, so the + // group repaints once instead of once per member. + expect(patchRuntimeTweenInPlace.mock.calls.map((call) => [call[1], call[4]])).toEqual([ + ["#a", true], + ["#b", true], + ["#c", false], + ]); expect(applySoftReload).not.toHaveBeenCalled(); expect(reloadPreview).not.toHaveBeenCalled(); }); + it("instantPatches: one patch that misses falls the whole batch back to the reload", () => { + patchRuntimeTweenInPlace.mockImplementation((_iframe, selector) => selector !== "#b"); + applySoftReload.mockReturnValue("applied"); + const reloadPreview = vi.fn(); + + applyPreviewSync( + FAKE_IFRAME, + result({ scriptText: "SCRIPT" }), + { + label: "Move animated layer (group)", + softReload: true, + instantPatches: [ + { selector: "#a", change: { kind: "set" as const, props: { x: 1 } } }, + { selector: "#b", change: { kind: "set" as const, props: { x: 2 } } }, + ], + }, + reloadPreview, + ); + + // A half-patched preview is worse than a reloaded one: "#a" landed, "#b" did + // not, so the reload repaints both from the written source. + expect(applySoftReload).toHaveBeenCalled(); + expect(trackStudioEvent).toHaveBeenCalledWith("gsap_instant_patch_fallback", { + selector: "#b", + }); + }); + it("instantPatch + patch fails: falls back to the soft reload, passing onAsyncFailure", () => { patchRuntimeTweenInPlace.mockReturnValue(false); applySoftReload.mockReturnValue("applied"); @@ -338,10 +401,16 @@ describe("runCommit — instantPatch wiring", () => { // The file already matched (changed:false) but the runtime patch deferred // from the paired first commit must still land. - expect(patchRuntimeTweenInPlace).toHaveBeenCalledWith(FAKE_IFRAME, "#a", { - kind: "set", - props: { x: 485, y: 311 }, - }); + expect(patchRuntimeTweenInPlace).toHaveBeenCalledWith( + FAKE_IFRAME, + "#a", + { + kind: "set", + props: { x: 485, y: 311 }, + }, + undefined, + false, + ); expect(deps.reloadPreview).not.toHaveBeenCalled(); }); diff --git a/packages/studio/src/hooks/useGsapScriptCommits.ts b/packages/studio/src/hooks/useGsapScriptCommits.ts index f1a21a3afe..fb042945d6 100644 --- a/packages/studio/src/hooks/useGsapScriptCommits.ts +++ b/packages/studio/src/hooks/useGsapScriptCommits.ts @@ -249,19 +249,28 @@ export function applyPreviewSync( options: CommitMutationOptions, reloadPreview: () => void, ): void { - if (options.instantPatch) { - const patched = patchRuntimeTweenInPlace( - iframe, - options.instantPatch.selector, - options.instantPatch.change, + const patches = options.instantPatches ?? (options.instantPatch ? [options.instantPatch] : []); + if (patches.length > 0) { + const deferSeek = options.deferPreviewSync === true; + const missed = patches.find( + (patch, index) => + !patchRuntimeTweenInPlace( + iframe, + patch.selector, + patch.change, + undefined, + deferSeek || index < patches.length - 1, + ), ); - // Patched in place — element is already correct on screen; no reload needed. - if (patched) return; + // Patched in place — elements are already correct on screen; no reload needed. + if (!missed) return; // The instant path couldn't patch in place — record the fallback so we can // track how often the fast path misses before the soft/full reload below. - trackStudioEvent("gsap_instant_patch_fallback", { selector: options.instantPatch.selector }); + trackStudioEvent("gsap_instant_patch_fallback", { selector: missed.selector }); // Fall through to the soft/full reload path below. } + // Written, but the caller has more writes to make and will render after the last. + if (options.deferPreviewSync) return; if (options.softReload && result.scriptText) { // A soft-reloadable edit escalates to a full iframe remount ONLY on the // PERMANENT "cannot-soft-reload" result (the preview is genuinely stale/ @@ -356,7 +365,12 @@ export function useGsapScriptCommits({ projectIdRef, activeCompPath, previewIfra ); if (!result) return; options.onResult?.(result); - await finalizeSuccessfulMutation(pid, compositionPath, last.selection, last.mutation, targetPath, result, options); + // Each call brings its own fast-path patch; the batch wrote them all, so the + // preview sync applies them all rather than just the last call's. + const instantPatches = calls + .map(({ options: callOptions }) => callOptions.instantPatch) + .filter((patch) => patch !== undefined); + await finalizeSuccessfulMutation(pid, compositionPath, last.selection, last.mutation, targetPath, result, instantPatches.length > 0 ? { ...options, instantPatches } : options); }, [showToast, finalizeSuccessfulMutation]); // Every GSAP-script commit is a read-modify-write of one file. Overlapping