Skip to content

perf(studio): virtualize timeline marquee selection #2707

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
miguel-heygen merged 1 commit into from
Aug 4, 2026
Merged

perf(studio): virtualize timeline marquee selection #2707

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions packages/studio/src/player/components/Timeline.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,7 @@ export const Timeline = memo(function Timeline({
handlePointerDown,
handlePointerMove,
handlePointerUp,
handlePointerCancel,
} = useTimelineRangeSelection({
scrollRef,
ppsRef,
Expand All @@ -410,10 +411,11 @@ export const Timeline = memo(function Timeline({
isDragging,
setShowPopover,
elementsRef: expandedElementsRef,
trackOrderRef,
clipIndex,
rowGeometryRef,
onSelectElement,
contentOrigin,
sessionEpoch,
});
setRangeSelectionRef.current = setRangeSelection; // stable ref consumed by useTimelineClipDrag

Expand Down Expand Up @@ -484,7 +486,8 @@ export const Timeline = memo(function Timeline({
}}
onPointerMove={handlePointerMove}
onPointerUp={handlePointerUp}
onLostPointerCapture={handlePointerUp}
onPointerCancel={handlePointerCancel}
onLostPointerCapture={handlePointerCancel}

@james-russo-rames-d-jusso james-russo-rames-d-jusso Aug 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 (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

>
<TimelineCanvas
major={major}
Expand Down
98 changes: 73 additions & 25 deletions packages/studio/src/player/components/timelineMarquee.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,19 @@ import {
isTimelineRulerPress,
getMarqueeRect,
getTimelineClipRect,
getMarqueeClipCandidates,
computeMarqueeSelection,
} from "./timelineMarquee";
import { createTimelineClipIndex } from "../lib/timelineClipIndex";
import type { TimelineElement } from "../store/playerStore";
import {
GUTTER,
LANE_H,
TRACK_H,
RULER_H,
CLIP_Y,
TRACKS_LEFT_PAD,
createTimelineRowGeometry,
getTimelineRowTop,
} from "./timelineLayout";

Expand Down Expand Up @@ -95,9 +99,13 @@ describe("getMarqueeRect", () => {

describe("getTimelineClipRect", () => {
const trackOrder = [0, 2, 5];
const geometry = createTimelineRowGeometry(
trackOrder,
trackOrder.map(() => TRACK_H),
);

it("maps start/duration to x via pps and the track row to y via the shared row→y helper", () => {
const rect = getTimelineClipRect({ start: 2, duration: 3, track: 2 }, trackOrder, 100, GUTTER);
const rect = getTimelineClipRect({ start: 2, duration: 3, track: 2 }, geometry, 100, GUTTER);
expect(rect).toEqual({
left: GUTTER + 200,
top: getTimelineRowTop(1) + CLIP_Y,
Expand All @@ -107,61 +115,59 @@ describe("getTimelineClipRect", () => {
});

it("places the first visible track below the ruler + top breathing pad", () => {
const rect = getTimelineClipRect({ start: 0, duration: 1, track: 0 }, trackOrder, 50, GUTTER);
const rect = getTimelineClipRect({ start: 0, duration: 1, track: 0 }, geometry, 50, GUTTER);
expect(rect?.top).toBe(getTimelineRowTop(0) + CLIP_Y);
expect(rect?.left).toBe(GUTTER);
});

it("uses the row index in trackOrder, not the raw track number", () => {
const rect = getTimelineClipRect({ start: 0, duration: 1, track: 5 }, trackOrder, 50, GUTTER);
const rect = getTimelineClipRect({ start: 0, duration: 1, track: 5 }, geometry, 50, GUTTER);
expect(rect?.top).toBe(getTimelineRowTop(2) + CLIP_Y);
});

it("uses cumulative tops and the resolved height for an expanded row", () => {
const rowHeights = [TRACK_H + 2 * LANE_H, TRACK_H, TRACK_H];
const expandedGeometry = createTimelineRowGeometry(trackOrder, rowHeights);
const rect = getTimelineClipRect(
{ start: 0, duration: 1, track: 0 },
trackOrder,
expandedGeometry,
50,
GUTTER,
rowHeights,
);
expect(rect).toMatchObject({
top: getTimelineRowTop(0, rowHeights) + CLIP_Y,
height: rowHeights[0] - CLIP_Y * 2,
height: TRACK_H - CLIP_Y * 2,
});
expect(
getTimelineClipRect({ start: 0, duration: 1, track: 2 }, trackOrder, 50, GUTTER, rowHeights)
?.top,
getTimelineClipRect({ start: 0, duration: 1, track: 2 }, expandedGeometry, 50, GUTTER)?.top,
).toBe(getTimelineRowTop(1, rowHeights) + CLIP_Y);
});

it("enforces the 4px minimum rendered width", () => {
const rect = getTimelineClipRect(
{ start: 0, duration: 0.01, track: 0 },
trackOrder,
10,
GUTTER,
);
const rect = getTimelineClipRect({ start: 0, duration: 0.01, track: 0 }, geometry, 10, GUTTER);
expect(rect?.width).toBe(4);
});

it("returns null for a track that is not displayed or an invalid pps", () => {
expect(
getTimelineClipRect({ start: 0, duration: 1, track: 9 }, trackOrder, 100, GUTTER),
getTimelineClipRect({ start: 0, duration: 1, track: 9 }, geometry, 100, GUTTER),
).toBeNull();
expect(
getTimelineClipRect({ start: 0, duration: 1, track: 0 }, trackOrder, 0, GUTTER),
getTimelineClipRect({ start: 0, duration: 1, track: 0 }, geometry, 0, GUTTER),
).toBeNull();
expect(
getTimelineClipRect({ start: 0, duration: 1, track: 0 }, trackOrder, NaN, GUTTER),
getTimelineClipRect({ start: 0, duration: 1, track: 0 }, geometry, NaN, GUTTER),
).toBeNull();
});
});

describe("computeMarqueeSelection", () => {
// Two visible tracks: row 0 = track 0, row 1 = track 1. pps 100.
const trackOrder = [0, 1];
const rowGeometry = createTimelineRowGeometry(
trackOrder,
trackOrder.map(() => TRACK_H),
);
const pps = 100;
const clips = [
{ id: "a", start: 0, duration: 1, track: 0 }, // x [32,132], row 0
Expand All @@ -175,7 +181,7 @@ describe("computeMarqueeSelection", () => {
const marquee = { left: ORIGIN, top: row0Top, width: 50, height: 10 };
const { ids, primaryId } = computeMarqueeSelection({
clips,
trackOrder,
rowGeometry,
pps,
contentOrigin: ORIGIN,
marquee,
Expand All @@ -188,7 +194,7 @@ describe("computeMarqueeSelection", () => {
const marquee = { left: ORIGIN, top: row0Top, width: 60, height: row1Top - row0Top + 5 };
const { ids } = computeMarqueeSelection({
clips,
trackOrder,
rowGeometry,
pps,
contentOrigin: ORIGIN,
marquee,
Expand All @@ -200,7 +206,7 @@ describe("computeMarqueeSelection", () => {
const marquee = { left: ORIGIN + 140, top: row0Top, width: 50, height: 10 };
const { ids } = computeMarqueeSelection({
clips,
trackOrder,
rowGeometry,
pps,
contentOrigin: ORIGIN,
marquee,
Expand All @@ -212,7 +218,7 @@ describe("computeMarqueeSelection", () => {
const marquee = { left: GUTTER + 140, top: row0Top, width: 50, height: 10 };
const { ids, primaryId } = computeMarqueeSelection({
clips,
trackOrder,
rowGeometry,
pps,
contentOrigin: GUTTER,
marquee,
Expand All @@ -226,7 +232,7 @@ describe("computeMarqueeSelection", () => {
const marquee = { left: GUTTER, top: row1Top, width: 100, height: 10 };
const { ids, primaryId } = computeMarqueeSelection({
clips,
trackOrder,
rowGeometry,
pps,
contentOrigin: GUTTER,
marquee,
Expand All @@ -240,10 +246,11 @@ describe("computeMarqueeSelection", () => {
const wide = { left: ORIGIN, top: row0Top, width: 320, height: 10 };
const narrow = { left: ORIGIN, top: row0Top, width: 80, height: 10 };
expect(
computeMarqueeSelection({ clips, trackOrder, pps, contentOrigin: ORIGIN, marquee: wide }).ids,
computeMarqueeSelection({ clips, rowGeometry, pps, contentOrigin: ORIGIN, marquee: wide })
.ids,
).toEqual(new Set(["a", "b"]));
expect(
computeMarqueeSelection({ clips, trackOrder, pps, contentOrigin: ORIGIN, marquee: narrow })
computeMarqueeSelection({ clips, rowGeometry, pps, contentOrigin: ORIGIN, marquee: narrow })
.ids,
).toEqual(new Set(["a"]));
});
Expand All @@ -252,11 +259,52 @@ describe("computeMarqueeSelection", () => {
const marquee = { left: 0, top: 0, width: 10000, height: 10000 };
const { ids } = computeMarqueeSelection({
clips: [{ id: "x", start: 0, duration: 1, track: 7 }],
trackOrder,
rowGeometry,
pps,
contentOrigin: GUTTER,
marquee,
});
expect(ids).toEqual(new Set());
});
});

describe("getMarqueeClipCandidates", () => {
it("queries only the intersecting rows and time span", () => {
const rowGeometry = createTimelineRowGeometry([0, 1, 2], [TRACK_H, TRACK_H, TRACK_H]);
const near: TimelineElement = { id: "near", tag: "div", start: 1, duration: 1, track: 1 };
const wrongTime: TimelineElement = {
id: "wrong-time",
tag: "div",
start: 20,
duration: 1,
track: 1,
};
const wrongRow: TimelineElement = {
id: "wrong-row",
tag: "div",
start: 1,
duration: 1,
track: 2,
};
const clipIndex = createTimelineClipIndex([
[0, []],
[1, [near, wrongTime]],
[2, [wrongRow]],
]);

expect(
getMarqueeClipCandidates({
clipIndex,
rowGeometry,
marquee: {
left: ORIGIN + 100,
top: getTimelineRowTop(1),
width: 100,
height: TRACK_H - 1,
},
pps: 100,
contentOrigin: ORIGIN,
}),
).toEqual([near]);
});
});
60 changes: 44 additions & 16 deletions packages/studio/src/player/components/timelineMarquee.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { RULER_H, CLIP_Y, getTimelineRowHeight, getTimelineRowTop } from "./timelineLayout";
import { RULER_H, CLIP_Y, TRACK_H, type TimelineRowGeometry } from "./timelineLayout";
import { rectsOverlap, type Rect } from "../../utils/marqueeGeometry";
import { queryTimelineClipIndex, type TimelineClipIndex } from "../lib/timelineClipIndex";
import type { TimelineElement } from "../store/playerStore";

/** Pointer must travel at least this far (either axis) before a pointerdown on
* the empty timeline body becomes a marquee drag instead of a plain click. */
Expand Down Expand Up @@ -62,23 +64,22 @@ export function getMarqueeRect(
/**
* A clip's rendered rect in canvas/content coordinates (the same space the
* marquee rect lives in): x from the shared content origin + start * pps, y from the clip's row
* index within the visible track order (cumulative row top + CLIP_Y).
* index within the canonical row geometry (cumulative row top + CLIP_Y).
* Returns null when the clip's track is not currently displayed.
*/
export function getTimelineClipRect(
clip: Pick<MarqueeClipInput, "start" | "duration" | "track">,
trackOrder: number[],
rowGeometry: TimelineRowGeometry,
pps: number,
contentOrigin: number,
rowHeights: readonly number[] = [],
): Rect | null {
const row = trackOrder.indexOf(clip.track);
const row = rowGeometry.getRowIndex(clip.track);
if (row < 0 || !Number.isFinite(pps) || pps <= 0) return null;
return {
left: contentOrigin + clip.start * pps,
top: getTimelineRowTop(row, rowHeights) + CLIP_Y,
top: rowGeometry.getRowTop(row) + CLIP_Y,
width: Math.max(clip.duration * pps, MIN_CLIP_W),
height: getTimelineRowHeight(row, rowHeights) - CLIP_Y * 2,
height: TRACK_H - CLIP_Y * 2,

@james-russo-rames-d-jusso james-russo-rames-d-jusso Aug 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 (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

};
}

Expand All @@ -90,29 +91,56 @@ export interface MarqueeSelectionResult {
primaryId: string | null;
}

/** Narrow a marquee hit test to the intersecting logical rows and time span. */
export function getMarqueeClipCandidates(input: {
clipIndex: TimelineClipIndex;
rowGeometry: TimelineRowGeometry;
marquee: Rect;
pps: number;
contentOrigin: number;
}): readonly TimelineElement[] {
if (!(input.pps > 0) || input.marquee.width <= 0 || input.marquee.height <= 0) return [];
const lastRow = input.rowGeometry.rowKeys.length - 1;
const first = Math.max(0, Math.floor(input.rowGeometry.getRowFromY(input.marquee.top)));
const last = Math.min(
lastRow,
Math.floor(input.rowGeometry.getRowFromY(input.marquee.top + input.marquee.height)),
);
if (first > last) return [];
const paddingSeconds = MIN_CLIP_W / input.pps;
const start = Math.max(
0,
(input.marquee.left - input.contentOrigin) / input.pps - paddingSeconds,
);
const end =
(input.marquee.left + input.marquee.width - input.contentOrigin) / input.pps + paddingSeconds;
if (end <= start) return [];

const candidates: TimelineElement[] = [];
for (let row = first; row <= last; row += 1) {
const rowKey = input.rowGeometry.rowKeys[row];
if (rowKey === undefined) continue;

@james-russo-rames-d-jusso james-russo-rames-d-jusso Aug 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 (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

candidates.push(...queryTimelineClipIndex(input.clipIndex, rowKey, { start, end }));
}
return candidates;
}

/**
* Live marquee selection: every clip whose rendered rect intersects the marquee.
* `baseSelection` (shift/cmd-additive) is unioned in but never affects primaryId.
*/
export function computeMarqueeSelection(input: {
clips: MarqueeClipInput[];
trackOrder: number[];
rowGeometry: TimelineRowGeometry;
pps: number;
contentOrigin: number;
marquee: Rect;
baseSelection?: Iterable<string>;
rowHeights?: readonly number[];
}): MarqueeSelectionResult {
const ids = new Set<string>(input.baseSelection ?? []);
let primaryId: string | null = null;
for (const clip of input.clips) {
const rect = getTimelineClipRect(
clip,
input.trackOrder,
input.pps,
input.contentOrigin,
input.rowHeights,
);
const rect = getTimelineClipRect(clip, input.rowGeometry, input.pps, input.contentOrigin);
if (rect && rectsOverlap(rect, input.marquee)) {
ids.add(clip.id);
primaryId = clip.id;
Expand Down
Loading
Loading