diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 75dc5cdbace..dd2732a6df0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -85,10 +85,20 @@ jobs: - name: Reject accidental file deletions if: github.event_name == 'pull_request' run: node scripts/check-no-main-deletions.mjs --base origin/main + # HEAD is the PR merge ref; its first parent is the current base tip, unlike the payload's + # base.sha, which goes stale and would count the base's newer commits as the PR's changes. + - name: Find the pull request's current base + id: base + if: github.event_name == 'pull_request' + run: | + git rev-parse --verify -q HEAD^2 > /dev/null || { echo "::error::HEAD is not the pull request merge commit"; exit 1; } + sha="$(git rev-parse --verify HEAD^1)" + echo "sha=$sha" >> "$GITHUB_OUTPUT" - uses: dorny/paths-filter@ceb8a2b8f2d89434be7ff52d3de7ec3738c5cc9d # v4 id: filter with: token: "" + base: ${{ steps.base.outputs.sha }} filters: | catalog_index: - "registry/**" @@ -823,6 +833,10 @@ jobs: needs: [changes] if: needs.changes.outputs.studio == 'true' runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + sample: [1, 2, 3] timeout-minutes: 18 steps: - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 @@ -1039,7 +1053,7 @@ jobs: if: always() uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 with: - name: timeline-viewport-gate-evidence + name: timeline-viewport-gate-evidence-${{ matrix.sample }} path: | /tmp/timeline-gate-*.json /tmp/studio-open-counts.json diff --git a/.github/workflows/player-perf.yml b/.github/workflows/player-perf.yml index f8310a70f1b..811f9a41ac5 100644 --- a/.github/workflows/player-perf.yml +++ b/.github/workflows/player-perf.yml @@ -30,10 +30,20 @@ jobs: - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 with: fetch-depth: 0 + # HEAD is the PR merge ref; its first parent is the current base tip, unlike the payload's + # base.sha, which goes stale and would count the base's newer commits as the PR's changes. + - name: Find the pull request's current base + id: base + if: github.event_name == 'pull_request' + run: | + git rev-parse --verify -q HEAD^2 > /dev/null || { echo "::error::HEAD is not the pull request merge commit"; exit 1; } + sha="$(git rev-parse --verify HEAD^1)" + echo "sha=$sha" >> "$GITHUB_OUTPUT" - uses: dorny/paths-filter@ceb8a2b8f2d89434be7ff52d3de7ec3738c5cc9d # v4 id: filter with: token: "" + base: ${{ steps.base.outputs.sha }} filters: | perf: - "packages/player/**" diff --git a/.github/workflows/preview-regression.yml b/.github/workflows/preview-regression.yml index f7666d8361f..ee66a154ead 100644 --- a/.github/workflows/preview-regression.yml +++ b/.github/workflows/preview-regression.yml @@ -29,10 +29,20 @@ jobs: - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 with: fetch-depth: 0 + # HEAD is the PR merge ref; its first parent is the current base tip, unlike the payload's + # base.sha, which goes stale and would count the base's newer commits as the PR's changes. + - name: Find the pull request's current base + id: base + if: github.event_name == 'pull_request' + run: | + git rev-parse --verify -q HEAD^2 > /dev/null || { echo "::error::HEAD is not the pull request merge commit"; exit 1; } + sha="$(git rev-parse --verify HEAD^1)" + echo "sha=$sha" >> "$GITHUB_OUTPUT" - uses: dorny/paths-filter@ceb8a2b8f2d89434be7ff52d3de7ec3738c5cc9d # v4 id: filter with: token: "" + base: ${{ steps.base.outputs.sha }} filters: | preview: - "packages/core/**" diff --git a/.github/workflows/regression.yml b/.github/workflows/regression.yml index 5b42d5515ef..9f8832bcc3a 100644 --- a/.github/workflows/regression.yml +++ b/.github/workflows/regression.yml @@ -50,11 +50,21 @@ jobs: - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 with: fetch-depth: 0 + # HEAD is the PR merge ref; its first parent is the current base tip, unlike the payload's + # base.sha, which goes stale and would count the base's newer commits as the PR's changes. + - name: Find the pull request's current base + id: base + if: github.event_name == 'pull_request' + run: | + git rev-parse --verify -q HEAD^2 > /dev/null || { echo "::error::HEAD is not the pull request merge commit"; exit 1; } + sha="$(git rev-parse --verify HEAD^1)" + echo "sha=$sha" >> "$GITHUB_OUTPUT" - uses: dorny/paths-filter@ceb8a2b8f2d89434be7ff52d3de7ec3738c5cc9d # v4 id: filter if: github.event_name != 'schedule' with: token: "" + base: ${{ steps.base.outputs.sha }} filters: | code: - "packages/core/**" diff --git a/.github/workflows/windows-render.yml b/.github/workflows/windows-render.yml index 6b79739d685..7c717fab650 100644 --- a/.github/workflows/windows-render.yml +++ b/.github/workflows/windows-render.yml @@ -53,10 +53,20 @@ jobs: - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 with: fetch-depth: 0 + # HEAD is the PR merge ref; its first parent is the current base tip, unlike the payload's + # base.sha, which goes stale and would count the base's newer commits as the PR's changes. + - name: Find the pull request's current base + id: base + if: github.event_name == 'pull_request' + run: | + git rev-parse --verify -q HEAD^2 > /dev/null || { echo "::error::HEAD is not the pull request merge commit"; exit 1; } + sha="$(git rev-parse --verify HEAD^1)" + echo "sha=$sha" >> "$GITHUB_OUTPUT" - uses: dorny/paths-filter@ceb8a2b8f2d89434be7ff52d3de7ec3738c5cc9d # v4 id: filter with: token: "" + base: ${{ steps.base.outputs.sha }} # A file counts only if it matches every pattern. Player and Studio `src/` run in a browser # and no code reads a package README, so a diff confined to them skips Windows. predicate-quantifier: every diff --git a/packages/studio/src/player/components/timelineLayout.test.ts b/packages/studio/src/player/components/timelineLayout.test.ts index 3a5c11c0416..11fce8c420a 100644 --- a/packages/studio/src/player/components/timelineLayout.test.ts +++ b/packages/studio/src/player/components/timelineLayout.test.ts @@ -27,13 +27,19 @@ import { resolveInsertRow } from "./timelineCollision"; import { getTimelineRenderTimeRange } from "./timelineViewportGeometry"; describe("horizontal timeline window", () => { - it("adds the shared half-viewport overscan on each side and clamps to duration", () => { + it("adds a quarter-viewport overscan on each side at rest and clamps to duration", () => { expect(getTimelineRenderTimeRange({ scrollLeft: 300, clientWidth: 500 }, 100, 200, 20)).toEqual( - { start: 0, end: 8.5 }, + { start: 0, end: 7.25 }, ); expect( getTimelineRenderTimeRange({ scrollLeft: 1_900, clientWidth: 500 }, 100, 200, 20), - ).toEqual({ start: 14.5, end: 20 }); + ).toEqual({ start: 15.75, end: 20 }); + }); + + it("takes the wider overscan a zoom preview asks for", () => { + expect( + getTimelineRenderTimeRange({ scrollLeft: 1_000, clientWidth: 500 }, 100, 200, 20, 0.5), + ).toEqual({ start: 5.5, end: 15.5 }); }); it("generates globally aligned ticks directly inside the bounded window", () => { diff --git a/packages/studio/src/player/components/timelineViewportGeometry.ts b/packages/studio/src/player/components/timelineViewportGeometry.ts index 6c408a01280..f00efab8083 100644 --- a/packages/studio/src/player/components/timelineViewportGeometry.ts +++ b/packages/studio/src/player/components/timelineViewportGeometry.ts @@ -8,11 +8,12 @@ export function getTimelineRenderTimeRange( pixelsPerSecond: number, contentOrigin: number, duration: number, + overscanRatio = TIMELINE_VIEWPORT_BUDGETS.timeOverscanViewportRatio, ): TimelineTimeRange { if (!(pixelsPerSecond > 0) || !(duration > 0) || !(viewport.clientWidth > 0)) { return { start: 0, end: 0 }; } - const overscanPx = viewport.clientWidth * TIMELINE_VIEWPORT_BUDGETS.timeOverscanViewportRatio; + const overscanPx = viewport.clientWidth * overscanRatio; const startPx = viewport.scrollLeft - contentOrigin - overscanPx; const endPx = viewport.scrollLeft + viewport.clientWidth - contentOrigin + overscanPx; return { diff --git a/packages/studio/src/player/components/timelineZoomInput.test.ts b/packages/studio/src/player/components/timelineZoomInput.test.ts index f7ee6c8fa98..ba1d4e2af11 100644 --- a/packages/studio/src/player/components/timelineZoomInput.test.ts +++ b/packages/studio/src/player/components/timelineZoomInput.test.ts @@ -2,10 +2,13 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { usePlayerStore } from "../store/playerStore"; +import { TIMELINE_VIEWPORT_BUDGETS } from "../lib/timelineViewportBudgets"; import { isTimelineMoving, subscribeTimelineMotion } from "./timelineMotion"; import { cancelTimelineZoom, isTimelineZoomPreviewing, + isTimelineZoomWindowWide, + markTimelineZoomWindowMounted, subscribeTimelineZoomPreview, redrawTimelineZoomPreview, currentTimelineRange, @@ -64,6 +67,18 @@ function viewport(scrollLeft = 0, scrollWidth = 20_000) { return { scroll, row }; } +/** What the timeline's render window does: mounts the zoom's wider window while a preview shows. */ +let unmountZoomWindow = () => {}; +beforeEach(() => { + let mounted = false; + unmountZoomWindow = subscribeTimelineZoomPreview(() => { + if (isTimelineZoomWindowWide() === mounted) return; + mounted = isTimelineZoomWindowWide(); + if (mounted) markTimelineZoomWindowMounted(TIMELINE_VIEWPORT_BUDGETS.zoomOverscanViewportRatio); + }); +}); +afterEach(() => unmountZoomWindow()); + /** Where `time` lands on screen once the committed zoom is laid out. */ const laidOutX = (time: number) => { const anchor = takeTimelineZoomAnchor(); @@ -140,6 +155,21 @@ describe("requestTimelineZoom", () => { expect(usePlayerStore.getState().timelinePps).toBe(100); }); + it("lays the same zoom-out out at once while only the rest window is mounted", () => { + unmountZoomWindow(); + usePlayerStore.setState({ + duration: 100, + zoomMode: "manual", + manualZoomPercent: 1000, + timelinePps: 100, + }); + viewport(5000); + // Mounted 46.98..63.18 s at rest, short of the 46.5..63.97 s the preview would show. + requestTimelineZoom(600, { time: 55.24, x: 556 }); + vi.advanceTimersToNextFrame(); + expect(usePlayerStore.getState().timelinePps).toBe(60); + }); + it("lays out a zoom-out before it shows past the window ruler ticks are drawn in", () => { // 50 s of clips in content 1996 s wide: ticks are drawn to 157 s, a view and a half in. usePlayerStore.setState({ duration: 50 }); diff --git a/packages/studio/src/player/components/timelineZoomInput.ts b/packages/studio/src/player/components/timelineZoomInput.ts index a2e82866f06..8a0a3631200 100644 --- a/packages/studio/src/player/components/timelineZoomInput.ts +++ b/packages/studio/src/player/components/timelineZoomInput.ts @@ -52,6 +52,7 @@ let viewport: TimelineZoomViewport | null = null; /** While a preview shows, scales rows and strips React mounts into the timeline before they paint. */ let mounts: MutationObserver | null = null; let preview: ZoomPreview | null = null; +let wideWindow = false; let frame = 0; let restTimer: ReturnType | null = null; let anchorForCommit: TimelineZoomAnchor | null = null; @@ -88,6 +89,7 @@ export function timelineZoomMapping(pps: number, contentOrigin: number) { /** Whether a zoom is drawn scaled right now, so boxes read off the page are scaled too. */ export const isTimelineZoomPreviewing = (): boolean => preview !== null; +export const isTimelineZoomWindowWide = (): boolean => wideWindow; /** Called each frame a zoom preview moves, and once when it is laid out or dropped. */ export function subscribeTimelineZoomPreview(listener: () => void): () => void { @@ -154,9 +156,20 @@ function previewNeedsLayout(p: ZoomPreview, scroll: HTMLDivElement, contentOrigi } /** The times laid out now: the render window clips, ruler ticks and beat lines are drawn in. */ -function drawnRange(scroll: HTMLDivElement, pps: number, contentOrigin: number): TimelineTimeRange { +function drawnRange( + scroll: HTMLDivElement, + pps: number, + contentOrigin: number, + overscanRatio?: number, +): TimelineTimeRange { const contentEnd = (scroll.scrollWidth - contentOrigin) / pps; - return getTimelineRenderTimeRange(scroll, pps, contentOrigin, contentEnd); + return getTimelineRenderTimeRange(scroll, pps, contentOrigin, contentEnd, overscanRatio); +} + +export function markTimelineZoomWindowMounted(overscanRatio: number) { + const view = viewport; + if (!preview || !view) return; + preview.drawn = drawnRange(view.scroll, preview.basePps, view.contentOrigin, overscanRatio); } function scalePreview(scroll: HTMLElement) { @@ -202,6 +215,7 @@ function commitPreview() { const done = preview; if (!view || !done) return; const left = view.scroll.scrollLeft - done.shift; + wideWindow = false; // Ending at the scale already laid out (an eased zoom-out) needs a scroll, not a layout. if (done.pps !== done.basePps) { flushSync(() => @@ -237,6 +251,7 @@ function request(percent: number, anchor: TimelineZoomAnchor | null, byPerson: b const { scroll, contentOrigin } = view; const now = shown(scroll); const at = anchor ?? defaultAnchor(view, now.pps, now.left); + const starting = !preview; preview ??= { percent: clamped, pps, @@ -254,6 +269,11 @@ function request(percent: number, anchor: TimelineZoomAnchor | null, byPerson: b preview.pps = pps; preview.shift = scroll.scrollLeft - left; preview.byPerson ||= byPerson; + // Before the first frame, so the timeline mounts the preview's wider window in time to show it. + if (starting) { + wideWindow = true; + emitPreview(); + } if (!frame) frame = requestAnimationFrame(drawPreview); if (restTimer) clearTimeout(restTimer); restTimer = setTimeout(commitPreview, TIMELINE_REST_MS); @@ -288,6 +308,7 @@ function dropPreview() { if (!preview) return; if (viewport) clearScaled(viewport.scroll); preview = null; + wideWindow = false; emitPreview(); } @@ -411,6 +432,8 @@ function easeZoom( const holdsOld = old.start >= start && old.end <= start + (old.end - old.start) * (from.pps / toPps); let layoutFirst = toPps < from.pps && from.pps / toPps <= MAX_PREVIEW_SCALE && holdsOld; + // A preview at the current scale, so the timeline mounts the zoom's wider window before frame one. + if (!layoutFirst) request(fromPercent, anchorAt(0), byPerson); const began = performance.now(); easingTo = toPercent; const step = (now: number) => { diff --git a/packages/studio/src/player/components/useTimelineClipRenderWindow.test.tsx b/packages/studio/src/player/components/useTimelineClipRenderWindow.test.tsx new file mode 100644 index 00000000000..d016fabd56d --- /dev/null +++ b/packages/studio/src/player/components/useTimelineClipRenderWindow.test.tsx @@ -0,0 +1,111 @@ +// @vitest-environment happy-dom +import React, { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { afterEach, beforeEach, expect, it, vi } from "vitest"; +import { usePlayerStore } from "../store/playerStore"; +import { + cancelTimelineZoom, + registerTimelineZoomViewport, + requestTimelineZoom, + settleTimelineZoom, + zoomTimelineToRange, +} from "./timelineZoomInput"; +import { getTimelineRenderTimeRange } from "./timelineViewportGeometry"; +import { useTimelineClipRenderWindow } from "./useTimelineClipRenderWindow"; +import type { TimelineScrollViewportSnapshot } from "./useTimelineScrollViewport"; + +(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +let root: Root | null = null; +let unregisterViewport = () => {}; + +beforeEach(() => { + vi.useFakeTimers({ + toFake: [ + "requestAnimationFrame", + "cancelAnimationFrame", + "setTimeout", + "clearTimeout", + "performance", + ], + }); + // 1000% of a 10 px/s fit, scrolled to 50 s in a 1080px viewport with 32px of track headers. + usePlayerStore.setState({ + zoomMode: "manual", + manualZoomPercent: 1000, + timelineFitPps: 10, + timelinePps: 100, + duration: 100, + currentTime: 0, + }); + const scroll = document.createElement("div"); + Object.defineProperties(scroll, { + clientWidth: { value: 1080 }, + scrollWidth: { value: 10_032 }, + scrollLeft: { value: 5000, writable: true }, + }); + unregisterViewport = registerTimelineZoomViewport({ scroll, contentOrigin: 32 }); +}); + +afterEach(() => { + act(() => cancelTimelineZoom()); + act(() => root?.unmount()); + root = null; + unregisterViewport(); + vi.useRealTimers(); +}); + +const SNAPSHOT = { scrollLeft: 5000, clientWidth: 1080 } as TimelineScrollViewportSnapshot; + +function renderWindow(renders: { start: number; end: number }[] = []) { + let range = { start: 0, end: 0 }; + function Harness() { + const pixelsPerSecond = usePlayerStore((state) => state.timelinePps); + ({ renderTimeRange: range } = useTimelineClipRenderWindow({ + tracks: [], + viewport: SNAPSHOT, + pixelsPerSecond, + contentOrigin: 32, + duration: 100, + })); + renders.push(range); + return null; + } + const host = document.body.appendChild(document.createElement("div")); + root = createRoot(host); + act(() => root?.render(React.createElement(Harness))); + return () => range; +} + +it("mounts a quarter viewport each side at rest and half while a zoom previews", () => { + const range = renderWindow(); + expect(range()).toEqual({ start: 46.98, end: 63.18 }); + act(() => requestTimelineZoom(600, { time: 55.24, x: 556 })); + expect(range()).toEqual({ start: 44.28, end: 65.88 }); + act(() => cancelTimelineZoom()); + expect(range()).toEqual({ start: 46.98, end: 63.18 }); +}); + +it("lets a zoom-out preview show the wider window it mounted", () => { + renderWindow(); + // At 60 px/s about 55.24 s the view shows 46.5..63.97 s: past the rest window, inside the zoom's. + act(() => requestTimelineZoom(600, { time: 55.24, x: 556 })); + act(() => vi.advanceTimersToNextFrame()); + expect(usePlayerStore.getState().timelinePps).toBe(100); +}); + +it("lays a zoom out in one render, with the rest window", () => { + const renders: { start: number; end: number }[] = []; + renderWindow(renders); + act(() => requestTimelineZoom(600, { time: 55.24, x: 556 })); + renders.length = 0; + act(() => settleTimelineZoom()); + expect(renders).toEqual([getTimelineRenderTimeRange(SNAPSHOT, 60, 32, 100)]); +}); + +it("keeps an eased zoom-out a preview in its first frame", () => { + renderWindow(); + act(() => void zoomTimelineToRange(60, 80, { smooth: true })); + act(() => vi.advanceTimersToNextFrame()); + expect(usePlayerStore.getState().timelinePps).toBe(100); +}); diff --git a/packages/studio/src/player/components/useTimelineClipRenderWindow.ts b/packages/studio/src/player/components/useTimelineClipRenderWindow.ts index 13d968a0951..eec9bdf6d06 100644 --- a/packages/studio/src/player/components/useTimelineClipRenderWindow.ts +++ b/packages/studio/src/player/components/useTimelineClipRenderWindow.ts @@ -1,10 +1,16 @@ -import { useMemo } from "react"; +import { useLayoutEffect, useMemo, useSyncExternalStore } from "react"; import { createTimelineClipIndex } from "../lib/timelineClipIndex"; +import { TIMELINE_VIEWPORT_BUDGETS } from "../lib/timelineViewportBudgets"; import { getTimelineRenderTimeRange, getTimelineVisibleTimeRange, } from "./timelineViewportGeometry"; import type { TimelineScrollViewportSnapshot } from "./useTimelineScrollViewport"; +import { + isTimelineZoomWindowWide, + markTimelineZoomWindowMounted, + subscribeTimelineZoomPreview, +} from "./timelineZoomInput"; interface UseTimelineClipRenderWindowInput { tracks: Parameters[0]; @@ -36,10 +42,18 @@ export function useTimelineClipRenderWindow({ keyframeContextMenuElementId, }: UseTimelineClipRenderWindowInput) { const clipIndex = useMemo(() => createTimelineClipIndex(tracks), [tracks]); + const zooming = useSyncExternalStore(subscribeTimelineZoomPreview, isTimelineZoomWindowWide); + const overscanRatio = zooming + ? TIMELINE_VIEWPORT_BUDGETS.zoomOverscanViewportRatio + : TIMELINE_VIEWPORT_BUDGETS.timeOverscanViewportRatio; const renderTimeRange = useMemo( - () => getTimelineRenderTimeRange(viewport, pixelsPerSecond, contentOrigin, duration), - [contentOrigin, duration, pixelsPerSecond, viewport], + () => + getTimelineRenderTimeRange(viewport, pixelsPerSecond, contentOrigin, duration, overscanRatio), + [contentOrigin, duration, overscanRatio, pixelsPerSecond, viewport], ); + useLayoutEffect(() => { + if (zooming) markTimelineZoomWindowMounted(overscanRatio); + }, [overscanRatio, zooming]); const visibleTimeRange = useMemo( () => getTimelineVisibleTimeRange(viewport, pixelsPerSecond, contentOrigin, duration), [contentOrigin, duration, pixelsPerSecond, viewport], diff --git a/packages/studio/src/player/components/useTimelinePlayhead.test.tsx b/packages/studio/src/player/components/useTimelinePlayhead.test.tsx index 528f3fb43a5..01e418adf9a 100644 --- a/packages/studio/src/player/components/useTimelinePlayhead.test.tsx +++ b/packages/studio/src/player/components/useTimelinePlayhead.test.tsx @@ -29,6 +29,10 @@ interface HarnessProps { zoomMode?: ZoomMode; } +/** The scroll positions the hook published, read at the moment it published them. */ +const published: number[] = []; +const syncScrollViewport = (el: HTMLDivElement) => published.push(el.scrollLeft); + function Harness({ pps: fixedPps, scroll, dragging = false, zoomMode = "manual" }: HarnessProps) { const storePps = usePlayerStore((s) => s.timelinePps); const pps = fixedPps ?? storePps; @@ -50,6 +54,7 @@ function Harness({ pps: fixedPps, scroll, dragging = false, zoomMode = "manual" timelineReady: true, elementsLength: 1, contentOrigin: ORIGIN, + syncScrollViewport, }); return null; } @@ -67,6 +72,7 @@ function mount(props: HarnessProps) { } beforeEach(() => { + published.length = 0; usePlayerStore.setState({ currentTime: 0, isPlaying: false, beatDragging: false }); }); afterEach(() => { @@ -103,6 +109,16 @@ describe("useTimelinePlayhead zoom anchor", () => { expect(scroll.scrollLeft).toBe(0); }); + it("publishes the scroll a zoom lands on, so the clips mounted for it are the ones shown", () => { + usePlayerStore.setState({ currentTime: 6 }); + const scroll = scrollBox(400); + const rezoom = mount({ pps: 100, scroll }); + published.length = 0; + rezoom({ pps: 200 }, true); + expect(published).toEqual([scroll.scrollLeft]); + expect(scroll.scrollLeft).not.toBe(400); + }); + it("keeps the playhead where it is on screen when the toolbar zooms", () => { usePlayerStore.setState({ currentTime: 6 }); const scroll = scrollBox(400); diff --git a/packages/studio/src/player/components/useTimelinePlayhead.ts b/packages/studio/src/player/components/useTimelinePlayhead.ts index 6310fb537e1..e86713d19ec 100644 --- a/packages/studio/src/player/components/useTimelinePlayhead.ts +++ b/packages/studio/src/player/components/useTimelinePlayhead.ts @@ -56,6 +56,7 @@ interface UseTimelinePlayheadInput { elementsLength: number; onSeek?: (time: number) => void; contentOrigin: number; + syncScrollViewport: (el: HTMLDivElement) => void; } export function useTimelinePlayhead({ @@ -75,6 +76,7 @@ export function useTimelinePlayhead({ elementsLength, onSeek, contentOrigin, + syncScrollViewport, }: UseTimelinePlayheadInput) { const dragScrollRaf = useRef(0); const previousZoomModeRef = useRef(zoomMode); @@ -100,6 +102,7 @@ export function useTimelinePlayhead({ const maxScrollLeft = Math.max(0, scroll.scrollWidth - scroll.clientWidth); const left = anchor.time * pps + contentOrigin - anchor.x; scroll.scrollLeft = Math.max(0, Math.min(maxScrollLeft, left)); + syncScrollViewport(scroll); return; } const zoomed = userZoomCount !== prevZoomCount; @@ -121,7 +124,8 @@ export function useTimelinePlayhead({ scroll.scrollLeft = zoomed ? revealPlayheadScrollLeft(scroll, contentOrigin + time * pps, contentOrigin, anchored) : anchored; - }, [pps, userZoomCount, scrollRef, durationRef, contentOrigin]); + syncScrollViewport(scroll); + }, [pps, userZoomCount, scrollRef, durationRef, contentOrigin, syncScrollViewport]); const syncPlayheadPosition = useCallback( (time: number) => { diff --git a/packages/studio/src/player/components/useTimelineProviderState.tsx b/packages/studio/src/player/components/useTimelineProviderState.tsx index 96c441381ad..387051b3381 100644 --- a/packages/studio/src/player/components/useTimelineProviderState.tsx +++ b/packages/studio/src/player/components/useTimelineProviderState.tsx @@ -350,6 +350,7 @@ export function useTimelineProviderState({ elementsLength: timelineElements.length, onSeek, contentOrigin, + syncScrollViewport, }); const { razorGuideX, updateRazorGuide, clearRazorGuide, splitAllAtPointer } = useTimelineRazorInteraction({ diff --git a/packages/studio/src/player/lib/timelineViewportBudgets.test.ts b/packages/studio/src/player/lib/timelineViewportBudgets.test.ts index 9972db441cb..a5448b2f607 100644 --- a/packages/studio/src/player/lib/timelineViewportBudgets.test.ts +++ b/packages/studio/src/player/lib/timelineViewportBudgets.test.ts @@ -9,7 +9,8 @@ describe("timeline viewport budgets", () => { expect(TIMELINE_VIEWPORT_BUDGETS).toMatchObject({ directScrollSafetyPx: 8_000_000, rowOverscanPerSide: 2, - timeOverscanViewportRatio: 0.5, + timeOverscanViewportRatio: 0.25, + zoomOverscanViewportRatio: 0.5, maxMountedRows: 64, maxMountedClipRoots: 512, maxMountedClipRootsPerRow: 128, diff --git a/packages/studio/src/player/lib/timelineViewportBudgets.ts b/packages/studio/src/player/lib/timelineViewportBudgets.ts index d51ac9baec9..7f5ecec2455 100644 --- a/packages/studio/src/player/lib/timelineViewportBudgets.ts +++ b/packages/studio/src/player/lib/timelineViewportBudgets.ts @@ -2,6 +2,7 @@ export interface TimelineViewportBudgets { directScrollSafetyPx: number; rowOverscanPerSide: number; timeOverscanViewportRatio: number; + zoomOverscanViewportRatio: number; maxMountedRows: number; maxMountedClipRoots: number; maxMountedClipRootsPerRow: number; @@ -61,7 +62,8 @@ export const MAX_VISIBLE_THUMBNAIL_FRAMES = Math.ceil(3840 / (66 * (16 / 9))); / export const TIMELINE_VIEWPORT_BUDGETS: Readonly = Object.freeze({ directScrollSafetyPx: 8_000_000, rowOverscanPerSide: 2, - timeOverscanViewportRatio: 0.5, + timeOverscanViewportRatio: 0.25, + zoomOverscanViewportRatio: 0.5, maxMountedRows: 64, maxMountedClipRoots: 512, maxMountedClipRootsPerRow: 128, diff --git a/packages/studio/tests/e2e/timeline-viewport-verdict.mjs b/packages/studio/tests/e2e/timeline-viewport-verdict.mjs index 1e7bcfdf78f..c71aa22a264 100644 --- a/packages/studio/tests/e2e/timeline-viewport-verdict.mjs +++ b/packages/studio/tests/e2e/timeline-viewport-verdict.mjs @@ -63,7 +63,7 @@ export function attemptPassed({ responsivenessPassed, passingRuns, requiredPassi return responsivenessPassed && passingRuns >= requiredPassingRuns; } -export function gatePassed({ directScrollApproved, attempts, memoryReturned }) { +export function gatePassed({ directScrollApproved, attempts, memoryReturned, zoomOutBlankFrames }) { const timingPassed = attempts.slice(0, TIMING_ATTEMPTS).some((attempt) => attempt.passed); - return directScrollApproved && timingPassed && memoryReturned; + return directScrollApproved && timingPassed && memoryReturned && zoomOutBlankFrames === 0; } diff --git a/packages/studio/tests/e2e/timeline-viewport-verdict.test.mjs b/packages/studio/tests/e2e/timeline-viewport-verdict.test.mjs index 91c52153ef4..6506de7ae8e 100644 --- a/packages/studio/tests/e2e/timeline-viewport-verdict.test.mjs +++ b/packages/studio/tests/e2e/timeline-viewport-verdict.test.mjs @@ -131,12 +131,18 @@ describe("attemptPassed", () => { describe("gatePassed", () => { const pass = { passed: true }; const fail = { passed: false }; - const passing = { directScrollApproved: true, attempts: [pass], memoryReturned: true }; + const passing = { + directScrollApproved: true, + attempts: [pass], + memoryReturned: true, + zoomOutBlankFrames: 0, + }; it("passes only when every check holds", () => { expect(gatePassed(passing)).toBe(true); expect(gatePassed({ ...passing, directScrollApproved: false })).toBe(false); expect(gatePassed({ ...passing, memoryReturned: false })).toBe(false); + expect(gatePassed({ ...passing, zoomOutBlankFrames: 1 })).toBe(false); }); it("fails timing only when the attempt and its one rerun both fail", () => { diff --git a/packages/studio/tests/e2e/timeline-virtualization.mjs b/packages/studio/tests/e2e/timeline-virtualization.mjs index b53c07065a7..eec1dba7cb5 100644 --- a/packages/studio/tests/e2e/timeline-virtualization.mjs +++ b/packages/studio/tests/e2e/timeline-virtualization.mjs @@ -352,6 +352,14 @@ try { ); } + const zoomOut = { + ctrlWheel: await countZoomOutBlankFrames(page, { steps: 16, deltaY: 100 }), + pinch: await countZoomOutBlankFrames(page, { steps: 60, deltaY: 8 }), + }; + console.error( + `timeline zoom-out blank frames: Ctrl+wheel ${zoomOut.ctrlWheel}, pinch ${zoomOut.pinch}`, + ); + await page.evaluate(() => window.__studioTest.resetTimelinePerformanceFixture()); await page.waitForFunction( () => document.querySelector('[aria-label="Timeline track view"]') === null, @@ -406,6 +414,7 @@ try { }, }, directScrollGate, + zoomOut, attempts, aggregate: { timingPassed: attempts.some((attempt) => attempt.passed), @@ -419,6 +428,7 @@ try { directScrollApproved: directScrollGate.decision === "approved", attempts, memoryReturned, + zoomOutBlankFrames: zoomOut.ctrlWheel + zoomOut.pinch, }) ? 0 : 1; @@ -427,6 +437,63 @@ try { } process.exit(exitCode); +/** + * Zooms in with the toolbar, then out with a Ctrl+wheel gesture (`deltaY` 8 is a trackpad pinch), and + * counts frames until the zoom rests with no ruler tick in view: the ticks are drawn from the render window. + */ +async function countZoomOutBlankFrames(page, { steps, deltaY }) { + for (let i = 0; i < 7; i += 1) await page.click('button[aria-label="Zoom in"]'); + await waitForZoomRest(page); + // Far from the start, where a zoom laid out against the old scroll would show nothing. + await page.evaluate(() => { + const view = document.querySelector("[data-timeline-scroll-viewport]"); + view.scrollLeft = (view.scrollWidth - view.clientWidth) / 2; + }); + await waitForZoomRest(page); + const box = await (await page.$("[data-timeline-scroll-viewport]")).boundingBox(); + await page.mouse.move(box.x + box.width / 2, box.y + box.height / 3); + await page.evaluate(() => { + const view = document.querySelector("[data-timeline-scroll-viewport]"); + const blank = { frames: 0, running: true }; + const tick = () => { + const r = view.getBoundingClientRect(); + const shown = [...view.querySelectorAll("[data-timeline-grid-cell]")].some((tick) => { + const t = tick.getBoundingClientRect(); + return t.right > r.left && t.left < r.right; + }); + if (!shown) blank.frames += 1; + if (blank.running) requestAnimationFrame(tick); + }; + window.__zoomOutBlank = blank; + requestAnimationFrame(tick); + }); + await page.keyboard.down("Control"); + for (let i = 0; i < steps; i += 1) { + await page.mouse.wheel({ deltaY }); + await page.evaluate(() => new Promise((resolve) => requestAnimationFrame(resolve))); + } + await page.keyboard.up("Control"); + await waitForZoomRest(page); + return page.evaluate(() => { + window.__zoomOutBlank.running = false; + return window.__zoomOutBlank.frames; + }); +} + +/** Until the zoom label holds for 30 frames: a zoom lays out about 150 ms after its last input. */ +async function waitForZoomRest(page) { + await page.evaluate(async () => { + const label = () => document.querySelector('[aria-label="Timeline zoom level"]')?.textContent; + const nextFrame = () => new Promise((resolve) => requestAnimationFrame(resolve)); + for (let held = 0, last = label(); held < 30; ) { + await nextFrame(); + const now = label(); + held = now === last ? held + 1 : 0; + last = now; + } + }); +} + async function waitForFixtureRender(page, elementCount) { const deadline = Date.now() + 60_000; let observed = null;