perf(studio): virtualize timeline marquee selection

[miguel-heygen]

Summary

Makes timeline range/marquee selection operate on logical timeline coordinates rather than mounted DOM rows. Selection can therefore include eligible clips outside the current virtualized window while rendering only the bounded overlay work that is visible.

Changes

• Resolve selection candidates from timeline geometry and indexed clip data.
• Keep one window-owned pointer lifecycle through autoscroll and row unmounts.
• Preserve additive selection and range scrubbing behavior.
• Cover off-window candidates, row unmounts, cancellation, and autoscroll.

Stack

Family E, 4 of 7. Base: #2706. Next: #2708.

Validation

• Full Family E tip: 301 Studio test files passed; 3,271 tests passed; 18 todos.
• Studio typecheck, build, oxlint, oxfmt, diff checks, and Fallow audit pass with no new issues.
• Manual QA selected and displayed an exact timeline range with virtualization enabled.

[miguel-heygen]

• fix(studio): scope timeline ease focus lifecycle #2710
• perf(studio): stabilize virtualized beat gestures #2709
• fix(studio): scope timeline context targets #2708
• perf(studio): virtualize timeline marquee selection #2707 👈 (View in Graphite)
• perf(studio): stabilize virtualized timeline drops #2706
• perf(studio): stabilize virtualized keyframe retiming #2705
• perf(studio): stabilize virtualized clip gestures #2704
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[vanceingalls]

Verdict: APPROVE at e90c4d603b.

Consistent design language with the merged Family E stack — cancelActiveGesture central cleanup, session-epoch invalidation, foreign-pointer isolation, snapshot-before-clear on gesture start, live marquee selection driven by logical clip index (not mounted DOM rows). Writes go through the store singleton, so I explicitly checked for the "callback captures a per-row ref that goes stale on unmount" class I found on #2705 — none present here. No blockers.

What I verified

Gesture ownership is window-owned, not per-row (useTimelineRangeSelection.ts:318-347)

handlePointerDown captures on the scroll viewport via setPointerCapture(pointerId) (line 327), sets activePointerIdRef + gestureEpochRef at gesture start. All subsequent handlers filter by activePointerIdRef (line 400) so foreign pointers can't perturb the active gesture. Same pattern as #2704's pointerMatchesGesture.

Session-epoch double-gate (:165-170, :319-320, :575-579)

isGestureSessionCurrent() requires BOTH gestureEpochRef === sessionEpochRef.current (prop-derived) AND gestureEpochRef === usePlayerStore.getState().timelineSessionEpoch (store-derived). Effect at line 575 fires on prop-change → cancelActiveGesture(true, false) — critically, restoreSelection = false on session change (don't restore a marquee's pre-drag selection to a project that just switched). Correct semantics.

Marquee selection uses logical geometry, not mounted DOM (timelineMarquee.ts + commitMarqueeSelection:75-104)

getMarqueeClipCandidates({clipIndex, rowGeometry, marquee: rect, pps, contentOrigin}) queries the TimelineClipIndex (indexed clip data), NOT the mounted rows. A clip whose row is scrolled off-viewport still appears in the selection candidates as long as its geometry intersects the marquee rect. Directly implements the "selection can include eligible clips outside the current virtualized window" claim from the PR body.

Snapshot-before-clear (:306-316)

beginMarquee snapshots pre-drag selection via snapshotSelection() into marquee.baseIds + marquee.basePrimary. On Escape-cancel with restoreSelection=true (:522-526), the store is written back to the snapshot. Additive drag (shift/cmd/ctrl) unions new hits with baseIds instead of replacing. Correct semantic for both cancel and additive paths.

Cleanup convergence (cancelActiveGesture:506-534)

One centralized cleanup covers: activePointerIdRef + gestureEpochRef reset, all three RAF handles cancelled (marqueeScrollRaf, seekRafRef, dragScrollRaf), marquee state cleared, optional restoreSelection, optional UI reset. Called from four terminal paths:

• handlePointerCancel (:536-547) with restoreSelection = isGestureSessionCurrent()
• Escape keydown effect (:551-572) with restoreSelection = true when a marquee is active
• sessionEpoch change effect (:575-579) with restoreSelection = false
• Unmount cleanup (:581-586) with restoreSelection = isGestureSessionCurrent()

All four paths funnel through the same cleanup. No divergence.

Source-existence check on commit (finishMarquee:460-463)

Primary lookup uses elementsRef.current.find(el => (el.key ?? el.id) === primaryKey) ?? null — a deleted primary maps to null before calling onSelectElement(primary). Consistent with the Family E identity-keying pattern.

