Skip to content

perf(studio): virtualize timeline clip windows - #2703

Open
miguel-heygen wants to merge 1 commit into
codex/studio-timeline-d-clip-index-v2from
codex/studio-timeline-d-clip-window-v2
Open

perf(studio): virtualize timeline clip windows#2703
miguel-heygen wants to merge 1 commit into
codex/studio-timeline-d-clip-index-v2from
codex/studio-timeline-d-clip-window-v2

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

What

Render only visible clip roots and defer horizontal reveal until virtual scroll geometry is ready.

Why

The final scalability boundary must bound clips as well as rows without hiding selected or active clips, losing reveal requests, shifting ticks, or allowing an early reveal to use stale scroll geometry.

How

  • Query the D7 interval index for each visible row and time window.
  • Pin selected, dragged, resized, context-menu, ease-focus, and reveal actors outside the normal window.
  • Keep active-clip lookup, ticks, and reveal behavior stable as clip roots mount and unmount.
  • Defer horizontal reveal until the current virtual scroll snapshot is ready.

This is D8 of the Family D draft review sequence. It targets current main after Family C landed; its Family D patch was replayed without semantic changes. It supersedes legacy PR #2640, which remains open for audit until landing. The unpublished local scroll-deferral source was folded here because the combined diff is 971 changed lines and both changes own the same lifecycle boundary.

Test plan

  • Unit tests added/updated
  • Manual testing performed
  • Documentation updated (not applicable)

Validated on the exact Family D tip with virtualization, active-clip, reveal, and player-seek coverage; the full Studio suite (2,939 passed; 18 todos); Studio Server files tests (67 passed); both typechecks; formatting/lint/file-size/Fallow gates; and two consecutive strict 50,000-element Chromium passes bounded to 207 mounted clip roots.

2026-07-29 rebase verification

  • Replayed D1-D8 onto current main after Family C landed; git range-diff marks every old/new Family D commit exactly equivalent (=).
  • oxfmt, oxlint, changed-file size gate, Studio typecheck, and Studio Server typecheck passed.
  • Full Studio suite: 288 files passed, 3,168 tests passed, 18 todos. Full Studio Server suite: 29 files and 398 tests passed.
  • Fallow CI audit exited successfully; its five minor observations are local duplication in existing test harness setup, not duplicated production decisions.
  • Chrome 150 browser evidence: 1,000-element control passed 5/5; strict 50,000-element gate passed its 4/5 quorum with 10 mounted rows, 240 mounted clip roots, 995 timeline descendants, approved direct scrolling, and memory returned. Repeated samples showed the same host-timing outlier pattern on the exact pre-rebase control.

No merge was performed; this PR remains a draft.

@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-clip-window-v2 branch from 0afcb3c to b7a9c06 Compare July 27, 2026 20:51
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-clip-index-v2 branch from 6883a5e to 99125c6 Compare July 28, 2026 20:43
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-clip-window-v2 branch from b7a9c06 to da7adb9 Compare July 28, 2026 20:43
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-clip-index-v2 branch from 99125c6 to 994baf5 Compare July 28, 2026 22:10
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-clip-window-v2 branch from da7adb9 to 4d007db Compare July 28, 2026 22:10
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-clip-index-v2 branch from 994baf5 to ef83c66 Compare July 28, 2026 22:36
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-clip-window-v2 branch from 4d007db to 6826c9f Compare July 28, 2026 22:36
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-clip-index-v2 branch from ef83c66 to 2dd814a Compare July 28, 2026 23:02
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-clip-window-v2 branch from 6826c9f to 1964f10 Compare July 28, 2026 23:02
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-clip-index-v2 branch from 2dd814a to b09e9f9 Compare July 28, 2026 23:22
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-clip-window-v2 branch from 1964f10 to 0fad6e5 Compare July 28, 2026 23:23
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-clip-index-v2 branch from b09e9f9 to c2cae90 Compare July 28, 2026 23:53
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-clip-window-v2 branch from 0fad6e5 to 0c6561c Compare July 28, 2026 23:53
@miguel-heygen
miguel-heygen changed the base branch from codex/studio-timeline-d-clip-index-v2 to main July 29, 2026 02:07
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-clip-window-v2 branch 2 times, most recently from 236fc38 to 1761957 Compare July 29, 2026 14:46
@miguel-heygen
miguel-heygen changed the base branch from main to codex/studio-timeline-d-clip-index-v2 July 29, 2026 15:04
@miguel-heygen
miguel-heygen marked this pull request as ready for review July 29, 2026 15:06
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-clip-window-v2 branch from 1761957 to 79f7704 Compare July 29, 2026 23:35
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-clip-index-v2 branch from c588a64 to 143d281 Compare July 29, 2026 23:35

@vanceingalls vanceingalls left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Verdict: APPROVE — grade A — rubric CORRECT

Role in stack: D8 payoff — clip-window virtualization delivering the -99.3% / -93.3% / -35.3% wins on the D1-D8 stack.

