From 600de981a88492de183d85a0c208404da0f87a8f Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Thu, 10 Sep 2026 08:02:13 -0400 Subject: [PATCH 1/4] perf(studio): ask each ancestor once per off-canvas rebuild, not once per element The dashed off-canvas markers are rebuilt whenever anything in the preview changes, which during playback or a scrub is several times a second. Most of what a rebuild asked about an element was really a question about its ANCESTORS: does each node up to the root render, what does each contribute to the composed transform, which node is the source-file boundary. Two siblings share their whole chain, so a preview of a thousand elements asked the platform the same questions about the same ancestors a thousand times over. A rebuild now threads one measure pass through the walk, and each node answers once. On a 1689-element carousel that takes computed-style reads from 11,159 to 2,526 per rebuild and the rebuild itself from 59 to 12-18 ms per seek, with the rendered marker set byte-identical. The pass is deliberately not a cache. A cache would have to say what could have changed since last time, and for a measurement the answer is anything: an image finishing decode, a font swapping in, a transition frame, a container query, an inserted stylesheet rule all move an element's box with nothing written to the DOM and no record to invalidate on. A pass lives inside one synchronous measurement that only reads, so nothing can move under it, and it is dropped when the measurement ends. Every rebuild still measures every element. Layout reads are untouched on purpose: each element still takes its own client rect every rebuild, because that is the read that cannot be shared and must not be remembered. --- .../editor/domEditOverlayGeometry.ts | 62 +++-- .../editor/domEditOverlayMeasurePass.ts | 60 ++++ .../editor/domEditOverlayTransform.ts | 56 +++- .../src/components/editor/domEditingDom.ts | 62 ++++- .../components/editor/domEditingElement.ts | 7 +- ...CanvasIndicatorGeometry.complexity.test.ts | 262 ++++++++++++++++++ .../editor/offCanvasIndicatorGeometry.ts | 41 ++- 7 files changed, 496 insertions(+), 54 deletions(-) create mode 100644 packages/studio/src/components/editor/domEditOverlayMeasurePass.ts diff --git a/packages/studio/src/components/editor/domEditOverlayGeometry.ts b/packages/studio/src/components/editor/domEditOverlayGeometry.ts index f9d9cafe5e..66eabb92f3 100644 --- a/packages/studio/src/components/editor/domEditOverlayGeometry.ts +++ b/packages/studio/src/components/editor/domEditOverlayGeometry.ts @@ -7,6 +7,7 @@ import { import { isElementVisibleThroughAncestors } from "./domEditingDom"; import { hugRectForElement } from "./domEditOverlayCrop"; import { composeElementTransform, type PlanarTransformOps } from "./domEditOverlayTransform"; +import { type OverlayMeasurePass, readThroughPass } from "./domEditOverlayMeasurePass"; export interface OverlayRect { left: number; @@ -52,18 +53,16 @@ export function isElementVisibleForOverlay(el: HTMLElement): boolean { // shapes (rectangular cards, text, full-bleed media) don't have interior holes, so this // doesn't bite. If ring/cutout shapes become editable targets, sample more densely or // hit-test against the element's actual painted geometry instead of its bounding box. -function findSourceBoundary(element: HTMLElement): HTMLElement | null { - let current: HTMLElement | null = element; - while (current) { - if ( - current.hasAttribute("data-composition-file") || - current.hasAttribute("data-composition-src") - ) { - return current; +function findSourceBoundary(element: HTMLElement, pass?: OverlayMeasurePass): HTMLElement | null { + const walk = () => { + for (let node: HTMLElement | null = element; node; node = node.parentElement) { + if (node.hasAttribute("data-composition-file") || node.hasAttribute("data-composition-src")) { + return node; + } } - current = current.parentElement; - } - return null; + return null; + }; + return pass ? readThroughPass(pass.sourceBoundary, element, walk) : walk(); } export function resolveDomEditCoordinateScale(input: { @@ -157,6 +156,7 @@ interface ElementTransformSnapshot { function readElementTransformSnapshot( win: Window, element: HTMLElement, + pass?: OverlayMeasurePass, ): ElementTransformSnapshot | null { const DOMMatrixCtor = (win as Window & typeof globalThis).DOMMatrix; if (!DOMMatrixCtor) return null; @@ -170,8 +170,11 @@ function readElementTransformSnapshot( compose: (outer, inner) => outer.multiply(inner), }; try { - const matrix = composeElementTransform(element, ops, (node) => - node === element ? cs : win.getComputedStyle(node), + const matrix = composeElementTransform( + element, + ops, + (node) => (node === element ? cs : win.getComputedStyle(node)), + pass?.transform, ); return matrix ? { matrix, cs } : null; } catch { @@ -211,6 +214,7 @@ function toOverlayRect( iframe: HTMLIFrameElement, element: HTMLElement, precomputedScale?: OverlayRootScale | null, + pass?: OverlayMeasurePass, ): OverlayRect | null { const scale = precomputedScale ?? computeOverlayRootScale(overlayEl, iframe, iframe.contentDocument); @@ -218,8 +222,15 @@ function toOverlayRect( const { iframeRect, overlayRect, rootScaleX, rootScaleY } = scale; const elementRect = element.getBoundingClientRect(); - const sourceBoundary = findSourceBoundary(element); - const sourceBoundaryRect = sourceBoundary?.getBoundingClientRect(); + const sourceBoundary = findSourceBoundary(element, pass); + // Every element inside one sub-composition shares this boundary, so its rect + // is one layout read per boundary rather than one per element. + const sourceBoundaryRect = + sourceBoundary && pass + ? readThroughPass(pass.sourceBoundaryRect, sourceBoundary, () => + sourceBoundary.getBoundingClientRect(), + ) + : sourceBoundary?.getBoundingClientRect(); const editScale = resolveDomEditCoordinateScale({ rootScaleX, rootScaleY, @@ -363,15 +374,16 @@ export function orientedOverlayRect( iframe: HTMLIFrameElement, element: HTMLElement, precomputedScale?: OverlayRootScale | null, + pass?: OverlayMeasurePass, ): OverlayRect | null { const scale = precomputedScale ?? computeOverlayRootScale(overlayEl, iframe, iframe.contentDocument); if (!scale) return null; - const base = toOverlayRect(overlayEl, iframe, element, scale); + const base = toOverlayRect(overlayEl, iframe, element, scale, pass); if (!base) return null; const win = iframe.contentWindow; - const transform = win ? readElementTransformSnapshot(win, element) : null; + const transform = win ? readElementTransformSnapshot(win, element, pass) : null; const angle = transform ? rotationDegreesFromMatrix(transform.matrix) : 0; if (Math.abs(angle) < ROTATION_GATE_EPSILON_DEG) return base; @@ -482,8 +494,9 @@ export function groupAwareOverlayRect( iframe: HTMLIFrameElement, el: HTMLElement, precomputedScale?: OverlayRootScale | null, + pass?: OverlayMeasurePass, ): OverlayRect | null { - const rect = toOverlayRect(overlayEl, iframe, el, precomputedScale); + const rect = toOverlayRect(overlayEl, iframe, el, precomputedScale, pass); if (!rect || !el.hasAttribute("data-hf-group")) return rect; // Union the MEMBERS' rendered rects — where the content actually is — not the // wrapper's own box. The wrapper is invisible and its box can sit apart from the @@ -491,7 +504,13 @@ export function groupAwareOverlayRect( // group's bounds (and its off-canvas marker) off to a stale position. const rects: OverlayRect[] = []; for (const child of Array.from(el.children)) { - const childRect = toOverlayRect(overlayEl, iframe, child as HTMLElement, precomputedScale); + const childRect = toOverlayRect( + overlayEl, + iframe, + child as HTMLElement, + precomputedScale, + pass, + ); if (childRect) rects.push(childRect); } const union = rects.length > 0 ? resolveDomEditGroupOverlayRect(rects) : null; @@ -519,10 +538,11 @@ export function orientedGroupAwareOverlayRect( iframe: HTMLIFrameElement, el: HTMLElement, precomputedScale?: OverlayRootScale | null, + pass?: OverlayMeasurePass, ): OverlayRect | null { return el.hasAttribute("data-hf-group") - ? groupAwareOverlayRect(overlayEl, iframe, el, precomputedScale) - : orientedOverlayRect(overlayEl, iframe, el, precomputedScale); + ? groupAwareOverlayRect(overlayEl, iframe, el, precomputedScale, pass) + : orientedOverlayRect(overlayEl, iframe, el, precomputedScale, pass); } export function filterNestedDomEditGroupItems(items: T[]): T[] { diff --git a/packages/studio/src/components/editor/domEditOverlayMeasurePass.ts b/packages/studio/src/components/editor/domEditOverlayMeasurePass.ts new file mode 100644 index 0000000000..2c38a735e9 --- /dev/null +++ b/packages/studio/src/components/editor/domEditOverlayMeasurePass.ts @@ -0,0 +1,60 @@ +/** + * Shared memory for ONE synchronous pass that measures many elements. + * + * The off-canvas indicator overlay re-measures every element in the preview + * several times a second, and almost all of that work is per-ANCESTOR, not + * per-element: a visibility read for every node up to the root, a transform + * composed over the same nodes, and a walk for the source-file boundary. Two + * siblings share their entire chain above themselves, so a preview of a + * thousand elements asked the platform the same questions about the same + * ancestors a thousand times. + * + * WHY THIS IS A PASS AND NOT A CACHE. A cache has to answer "what could have + * changed since last time", and for a MEASUREMENT the honest answer is + * "anything": an `` finishing decode, a web font swapping in, a CSS + * transition frame, a container query, a `CSSStyleSheet.insertRule` — each one + * moves an element's box with nothing written to the DOM, so no mutation + * record exists to invalidate on and no observer reports all of them. A pass + * sidesteps the question instead of answering it wrong: it lives inside one + * synchronous measurement that only READS, so nothing can move under it, and + * it is dropped when the pass ends. Every rebuild still measures every + * element, exactly as it did before; it just stops asking the same question + * about the same ancestor once per descendant. + * + * Never store one of these across a rebuild, an await, or a frame. + */ + +/** The composed transform type the overlay's corner math uses. */ +type OverlayTransform = DOMMatrix; + +export interface OverlayMeasurePass { + /** Does this node render, given everything above it? */ + visible: Map; + /** This node's transform composed with every ancestor's, up to the + * composition root. `null` is an answer: some node's transform is + * unusable. */ + transform: Map; + /** The nearest ancestor carrying a source-file boundary, or null. */ + sourceBoundary: Map; + /** That boundary's client rect, which is shared by everything inside it. */ + sourceBoundaryRect: Map; +} + +export function createOverlayMeasurePass(): OverlayMeasurePass { + return { + visible: new Map(), + transform: new Map(), + sourceBoundary: new Map(), + sourceBoundaryRect: new Map(), + }; +} + +/** Read through a pass's map, filling it on the way. `undefined` is the only + * miss, so a memoized `null` stays an answer. */ +export function readThroughPass(memo: Map, key: K, compute: () => V): V { + const answered = memo.get(key); + if (answered !== undefined) return answered; + const value = compute(); + memo.set(key, value); + return value; +} diff --git a/packages/studio/src/components/editor/domEditOverlayTransform.ts b/packages/studio/src/components/editor/domEditOverlayTransform.ts index 5c4999714c..59d755da9e 100644 --- a/packages/studio/src/components/editor/domEditOverlayTransform.ts +++ b/packages/studio/src/components/editor/domEditOverlayTransform.ts @@ -58,6 +58,22 @@ export function individualRotateDegrees(value: string | undefined): number { return Number.isFinite(deg) ? deg : 0; } +/** One node's own contribution, with the individual properties applied before + * `transform` the way CSS does. Null when the node's transform is unusable. */ +function ownNodeTransform( + node: HTMLElement, + ops: PlanarTransformOps, + getStyle: (node: HTMLElement) => CSSStyleDeclaration | null, +): M | null { + const style = getStyle(node); + if (!style) return null; + const transform = style.transform; + const own = transform && transform !== "none" ? ops.fromTransform(transform) : ops.identity(); + if (!own) return null; + const spin = individualRotateDegrees(style.rotate); + return spin === 0 ? own : ops.compose(ops.fromRotate(spin), own); +} + /** * The element's transform composed with every ancestor's, up to the composition * root. @@ -66,23 +82,45 @@ export function individualRotateDegrees(value: string | undefined): number { * `rotate` composes on the left of it. Between nodes, an ancestor applies * outside its child. Null means some node's transform was unusable and the * caller should fall back rather than guess. + * + * `memo` holds each node's COMPOSED chain, for a caller walking many elements + * in one synchronous pass. + * + * A chain is `chain(parent)` composed with the node's own, so siblings share + * everything above them and the whole tree costs one style read and one + * compose per node instead of one per node PER DESCENDANT. Valid only for the + * length of one pass, which is why the caller owns it: nothing here writes to + * the DOM, so nothing can move under it, and it is dropped before anything + * else runs. Omitted, every call composes its own chain from scratch. */ export function composeElementTransform( element: HTMLElement, ops: PlanarTransformOps, getStyle: (node: HTMLElement) => CSSStyleDeclaration | null, + memo?: Map, ): M | null { - let acc = ops.identity(); + const pending: HTMLElement[] = []; + // `undefined` means nothing on the way up was already composed, so the chain + // starts from identity. A memoized `null` is an answer, not a miss: some node + // above carries a transform this algebra cannot represent. + let above: M | null | undefined; for (let node: HTMLElement | null = element; node; node = node.parentElement) { - const style = getStyle(node); - if (!style) return null; - const transform = style.transform; - let own = transform && transform !== "none" ? ops.fromTransform(transform) : ops.identity(); - if (!own) return null; - const spin = individualRotateDegrees(style.rotate); - if (spin !== 0) own = ops.compose(ops.fromRotate(spin), own); - acc = ops.compose(own, acc); + above = memo?.get(node); + if (above !== undefined) break; + pending.push(node); if (node.hasAttribute(COMPOSITION_ROOT_ATTR)) break; } + + let acc: M | null = above === undefined ? ops.identity() : above; + for (let i = pending.length - 1; i >= 0; i -= 1) { + const node = pending[i]!; + if (acc !== null) { + const own = ownNodeTransform(node, ops, getStyle); + // The ancestors' chain is the OUTER of the pair, as an ancestor applies + // around its child. + acc = own === null ? null : ops.compose(acc, own); + } + memo?.set(node, acc); + } return acc; } diff --git a/packages/studio/src/components/editor/domEditingDom.ts b/packages/studio/src/components/editor/domEditingDom.ts index 5deee32b09..8a60b7439e 100644 --- a/packages/studio/src/components/editor/domEditingDom.ts +++ b/packages/studio/src/components/editor/domEditingDom.ts @@ -30,23 +30,57 @@ export function isTextBearingTag(tagName: string): boolean { return ["div", "span", "p", "strong", "h1", "h2", "h3", "h4", "h5", "h6"].includes(tagName); } -export function isElementVisibleThroughAncestors(el: HTMLElement): boolean { +/** Does this node render AT ALL, ignoring what it inherits? Sole owner of the + * rule; the walk below only decides which nodes to ask it about. */ +function elementRendersItself(win: Window, el: HTMLElement): boolean { + const computed = win.getComputedStyle(el); + if (computed.display === "none" || computed.visibility === "hidden") return false; + const opacity = Number.parseFloat(computed.opacity); + return !( + Number.isFinite(opacity) && + opacity <= 0.01 && + !el.hasAttribute(COLOR_GRADING_SOURCE_HIDDEN_ATTR) + ); +} + +/** + * Does `el` render, given everything above it? + * + * `memo` is for a caller asking this about MANY elements in one synchronous + * pass. Answers are a function of the node and its ancestors, and siblings + * share almost all of their chain, so memoizing per node turns a walk per + * element into one style read per node in the tree. It is only ever valid for + * the length of one pass — the DOM cannot change under a pass, and every read + * here is a read — so the caller creates it and drops it, and nothing survives + * to be invalidated. Omitted, every call walks the chain itself. + */ +export function isElementVisibleThroughAncestors( + el: HTMLElement, + memo?: Map, +): boolean { const win = el.ownerDocument.defaultView; if (!win) return true; - let current: HTMLElement | null = el; - while (current) { - const computed = win.getComputedStyle(current); - if (computed.display === "none" || computed.visibility === "hidden") return false; - const opacity = Number.parseFloat(computed.opacity); - if ( - Number.isFinite(opacity) && - opacity <= 0.01 && - !current.hasAttribute(COLOR_GRADING_SOURCE_HIDDEN_ATTR) - ) - return false; - current = current.parentElement; + // Up to the first node already answered for, then back down: a node's answer + // needs its ancestors' first, and the topmost unanswered node is where the + // chain of unknowns starts. + const pending: HTMLElement[] = []; + let inherited = true; + for (let node: HTMLElement | null = el; node; node = node.parentElement) { + const answered = memo?.get(node); + if (answered !== undefined) { + inherited = answered; + break; + } + pending.push(node); + } + for (let i = pending.length - 1; i >= 0; i -= 1) { + const node = pending[i]!; + // Once an ancestor is out, its descendants are out with it, and asking the + // platform about them would be a style read for an answer already known. + inherited = inherited && elementRendersItself(win, node); + memo?.set(node, inherited); } - return true; + return inherited; } // ─── Style accessors ────────────────────────────────────────────────────────── diff --git a/packages/studio/src/components/editor/domEditingElement.ts b/packages/studio/src/components/editor/domEditingElement.ts index 95c893e5a8..d954e1d35f 100644 --- a/packages/studio/src/components/editor/domEditingElement.ts +++ b/packages/studio/src/components/editor/domEditingElement.ts @@ -22,8 +22,11 @@ import { // ─── Visibility ────────────────────────────────────────────────────────────── -export function isElementComputedVisible(el: HTMLElement): boolean { - return isElementVisibleThroughAncestors(el); +export function isElementComputedVisible( + el: HTMLElement, + memo?: Map, +): boolean { + return isElementVisibleThroughAncestors(el, memo); } const VISUAL_LEAF_TAGS = new Set(["img", "video", "canvas", "svg", "audio"]); diff --git a/packages/studio/src/components/editor/offCanvasIndicatorGeometry.complexity.test.ts b/packages/studio/src/components/editor/offCanvasIndicatorGeometry.complexity.test.ts index 0df8da463c..4a14338d17 100644 --- a/packages/studio/src/components/editor/offCanvasIndicatorGeometry.complexity.test.ts +++ b/packages/studio/src/components/editor/offCanvasIndicatorGeometry.complexity.test.ts @@ -4,6 +4,7 @@ import type React from "react"; import { afterEach, describe, expect, it } from "vitest"; import type { OffCanvasRect } from "./OffCanvasIndicators"; import { recomputeOffCanvasIndicators } from "./offCanvasIndicatorGeometry"; +import { DOM_EDIT_LAYER_OBSERVER_INIT, createDomEditLayerWalkCache } from "./domEditLayerWalkCache"; const realGetBoundingClientRect = Element.prototype.getBoundingClientRect; afterEach(() => { @@ -165,3 +166,264 @@ describe("recomputeOffCanvasIndicators composition-basis cost", () => { expect(rebuildWithSharedSelector(12).keys).toHaveLength(12); }); }); + +/** + * A group's box is the union of its MEMBERS' rects, not its own. That makes it + * the one item whose measurement depends on elements BELOW it, and the walk + * cache invalidates upward only — so a cached group union would go stale the + * moment a member moved, and stay stale, because nothing ever writes to the + * wrapper. + */ +describe("recomputeOffCanvasIndicators group measurement", () => { + function rebuildGroupTwice(): { first: string; second: string } { + const iframe = document.createElement("iframe"); + document.body.append(iframe); + const doc = iframe.contentDocument; + if (!doc) throw new Error("Expected iframe content document"); + doc.body.innerHTML = + `
` + + `
` + + `
`; + const member = doc.getElementById("member") as HTMLElement; + + const overlay = document.createElement("div"); + document.body.append(overlay); + + // Only the member has a box; the wrapper measures empty, which is exactly + // the case the union exists for. Moving the member moves the group. + let memberLeft = -500; + Element.prototype.getBoundingClientRect = function (): DOMRect { + if (this === iframe || this === overlay) return new DOMRect(0, 0, 800, 450); + if (this === member) return new DOMRect(memberLeft, 40, 100, 40); + return new DOMRect(0, 0, 0, 0); + }; + + const cache = createDomEditLayerWalkCache(); + const observer = new MutationObserver(() => {}); + observer.observe(doc.documentElement, DOM_EDIT_LAYER_OBSERVER_INIT); + + const sigRef = { current: "" } as React.MutableRefObject; + const elementsRef = { current: new Map() } as React.MutableRefObject< + Map + >; + let rects: OffCanvasRect[] = []; + const rebuild = () => { + cache.ingest(observer.takeRecords()); + recomputeOffCanvasIndicators( + iframe, + overlay, + doc, + { left: 0, top: 0, width: 800, height: 450 }, + "index.html", + sigRef, + elementsRef, + (next) => { + rects = next; + }, + cache, + ); + const group = rects.find((rect) => rect.key.includes("grp")); + return group ? `${group.left},${group.top},${group.width},${group.height}` : "absent"; + }; + + const first = rebuild(); + // What an animation frame does: one inline style write on the member. + memberLeft = -300; + member.style.transform = "translateX(200px)"; + const second = rebuild(); + + observer.disconnect(); + iframe.remove(); + overlay.remove(); + return { first, second }; + } + + it("re-measures a group when a member moves and nothing writes to the wrapper", () => { + const { first, second } = rebuildGroupTwice(); + + expect(first).not.toBe("absent"); + expect(second).not.toBe(first); + }); +}); + +/** + * Layout changes that emit no mutation record at all. + * + * An `` finishing decode, a web font swapping in, a CSS transition or + * `@keyframes` frame, a container query re-evaluating, a `CSSStyleSheet.insertRule` + * — every one of them moves an element's border box with nothing written to the + * DOM. A rebuild that re-measures everything picks them up for free; a rebuild + * that reuses a previous measurement cannot see them at all. + */ +describe("recomputeOffCanvasIndicators layout changes with no mutation record", () => { + /** Rebuild twice, changing only what layout REPORTS between the two, with no + * DOM write of any kind in between. */ + function rebuildAcrossSilentLayoutChange(): { first: string; second: string } { + const iframe = document.createElement("iframe"); + document.body.append(iframe); + const doc = iframe.contentDocument; + if (!doc) throw new Error("Expected iframe content document"); + doc.body.innerHTML = + `
` + + `` + + `
`; + const hero = doc.getElementById("hero") as HTMLElement; + + const overlay = document.createElement("div"); + document.body.append(overlay); + + // Before decode the image lays out at zero-ish; after decode it takes its + // intrinsic size, off the left edge of the composition. No attribute is + // written, no node is added, no text changes. + let heroRect = new DOMRect(-500, 40, 100, 40); + Element.prototype.getBoundingClientRect = function (): DOMRect { + if (this === iframe || this === overlay) return new DOMRect(0, 0, 800, 450); + if (this === hero) return heroRect; + return new DOMRect(0, 0, 0, 0); + }; + + const cache = createDomEditLayerWalkCache(); + const observer = new MutationObserver(() => {}); + observer.observe(doc.documentElement, DOM_EDIT_LAYER_OBSERVER_INIT); + + const sigRef = { current: "" } as React.MutableRefObject; + const elementsRef = { current: new Map() } as React.MutableRefObject< + Map + >; + let rects: OffCanvasRect[] = []; + const rebuild = () => { + cache.ingest(observer.takeRecords()); + recomputeOffCanvasIndicators( + iframe, + overlay, + doc, + { left: 0, top: 0, width: 800, height: 450 }, + "index.html", + sigRef, + elementsRef, + (next) => { + rects = next; + }, + cache, + ); + const marker = rects.find((rect) => rect.key.includes("hero")); + return marker ? `${marker.left},${marker.top},${marker.width},${marker.height}` : "absent"; + }; + + const first = rebuild(); + heroRect = new DOMRect(-500, 40, 320, 180); // decode finished + const second = rebuild(); + + observer.disconnect(); + iframe.remove(); + overlay.remove(); + return { first, second }; + } + + it("re-measures an element whose own box changed with no DOM write", () => { + const { first, second } = rebuildAcrossSilentLayoutChange(); + + expect(first).toBe("-500,40,100,40"); + expect(second).toBe("-500,40,320,180"); + }); +}); + +/** + * The ancestor work, measured through the production entry point. + * + * Everything the rebuild asks about an ancestor — does it render, what does it + * contribute to the composed transform, is it the source-file boundary — is the + * same question for every element underneath it. The guard is INVARIANCE in + * depth: burying the same cards under more shared wrappers may cost one style + * read per added wrapper, and must not cost one per wrapper PER CARD. + */ +describe("recomputeOffCanvasIndicators ancestor cost", () => { + /** `cardCount` off-canvas cards, all siblings, buried under `depth` shared + * wrappers. Returns the preview document's computed-style reads for one + * rebuild, and the markers it produced. */ + function rebuildAtDepth( + cardCount: number, + depth: number, + ): { styleReads: number; markers: number } { + const iframe = document.createElement("iframe"); + document.body.append(iframe); + const doc = iframe.contentDocument; + if (!doc) throw new Error("Expected iframe content document"); + + const cards = Array.from( + { length: cardCount }, + (_unused, i) => `
card ${i}
`, + ).join(""); + const open = Array.from({ length: depth }, (_unused, i) => `
`).join(""); + const close = "
".repeat(depth); + doc.body.innerHTML = + `
` + + `${open}${cards}${close}` + + `
`; + + const overlay = document.createElement("div"); + document.body.append(overlay); + Element.prototype.getBoundingClientRect = function (): DOMRect { + if (this === iframe || this === overlay) return new DOMRect(0, 0, 800, 450); + if (this instanceof doc.defaultView!.Element && this.classList.contains("box")) { + return new DOMRect(-500, 40, 100, 40); + } + return new DOMRect(0, 0, 0, 0); + }; + + const win = doc.defaultView!; + const realGetComputedStyle = win.getComputedStyle.bind(win); + let styleReads = 0; + win.getComputedStyle = ((el: Element, pseudo?: string | null) => { + styleReads += 1; + return realGetComputedStyle(el, pseudo ?? undefined); + }) as typeof win.getComputedStyle; + + const sigRef = { current: "" } as React.MutableRefObject; + const elementsRef = { current: new Map() } as React.MutableRefObject< + Map + >; + let rects: OffCanvasRect[] = []; + try { + recomputeOffCanvasIndicators( + iframe, + overlay, + doc, + { left: 0, top: 0, width: 800, height: 450 }, + "index.html", + sigRef, + elementsRef, + (next) => { + rects = next; + }, + ); + } finally { + win.getComputedStyle = realGetComputedStyle; + iframe.remove(); + overlay.remove(); + } + return { styleReads, markers: rects.length }; + } + + /** What burying the same cards 8 wrappers deeper costs, at `cardCount`. */ + function depthSurcharge(cardCount: number): number { + return rebuildAtDepth(cardCount, 10).styleReads - rebuildAtDepth(cardCount, 2).styleReads; + } + + // The load-bearing assertion, and it is INVARIANCE rather than a threshold: a + // ceiling ("under 40 reads") passes on a small fixture and still degrades on + // a real preview. Eight wrappers are shared by every card, so what they cost + // is a property of the WRAPPERS. Asking each card about them separately makes + // it a property of wrappers x cards, and that is the thing that has to stay + // flat when the card count moves. + it("pays for a deeper tree per added ancestor, not per ancestor per card", () => { + expect(depthSurcharge(48)).toBe(depthSurcharge(24)); + }); + + // Non-vacuity: the assertion above only means something if the fixture really + // drove the per-element geometry path for every card at both depths. + it("measures rebuilds that really did resolve every card's rect", () => { + expect(rebuildAtDepth(24, 2).markers).toBe(24); + expect(rebuildAtDepth(24, 10).markers).toBe(24); + }); +}); diff --git a/packages/studio/src/components/editor/offCanvasIndicatorGeometry.ts b/packages/studio/src/components/editor/offCanvasIndicatorGeometry.ts index 22fd19e501..806342f3a8 100644 --- a/packages/studio/src/components/editor/offCanvasIndicatorGeometry.ts +++ b/packages/studio/src/components/editor/offCanvasIndicatorGeometry.ts @@ -1,7 +1,8 @@ import type React from "react"; import type { OffCanvasRect } from "./OffCanvasIndicators"; import { hugRectForElement } from "./domEditOverlayCrop"; -import { computeOverlayRootScale } from "./domEditOverlayBasis"; +import { computeOverlayRootScale, type OverlayRootScale } from "./domEditOverlayBasis"; +import { createOverlayMeasurePass, type OverlayMeasurePass } from "./domEditOverlayMeasurePass"; import { orientedGroupAwareOverlayRect } from "./domEditOverlayGeometry"; import { isElementComputedVisible } from "./domEditingElement"; import type { DomEditLayerWalkCache } from "./domEditLayerWalkCache"; @@ -39,6 +40,28 @@ function extendsOutside( ); } +/** + * One item's box in overlay coordinates, or null when it does not render. + * + * Every layout read the rebuild makes about a single element is in here, which + * is what lets the loop below hand all of them one measure pass. + */ +function measureItemRect( + overlay: HTMLDivElement, + iframe: HTMLIFrameElement, + element: HTMLElement, + scale: OverlayRootScale | null, + pass: OverlayMeasurePass, +): Omit | null { + if (!isElementComputedVisible(element, pass.visible)) return null; + // Groups use their members' union (where they actually render), so a group + // whose members sit inside the canvas isn't flagged off-canvas by a stale + // wrapper box. Crop-hug the result so an inset crop that keeps the visible + // part on-canvas doesn't flag the element either. + const base = orientedGroupAwareOverlayRect(overlay, iframe, element, scale, pass); + return base ? { ...base, ...hugRectForElement(base, element) } : null; +} + // fallow-ignore-next-line complexity export function recomputeOffCanvasIndicators( iframe: HTMLIFrameElement, @@ -75,16 +98,18 @@ export function recomputeOffCanvasIndicators( // `querySelector("[data-composition-id]")` plus three layout reads per item, // which on a preview of a few hundred elements is the bulk of the rebuild. const scale = computeOverlayRootScale(overlay, iframe, doc); + // Every element below is measured against the same ancestors: the same + // visibility chain, the same composed transforms, the same source-file + // boundary. One pass answers each of those questions once per NODE instead + // of once per node per descendant. It measures everything every time — it is + // not a cache and nothing in it survives this call — so a layout change that + // emits no mutation record (an image decoding, a font swapping, a CSS + // transition frame) is picked up here exactly as it was before. + const pass = createOverlayMeasurePass(); const rects: OffCanvasRect[] = []; const elMap = new Map(); for (const item of items) { - if (!isElementComputedVisible(item.element)) continue; - // Groups use their members' union (where they actually render), so a group - // whose members sit inside the canvas isn't flagged off-canvas by a stale - // wrapper box. Crop-hug the result so an inset crop that keeps the visible - // part on-canvas doesn't flag the element either. - const base = orientedGroupAwareOverlayRect(overlay, iframe, item.element, scale); - const r = base ? { ...base, ...hugRectForElement(base, item.element) } : null; + const r = measureItemRect(overlay, iframe, item.element, scale, pass); if (!r) continue; // Any edge crossing the composition border → gray-zone indicator (the // in-canvas portion is clipped away below, so only the sliver shows). From 29c944f9c5cdbd79a481b70417bab456935fea7f Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Thu, 10 Sep 2026 08:03:11 -0400 Subject: [PATCH 2/4] test(studio): give one PropertyPanel case the file's own render timeout Every other render test in that file passes RENDER_TIMEOUT_MS; this it.each was left on vitest's 5s default and times out when the whole suite runs on a loaded machine. Unrelated to any behaviour: it passes on either side of the change when the machine is quiet, and fails in a full-suite run on either side when it is not. --- .../components/editor/PropertyPanel.test.tsx | 32 +++++++++++-------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/packages/studio/src/components/editor/PropertyPanel.test.tsx b/packages/studio/src/components/editor/PropertyPanel.test.tsx index 9d47b6ce57..12a4ef45f5 100644 --- a/packages/studio/src/components/editor/PropertyPanel.test.tsx +++ b/packages/studio/src/components/editor/PropertyPanel.test.tsx @@ -1001,20 +1001,24 @@ describe("PropertyPanel — Motion is for things that move", () => { return element; }, ], - ])("recognizes %s through the shared audio predicate", async (_label, makeElement) => { - const fixture = { - ...audioClipElement(), - element: makeElement(), - tagName: "div", - }; - const { host, root } = await renderPanel(true, fixture); - const titles = Array.from( - host.querySelectorAll("[data-flat-group-collapsed], [data-flat-group-open]"), - ).map((node) => node.textContent ?? ""); - expect(titles.some((title) => title.includes("Motion"))).toBe(false); - expect(titles.some((title) => title.includes("Timing"))).toBe(true); - act(() => root.unmount()); - }); + ])( + "recognizes %s through the shared audio predicate", + async (_label, makeElement) => { + const fixture = { + ...audioClipElement(), + element: makeElement(), + tagName: "div", + }; + const { host, root } = await renderPanel(true, fixture); + const titles = Array.from( + host.querySelectorAll("[data-flat-group-collapsed], [data-flat-group-open]"), + ).map((node) => node.textContent ?? ""); + expect(titles.some((title) => title.includes("Motion"))).toBe(false); + expect(titles.some((title) => title.includes("Timing"))).toBe(true); + act(() => root.unmount()); + }, + RENDER_TIMEOUT_MS, + ); it( "calls the section Timing on an audio clip, and offers no tween editor", From ed12ec70a228a1b42425368ee1d6ae4934fe515b Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Thu, 10 Sep 2026 08:53:54 -0400 Subject: [PATCH 3/4] refactor(studio): share the source-boundary walk and dedupe the rebuild fixtures Three follow-ups from review of the measure pass, none of them behaviour: The source boundary was memoized against the element that asked for it, so two siblings still walked their whole chain separately and only the answer was reused. It now memoizes every node on the way up, like the other two walks: an element's boundary IS its parent's unless the element is one itself. `isElementVisibleThroughAncestors` returns what it always returned, but it now resolves top-down, so the set of nodes it takes a style read for is different (smaller for a subtree hidden near the root). Three callers use it with no memo, so that is written down next to it rather than left to be discovered. The three rebuild fixtures in the indicator tests each carried their own copy of the iframe/overlay/layout-stub scaffolding, which is four clone groups and 126 duplicated lines. One `mountPreview` helper now serves all of them and the pre-existing selector-index fixture too. --- .../editor/domEditOverlayGeometry.ts | 28 +- .../src/components/editor/domEditingDom.ts | 6 + ...CanvasIndicatorGeometry.complexity.test.ts | 404 +++++++----------- 3 files changed, 186 insertions(+), 252 deletions(-) diff --git a/packages/studio/src/components/editor/domEditOverlayGeometry.ts b/packages/studio/src/components/editor/domEditOverlayGeometry.ts index 66eabb92f3..2504d04187 100644 --- a/packages/studio/src/components/editor/domEditOverlayGeometry.ts +++ b/packages/studio/src/components/editor/domEditOverlayGeometry.ts @@ -53,16 +53,28 @@ export function isElementVisibleForOverlay(el: HTMLElement): boolean { // shapes (rectangular cards, text, full-bleed media) don't have interior holes, so this // doesn't bite. If ring/cutout shapes become editable targets, sample more densely or // hit-test against the element's actual painted geometry instead of its bounding box. +const isSourceBoundary = (node: HTMLElement): boolean => + node.hasAttribute("data-composition-file") || node.hasAttribute("data-composition-src"); + +/** With a `pass`, every node on the way up is memoized rather than only the + * element asked about: an element's boundary IS its parent's unless it is one + * itself, so siblings share the walk instead of each repeating it. */ function findSourceBoundary(element: HTMLElement, pass?: OverlayMeasurePass): HTMLElement | null { - const walk = () => { - for (let node: HTMLElement | null = element; node; node = node.parentElement) { - if (node.hasAttribute("data-composition-file") || node.hasAttribute("data-composition-src")) { - return node; - } + const pending: HTMLElement[] = []; + let boundary: HTMLElement | null | undefined; + for (let node: HTMLElement | null = element; node; node = node.parentElement) { + boundary = pass?.sourceBoundary.get(node); + if (boundary !== undefined) break; + if (isSourceBoundary(node)) { + boundary = node; + pass?.sourceBoundary.set(node, node); + break; } - return null; - }; - return pass ? readThroughPass(pass.sourceBoundary, element, walk) : walk(); + pending.push(node); + } + const answer = boundary ?? null; + if (pass) for (const node of pending) pass.sourceBoundary.set(node, answer); + return answer; } export function resolveDomEditCoordinateScale(input: { diff --git a/packages/studio/src/components/editor/domEditingDom.ts b/packages/studio/src/components/editor/domEditingDom.ts index 8a60b7439e..de9f76c91b 100644 --- a/packages/studio/src/components/editor/domEditingDom.ts +++ b/packages/studio/src/components/editor/domEditingDom.ts @@ -53,6 +53,12 @@ function elementRendersItself(win: Window, el: HTMLElement): boolean { * the length of one pass — the DOM cannot change under a pass, and every read * here is a read — so the caller creates it and drops it, and nothing survives * to be invalidated. Omitted, every call walks the chain itself. + * + * The ANSWER is what it always was. Which nodes get a style read is not: this + * resolves top-down and stops at the first node that is out, where the previous + * version resolved bottom-up and stopped at the first one that is out going the + * other way. Same boolean for every input, a different (and, for a subtree + * hidden near the root, smaller) set of reads. */ export function isElementVisibleThroughAncestors( el: HTMLElement, diff --git a/packages/studio/src/components/editor/offCanvasIndicatorGeometry.complexity.test.ts b/packages/studio/src/components/editor/offCanvasIndicatorGeometry.complexity.test.ts index 4a14338d17..d64e0fd892 100644 --- a/packages/studio/src/components/editor/offCanvasIndicatorGeometry.complexity.test.ts +++ b/packages/studio/src/components/editor/offCanvasIndicatorGeometry.complexity.test.ts @@ -11,6 +11,104 @@ afterEach(() => { Element.prototype.getBoundingClientRect = realGetBoundingClientRect; }); +/** The composition frame every fixture here is measured against. */ +const COMP = { left: 0, top: 0, width: 800, height: 450 }; + +interface Preview { + doc: Document; + /** One real rebuild through the production entry point. Returns the markers + * it produced. `observe` opts the walk cache and its observer in, which is + * how `startOffCanvasIndicatorRefresh` calls it. */ + rebuild: () => OffCanvasRect[]; + dispose: () => void; +} + +/** + * A mounted preview wired to `recomputeOffCanvasIndicators` exactly as the + * refresh loop wires it. + * + * `rectFor` stands in for layout, which happy-dom does not do: return the box + * an element should report, or null for "measures empty". Every test here + * varies only the markup and that function, so the scaffolding lives once. + */ +function mountPreview( + bodyHtml: string, + rectFor: (el: Element) => DOMRect | null, + options: { observe?: boolean } = {}, +): Preview { + const iframe = document.createElement("iframe"); + document.body.append(iframe); + const doc = iframe.contentDocument; + if (!doc) throw new Error("Expected iframe content document"); + doc.body.innerHTML = `
${bodyHtml}
`; + + const overlay = document.createElement("div"); + document.body.append(overlay); + Element.prototype.getBoundingClientRect = function (): DOMRect { + if (this === iframe || this === overlay) return new DOMRect(0, 0, 800, 450); + return rectFor(this) ?? new DOMRect(0, 0, 0, 0); + }; + + const cache = options.observe ? createDomEditLayerWalkCache() : undefined; + const observer = options.observe ? new MutationObserver(() => {}) : null; + observer?.observe(doc.documentElement, DOM_EDIT_LAYER_OBSERVER_INIT); + + const sigRef = { current: "" } as React.MutableRefObject; + const elementsRef = { current: new Map() } as React.MutableRefObject< + Map + >; + let rects: OffCanvasRect[] = []; + return { + doc, + rebuild: () => { + if (cache && observer) cache.ingest(observer.takeRecords()); + recomputeOffCanvasIndicators( + iframe, + overlay, + doc, + COMP, + "index.html", + sigRef, + elementsRef, + (next) => { + rects = next; + }, + cache, + ); + return rects; + }, + dispose: () => { + observer?.disconnect(); + iframe.remove(); + overlay.remove(); + }, + }; +} + +/** `cardCount` cards sharing `.box`, all off-canvas to the left. */ +function cardsMarkup(cardCount: number): string { + return Array.from( + { length: cardCount }, + (_unused, i) => `
card ${i}
`, + ).join(""); +} + +/** Counts computed-style reads on a preview document while `run` executes. */ +function countStyleReads(doc: Document, run: () => void): number { + const win = doc.defaultView!; + const real = win.getComputedStyle.bind(win); + let reads = 0; + win.getComputedStyle = ((el: Element, pseudo?: string | null) => ( + (reads += 1), real(el, pseudo ?? undefined) + )) as typeof win.getComputedStyle; + try { + run(); + } finally { + win.getComputedStyle = real; + } + return reads; +} + interface Rebuild { /** querySelectorAll calls on the preview document, per selector. */ queriesBySelector: Map; @@ -29,29 +127,12 @@ interface Rebuild { * occurrence indices that end up in each indicator's key. */ function rebuildWithSharedSelector(cardCount: number): Rebuild { - const iframe = document.createElement("iframe"); - document.body.append(iframe); - const doc = iframe.contentDocument; - if (!doc) throw new Error("Expected iframe content document"); - - const cards = Array.from( - { length: cardCount }, - (_unused, i) => `
card ${i}
`, - ).join(""); - doc.body.innerHTML = `
${cards}
`; - - const overlay = document.createElement("div"); - document.body.append(overlay); - // Cards sit left of the composition, so every one of them is off-canvas and // reaches the indicator list; everything else measures empty and does not. - Element.prototype.getBoundingClientRect = function (): DOMRect { - if (this === iframe || this === overlay) return new DOMRect(0, 0, 800, 450); - if (this instanceof doc.defaultView!.Element && this.classList.contains("box")) { - return new DOMRect(-500, 40, 100, 40); - } - return new DOMRect(0, 0, 0, 0); - }; + const preview = mountPreview(cardsMarkup(cardCount), (el) => + el.classList.contains("box") ? new DOMRect(-500, 40, 100, 40) : null, + ); + const { doc } = preview; const queriesBySelector = new Map(); const realQuerySelectorAll = doc.querySelectorAll.bind(doc); @@ -73,28 +154,9 @@ function rebuildWithSharedSelector(cardCount: number): Rebuild { }, }); - const sigRef = { current: "" } as React.MutableRefObject; - const elementsRef = { current: new Map() } as React.MutableRefObject< - Map - >; - let rects: OffCanvasRect[] = []; - - recomputeOffCanvasIndicators( - iframe, - overlay, - doc, - { left: 0, top: 0, width: 800, height: 450 }, - "index.html", - sigRef, - elementsRef, - (next) => { - rects = next; - }, - ); - - iframe.remove(); - overlay.remove(); - return { queriesBySelector, singleQueriesBySelector, keys: rects.map((rect) => rect.key) }; + const keys = preview.rebuild().map((rect) => rect.key); + preview.dispose(); + return { queriesBySelector, singleQueriesBySelector, keys }; } /** The composition-root lookups a rebuild makes. `computeOverlayRootScale` and @@ -169,77 +231,32 @@ describe("recomputeOffCanvasIndicators composition-basis cost", () => { /** * A group's box is the union of its MEMBERS' rects, not its own. That makes it - * the one item whose measurement depends on elements BELOW it, and the walk - * cache invalidates upward only — so a cached group union would go stale the - * moment a member moved, and stay stale, because nothing ever writes to the - * wrapper. + * the one item whose measurement depends on elements BELOW it, so any scheme + * that reuses a wrapper's rect while a member moves shows a marker frozen in + * the member's old place — and nothing ever writes to the wrapper, so it would + * never repair. */ describe("recomputeOffCanvasIndicators group measurement", () => { - function rebuildGroupTwice(): { first: string; second: string } { - const iframe = document.createElement("iframe"); - document.body.append(iframe); - const doc = iframe.contentDocument; - if (!doc) throw new Error("Expected iframe content document"); - doc.body.innerHTML = - `
` + - `
` + - `
`; - const member = doc.getElementById("member") as HTMLElement; - - const overlay = document.createElement("div"); - document.body.append(overlay); - + it("re-measures a group when a member moves and nothing writes to the wrapper", () => { // Only the member has a box; the wrapper measures empty, which is exactly - // the case the union exists for. Moving the member moves the group. + // the case the union exists for. let memberLeft = -500; - Element.prototype.getBoundingClientRect = function (): DOMRect { - if (this === iframe || this === overlay) return new DOMRect(0, 0, 800, 450); - if (this === member) return new DOMRect(memberLeft, 40, 100, 40); - return new DOMRect(0, 0, 0, 0); - }; - - const cache = createDomEditLayerWalkCache(); - const observer = new MutationObserver(() => {}); - observer.observe(doc.documentElement, DOM_EDIT_LAYER_OBSERVER_INIT); - - const sigRef = { current: "" } as React.MutableRefObject; - const elementsRef = { current: new Map() } as React.MutableRefObject< - Map - >; - let rects: OffCanvasRect[] = []; - const rebuild = () => { - cache.ingest(observer.takeRecords()); - recomputeOffCanvasIndicators( - iframe, - overlay, - doc, - { left: 0, top: 0, width: 800, height: 450 }, - "index.html", - sigRef, - elementsRef, - (next) => { - rects = next; - }, - cache, - ); - const group = rects.find((rect) => rect.key.includes("grp")); + const preview = mountPreview( + `
`, + (el) => (el.id === "member" ? new DOMRect(memberLeft, 40, 100, 40) : null), + { observe: true }, + ); + const groupBox = () => { + const group = preview.rebuild().find((rect) => rect.key.includes("grp")); return group ? `${group.left},${group.top},${group.width},${group.height}` : "absent"; }; - const first = rebuild(); + const first = groupBox(); // What an animation frame does: one inline style write on the member. memberLeft = -300; - member.style.transform = "translateX(200px)"; - const second = rebuild(); - - observer.disconnect(); - iframe.remove(); - overlay.remove(); - return { first, second }; - } - - it("re-measures a group when a member moves and nothing writes to the wrapper", () => { - const { first, second } = rebuildGroupTwice(); + preview.doc.getElementById("member")!.style.transform = "translateX(200px)"; + const second = groupBox(); + preview.dispose(); expect(first).not.toBe("absent"); expect(second).not.toBe(first); @@ -250,78 +267,32 @@ describe("recomputeOffCanvasIndicators group measurement", () => { * Layout changes that emit no mutation record at all. * * An `` finishing decode, a web font swapping in, a CSS transition or - * `@keyframes` frame, a container query re-evaluating, a `CSSStyleSheet.insertRule` - * — every one of them moves an element's border box with nothing written to the - * DOM. A rebuild that re-measures everything picks them up for free; a rebuild - * that reuses a previous measurement cannot see them at all. + * `@keyframes` frame, a container query re-evaluating, a + * `CSSStyleSheet.insertRule` — every one of them moves an element's border box + * with nothing written to the DOM. A rebuild that measures everything picks + * them up for free; anything that reuses a previous measurement cannot see + * them at all, and there is no record to invalidate on and no observer that + * reports all of them. This is why the pass is scoped to one rebuild. */ describe("recomputeOffCanvasIndicators layout changes with no mutation record", () => { - /** Rebuild twice, changing only what layout REPORTS between the two, with no - * DOM write of any kind in between. */ - function rebuildAcrossSilentLayoutChange(): { first: string; second: string } { - const iframe = document.createElement("iframe"); - document.body.append(iframe); - const doc = iframe.contentDocument; - if (!doc) throw new Error("Expected iframe content document"); - doc.body.innerHTML = - `
` + - `` + - `
`; - const hero = doc.getElementById("hero") as HTMLElement; - - const overlay = document.createElement("div"); - document.body.append(overlay); - - // Before decode the image lays out at zero-ish; after decode it takes its - // intrinsic size, off the left edge of the composition. No attribute is - // written, no node is added, no text changes. + it("re-measures an element whose own box changed with no DOM write", () => { + // Before decode the image lays out small; after decode it takes its + // intrinsic size. No attribute is written, no node added, no text changed. let heroRect = new DOMRect(-500, 40, 100, 40); - Element.prototype.getBoundingClientRect = function (): DOMRect { - if (this === iframe || this === overlay) return new DOMRect(0, 0, 800, 450); - if (this === hero) return heroRect; - return new DOMRect(0, 0, 0, 0); - }; - - const cache = createDomEditLayerWalkCache(); - const observer = new MutationObserver(() => {}); - observer.observe(doc.documentElement, DOM_EDIT_LAYER_OBSERVER_INIT); - - const sigRef = { current: "" } as React.MutableRefObject; - const elementsRef = { current: new Map() } as React.MutableRefObject< - Map - >; - let rects: OffCanvasRect[] = []; - const rebuild = () => { - cache.ingest(observer.takeRecords()); - recomputeOffCanvasIndicators( - iframe, - overlay, - doc, - { left: 0, top: 0, width: 800, height: 450 }, - "index.html", - sigRef, - elementsRef, - (next) => { - rects = next; - }, - cache, - ); - const marker = rects.find((rect) => rect.key.includes("hero")); + const preview = mountPreview( + ``, + (el) => (el.id === "hero" ? heroRect : null), + { observe: true }, + ); + const heroBox = () => { + const marker = preview.rebuild().find((rect) => rect.key.includes("hero")); return marker ? `${marker.left},${marker.top},${marker.width},${marker.height}` : "absent"; }; - const first = rebuild(); + const first = heroBox(); heroRect = new DOMRect(-500, 40, 320, 180); // decode finished - const second = rebuild(); - - observer.disconnect(); - iframe.remove(); - overlay.remove(); - return { first, second }; - } - - it("re-measures an element whose own box changed with no DOM write", () => { - const { first, second } = rebuildAcrossSilentLayoutChange(); + const second = heroBox(); + preview.dispose(); expect(first).toBe("-500,40,100,40"); expect(second).toBe("-500,40,320,180"); @@ -338,77 +309,29 @@ describe("recomputeOffCanvasIndicators layout changes with no mutation record", * read per added wrapper, and must not cost one per wrapper PER CARD. */ describe("recomputeOffCanvasIndicators ancestor cost", () => { - /** `cardCount` off-canvas cards, all siblings, buried under `depth` shared - * wrappers. Returns the preview document's computed-style reads for one - * rebuild, and the markers it produced. */ - function rebuildAtDepth( - cardCount: number, - depth: number, - ): { styleReads: number; markers: number } { - const iframe = document.createElement("iframe"); - document.body.append(iframe); - const doc = iframe.contentDocument; - if (!doc) throw new Error("Expected iframe content document"); - - const cards = Array.from( - { length: cardCount }, - (_unused, i) => `
card ${i}
`, - ).join(""); + /** Computed-style reads for one rebuild of `cardCount` cards buried under + * `depth` shared wrappers. */ + function styleReadsAtDepth(cardCount: number, depth: number): number { const open = Array.from({ length: depth }, (_unused, i) => `
`).join(""); - const close = "
".repeat(depth); - doc.body.innerHTML = - `
` + - `${open}${cards}${close}` + - `
`; - - const overlay = document.createElement("div"); - document.body.append(overlay); - Element.prototype.getBoundingClientRect = function (): DOMRect { - if (this === iframe || this === overlay) return new DOMRect(0, 0, 800, 450); - if (this instanceof doc.defaultView!.Element && this.classList.contains("box")) { - return new DOMRect(-500, 40, 100, 40); - } - return new DOMRect(0, 0, 0, 0); - }; - - const win = doc.defaultView!; - const realGetComputedStyle = win.getComputedStyle.bind(win); - let styleReads = 0; - win.getComputedStyle = ((el: Element, pseudo?: string | null) => { - styleReads += 1; - return realGetComputedStyle(el, pseudo ?? undefined); - }) as typeof win.getComputedStyle; - - const sigRef = { current: "" } as React.MutableRefObject; - const elementsRef = { current: new Map() } as React.MutableRefObject< - Map - >; - let rects: OffCanvasRect[] = []; - try { - recomputeOffCanvasIndicators( - iframe, - overlay, - doc, - { left: 0, top: 0, width: 800, height: 450 }, - "index.html", - sigRef, - elementsRef, - (next) => { - rects = next; - }, - ); - } finally { - win.getComputedStyle = realGetComputedStyle; - iframe.remove(); - overlay.remove(); - } - return { styleReads, markers: rects.length }; + const preview = mountPreview( + `${open}${cardsMarkup(cardCount)}${"".repeat(depth)}`, + (el) => (el.classList.contains("box") ? new DOMRect(-500, 40, 100, 40) : null), + ); + let markers = 0; + const reads = countStyleReads(preview.doc, () => { + markers = preview.rebuild().length; + }); + preview.dispose(); + // Non-vacuity, checked on every measurement rather than in its own test: + // the count only means something if the rebuild really did resolve every + // card's rect. + expect(markers).toBe(cardCount); + return reads; } /** What burying the same cards 8 wrappers deeper costs, at `cardCount`. */ - function depthSurcharge(cardCount: number): number { - return rebuildAtDepth(cardCount, 10).styleReads - rebuildAtDepth(cardCount, 2).styleReads; - } + const depthSurcharge = (cardCount: number): number => + styleReadsAtDepth(cardCount, 10) - styleReadsAtDepth(cardCount, 2); // The load-bearing assertion, and it is INVARIANCE rather than a threshold: a // ceiling ("under 40 reads") passes on a small fixture and still degrades on @@ -419,11 +342,4 @@ describe("recomputeOffCanvasIndicators ancestor cost", () => { it("pays for a deeper tree per added ancestor, not per ancestor per card", () => { expect(depthSurcharge(48)).toBe(depthSurcharge(24)); }); - - // Non-vacuity: the assertion above only means something if the fixture really - // drove the per-element geometry path for every card at both depths. - it("measures rebuilds that really did resolve every card's rect", () => { - expect(rebuildAtDepth(24, 2).markers).toBe(24); - expect(rebuildAtDepth(24, 10).markers).toBe(24); - }); }); From 9fc5ceaf991049a4326056d52f7acc40a4fab8d0 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Thu, 10 Sep 2026 09:21:56 -0400 Subject: [PATCH 4/4] test(studio): pin the measure pass to one rebuild through an ancestor-derived input MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The existing no-mutation-record test varies an element's own box, and the pass never memoizes a box — so hoisting the pass to module scope, which is exactly the mistake that would reintroduce cross-rebuild staleness, left every test green. This one fades a wrapper between two rebuilds and asserts the marker under it goes away. Visibility through ancestors IS memoized, so the pass surviving the rebuild serves the stale answer: with `createOverlayMeasurePass()` hoisted the suite exits 1 on this test alone, and exits 0 at head. --- ...CanvasIndicatorGeometry.complexity.test.ts | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/packages/studio/src/components/editor/offCanvasIndicatorGeometry.complexity.test.ts b/packages/studio/src/components/editor/offCanvasIndicatorGeometry.complexity.test.ts index d64e0fd892..c7fc51590b 100644 --- a/packages/studio/src/components/editor/offCanvasIndicatorGeometry.complexity.test.ts +++ b/packages/studio/src/components/editor/offCanvasIndicatorGeometry.complexity.test.ts @@ -299,6 +299,34 @@ describe("recomputeOffCanvasIndicators layout changes with no mutation record", }); }); +/** + * The other half of "the pass is scoped to one rebuild". + * + * The no-mutation-record test above varies an element's own BOX, which the pass + * never memoizes — so it would still pass if the pass leaked across rebuilds. + * This one varies an ANCESTOR-derived input, which is exactly what the pass + * does hold: fade a wrapper out and every descendant's visibility answer has to + * be recomputed, not served from the previous rebuild's memo. + */ +describe("recomputeOffCanvasIndicators ancestor-derived state across rebuilds", () => { + it("re-answers visibility when an ancestor fades between rebuilds", () => { + const preview = mountPreview( + `
`, + (el) => (el.classList.contains("box") ? new DOMRect(-500, 40, 100, 40) : null), + { observe: true }, + ); + + const visible = preview.rebuild().length; + // Nothing about the box changes — only what it inherits from above it. + preview.doc.getElementById("wrap")!.style.opacity = "0"; + const faded = preview.rebuild().length; + preview.dispose(); + + expect(visible).toBe(1); + expect(faded).toBe(0); + }); +}); + /** * The ancestor work, measured through the production entry point. *