From a4c58c28486a129a83ea78f86ce129150e52201f Mon Sep 17 00:00:00 2001 From: Arham Wani Date: Fri, 28 Aug 2026 07:52:07 +0530 Subject: [PATCH 1/2] fix(timeline): coalesce scrub state updates per frame --- .../v4/V4Timeline.geometry.test.tsx | 72 +++++++++++++++++-- src/components/ai-edition/v4/V4Timeline.tsx | 6 +- 2 files changed, 70 insertions(+), 8 deletions(-) diff --git a/src/components/ai-edition/v4/V4Timeline.geometry.test.tsx b/src/components/ai-edition/v4/V4Timeline.geometry.test.tsx index 5119eedae..3a0e19044 100644 --- a/src/components/ai-edition/v4/V4Timeline.geometry.test.tsx +++ b/src/components/ai-edition/v4/V4Timeline.geometry.test.tsx @@ -1,7 +1,8 @@ // @vitest-environment jsdom import "@testing-library/jest-dom"; -import { fireEvent, render, screen } from "@testing-library/react"; -import { beforeAll, describe, expect, it, vi } from "vitest"; +import { act, fireEvent, render, screen } from "@testing-library/react"; +import { Profiler, type ProfilerOnRenderCallback } from "react"; +import { afterEach, beforeAll, describe, expect, it, vi } from "vitest"; // The regression under test is geometric, so the environment has to have a size: // jsdom reports 0 for every box, which would leave `pxPerSec` at 0 (the @@ -76,6 +77,7 @@ function renderTimeline( clips = [clip(0, TOTAL_SEC)], annotation = { id: "ann1", startMs: 10_000, endMs: 11_000 }, assets: Array> = [NO_CAMERA_ASSET], + onRender?: ProfilerOnRenderCallback, ) { const tl = { clips, @@ -104,13 +106,14 @@ function renderTimeline( /* the toolbar only awaits it */ }), }; - render( + const setCurrentTime = vi.fn(); + const timeline = ( } - setCurrentTime={vi.fn()} + setCurrentTime={setCurrentTime} playing={false} onTogglePlay={vi.fn()} onPrevClip={vi.fn()} @@ -118,15 +121,29 @@ function renderTimeline( onEditClip={vi.fn()} onAddVoiceover={vi.fn()} /> - , + + ); + render( + onRender ? ( + + {timeline} + + ) : ( + timeline + ), ); return { pill: screen.getByTitle("toolbar.newAnnotation"), clipEls: Array.from(document.querySelectorAll("[data-clip-id]")), tl, + setCurrentTime, }; } +afterEach(() => { + vi.unstubAllGlobals(); +}); + /** Drag a handle by `dxPx`. The move/up listeners live on `window`, so the drag * is driven by pointer deltas alone — the handle may re-mount under it. */ function dragHandle(handle: Element, dxPx: number) { @@ -147,6 +164,51 @@ function zoomIn(notches: number) { wheelZoomOn(document.querySelector("[class*=tlTracks]") as HTMLElement, notches); } +describe("V4Timeline scrubbing", () => { + it("publishes at most one React scrub-state update per animation frame", () => { + const frames = new Map(); + let nextFrameId = 1; + vi.stubGlobal("requestAnimationFrame", (callback: FrameRequestCallback) => { + const frameId = nextFrameId++; + frames.set(frameId, callback); + return frameId; + }); + vi.stubGlobal("cancelAnimationFrame", (frameId: number) => { + frames.delete(frameId); + }); + const onRender = vi.fn(); + const { setCurrentTime } = renderTimeline( + [clip(0, TOTAL_SEC)], + { id: "ann1", startMs: 10_000, endMs: 11_000 }, + [NO_CAMERA_ASSET], + onRender, + ); + const ruler = document.querySelector("[class*=tlRulerRow]"); + expect(ruler).not.toBeNull(); + + fireEvent.pointerDown(ruler as HTMLElement, { button: 0, clientX: 90 }); + const commitsAfterPointerDown = onRender.mock.calls.length; + setCurrentTime.mockClear(); + + fireEvent.pointerMove(window, { clientX: 180 }); + fireEvent.pointerMove(window, { clientX: 270 }); + fireEvent.pointerMove(window, { clientX: 360 }); + + expect(onRender).toHaveBeenCalledTimes(commitsAfterPointerDown); + expect(setCurrentTime).not.toHaveBeenCalled(); + expect(frames.size).toBe(1); + + const [[frameId, frame]] = frames; + frames.delete(frameId); + act(() => frame(0)); + + expect(onRender).toHaveBeenCalledTimes(commitsAfterPointerDown + 1); + expect(setCurrentTime).toHaveBeenCalledTimes(1); + expect(setCurrentTime).toHaveBeenCalledWith(720); + fireEvent.pointerUp(window); + }); +}); + describe("V4Timeline lane pills", () => { it("draws a pill exactly as wide as its region, at any zoom", () => { // 1 s of 1800 s. The old `Math.max(1.5, …)` floor drew this as 1.5% — 27 diff --git a/src/components/ai-edition/v4/V4Timeline.tsx b/src/components/ai-edition/v4/V4Timeline.tsx index cd2edb07f..502122a84 100644 --- a/src/components/ai-edition/v4/V4Timeline.tsx +++ b/src/components/ai-edition/v4/V4Timeline.tsx @@ -773,8 +773,6 @@ export function V4Timeline({ playheadElRef.current.style.left = `${pct * 100}%`; } - // Optimistic local UI state update - setScrubbingTimeSec(targetTime); pendingSeekTimeRef.current = targetTime; if (isImmediate) { @@ -782,15 +780,17 @@ export function V4Timeline({ cancelAnimationFrame(rafSeekRef.current); rafSeekRef.current = 0; } + setScrubbingTimeSec(targetTime); setCurrentTime(targetTime); return; } - // Throttled store update / D3D seek via rAF to avoid IPC flooding + // Throttled React state + store update / D3D seek via rAF to avoid re-render and IPC floods. if (rafSeekRef.current === 0) { rafSeekRef.current = requestAnimationFrame(() => { rafSeekRef.current = 0; if (pendingSeekTimeRef.current !== null) { + setScrubbingTimeSec(pendingSeekTimeRef.current); setCurrentTime(pendingSeekTimeRef.current); } }); From ae186689a44778384fadfe56a98cd38ad3aa11ac Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Mon, 14 Sep 2026 18:17:47 +0200 Subject: [PATCH 2/2] test(timeline): measure per-frame scrub commits against one update's cost Main's toolbar tooltips (Radix asChild triggers) re-attach their ref on every commit on React 18, so each V4Timeline commit is followed by a nested one and the hardcoded +1 no longer held. The test now measures what one scrub-state update costs at pointer-down and asserts, over two frames, that pointer moves add no commit and each frame adds exactly that cost. --- .../v4/V4Timeline.geometry.test.tsx | 45 ++++++++++++------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/src/components/ai-edition/v4/V4Timeline.geometry.test.tsx b/src/components/ai-edition/v4/V4Timeline.geometry.test.tsx index 3a0e19044..6cbc72def 100644 --- a/src/components/ai-edition/v4/V4Timeline.geometry.test.tsx +++ b/src/components/ai-edition/v4/V4Timeline.geometry.test.tsx @@ -186,25 +186,36 @@ describe("V4Timeline scrubbing", () => { const ruler = document.querySelector("[class*=tlRulerRow]"); expect(ruler).not.toBeNull(); + // What ONE scrub-state update costs in commits, measured on the pointer-down seek + // (which sets it once, synchronously) rather than hardcoded: the toolbar's Radix + // tooltip triggers re-attach their ref on every commit and add a nested one. + const commitsBeforePointerDown = onRender.mock.calls.length; fireEvent.pointerDown(ruler as HTMLElement, { button: 0, clientX: 90 }); - const commitsAfterPointerDown = onRender.mock.calls.length; - setCurrentTime.mockClear(); - - fireEvent.pointerMove(window, { clientX: 180 }); - fireEvent.pointerMove(window, { clientX: 270 }); - fireEvent.pointerMove(window, { clientX: 360 }); - - expect(onRender).toHaveBeenCalledTimes(commitsAfterPointerDown); - expect(setCurrentTime).not.toHaveBeenCalled(); - expect(frames.size).toBe(1); - - const [[frameId, frame]] = frames; - frames.delete(frameId); - act(() => frame(0)); + const commitsPerUpdate = onRender.mock.calls.length - commitsBeforePointerDown; + expect(commitsPerUpdate).toBeGreaterThan(0); + + // Over two frames: no pointer move commits, and each frame costs exactly one + // update however many moves it coalesced. + for (const [clientXs, expectedSec] of [ + [[180, 270, 360], 720], + [[450, 540], 1080], + ] as const) { + const commitsBeforeFrame = onRender.mock.calls.length; + setCurrentTime.mockClear(); + for (const clientX of clientXs) fireEvent.pointerMove(window, { clientX }); + + expect(onRender).toHaveBeenCalledTimes(commitsBeforeFrame); + expect(setCurrentTime).not.toHaveBeenCalled(); + expect(frames.size).toBe(1); + + const [[frameId, frame]] = frames; + frames.delete(frameId); + act(() => frame(0)); - expect(onRender).toHaveBeenCalledTimes(commitsAfterPointerDown + 1); - expect(setCurrentTime).toHaveBeenCalledTimes(1); - expect(setCurrentTime).toHaveBeenCalledWith(720); + expect(onRender).toHaveBeenCalledTimes(commitsBeforeFrame + commitsPerUpdate); + expect(setCurrentTime).toHaveBeenCalledTimes(1); + expect(setCurrentTime).toHaveBeenCalledWith(expectedSec); + } fireEvent.pointerUp(window); }); });