Root-cause fit: TimelineLanes.tsx:122 iterates virtualRows.map(...) (bounded row set), and inside each row queryTimelineClipIndex(clipIndex, trackNum, renderTimeRange, pinnedClipIdentities) (:129) returns only overlap + pinned clips. Off-window clips genuinely unmount: renderElements = els.filter(el => indexedRenderSet.has(el) || <multi-drag passenger>). No accidental keep-alive path.

Perf claim verification: PR body cites two consecutive strict 50k-element Chromium passes at 207 mounted clip roots (rebase update: 240) against maxMountedClipRoots: 512 / maxMountedClipRootsPerRow: 128 (timelineViewportBudgets.ts). Budgets internally consistent; observed well under cap. Overscan timeOverscanViewportRatio: 0.5 (2× viewport window), rowOverscanPerSide: 4. player-perf + preview-regression + Preview parity all green. Magnitudes plausible; independent Chromium re-run not attempted per read-only scope.

Claims verified (file:line at HEAD 79f77049):

  • Pinned actors — selected/dragged/resized/reveal/focused-ease/clip-context/keyframe-context — all seven: useTimelineClipRenderWindow.ts:53-75, threaded from Timeline.tsx:339-345.
  • Scrollbar accuracy: canvas outer at TimelineCanvas.tsx:114 uses totalH + full trackContentWidth, list at TimelineLanes.tsx:118 overlays absolute inset-0.
  • Sticky ticks: useTimelineTicks.ts:14 takes renderTimeRange and generateTicks filters to window (Timeline.tsx:436). Major-tick interval derived from full duration, so labels don't shift as the window scrolls.
  • Active-clip lookup stable across mount churn: useTimelineActiveClips.ts:117 calls applyActiveClipDiff(..., syncAll=true) on elementStateVersion change; a remounted clip regains data-active.
  • Reveal defer / coordinate-first: useTimelineRevealClip.ts:33-49 scrolls from model coordinates (no getBoundingClientRect); scrolledRequestRef gate + viewportVersion dep pins the request across scroll→mount cycle.
  • Drag survival: pinnedClipIdentities includes draggedElementId AND drag listeners are on window (timelineClipDragGestureLifecycle.ts:250-253) — two independent defenses.
  • Selection: selectedElementIds lives in usePlayerStore, not DOM. Test at Timeline.virtualization.test.tsx:198,224,242,252 scrolls 8000px away from clip-490 and asserts it stays mounted (via pin) AND reveal from off-window works.

Adversarial findings (all non-blocking):

  • No direct test asserts a mid-drag clip survives virtualization. Two orthogonal defenses (pin + window listeners) cover it; a "scroll during drag" assertion would close the loop.
  • Only reveal pins revealElementId; a clip that gained focus via ordinary click and then scrolls out has no focusedClipIdentity pin, so focus falls to <body>. Clips are tabIndex={-1} so focus is programmatic-only — practical hit small, but a document.activeElement-derived pin would close it.
  • useTimelineRevealClip.ts:49 synthesizes new Event("scroll") after programmatic scrollLeft/scrollTop, which also emit their own native scroll. Likely double-fires the snapshot listener; the debounce probably absorbs it, worth confirming.
  • renderClipContent={viewport.isScrolling ? undefined : renderClipContent} (Timeline.tsx:529) is a nice second-order defense against rich-content churn during scroll.

Standards: No any, no new empty catch, no .skip/.todo, no TODO/FIXME. File-list matches (16 files); +775/-228 arithmetic checks.

CI state: All required green on 79f77049. Body claims 288 studio files / 3,168 tests pass and 29 server files / 398 tests pass on rebase.

Suggested next step: Land. Optional follow-ups: (1) explicit drag-across-virtualization test, (2) focusedClipIdentity pin off document.activeElement, (3) verify the synthetic "scroll" dispatch isn't duplicate-firing. None gate D8.

Review by Via

@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-clip-index-v2 branch from 143d281 to 4f820c4 Compare July 30, 2026 00:23
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-clip-window-v2 branch from 79f7704 to 76db7a2 Compare July 30, 2026 00:23
@miguel-heygen

Copy link
Copy Markdown
Collaborator Author

Addressed at 76db7a269.

  • Consolidated the D8 render-window, pin, reveal, lane, and drag identity reads through the canonical timeline element identity helper.
  • Preserved all seven pinned actors and both drag-survival defenses.
  • Virtualization, lane, reveal, index, full Studio, and typecheck coverage pass on the rebuilt stack tip.

@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-clip-index-v2 branch from 4f820c4 to e5ce10a Compare July 30, 2026 01:31
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-clip-window-v2 branch from 76db7a2 to 46c0d25 Compare July 30, 2026 01:31
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-clip-index-v2 branch from e5ce10a to 06c0f1d Compare July 30, 2026 02:19
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-clip-window-v2 branch from 46c0d25 to 383514d Compare July 30, 2026 02:19

@miguel-heygen miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

🔴 Changes requested at exact head 383514d48407a55d40670a926b4bf292cced6a1f.

