From 843a29d0aa44bed346fbf425e25339dbeec9beb3 Mon Sep 17 00:00:00 2001 From: Muzzy5150 Date: Sat, 31 Oct 2026 15:09:10 -0700 Subject: [PATCH] perf(core): avoid false motion blur callback dependencies Compare eventful and silent geometry by stable element identity. This prevents style-only callbacks and DOM insertion from forcing eventful pose reads. --- .../captureMotionBlur/discover.test.ts | 56 +++++++++++++- .../src/runtime/captureMotionBlur/discover.ts | 76 +++++++++++++------ .../runtime/captureMotionBlur/index.test.ts | 36 +++++++++ 3 files changed, 142 insertions(+), 26 deletions(-) diff --git a/packages/core/src/runtime/captureMotionBlur/discover.test.ts b/packages/core/src/runtime/captureMotionBlur/discover.test.ts index 348f401db73..0e5db665379 100644 --- a/packages/core/src/runtime/captureMotionBlur/discover.test.ts +++ b/packages/core/src/runtime/captureMotionBlur/discover.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { discoverMovers } from "./discover"; +import { discoverMovers, scanMovers } from "./discover"; // jsdom lays nothing out: each element's rect follows the x its fake timeline writes. function element(parent: Element, id: string, size = 10, tag = "div"): HTMLElement { @@ -304,6 +304,60 @@ describe("discoverMovers", () => { }); }); +describe("scanMovers callback dependency", () => { + it("ignores a callback-only style change whose box stays fixed", () => { + const scene = document.createElement("div"); + const mover = element(scene, "mover"); + const label = element(scene, "label"); + let callbacks = 0; + let seeks = 0; + const seek = (t: number, suppressEvents: boolean): void => { + seeks++; + mover.style.transform = `translateX(${300 * t}px)`; + if (!suppressEvents) { + callbacks++; + label.style.opacity = String(t); + } + }; + + const found = scanMovers({ scene, durationSeconds: 1, fps: 30, seek, events: true }); + + expect(found.movers).toEqual([mover]); + expect(found.readEvents).toBe(false); + expect({ callbacks, seeks }).toEqual({ callbacks: 31, seeks: 62 }); + expect(mover.getBoundingClientRect().left).toBe(300); + expect(label.getBoundingClientRect().left).toBe(0); + }); + + it("compares the same mover by identity after a callback inserts an earlier sibling", () => { + const scene = document.createElement("div"); + const mover = element(scene, "mover"); + let inserted: HTMLElement | undefined; + let callbacks = 0; + let seeks = 0; + const seek = (t: number, suppressEvents: boolean): void => { + seeks++; + mover.style.transform = `translateX(${300 * t}px)`; + if (!suppressEvents) { + callbacks++; + if (t > 0 && !inserted) { + inserted = document.createElement("span"); + inserted.id = "inserted"; + scene.insertBefore(inserted, mover); + } + } + }; + + const found = scanMovers({ scene, durationSeconds: 1, fps: 30, seek, events: true }); + + expect(found.movers).toEqual([mover]); + expect(found.readEvents).toBe(false); + expect({ callbacks, seeks }).toEqual({ callbacks: 31, seeks: 62 }); + expect([...scene.children]).toEqual([inserted, mover]); + expect(mover.getBoundingClientRect().left).toBe(300); + }); +}); + describe("discoverMovers translationOnly", () => { // jsdom resolves no transforms: give computed style the inline matrix, as Chrome's would be. beforeEach(() => { diff --git a/packages/core/src/runtime/captureMotionBlur/discover.ts b/packages/core/src/runtime/captureMotionBlur/discover.ts index 8fb6f36a827..be85affe919 100644 --- a/packages/core/src/runtime/captureMotionBlur/discover.ts +++ b/packages/core/src/runtime/captureMotionBlur/discover.ts @@ -38,7 +38,8 @@ export interface DiscoverOptions { /** Seek the composition (timeline + adapters) to a time. */ seek: (t: number) => void; onStep?: (i: number, steps: number) => void; - onStepBoxes?: (i: number, boxes: ReadonlyArray) => void; + onStepBoxes?: (i: number, boxes: ReadonlyMap) => void; + trackBoxes?: ReadonlySet; } export interface Discovery { @@ -50,6 +51,33 @@ export interface Discovery { translationOnly: boolean; } +function readStepGeometry( + all: ReadonlyArray, + first: ReadonlyArray, + dynamic: Set, + trackBoxes: ReadonlySet | undefined, +): { + rects: Array; + fresh: number[]; + boxes: ReadonlyMap; +} { + const rects: Array = []; + const fresh: number[] = []; + const boxes = new Map(); + all.forEach((el, j) => { + const changed = styleKey(el) !== first[j]; + if (!trackBoxes?.has(el) && !dynamic.has(el) && !changed) return; + if (!dynamic.has(el) && changed) { + fresh.push(j); + dynamic.add(el); + } + const box = el.getBoundingClientRect(); + rects[j] = box; + boxes.set(el, box); + }); + return { rects, fresh, boxes }; +} + // The whole inline style in one read; opacity or colour tweens only cost a rect check. const styleKey = (el: HTMLElement): string => el.getAttribute("style") ?? ""; @@ -189,6 +217,7 @@ export function discoverMovers({ seek, onStep, onStepBoxes, + trackBoxes, }: DiscoverOptions): Discovery { seek(0); const all = [...scene.querySelectorAll("*")].filter(isHtmlElement); @@ -224,15 +253,8 @@ export function discoverMovers({ onStep?.(i, steps); // Rects only for elements whose style has changed; a first change is judged against its // own baseline style at this time, so an ancestor's pan is not its move. - const rects: Array = []; - const fresh: number[] = []; - all.forEach((el, j) => { - if (!dynamic.has(el) && styleKey(el) === first[j]) return; - if (!dynamic.has(el)) fresh.push(j); - dynamic.add(el); - rects[j] = el.getBoundingClientRect(); - }); - onStepBoxes?.(i, rects); + const { rects, fresh, boxes } = readStepGeometry(all, first, dynamic, trackBoxes); + onStepBoxes?.(i, boxes); const away = fresh.filter( (j) => edgeShift(baseRects[j], rects[j]) / framesPerStep >= MIN_SPEED, ); @@ -279,20 +301,23 @@ export interface ScanOptions extends Omit { events: boolean; } -/** Every changing element's box, at 1/64 px, as one number per step. */ -function boxesKey(boxes: ReadonlyArray): number { - let h = 0x811c9dc5; - const mix = (v: number): void => { - h = Math.imul(h ^ v, 0x01000193); - }; - boxes.forEach((b, j) => { - if (!b) return; - for (const v of [j, b.left, b.top, b.width, b.height]) mix(Math.round(v * 64)); - }); - return h; +/** Whether the same elements occupy different boxes, at 1/64 px. */ +function boxesDiffer( + loud: ReadonlyMap | undefined, + quiet: ReadonlyMap, +): boolean { + if (!loud) return true; + for (const [el, a] of loud) { + const b = quiet.get(el); + if (!b) return true; + for (const key of ["left", "top", "width", "height"] as const) { + if (Math.round(a[key] * 64) !== Math.round(b[key] * 64)) return true; + } + } + return false; } -/** Discovery, and whether pose reads need callbacks: when they change an element's style or box. */ +/** Discovery, and whether pose reads need callbacks to reproduce its geometry. */ export function scanMovers({ seek, events, @@ -302,21 +327,22 @@ export function scanMovers({ const quiet = (t: number): void => seek(t, true); if (!events) return { ...discoverMovers({ ...scan, seek: quiet, onStep }), readEvents: false }; // Loud first: run after the silent scan, it starts at the end and fires a one-off call once more. - const loud: number[] = []; + const loud: Array> = []; const found = discoverMovers({ ...scan, seek: (t) => seek(t, false), onStep, onStepBoxes: (i, boxes) => { - loud[i] = boxesKey(boxes); + loud[i] = boxes; }, }); let differs = false; discoverMovers({ ...scan, seek: quiet, + trackBoxes: found.dynamic, onStepBoxes: (i, boxes) => { - differs ||= boxesKey(boxes) !== loud[i]; + differs ||= boxesDiffer(loud[i], boxes); }, }); return { ...found, readEvents: differs }; diff --git a/packages/core/src/runtime/captureMotionBlur/index.test.ts b/packages/core/src/runtime/captureMotionBlur/index.test.ts index 62030e8f3c8..ac867d5095f 100644 --- a/packages/core/src/runtime/captureMotionBlur/index.test.ts +++ b/packages/core/src/runtime/captureMotionBlur/index.test.ts @@ -939,6 +939,42 @@ describe("painting", () => { expect(reads.every(([, suppress]) => suppress === true)).toBe(true); }); + it.each(["capture", "filter"] as const)( + "does not replay a style-only callback during %s pose reads", + (tier) => { + installHtmlInCanvas(); + if (tier === "filter") setFilterFallbackForTests(true); + const { root, el, h } = movingHost({ + "data-motion-blur": "on", + "data-motion-blur-events": "on", + "data-motion-blur-tier": tier, + }); + const label = document.createElement("span"); + root.append(label); + const slide = h.seek; + let callbacks = 0; + const seek = vi.fn((t: number, suppressEvents: boolean) => { + slide(t, suppressEvents); + if (!suppressEvents) { + callbacks++; + label.style.opacity = String(t); + } + }); + initCaptureMotionBlur({ ...h, seek }); + + win.__hfMotionBlur?.probe(); + expect(win.__hfMotionBlur?.report()).toMatchObject({ tier, movers: 1 }); + expect({ callbacks, seeks: seek.mock.calls.length }).toEqual({ callbacks: 32, seeks: 63 }); + + callbacks = 0; + seek.mockClear(); + void paintCaptureMotionBlur(0.5, PLAY); + expect(callbacks).toBe(0); + expect(seek).toHaveBeenCalledTimes(tier === "capture" ? 6 : 5); + expect(el.getBoundingClientRect().left).toBe(150); + }, + ); + it("lands silent reads back on the frame through the host's holdFrame, taken before the first read", () => { installHtmlInCanvas(); const { h } = movingHost({ "data-motion-blur": "on", "data-motion-blur-events": "on" });