No stale-ref closures over unmount-able components. I explicitly checked commitMarqueeSelection (pure function, takes all inputs as parameters), applyMarqueeAtClient (writes to store singleton via usePlayerStore.getState()), and stepMarqueeAutoScroll (reads refs owned by the hook itself, not by any per-row component). This is the class I flagged on #2705 — none present here because the hook lives in Timeline.tsx (a stable ancestor), not in individual row components.

Non-blockers

1. marquee.baseIds doesn't filter deleted elements mid-drag. If an element in the pre-drag selection is deleted between marquee-start and drop, its ID stays in marquee.baseIds and gets unioned back via the additive path. setSelectedElementIds writes the stale ID; downstream render just doesn't highlight anything for that ID (no element to draw the border on). Non-corrupting, but the ID lingers in the selection set. Optional: filter marquee.baseIds against elementsRef.current at commit time.

2. Escape keydown listener uses default (bubble) phase (:570). No preventDefault() / stopPropagation(). #2704 uses capture-phase for Esc; if a modal-close handler ever registers with capture=false, the two would race depending on registration order. Cross-stack consistency concern, not this PR's regression.

3. Live marquee selection writes to the store on every pointermove during an active drag (via applyMarqueeAtClient → commitMarqueeSelection). No frame throttling. For a marquee with N candidate clips, this is one full selection recompute + two store writes per pointermove. Existing pattern from the family — the clipIndex-based recompute is O(candidates_in_rect), not O(all_clips), so should scale. Non-blocker but worth noting if perf ever surfaces at 60fps.

4. sessionEpoch is a prop (:39), same shape as #2704 and #2706. Same cross-stack observation I raised earlier — verify Timeline.tsx passes the SAME usePlayerStore.getState().timelineSessionEpoch-derived value to all four hooks, not one variant here and a different one elsewhere.

5. isMarqueePress uses viewport-space y (:118-126) — deliberately, per the comment at :335-337 explaining that sticky ruler decisions need viewport-y (content-y would break once the body scrolls down and the ruler visually overlays scrolled-away rows). Correct design choice; noting for anyone touching this in the future.

Family E stack ack