I reviewed this as the exact #2703 delta (06c0f1df..383514d4), not as a blended stack. Two assumptions in the core windowing path do not hold.

Findings

P1 — the interval index is followed by a full-row scan on every render

TimelineLanes.tsx:128-155 queries queryTimelineClipIndex(...), converts the result to a Set, and then calls els.filter(...) across every clip in the row. That makes the hot scroll/render path O(n) in the total clips on each visible row, even though #2702 built the interval index specifically to make the same query proportional to the visible result.

The index already restores matches to original projection order (timelineClipIndex.ts:146-173, ordinal sort), so the full scan is not needed for z/DOM order. The only extra behavior in the filter is multi-drag passenger projection. That can be merged only when a multi-drag preview exists, without making every normal scroll walk the full row.

The browser gate cannot catch this shape: the 50k dense-short fixture intentionally distributes clips so each row has at most 128 roots (timelinePerformanceFixture.ts:63-69, and its test pins <= 128). A real long track with thousands of sequential clips still performs a thousands-element scan per visible row per scroll update.

Please render directly from the indexed result on the normal path, merge projected multi-drag passengers separately when active, and add a regression/perf fixture with one long row containing thousands of off-window clips.

P1 — an early reveal can be consumed before the viewport has usable geometry

useTimelineRevealClip.ts:117-133 marks a request as scrolled, focuses the pinned clip, and clears the request without requiring a non-degenerate viewport. On initial/hidden layout, clientWidth / clientHeight can still be zero. computeRevealScroll deliberately returns { left: null, top: null } for that geometry, but the reveal target is pinned so it can mount and focus successfully; the hook then clears the request. A later ResizeObserver / viewportVersion update has nothing left to retry, so the requested clip remains off-screen.

The new test sets clientWidth=300 and clientHeight=100 synchronously in the ref callback, so it never exercises the zero-size → ready transition.

Please keep the request pending until the relevant viewport axes are usable (or pass readiness explicitly), and add a regression test that starts at zero dimensions, mounts/focuses the pinned clip, then resizes and verifies the reveal scroll occurs before consumption.

Verification

  • git diff --check passes for the exact delta.
  • Current exact-head checks have no active failure, but the stacked branch only exposes the preview/regression subset; this review does not treat that as proof of the two missing shapes above.

Coverage ledger

Audited: all 16 changed files — Timeline.tsx, Timeline.virtualization.test.tsx, TimelineCanvas.tsx, TimelineClip.tsx, TimelineLanes.test.tsx, TimelineLanes.tsx, timelineClipDragGestureLifecycle.ts, timelineLaneProps.ts, useTimelineActiveClips.test.ts, useTimelineActiveClips.ts, useTimelineClipRenderWindow.ts, useTimelineRevealClip.test.tsx, useTimelineRevealClip.ts, useTimelineTicks.ts, useTimelinePlayer.seek.test.ts, useTimelinePlayer.ts.

Trusting: none.

— Magi

@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-clip-window-v2 branch 2 times, most recently from 9b5b8b2 to b8c4dd9 Compare July 30, 2026 03:15
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-clip-index-v2 branch from 06c0f1d to 83f737a Compare July 30, 2026 03:15
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-clip-window-v2 branch from b8c4dd9 to 2922d03 Compare July 30, 2026 03:47
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-clip-index-v2 branch 2 times, most recently from 3a8a488 to e2d7d99 Compare July 30, 2026 04:05
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-clip-window-v2 branch from 2922d03 to 027a269 Compare July 30, 2026 04:05
@miguel-heygen

Copy link
Copy Markdown
Collaborator Author

Addressed both exact-delta blockers on the current head 027a26971:

  • Removed the post-index Set + els.filter(...) path. Virtualized rows now render the interval index result directly in original projection order, so a scroll query does not scan every clip in the row. The balanced max tree prunes non-overlapping branches and unions only visible clips, pins, and shifted drag actors.
  • Added adversarial 50k single-row coverage for the early-long-clip case and a shifted multi-drag window, plus deterministic randomized oracle coverage.
  • A reveal requested while the viewport is 0×0 now remains pending. The existing viewport ResizeObserver/version change retries it once geometry is ready; the zero→ready regression asserts scroll, focus, and request consumption.

Verification: 84 focused tests pass, Studio typecheck passes, and the exact stacked 50k browser gate mounts 160 clip roots / 707 timeline descendants (4/5 hosted-runner quorum; all DOM budgets pass). Exact job: https://github.com/heygen-com/hyperframes/actions/runs/30512973405/job/90776821797

@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-clip-window-v2 branch from 027a269 to df935c5 Compare July 30, 2026 15:25
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-clip-index-v2 branch from e2d7d99 to 029971e Compare July 30, 2026 15:25
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-clip-index-v2 branch from 029971e to 65a82ab Compare July 30, 2026 17:02
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-clip-window-v2 branch from df935c5 to 20c1f09 Compare July 30, 2026 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants