Skip to content

Commit 723d338

Browse files
fix(studio): keep dense keyframes readable (#2925)
* perf(studio): define timeline viewport budgets and fixtures * test(studio): gate timeline viewport performance in Chromium * refactor(studio): isolate clip drag lifecycle * refactor(studio): extract timeline render contracts * perf(studio): centralize timeline viewport geometry * perf(studio): follow playhead across virtualized rows * perf(studio): add timeline clip-window index primitive * perf(studio): virtualize timeline clip windows * perf(studio): stop timeline scroll work when row virtualization is off The row virtualization stack made the timeline publish a viewport snapshot on every scroll frame and swap `renderClipContent` across every mounted clip at gesture start and settle. Both are windowing concessions, and neither was gated on the flag, so the build users actually run paid for them while mounting all 1,000 clips anyway. Measured on a 3,000-clip project: median scroll step 16.6ms to 76.9ms, p95 17.9ms to 189.4ms, 40 long tasks to 247. Gate both on the row virtualization flag. The scroll path now stops at the door when the flag is off, so `isScrolling` stays false and resize-driven and programmatic syncs still publish through the immediate path. The flag moves into its own module: the scroll-viewport hook needs to read it, and the virtualization hook already imports the viewport snapshot type back, which would have closed an import cycle. Also release the perf fixture lease from the fixture rather than from the test-hook effect. Loading a fixture writes player state, which changed that effect's dependency identities and tore it down on the next frame, so the lease was revoked moments after it was taken and live iframe discovery overwrote the fixture before the gate could measure it. The e2e gate gains a flag-off arm (`test:timeline-default`, 1,000 elements) next to the existing flag-on one. It refuses the 50,000-element combination, verifies from the mounted DOM that the server under test matches the requested flag, and skips the DOM-size budgets for the unvirtualized build rather than relaxing them, so a skipped budget never reads as a passed one. Verified against a live Studio dev server on the fixture project: flag off, before: interactionP95 303.1ms, longest task 194ms, 0/5 runs pass flag off, after: interactionP95 33.6ms, longest task 0ms, 5/5 runs pass flag on, after: interactionP95 33.2ms, 4/5 runs pass, exit 0 The flag-on arm's fourth run reproducibly reports a 55-58ms long task against a 50ms budget. That is the residual tail of the window swap itself, tracked separately and not addressed here. * ci(studio): run the timeline viewport gate on studio changes The gate has existed since the row virtualization stack landed but nothing under `.github/` referenced it, so it only ever ran when someone ran it by hand. That is how the flag-off scroll regression reached eight merged-ready PRs without anything noticing. Adds a `studio-timeline-viewport` job that boots two Studio dev servers, one per flag state, and runs both arms of the gate against them. Two servers are needed because row virtualization is read from `import.meta.env` at module load, so one process cannot serve both builds. Scoped to a new `studio` paths filter rather than the broad `code` one: the gate only says anything about `packages/studio`, `packages/core` and `packages/studio-server`. Adds a `ci` tier. It applies the constrained budgets without any emulation, because a hosted runner is already slower and noisier than the machine the strict numbers were recorded on, while the existing `low-resource` tier would throttle it a further 4x and measure the throttle rather than the build. The fixture composition is tracked under `tests/e2e/fixtures` but Studio resolves projects from the gitignored `data/projects`, so the job copies it into place instead of a project directory being committed. Both arms run in about 7 seconds each locally, so the job cost is almost entirely dependency install and the workspace build it shares with `studio-load-smoke`. * fix(ci): preserve both timeline gate evidence arms * ci(studio): report timeline gate arm statuses * ci(studio): require timeline gate evidence artifacts * fix(studio): keep dense keyframes readable * fix(ci): resolve timeline stack audit findings
1 parent fbfffb1 commit 723d338

82 files changed

Lines changed: 5754 additions & 1084 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.fallowrc.jsonc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -631,6 +631,11 @@
631631
// complexity pre-dates the computed-timeline work. Exempted at file level
632632
// rather than refactored as scope creep.
633633
"ignore": [
634+
// useGestureRecording.ts: readBasePosition/connectGsapRuntime/tick are
635+
// inherited gesture-runtime control flow. This stack only changes
636+
// recordSample to coalesce display-rate events onto authored frames;
637+
// the import and helper insertion shift the untouched functions' lines.
638+
"packages/studio/src/hooks/useGestureRecording.ts",
634639
// sourcePatcher.ts: resolveSourceFile / splitInlineStyleDeclarations /
635640
// patch*InTag pre-date this PR; only the PatchOperation type gained two
636641
// optional fields, but the line-shift fingerprint re-flags the inherited

.github/workflows/ci.yml

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ jobs:
3636
skills: ${{ steps.filter.outputs.skills }}
3737
codex_plugin: ${{ steps.filter.outputs.codex_plugin }}
3838
gcp_beginframe: ${{ steps.filter.outputs.gcp_beginframe }}
39+
studio: ${{ steps.filter.outputs.studio }}
3940
steps:
4041
# Force git-based change detection instead of the pull_request REST API.
4142
# The API path can fail the whole workflow on transient listFiles
@@ -77,6 +78,12 @@ jobs:
7778
- "scripts/package-codex-plugin.mjs"
7879
- "package.json"
7980
- ".github/workflows/ci.yml"
81+
studio:
82+
- "packages/studio/**"
83+
- "packages/core/**"
84+
- "packages/studio-server/**"
85+
- "bun.lock"
86+
- ".github/workflows/ci.yml"
8087
gcp_beginframe:
8188
- "packages/gcp-cloud-run/Dockerfile"
8289
- "packages/aws-lambda/scripts/probe-beginframe.ts"
@@ -496,6 +503,97 @@ jobs:
496503
497504
kill $SERVER_PID 2>/dev/null || true
498505
506+
studio-timeline-viewport:
507+
name: "Studio: timeline viewport gate"
508+
needs: [changes]
509+
if: needs.changes.outputs.studio == 'true'
510+
runs-on: ubuntu-latest
511+
timeout-minutes: 12
512+
steps:
513+
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
514+
- uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2
515+
- uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4
516+
with:
517+
node-version: 22
518+
- run: bun install --frozen-lockfile
519+
# Same reason as studio-load-smoke: vite.config.ts is loaded by Node and
520+
# resolves the workspace packages through their "node" export condition.
521+
- run: bun run --filter '@hyperframes/{parsers,lint,studio-server}' build
522+
- run: bun run --cwd packages/core build
523+
- run: bun run --cwd packages/core build:hyperframes-runtime
524+
- name: Install the fixture as a Studio project
525+
# Studio resolves projects from packages/studio/data/projects, which is
526+
# gitignored. The fixture composition is tracked under tests/e2e, so
527+
# copy it into place rather than committing a project directory.
528+
run: |
529+
mkdir -p packages/studio/data/projects
530+
cp -R packages/studio/tests/e2e/fixtures/timeline-virtualization \
531+
packages/studio/data/projects/timeline-virtualization
532+
- name: Run both arms of the timeline viewport gate
533+
run: |
534+
set -euo pipefail
535+
536+
# Two servers, because row virtualization is read from import.meta.env
537+
# at module load: one process cannot serve both builds.
538+
bun run --cwd packages/studio dev -- --port 5313 --strictPort &
539+
DEFAULT_PID=$!
540+
VITE_STUDIO_TIMELINE_ROW_VIRTUALIZATION_ENABLED=1 \
541+
bun run --cwd packages/studio dev -- --port 5314 --strictPort &
542+
VIRTUALIZED_PID=$!
543+
trap 'kill $DEFAULT_PID $VIRTUALIZED_PID 2>/dev/null || true' EXIT
544+
545+
for i in $(seq 1 60); do
546+
if curl -sf http://localhost:5313/ >/dev/null 2>&1 \
547+
&& curl -sf http://localhost:5314/ >/dev/null 2>&1; then break; fi
548+
sleep 1
549+
done
550+
if ! curl -sf http://localhost:5313/ >/dev/null 2>&1 \
551+
|| ! curl -sf http://localhost:5314/ >/dev/null 2>&1; then
552+
echo "FAIL: studio dev servers did not start"
553+
exit 1
554+
fi
555+
556+
# The default build first. It is the one users get, and the arm that
557+
# caught the regression this gate exists for.
558+
# Capture both statuses so either failure still leaves two evidence files.
559+
DEFAULT_STATUS=0
560+
STUDIO_URL="http://localhost:5313/#project/timeline-virtualization" \
561+
TIMELINE_ROW_VIRTUALIZATION=off \
562+
TIMELINE_ELEMENT_COUNT=1000 \
563+
TIMELINE_TIER=ci \
564+
node packages/studio/tests/e2e/timeline-virtualization.mjs \
565+
| tee /tmp/timeline-gate-default.json \
566+
|| DEFAULT_STATUS=$?
567+
568+
VIRTUALIZED_STATUS=0
569+
STUDIO_URL="http://localhost:5314/#project/timeline-virtualization" \
570+
TIMELINE_ROW_VIRTUALIZATION=on \
571+
TIMELINE_ELEMENT_COUNT=50000 \
572+
TIMELINE_TIER=ci \
573+
node packages/studio/tests/e2e/timeline-virtualization.mjs \
574+
| tee /tmp/timeline-gate-virtualized.json \
575+
|| VIRTUALIZED_STATUS=$?
576+
577+
{
578+
echo "### Timeline viewport gate"
579+
echo "- Default arm exit: ${DEFAULT_STATUS}"
580+
echo "- Virtualized arm exit: ${VIRTUALIZED_STATUS}"
581+
} >> "$GITHUB_STEP_SUMMARY"
582+
583+
if (( DEFAULT_STATUS != 0 || VIRTUALIZED_STATUS != 0 )); then
584+
echo "FAIL: default=${DEFAULT_STATUS}, virtualized=${VIRTUALIZED_STATUS}"
585+
exit 1
586+
fi
587+
- name: Upload gate evidence
588+
# The gate's whole output is machine-readable evidence, and a red run is
589+
# exactly when someone needs to read it. Keep it on failure too.
590+
if: always()
591+
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4
592+
with:
593+
name: timeline-viewport-gate-evidence
594+
path: /tmp/timeline-gate-*.json
595+
if-no-files-found: error
596+
499597
smoke-global-install:
500598
name: "Smoke: global install"
501599
needs: [changes, build]

bun.lock

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/studio/package.json

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,10 @@
4949
"build": "vite build && tsup",
5050
"typecheck": "tsc --noEmit",
5151
"test": "vitest run",
52+
"test:timeline-virtualization": "TIMELINE_ROW_VIRTUALIZATION=on TIMELINE_ELEMENT_COUNT=50000 node tests/e2e/timeline-virtualization.mjs",
5253
"test:watch": "vitest",
53-
"report:sdk-cutover": "bun src/utils/sdkCutoverPolicy.report.ts"
54+
"report:sdk-cutover": "bun src/utils/sdkCutoverPolicy.report.ts",
55+
"test:timeline-default": "TIMELINE_ROW_VIRTUALIZATION=off TIMELINE_ELEMENT_COUNT=1000 node tests/e2e/timeline-virtualization.mjs"
5456
},
5557
"dependencies": {
5658
"@codemirror/autocomplete": "^6.20.1",
@@ -70,6 +72,7 @@
7072
"@hyperframes/sdk": "workspace:*",
7173
"@hyperframes/studio-server": "workspace:*",
7274
"@phosphor-icons/react": "^2.1.10",
75+
"@tanstack/react-virtual": "^3.14.6",
7376
"bpm-detective": "^2.0.5",
7477
"dompurify": "^3.2.4",
7578
"gsap": "^3.13.0",

packages/studio/src/components/TimelineToolbar.tsx

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,13 @@ export function TimelineToolbar({ domEditSession, onSplitElement }: TimelineTool
118118
// the selection changes, not only on the next playhead tick.
119119
const selectedElementId = usePlayerStore((s) => s.selectedElementId);
120120
const elements = usePlayerStore((s) => s.elements);
121+
const timelineFitPps = usePlayerStore((s) => s.timelineFitPps);
121122
const { zoomMode, manualZoomPercent, setZoomMode, setManualZoomPercent } = useTimelineZoom();
122-
const displayedTimelineZoomPercent = getTimelineZoomPercent(zoomMode, manualZoomPercent);
123+
const displayedTimelineZoomPercent = getTimelineZoomPercent(
124+
zoomMode,
125+
manualZoomPercent,
126+
timelineFitPps,
127+
);
123128
const {
124129
state: keyframeState,
125130
isMotionPath: keyframeIsMotionPath,
@@ -418,7 +423,7 @@ export function TimelineToolbar({ domEditSession, onSplitElement }: TimelineTool
418423
onClick={() => {
419424
setZoomMode("manual");
420425
setManualZoomPercent(
421-
getNextTimelineZoomPercent("out", zoomMode, manualZoomPercent),
426+
getNextTimelineZoomPercent("out", zoomMode, manualZoomPercent, timelineFitPps),
422427
);
423428
}}
424429
className={flatIdle}
@@ -430,12 +435,14 @@ export function TimelineToolbar({ domEditSession, onSplitElement }: TimelineTool
430435
type="range"
431436
min="0"
432437
max="100"
433-
value={timelineZoomPercentToSlider(displayedTimelineZoomPercent)}
438+
value={timelineZoomPercentToSlider(displayedTimelineZoomPercent, timelineFitPps)}
434439
title={`${displayedTimelineZoomPercent}%`}
435440
aria-label="Timeline zoom"
436441
onChange={(e) => {
437442
setZoomMode("manual");
438-
setManualZoomPercent(timelineSliderToZoomPercent(Number(e.target.value)));
443+
setManualZoomPercent(
444+
timelineSliderToZoomPercent(Number(e.target.value), timelineFitPps),
445+
);
439446
}}
440447
// h-6 on the input is the 24x24 WCAG 2.2 (2.5.8) target: the visible
441448
// track stays 2px and the thumb 10px, only the pointer box grows.
@@ -447,7 +454,9 @@ export function TimelineToolbar({ domEditSession, onSplitElement }: TimelineTool
447454
aria-label="Zoom in"
448455
onClick={() => {
449456
setZoomMode("manual");
450-
setManualZoomPercent(getNextTimelineZoomPercent("in", zoomMode, manualZoomPercent));
457+
setManualZoomPercent(
458+
getNextTimelineZoomPercent("in", zoomMode, manualZoomPercent, timelineFitPps),
459+
);
451460
}}
452461
className={flatIdle}
453462
>

packages/studio/src/components/nle/NLEContext.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ export interface NLEContextValue {
4848
compositionLoading: boolean;
4949
setCompositionLoading: (loading: boolean) => void;
5050
timelineDisabled: boolean;
51+
timelineSessionEpoch: number;
5152
hasLoadedOnceRef: React.MutableRefObject<boolean>;
5253
// preview composition size (for preview block drop)
5354
previewCompositionSize: { width: number; height: number } | null;
@@ -103,7 +104,7 @@ export function NLEProvider({
103104
// project would otherwise keep rendering (and re-fetching from) the old project
104105
// after switching.
105106
useEffect(() => {
106-
usePlayerStore.getState().reset();
107+
usePlayerStore.getState().beginTimelineSession(projectId);
107108
useAssetPreviewStore.getState().clearPreviewAsset();
108109
}, [projectId]);
109110

@@ -289,6 +290,7 @@ export function NLEProvider({
289290
setCompositionLoadingRaw(loading);
290291
}, []);
291292
const timelineDisabled = shouldDisableTimelineWhileCompositionLoading(compositionLoading);
293+
const timelineSessionEpoch = usePlayerStore((state) => state.timelineSessionEpoch);
292294

293295
useEffect(() => {
294296
onCompositionLoadingChange?.(compositionLoading);
@@ -319,6 +321,7 @@ export function NLEProvider({
319321
compositionLoading,
320322
setCompositionLoading,
321323
timelineDisabled,
324+
timelineSessionEpoch,
322325
hasLoadedOnceRef,
323326
previewCompositionSize,
324327
setPreviewCompositionSize,

packages/studio/src/components/nle/TimelinePane.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ export function TimelinePane({
127127
persistTimelineH,
128128
containerRef,
129129
timelineDisabled,
130+
timelineSessionEpoch,
130131
} = useNLEContext();
131132

132133
// Move/resize/split come from the timeline edit context, not props — the
@@ -271,6 +272,7 @@ export function TimelinePane({
271272
>
272273
<div className="flex-shrink-0">{timelineToolbar}</div>
273274
<Timeline
275+
sessionEpoch={timelineSessionEpoch}
274276
onSeek={seek}
275277
onDrillDown={handleDrillDown}
276278
renderClipContent={renderClipContent}

packages/studio/src/hooks/useGestureRecording.test.tsx

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import React, { act } from "react";
44
import { afterEach, describe, expect, it, vi } from "vitest";
55
import { mountReactHarness } from "./domSelectionTestHarness";
6-
import { useGestureRecording } from "./useGestureRecording";
6+
import { useGestureRecording, type GestureSample } from "./useGestureRecording";
77

88
Reflect.set(globalThis, "IS_REACT_ACT_ENVIRONMENT", true);
99

@@ -13,6 +13,58 @@ afterEach(() => {
1313
});
1414

1515
describe("useGestureRecording", () => {
16+
it("keeps only the latest gesture sample in each output frame", () => {
17+
const animationFrames: FrameRequestCallback[] = [];
18+
vi.stubGlobal(
19+
"requestAnimationFrame",
20+
vi.fn((callback: FrameRequestCallback) => {
21+
animationFrames.push(callback);
22+
return animationFrames.length;
23+
}),
24+
);
25+
vi.stubGlobal("cancelAnimationFrame", vi.fn());
26+
let nowMs = 0;
27+
vi.spyOn(performance, "now").mockImplementation(() => nowMs);
28+
29+
const iframe = document.createElement("iframe");
30+
document.body.append(iframe);
31+
const element = iframe.contentDocument!.createElement("div");
32+
element.id = "card";
33+
iframe.contentDocument!.body.append(element);
34+
35+
let recording: ReturnType<typeof useGestureRecording> | null = null;
36+
function Harness() {
37+
recording = useGestureRecording();
38+
return null;
39+
}
40+
const root = mountReactHarness(<Harness />);
41+
42+
act(() => recording?.startRecording(element, iframe));
43+
document.dispatchEvent(new MouseEvent("pointermove", { bubbles: true, clientX: 10 }));
44+
nowMs = 1;
45+
act(() => animationFrames.shift()?.(1));
46+
document.dispatchEvent(new MouseEvent("pointermove", { bubbles: true, clientX: 20 }));
47+
nowMs = 10;
48+
act(() => animationFrames.shift()?.(10));
49+
document.dispatchEvent(new MouseEvent("pointermove", { bubbles: true, clientX: 40 }));
50+
nowMs = 20;
51+
act(() => animationFrames.shift()?.(20));
52+
document.dispatchEvent(new MouseEvent("pointermove", { bubbles: true, clientX: 50 }));
53+
nowMs = 30;
54+
act(() => animationFrames.shift()?.(30));
55+
56+
let samples: GestureSample[] = [];
57+
act(() => {
58+
samples = recording?.stopRecording() ?? [];
59+
});
60+
61+
expect(samples).toEqual([
62+
{ time: 0, properties: { x: 10, y: 0 } },
63+
{ time: 1 / 30, properties: { x: 40, y: 0 } },
64+
]);
65+
act(() => root.unmount());
66+
});
67+
1668
it("releases the preview runtime when unmounted during a recording", () => {
1769
const cancelAnimationFrame = vi.fn();
1870
vi.stubGlobal(

packages/studio/src/hooks/useGestureRecording.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { useCallback, useEffect, useRef, useState } from "react";
22
import { usePlayerStore, liveTime } from "../player/store/playerStore";
3+
import { frameToSeconds, secondsToFrame } from "../player/lib/time";
34

45
export interface GestureSample {
56
time: number;
@@ -139,7 +140,18 @@ function recordSample(r: RecordingRefs, time: number, properties: Record<string,
139140
// stripStudioEditsFromTarget in studio-api), so the keyframes must already
140141
// include it. Subtracting it made the committed gesture play shoved off by the
141142
// offset (the offset was removed twice).
142-
r.samples.push({ time, properties: { ...properties } });
143+
const frame = secondsToFrame(time);
144+
const sample = { time: frameToSeconds(frame), properties: { ...properties } };
145+
const lastIndex = r.samples.length - 1;
146+
const lastSample = r.samples[lastIndex];
147+
// Gesture events follow the display refresh rate (often 60/120Hz), but the
148+
// Studio preview is authored at 30fps. Keep the latest pointer state for each
149+
// output frame so recording cannot create visually indistinguishable piles.
150+
if (lastSample?.time === sample.time) {
151+
r.samples[lastIndex] = sample;
152+
} else {
153+
r.samples.push(sample);
154+
}
143155
r.trail.push({ x: r.pointer.x, y: r.pointer.y });
144156
}
145157

0 commit comments

Comments
 (0)