Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -785,7 +785,7 @@ jobs:
cp -R packages/studio/tests/e2e/fixtures/studio-open \
packages/studio/data/projects/studio-open
- name: Read the base branch's work-count ceilings
# A ceiling may not rise or disappear against the branch this PR merges into.
# A ceiling may not disappear, or rise without a new reason, against the branch this PR merges into.
if: github.event_name == 'pull_request'
env:
BASE_REF: ${{ github.base_ref }}
Expand Down
94 changes: 76 additions & 18 deletions packages/studio/src/components/nle/NLEPreview.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,23 +93,25 @@ function renderPreview(
document.body.append(host);
const root = createRoot(host);
const iframeRef = createRef<HTMLIFrameElement>();

act(() => {
root.render(
React.createElement(NLEPreview, {
projectId: "timeline-edit-playground",
iframeRef,
onIframeLoad: () => {},
previewSlots,
onShadowIframeLoad: () => {},
onShadowReadyChange: () => {},
onShadowError: () => {},
setShadowIframeNode: () => {},
resetPreviewSlots: () => {},
fillBox,
}),
);
});
const render = (directUrl?: string) =>
act(() => {
root.render(
React.createElement(NLEPreview, {
projectId: "timeline-edit-playground",
directUrl,
iframeRef,
onIframeLoad: () => {},
previewSlots,
onShadowIframeLoad: () => {},
onShadowReadyChange: () => {},
onShadowError: () => {},
setShadowIframeNode: () => {},
resetPreviewSlots: () => {},
fillBox,
}),
);
});
render();

const viewport = host.querySelector('[aria-label="Composition preview"]') as HTMLDivElement;
const stage = host.querySelector('[data-testid="preview-zoom-stage"]') as HTMLDivElement;
Expand All @@ -124,6 +126,7 @@ function renderPreview(
return {
host,
root,
render,
viewport,
stage,
cleanup() {
Expand Down Expand Up @@ -296,12 +299,67 @@ describe("NLEPreview", () => {
expect(poster()?.style.zIndex).toBe("2");

act(() => livePlayerProps.onReadyToShowChange?.(false));
expect(poster()).not.toBeNull();
expect(poster()?.hidden).toBe(false);
act(() => livePlayerProps.onReadyToShowChange?.(true));
expect(poster()?.hidden).toBe(true);
act(() => poster()?.dispatchEvent(new Event("load")));
expect(poster()).toBeNull();
view.cleanup();
});

describe("a missing poster", () => {
const renderUrl =
"/api/projects/timeline-edit-playground/thumbnail/index.html?t=0&output=source";
const fetchSpy = vi.fn(() => Promise.resolve(new Response()));
beforeEach(() => {
fetchSpy.mockClear();
vi.stubGlobal("fetch", fetchSpy);
});
afterEach(() => vi.unstubAllGlobals());

const settle = (
view: ReturnType<typeof renderPreview>,
steps: Array<"ready" | "missing" | "loaded">,
) => {
for (const step of steps) {
const poster = view.stage.querySelector('[data-testid="preview-poster"]');
act(() =>
step === "ready"
? livePlayerProps.onReadyToShowChange?.(true)
: poster?.dispatchEvent(new Event(step === "missing" ? "error" : "load")),
);
}
};

it("is rendered for the next open when the live frame is ready first", () => {
const view = renderPreview();
settle(view, ["ready", "missing"]);
expect(fetchSpy.mock.calls).toEqual([[renderUrl]]);
expect(view.stage.querySelector('[data-testid="preview-poster"]')).toBeNull();
view.cleanup();
});

it("is rendered once the live frame is ready when it is missing first", () => {
const view = renderPreview();
settle(view, ["missing"]);
expect(fetchSpy).not.toHaveBeenCalled();
settle(view, ["ready", "ready"]);
expect(fetchSpy.mock.calls).toEqual([[renderUrl]]);
view.cleanup();
});

it("is rendered on a return from a sub-composition when the live frame is ready first", () => {
const view = renderPreview();
settle(view, ["loaded", "ready"]);
view.render("/api/projects/timeline-edit-playground/preview/comp/compositions/intro.html");
settle(view, ["ready"]);
view.render();
settle(view, ["ready", "missing"]);
expect(fetchSpy.mock.calls).toEqual([[renderUrl]]);
view.cleanup();
});
});

it("mounts the live player once when the composition switches", () => {
playerMounts.length = 0;
const Harness = ({ projectId }: { projectId: string }) => {
Expand Down
4 changes: 3 additions & 1 deletion packages/studio/src/components/nle/NLEPreview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -551,11 +551,13 @@ export const NLEPreview = memo(function NLEPreview({
/>
),
)}
{poster.showPoster && (
{poster.mountPoster && (
<PreviewPoster
key={activeKey}
projectId={projectId}
hidden={poster.hidePoster}
onSize={(size) => setCompositionSize((prev) => prev ?? size)}
onLoaded={poster.onPosterLoaded}
onMissing={poster.onPosterMissing}
/>
)}
Expand Down
52 changes: 38 additions & 14 deletions packages/studio/src/components/nle/PreviewPoster.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,15 @@ const PREVIEW_POSTER_STYLE: React.CSSProperties = {
/** Covers the live preview with the cached frame 0; reports its size so the stage fits it. */
export function PreviewPoster({
projectId,
hidden,
onSize,
onLoaded,
onMissing,
}: {
projectId: string;
hidden: boolean;
onSize: (size: PreviewCompositionSize) => void;
onLoaded: () => void;
onMissing: () => void;
}) {
return (
Expand All @@ -34,42 +38,62 @@ export function PreviewPoster({
alt=""
aria-hidden
data-testid="preview-poster"
hidden={hidden}
style={PREVIEW_POSTER_STYLE}
onLoad={(event) => {
const { naturalWidth: width, naturalHeight: height } = event.currentTarget;
if (width > 0 && height > 0) onSize({ width, height });
if (!hidden && width > 0 && height > 0) onSize({ width, height });
onLoaded();
}}
onError={onMissing}
/>
);
}

/** The poster covers the live slot until its first frame is ready to show; a missing poster is
* rendered after that, off the open path, for the next open. */
/** The poster covers the live slot until its first frame is ready; a missing poster is rendered
* after both are known, in either order, off the open path, for the next open. */
export function usePreviewPoster(
projectId: string,
activeKey: string,
directUrl: string | undefined,
) {
const [posterDoneFor, setPosterDoneFor] = useState<string | null>(null);
const posterMissingRef = useRef(false);
const visit = useVisitNumber(activeKey);
const [coverDoneFor, setCoverDoneFor] = useState<number | null>(null);
const [posterSettledFor, setPosterSettledFor] = useState<number | null>(null);
const missingForRef = useRef<number | null>(null);
const liveReadyForRef = useRef<number | null>(null);
const renderMissingPoster = useCallback(() => {
if (missingForRef.current !== visit || liveReadyForRef.current !== visit) return;
missingForRef.current = null;
void fetch(previewPosterUrl(projectId, false)).catch(() => {});
}, [visit, projectId]);
const onLiveReadyToShowChange = useCallback(
(ready: boolean) => {
if (!ready) return;
setPosterDoneFor(activeKey);
if (!posterMissingRef.current || directUrl) return;
posterMissingRef.current = false;
void fetch(previewPosterUrl(projectId, false)).catch(() => {});
setCoverDoneFor(visit);
liveReadyForRef.current = visit;
renderMissingPoster();
},
[activeKey, directUrl, projectId],
[visit, renderMissingPoster],
);
return {
showPoster: !directUrl && posterDoneFor !== activeKey,
mountPoster: !directUrl && (coverDoneFor !== visit || posterSettledFor !== visit),
hidePoster: coverDoneFor === visit,
onLiveReadyToShowChange,
onPreviewError: () => setPosterDoneFor(activeKey),
onPreviewError: () => setCoverDoneFor(visit),
onPosterLoaded: () => setPosterSettledFor(visit),
onPosterMissing: () => {
posterMissingRef.current = true;
setPosterDoneFor(activeKey);
setCoverDoneFor(visit);
setPosterSettledFor(visit);
missingForRef.current = visit;
renderMissingPoster();
},
};
}

function useVisitNumber(key: string): number {
const [seen, setSeen] = useState({ key, visit: 0 });
if (seen.key === key) return seen.visit;
setSeen({ key, visit: seen.visit + 1 });
return seen.visit + 1;
}
7 changes: 5 additions & 2 deletions packages/studio/tests/e2e/perf-ceilings.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,13 @@
"studio-open": {
"browser": "153",
"counts": {
"thumbnailRenders": 20,
"thumbnailRenders": 14,
"total.frameNavigations": 7,
"total.requests.document": 2,
"total.requests.thumbnail": 20
"total.requests.thumbnail": 22
},
"raised": {
"total.requests.thumbnail": "#4401 shows a cached frame-0 poster while the preview boots: a cache-only ask that misses on a fresh open, then a render of that poster for the next open (+2 requests)"
}
}
}
40 changes: 32 additions & 8 deletions packages/studio/tests/e2e/perf-ratchet.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@
/**
* Work-count ratchet: each gated counter has a ceiling in perf-ceilings.json that may only go
* down. A journey fails when a counter rises above its ceiling or falls below it (bank the
* improvement), or when a ceiling was raised or removed against the base branch's file.
* improvement), or when a ceiling was raised or removed against the base branch's file. A raise
* passes only when the journey's `raised` map gives that counter a reason the base did not have.
*
* node perf-ratchet.mjs check <ceilings.json> <journey> <evidence.json> [<base-ceilings.json>]
* node perf-ratchet.mjs lower <ceilings.json> <journey> <evidence.json>
* node perf-ratchet.mjs correlate <variant>=<evidence.json> ...
*
* Ceilings: `{ <journey>: { browser: "<Chrome major>", counts: { <counter>: n } } }`.
* Ceilings: `{ <journey>: { browser: "<Chrome major>", counts: { <counter>: n },
* raised?: { <counter>: "<why this ceiling went up>" } } }`.
* Evidence is a journey's JSON output: `workCounts`, `wallMs` and the browser it ran in.
*/
import { existsSync, readFileSync, realpathSync, writeFileSync } from "node:fs";
Expand All @@ -27,15 +29,17 @@ function measuredStatus(ceiling, value) {
return value > ceiling ? "rose" : "below";
}

function ceilingRow(counter, ceiling, value, base) {
if (raisedAgainst(ceiling, base)) return { counter, ceiling, base, status: "raised" };
function ceilingRow(counter, ceiling, value, base, accepted) {
if (raisedAgainst(ceiling, base) && !accepted.has(counter)) {
return { counter, ceiling, base, status: "raised" };
}
return { counter, ceiling, value, status: measuredStatus(ceiling, value) };
}

/** Passes only when every gated counter sits exactly at a ceiling no higher than the base's. */
export function checkCeilings(ceilings, counts, baseCeilings = {}) {
export function checkCeilings(ceilings, counts, baseCeilings = {}, accepted = new Set()) {
const rows = Object.entries(ceilings).map(([counter, ceiling]) =>
ceilingRow(counter, ceiling, counts[counter], baseCeilings[counter]),
ceilingRow(counter, ceiling, counts[counter], baseCeilings[counter], accepted),
);
if (rows.length === 0)
throw new Error("no gated counters: a ratchet that checks nothing passes nothing");
Expand All @@ -57,7 +61,7 @@ const ROW_TEXT = {
below: ({ counter, ceiling, value }) =>
`FAIL ${counter} fell ${ceiling} -> ${value}: bank it by setting its ceiling to ${value} (perf-ratchet.mjs lower)`,
raised: ({ counter, ceiling, base }) =>
`FAIL ${counter}: ceiling raised ${base} -> ${ceiling} against the base branch; ceilings only go down`,
`FAIL ${counter}: ceiling raised ${base} -> ${ceiling} against the base branch; a raise needs a new reason under "raised" in perf-ceilings.json`,
removed: ({ counter, base }) =>
`FAIL ${counter}: ceiling ${base} removed against the base branch; ceilings only go down`,
};
Expand Down Expand Up @@ -153,10 +157,24 @@ export function readBase(base, all, journey) {
assertBaseShape(base);
return {
counts: base[journey]?.counts,
raised: base[journey]?.raised,
removedJourneys: Object.keys(base).filter((name) => !Object.hasOwn(all, name)),
};
}

const normalized = (text) => (typeof text === "string" ? text.replace(/\s+/g, " ").trim() : "");
const isNewReason = (reason, baseReason) =>
normalized(reason) !== "" && normalized(reason) !== normalized(baseReason);

/** Counters raised above the base on purpose: a reason in `raised` the base branch did not give. */
export function acceptedRaises(entry, base) {
const reasons = entry.raised ?? {};
const onPurpose = (counter) =>
raisedAgainst(entry.counts[counter], base.counts?.[counter]) &&
isNewReason(reasons[counter], base.raised?.[counter]);
return new Set(Object.keys(reasons).filter(onPurpose));
}

function assertBaseShape(base) {
for (const [name, entry] of Object.entries(base)) {
if (!isObject(entry?.counts)) throw new Error(`base ceilings: "${name}" has no counts object`);
Expand Down Expand Up @@ -197,10 +215,16 @@ function printResult(journey, ok, rows, removedJourneys) {
function runCheck(args) {
const { all, journey, evidence, counts } = journeyInputs(args);
const base = loadBase(args[3], all, journey);
const { passed, rows } = checkCeilings(all[journey].counts, counts, base.counts);
const accepted = acceptedRaises(all[journey], base);
const { passed, rows } = checkCeilings(all[journey].counts, counts, base.counts, accepted);
const ok = passed && base.removedJourneys.length === 0;
printResult(journey, ok, rows, base.removedJourneys);
printNotes(all[journey], evidence, rows);
for (const counter of accepted) {
console.log(
`[perf-ratchet] note: ${counter} raised on purpose: ${all[journey].raised[counter]}`,
);
}
return ok ? 0 : 1;
}

Expand Down
Loading
Loading