mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-11 23:00:03 +00:00
fix(studio): capture the storyboard frame hero at full resolution (#3338)
The thumbnail route bounds every preview capture to 240x135. That bound came from the timeline, where thumbnails are small and numerous and their decoded bytes are budgeted. The storyboard reuses the same route for its frame detail hero, which is up to 900px wide, so the poster arrived at 240x135 and upscaled past 7x on a retina display. Headlines survived it; body copy, table labels and captions did not. That is the surface where it costs the most. references/review-loop.md sends the user here to confirm layout and real copy, and tells them to run no CLI in that pass: "the poster is the only picture this pass needs". Give the caller a way to ask for the composition's own dimensions, which the route already supports as `output=source`, and fold the choice into a single `surface` prop. Whether a poster is a tile or the hero decides both the crop and the capture density, so one prop owns both rather than two that can disagree. The contact sheet keeps the bounded capture: many tiles, and it is a contact sheet. The timeline is untouched. Reported with a reproduction and a correct read of the consequences in #3271. Co-authored-by: anikam13 <22992075+anikam13@users.noreply.github.com>
This commit is contained in:
@@ -0,0 +1,64 @@
|
|||||||
|
// @vitest-environment happy-dom
|
||||||
|
|
||||||
|
import React, { act } from "react";
|
||||||
|
import { createRoot, type Root } from "react-dom/client";
|
||||||
|
import { afterEach, describe, expect, it } from "vitest";
|
||||||
|
import { FramePoster } from "./FramePoster";
|
||||||
|
|
||||||
|
(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
|
||||||
|
|
||||||
|
const roots: Root[] = [];
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
act(() => roots.splice(0).forEach((root) => root.unmount()));
|
||||||
|
document.body.replaceChildren();
|
||||||
|
});
|
||||||
|
|
||||||
|
// A fresh host per render: several cases compare two surfaces side by side.
|
||||||
|
function renderPoster(surface?: "tile" | "hero"): HTMLImageElement {
|
||||||
|
const host = document.createElement("div");
|
||||||
|
document.body.appendChild(host);
|
||||||
|
const root = createRoot(host);
|
||||||
|
roots.push(root);
|
||||||
|
act(() => {
|
||||||
|
root.render(
|
||||||
|
<FramePoster
|
||||||
|
projectId="demo"
|
||||||
|
src="frames/01-hero.html"
|
||||||
|
seconds={3}
|
||||||
|
title="hero"
|
||||||
|
surface={surface}
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
const img = host.querySelector("img");
|
||||||
|
if (!img) throw new Error("poster did not render an <img>");
|
||||||
|
return img;
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("FramePoster", () => {
|
||||||
|
// Regression: the hero and the tile shared one bounded poster, so the frame
|
||||||
|
// detail view — the sketch pass's only picture — showed a 240x135 capture
|
||||||
|
// upscaled past 7x on a retina display, and body copy was unreadable.
|
||||||
|
it("captures the focus hero at the composition's own dimensions", () => {
|
||||||
|
const url = new URL(renderPoster("hero").src);
|
||||||
|
|
||||||
|
expect(url.searchParams.get("output")).toBe("source");
|
||||||
|
expect(url.pathname).toBe("/api/projects/demo/thumbnail/frames/01-hero.html");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("leaves the contact-sheet tile on the route's bounded preview capture", () => {
|
||||||
|
const url = new URL(renderPoster("tile").src);
|
||||||
|
|
||||||
|
expect(url.searchParams.has("output")).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("defaults to the tile surface", () => {
|
||||||
|
expect(new URL(renderPoster().src).search).toBe(new URL(renderPoster("tile").src).search);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("letterboxes only the hero, so a tile still fills its cell", () => {
|
||||||
|
expect(renderPoster("hero").className).toContain("object-contain");
|
||||||
|
expect(renderPoster("tile").className).toContain("object-cover");
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -8,8 +8,15 @@ export interface FramePosterProps {
|
|||||||
/** Time (seconds) to seek to for the poster. */
|
/** Time (seconds) to seek to for the poster. */
|
||||||
seconds: number;
|
seconds: number;
|
||||||
title: string;
|
title: string;
|
||||||
/** `cover` fills+crops (contact-sheet tile); `contain` letterboxes (focus hero). */
|
/**
|
||||||
fit?: "cover" | "contain";
|
* Where this poster is rendered. A contact-sheet tile is ~300px wide and there
|
||||||
|
* are many of them; the focus hero is up to 900px wide and there is exactly
|
||||||
|
* one. That single difference decides both the crop and how much resolution
|
||||||
|
* the server has to capture, so it is one prop rather than two that can
|
||||||
|
* disagree: `tile` fills+crops at the route's bounded preview density, `hero`
|
||||||
|
* letterboxes at the composition's own dimensions.
|
||||||
|
*/
|
||||||
|
surface?: "tile" | "hero";
|
||||||
/**
|
/**
|
||||||
* Project content signature to key the poster URL on. The thumbnail route
|
* Project content signature to key the poster URL on. The thumbnail route
|
||||||
* regenerates when the frame's source changes, but the browser only refetches
|
* regenerates when the frame's source changes, but the browser only refetches
|
||||||
@@ -30,14 +37,14 @@ export function FramePoster({
|
|||||||
src,
|
src,
|
||||||
seconds,
|
seconds,
|
||||||
title,
|
title,
|
||||||
fit = "cover",
|
surface = "tile",
|
||||||
posterVersion,
|
posterVersion,
|
||||||
}: FramePosterProps) {
|
}: FramePosterProps) {
|
||||||
const [failed, setFailed] = useState(false);
|
const [failed, setFailed] = useState(false);
|
||||||
// The <img> is reused (no key) when a tile/hero swaps to a different frame, so a
|
// The <img> is reused (no key) when a tile/hero swaps to a different frame, so a
|
||||||
// prior load error would stick. Reset when the poster target changes — including
|
// prior load error would stick. Reset when the poster target changes — including
|
||||||
// a new posterVersion, so a frame that failed mid-write retries once it settles.
|
// a new posterVersion, so a frame that failed mid-write retries once it settles.
|
||||||
useEffect(() => setFailed(false), [src, seconds, posterVersion]);
|
useEffect(() => setFailed(false), [src, seconds, posterVersion, surface]);
|
||||||
if (failed) {
|
if (failed) {
|
||||||
return (
|
return (
|
||||||
<div className="flex h-full w-full items-center justify-center text-[11px] text-neutral-600">
|
<div className="flex h-full w-full items-center justify-center text-[11px] text-neutral-600">
|
||||||
@@ -50,6 +57,11 @@ export function FramePoster({
|
|||||||
seekTime: seconds,
|
seekTime: seconds,
|
||||||
duration: 0,
|
duration: 0,
|
||||||
origin: window.location.origin,
|
origin: window.location.origin,
|
||||||
|
// The hero is the sketch pass's only picture (references/review-loop.md), and
|
||||||
|
// it is shown large. Bounded to the preview cap it arrives at 240x135 and
|
||||||
|
// upscales past 7x on a retina display, which is unreadable for exactly the
|
||||||
|
// body copy and labels this pass exists to confirm.
|
||||||
|
...(surface === "hero" ? { output: "source" as const } : {}),
|
||||||
});
|
});
|
||||||
if (posterVersion) {
|
if (posterVersion) {
|
||||||
const withVersion = new URL(url, window.location.origin);
|
const withVersion = new URL(url, window.location.origin);
|
||||||
@@ -63,7 +75,7 @@ export function FramePoster({
|
|||||||
draggable={false}
|
draggable={false}
|
||||||
loading="lazy"
|
loading="lazy"
|
||||||
onError={() => setFailed(true)}
|
onError={() => setFailed(true)}
|
||||||
className={`h-full w-full ${fit === "contain" ? "object-contain" : "object-cover"}`}
|
className={`h-full w-full ${surface === "hero" ? "object-contain" : "object-cover"}`}
|
||||||
/>
|
/>
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -224,7 +224,7 @@ export function StoryboardFrameFocus({
|
|||||||
src={frame.src}
|
src={frame.src}
|
||||||
seconds={posterTime(frame)}
|
seconds={posterTime(frame)}
|
||||||
title={title}
|
title={title}
|
||||||
fit="contain"
|
surface="hero"
|
||||||
posterVersion={posterVersion}
|
posterVersion={posterVersion}
|
||||||
/>
|
/>
|
||||||
) : (
|
) : (
|
||||||
|
|||||||
@@ -76,6 +76,18 @@ describe("buildCompositionThumbnailUrl", () => {
|
|||||||
"http://localhost:3000/api/projects/demo/thumbnail/index.html?t=2.00&v=v3&selector=.card&selectorIndex=2",
|
"http://localhost:3000/api/projects/demo/thumbnail/index.html?t=2.00&v=v3&selector=.card&selectorIndex=2",
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("asks for source density only when a caller opts in", () => {
|
||||||
|
const base = {
|
||||||
|
previewUrl: "/api/projects/demo/preview",
|
||||||
|
seekTime: 1,
|
||||||
|
duration: 0,
|
||||||
|
origin: "http://localhost:3000",
|
||||||
|
};
|
||||||
|
|
||||||
|
expect(buildCompositionThumbnailUrl(base)).not.toContain("output=");
|
||||||
|
expect(buildCompositionThumbnailUrl({ ...base, output: "source" })).toContain("output=source");
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe("CompositionThumbnail", () => {
|
describe("CompositionThumbnail", () => {
|
||||||
|
|||||||
@@ -31,6 +31,7 @@ export function buildCompositionThumbnailUrl({
|
|||||||
selector,
|
selector,
|
||||||
selectorIndex,
|
selectorIndex,
|
||||||
origin,
|
origin,
|
||||||
|
output,
|
||||||
}: {
|
}: {
|
||||||
previewUrl: string;
|
previewUrl: string;
|
||||||
seekTime?: number;
|
seekTime?: number;
|
||||||
@@ -38,6 +39,13 @@ export function buildCompositionThumbnailUrl({
|
|||||||
selector?: string;
|
selector?: string;
|
||||||
selectorIndex?: number;
|
selectorIndex?: number;
|
||||||
origin: string;
|
origin: string;
|
||||||
|
/**
|
||||||
|
* Capture density. Omitted, the route bounds the image to its preview cap —
|
||||||
|
* right for the timeline, where thumbnails are small and numerous and their
|
||||||
|
* decoded bytes are budgeted. `"source"` captures at the composition's own
|
||||||
|
* dimensions, for the rare surface that shows one poster large enough to read.
|
||||||
|
*/
|
||||||
|
output?: "source";
|
||||||
}): string {
|
}): string {
|
||||||
const thumbnailBase = previewUrl
|
const thumbnailBase = previewUrl
|
||||||
.replace("/preview/comp/", "/thumbnail/")
|
.replace("/preview/comp/", "/thumbnail/")
|
||||||
@@ -45,6 +53,7 @@ export function buildCompositionThumbnailUrl({
|
|||||||
const thumbnailUrl = new URL(thumbnailBase, origin);
|
const thumbnailUrl = new URL(thumbnailBase, origin);
|
||||||
thumbnailUrl.searchParams.set("t", (seekTime + duration / 2).toFixed(2));
|
thumbnailUrl.searchParams.set("t", (seekTime + duration / 2).toFixed(2));
|
||||||
thumbnailUrl.searchParams.set("v", THUMBNAIL_URL_VERSION);
|
thumbnailUrl.searchParams.set("v", THUMBNAIL_URL_VERSION);
|
||||||
|
if (output) thumbnailUrl.searchParams.set("output", output);
|
||||||
if (selector) {
|
if (selector) {
|
||||||
thumbnailUrl.searchParams.set("selector", selector);
|
thumbnailUrl.searchParams.set("selector", selector);
|
||||||
if (selectorIndex != null && selectorIndex > 0) {
|
if (selectorIndex != null && selectorIndex > 0) {
|
||||||
|
|||||||
Reference in New Issue
Block a user