Family E, 4 of 7. Base: main (post-#2704/#2705/#2706 merges). Next: #2708. 33 focused marquee/range-selection tests + typecheck + lint + format green per PR body. CI: 17 passing, 13 running, 0 failing at time of review.

— Review by Via

[james-russo-rames-d-jusso]

Reviewed at e90c4d603b (draft — no stamp per the earlier undraft-first gate, but flagging findings now so they can travel with the CI bake).

The core refactor is sound — getMarqueeClipCandidates narrowing via rowGeometry.getRowFromY + queryTimelineClipIndex correctly decouples selection from the virtualized DOM window, and the activePointerId/gestureEpoch/session-epoch triple carried over from the Family E lifecycle work is applied consistently here (all 8 pointer-lifecycle invariants — foreign-pointer isolation on down/move/up/cancel, activePointerId reset on all exit paths, sessionEpoch closure freshness, mid-drag epoch cancel, autoscroll survives row unmounts via scroll-viewport-scoped capture, RAF cancelled on all 6 exit paths, window-listener cleanup, isDragging propagation — hold at head). The new useTimelineRangeSelection.test.tsx (204 lines, 6 well-shaped it() cases) pins off-window selection, row-unmount, and cancellation correctly.

That said, a few things worth surfacing before merge — none blocking, but worth Miguel's eyes.

Headline concerns (inline)

1. TRACK_H hardcode narrows marquee hit-rect for expanded-lane rows (timelineMarquee.ts:82). The old code used getTimelineRowHeight(row, rowHeights); the new code hard-codes height: TRACK_H - CLIP_Y * 2. For rows with property-lane expansion (rowHeight = TRACK_H + laneCount * LANE_H), the marquee hit-rect for each clip is now bounded to the clip-bar band at the top of the row, not the full row. This matches the visible clip-bar (per TimelineLanes.tsx:335-338), so it's geometrically correct — but it's a user-visible tightening (marquee overlapping only a row's keyframe strips no longer selects that row's clips). The PR body doesn't mention it; worth flagging so QA/design isn't surprised.

2. onLostPointerCapture flipped from commit to cancel (Timeline.tsx:490). Was onLostPointerCapture={handlePointerUp} (commit-on-loss); now onLostPointerCapture={handlePointerCancel} → cancelActiveGesture(true, isGestureSessionCurrent()), which restores pre-drag selection for an active marquee and unconditionally nulls the live range. Mid-gesture capture-loss (fullscreen swap, right-click, iOS interruption) now silently discards in-progress work. The one covering test (does not clear a finalized range when capture is lost after pointerup) only exercises the post-pointerup path where the guard short-circuits — no test drives mid-gesture capture loss on the new semantics.

3. stepMarqueeAutoScroll RAF captures stale clipIndex / contentOrigin (useTimelineRangeSelection.ts:255). The RAF reschedules itself with requestAnimationFrame(stepMarqueeAutoScroll) (line 255) and calls the closure-captured applyMarqueeAtClient from useCallback (deps at :226 include clipIndex, contentOrigin). If clipIndex reference changes mid-drag — a clip added or removed while the pointer sits in the edge zone — the RAF loop continues committing selection against the stale index until the next pointermove refreshes the handler. Practical impact small (clipIndex is memoized off tracks so mid-drag mutation is rare), but the pattern is fragile; a latestApplyRef (mutable ref updated each render) closes the gap.

4. Autoscroll test coverage claimed but not delivered (useTimelineRangeSelection.test.tsx). PR body lists "off-window candidates, row unmounts, cancellation, and autoscroll" as coverage. The 6 new it() cases cover the first three; nothing drives syncMarqueeAutoScroll / stepMarqueeAutoScroll / resolveTimelineAutoScrollLoopAction. The configureTimelineTestViewport fixture sets 800×240 and dragMarquee moves the pointer to (400, OFFSCREEN_ROW_Y) — well inside those bounds, edge-zone never triggered, marqueeScrollRaf.current never set. ~40 lines around useTimelineRangeSelection.ts:243-266 effectively uncovered. Given the RAF stale-closure concern above, that's the exact surface most worth pinning under test.

5. primaryId iteration order silently changed (timelineMarquee.ts:122). computeMarqueeSelection walks input.clips and re-assigns primaryId = clip.id on every overlap, so the last iterated overlapping clip wins. Before: input.clips = toMarqueeClips(elementsRef.current) (store-model order). Now: input.clips = toMarqueeClips([...candidates]) where candidates is row-outer / per-row ordinal-sorted from getMarqueeClipCandidates. If store order differs from row-outer+ordinal, the primary picks a different clip than before. No new test asserts primary specifically (only expectSelectedIds(...) for the set). If the reordering is intentional (deterministic "last row, last ordinal wins"), worth a docstring on getMarqueeClipCandidates; if incidental, primary should be computed against elementsRef.current order.

Nits (inline)

Plus three inline nits (state-hygiene on showPopover post-cancel, redundant null-check in handlePointerCancel guard, bare as React.PointerEvent cast).

PR body corrections (non-blocking)

• "One window-owned pointer lifecycle" — actually scroll-viewport-owned (data-timeline-scroll-viewport, Timeline.tsx:465). Mechanism is correct (scroll viewport outlives row unmounts), but the wording is imprecise; teammates reading the PR body may look for a literal window listener.
• "Rendering only the bounded overlay work that is visible" — this refers to the SELECTION scan (getMarqueeClipCandidates narrows to intersecting rows), not the OVERLAY layer. The marquee overlay still lives inside the row-list subtree (TimelineCanvas, Timeline.tsx:504 receives marqueeRect), so every drag frame still re-renders the row subtree — unlike #2704's hoisted TimelineGestureOverlay. Flagging so we don't over-attribute the perf gain.

Nice execution on the candidate-narrowing shape and the multi-layer session-epoch gate. Autoscroll test + PR body clarifications are the main items from my side.

— Review by Rames D Jusso

[james-russo-rames-d-jusso]

🟠 (concern) height: TRACK_H - CLIP_Y * 2 swapped in for the previous getTimelineRowHeight(row, rowHeights) - CLIP_Y * 2. For rows with property-lane expansion where rowHeight = TRACK_H + laneCount * LANE_H (per TimelineLanes.tsx:335-338), the marquee hit-rect for each clip is now bounded to the clip-bar band at the top of the row, not the full row. This matches the visible clip-bar (correct geometrically), but the effective behavior tightens: a marquee that only intersects a row's keyframe/property-lane strips no longer selects the row's clips. Not called out in the PR body — worth a line so QA/design isn't surprised. — Rames D Jusso

[james-russo-rames-d-jusso]

🟠 (concern) onLostPointerCapture={handlePointerCancel} (was handlePointerUp on main). handlePointerCancel calls cancelActiveGesture(true, isGestureSessionCurrent()) which restores pre-drag selection for an active marquee and unconditionally nulls the live range (useTimelineRangeSelection.ts:506-531). Mid-gesture capture-loss — fullscreen swap, right-click during a marquee, iOS interruption — now silently discards in-progress work instead of committing the drawn selection. The one covering test (does not clear a finalized range when capture is lost after pointerup) exercises only the post-pointerup path where the guard at useTimelineRangeSelection.ts:539 short-circuits on activePointerIdRef.current === null — no test drives capture-loss WHILE the gesture is active. If the semantic change is intentional (safer default: don't commit an accidentally-truncated selection), worth a line in the PR body; if not, handlePointerUp is the pre-PR shape. — Rames D Jusso

[james-russo-rames-d-jusso]

🟠 (concern) stepMarqueeAutoScroll reschedules itself via marqueeScrollRaf.current = requestAnimationFrame(stepMarqueeAutoScroll) and calls closure-captured applyMarqueeAtClient. The useCallback at ~:213 depends on clipIndex and contentOrigin; if either changes mid-drag while the pointer sits in an autoscroll edge zone (a clip added/removed → clipIndex re-memos off tracks, or contentOrigin drifts on a resize), the RAF loop keeps committing selection against the stale reference until the next pointermove refreshes the callback chain via syncMarqueeAutoScroll. Practical impact tiny (clipIndex is rarely mutated mid-drag), but the pattern is fragile — a latestApplyRef (mutable ref updated each render) closes the gap without changing hot-path behavior. — Rames D Jusso

[james-russo-rames-d-jusso]

🟠 (concern) PR body promises coverage for autoscroll, but no it() in this file drives syncMarqueeAutoScroll / stepMarqueeAutoScroll / resolveTimelineAutoScrollLoopAction. configureTimelineTestViewport fixes viewport at 800×240, and every dragMarquee call keeps the pointer well inside those bounds (OFFSCREEN_ROW_Y=getTimelineRowTop(50)+40 sits at row 50's top+40, still logical-y not client-y after toContentPoint folds scroll — but the fixture never SCROLLS into an edge). marqueeScrollRaf.current is never set in any test. Given the RAF stale-closure concern on the sibling file, this is the exact surface most worth pinning. A fake-timer test that dispatches pointermove at clientY = scrollRect.top + scrollRect.height + 1 and then advances RAF ticks would cover the whole :243-266 block. — Rames D Jusso

[james-russo-rames-d-jusso]

🟠 (concern) computeMarqueeSelection sets primaryId = clip.id on every overlap iteration, so the last iterated overlapping clip wins. In this PR the caller (commitMarqueeSelection in useTimelineRangeSelection.ts) now passes toMarqueeClips([...candidates]) where candidates is row-outer / per-row ordinal-sorted from getMarqueeClipCandidates:109-126. Before, input.clips came from toMarqueeClips(elementsRef.current) (store-model order). If elementsRef.current order differs from row-outer+ordinal (very likely — store order is typically insertion / hf-id order), a multi-row marquee's selectedElementId (primary) picks a different clip than pre-PR. No new test asserts primary specifically; the tests only check the set (expectSelectedIds(...)). If the reordering is intentional (deterministic 'last row + last ordinal wins'), worth a docstring on getMarqueeClipCandidates or a comment above primaryId = clip.id; if incidental, compute primary against elementsRef.current order. — Rames D Jusso

[james-russo-rames-d-jusso]

🟢 (nit — state hygiene) cancelActiveGesture with updateUi=true clears marqueeRect, rangeSelection, isScrubbing, but not showPopover. Not a user-visible bug — TimelineOverlays.tsx:88 gates the popover on {showPopover && rangeSelection && (...)}, so the null rangeSelection hides the popover anyway. Still worth adding setShowPopover(false); alongside for state-hygiene / to keep the invariant showPopover ⇒ rangeSelection !== null explicit at the reset boundary. — Rames D Jusso

[james-russo-rames-d-jusso]

🟢 (nit) if (activePointerIdRef.current === null || (e && activePointerIdRef.current !== null && e.pointerId !== activePointerIdRef.current)) return; — the second clause's activePointerIdRef.current !== null is dead: if we reach the right side of the ||, the first clause's === null already failed. Reads the same as if (activePointerIdRef.current === null || (e && e.pointerId !== activePointerIdRef.current)) return; No functional impact. — Rames D Jusso

[james-russo-rames-d-jusso]

🟢 (nit — CONTRIBUTING.md convention) pointer() test helper returns { ... } as React.PointerEvent;. CONTRIBUTING.md#type-safety-conventions prefers as unknown as T at hard type-system boundaries (test-mocks qualify), paired with a one-line justification comment. Minor. — Rames D Jusso