mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-10 22:20:14 +00:00
fix(cli): accept portrait aspects for --resolution alias flag
The aspect-agnostic resolution aliases (`--resolution 1080p` / `hd` / `4k` / `uhd`) previously all normalized to a landscape preset, which rejected portrait 1080x1920 compositions with 'Output resolution incompatible'. Users had to specify the orientation-bearing alias (`1080p-portrait`) or render at native. This threads two new fields (`outputResolutionAspectAgnostic` + `outputResolutionRaw`) through the render pipeline. At the CLI layer we detect whether the user's flag was an aspect-agnostic alias; at the compile stage we re-map the preset to the composition's orientation via the existing `suggestMatchingPreset` sibling-lookup (formerly private). Explicit orientation-bearing aliases and canonical presets stay strict. Field signal: ts=1784176662 (darwin/arm64, CLI 0.7.59, `--resolution 1080p` on a 1080x1920 portrait comp). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> — Via
This commit is contained in:
@@ -43,7 +43,12 @@ import { isVideoFrameFormat } from "@hyperframes/engine";
|
||||
import { resolveRenderPaths } from "./utils/paths.js";
|
||||
import { defaultLogger, type ProducerLogger } from "./logger.js";
|
||||
import { Semaphore } from "./utils/semaphore.js";
|
||||
import { parseFps, normalizeResolutionFlag, type CanvasResolution } from "@hyperframes/core";
|
||||
import {
|
||||
parseFps,
|
||||
normalizeResolutionFlag,
|
||||
isAspectAgnosticResolutionAlias,
|
||||
type CanvasResolution,
|
||||
} from "@hyperframes/core";
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Types
|
||||
@@ -93,9 +98,17 @@ interface RenderInput {
|
||||
* Output resolution preset (e.g. `landscape-4k`). Drives the same
|
||||
* `resolveDeviceScaleFactor` supersampling path the local CLI uses — Chrome
|
||||
* renders at a higher devicePixelRatio so the captured screenshot lands at
|
||||
* the requested dimensions. Aspect ratio must match the composition.
|
||||
* the requested dimensions. Aspect ratio must match the composition unless
|
||||
* `outputResolutionAspectAgnostic` is set (see below).
|
||||
*/
|
||||
outputResolution?: CanvasResolution;
|
||||
/**
|
||||
* True when `outputResolution` was normalized from an aspect-agnostic alias
|
||||
* (`1080p`, `hd`, `4k`, `uhd`). The compile stage will adapt the preset to
|
||||
* the composition's orientation instead of rejecting portrait/square
|
||||
* compositions as an aspect-ratio mismatch.
|
||||
*/
|
||||
outputResolutionAspectAgnostic?: boolean;
|
||||
}
|
||||
|
||||
interface PreparedRenderInput {
|
||||
@@ -151,7 +164,8 @@ export function parseRenderOptions(body: Record<string, unknown>): Omit<RenderIn
|
||||
? body.videoFrameFormat
|
||||
: undefined;
|
||||
|
||||
const { variables, outputResolution } = parseRenderOverrides(body);
|
||||
const { variables, outputResolution, outputResolutionAspectAgnostic } =
|
||||
parseRenderOverrides(body);
|
||||
|
||||
return {
|
||||
outputPath,
|
||||
@@ -165,6 +179,7 @@ export function parseRenderOptions(body: Record<string, unknown>): Omit<RenderIn
|
||||
format,
|
||||
variables,
|
||||
outputResolution,
|
||||
outputResolutionAspectAgnostic,
|
||||
videoFrameFormat,
|
||||
};
|
||||
}
|
||||
@@ -182,15 +197,23 @@ function isPlainObject(value: unknown): value is Record<string, unknown> {
|
||||
function parseRenderOverrides(body: Record<string, unknown>): {
|
||||
variables?: Record<string, unknown>;
|
||||
outputResolution?: CanvasResolution;
|
||||
outputResolutionAspectAgnostic?: boolean;
|
||||
} {
|
||||
// Only forward a plain JSON object. Arrays / primitives / null → undefined.
|
||||
const variables = isPlainObject(body.variables) ? body.variables : undefined;
|
||||
// Accept canonical presets and aliases ("4k", "landscape-4k", …).
|
||||
const outputResolution =
|
||||
typeof body.outputResolution === "string"
|
||||
? normalizeResolutionFlag(body.outputResolution)
|
||||
: undefined;
|
||||
return { variables, outputResolution };
|
||||
const rawOutputResolution =
|
||||
typeof body.outputResolution === "string" ? body.outputResolution : undefined;
|
||||
const outputResolution = rawOutputResolution
|
||||
? normalizeResolutionFlag(rawOutputResolution)
|
||||
: undefined;
|
||||
// Preserve the "raw shape was tier-only" signal so the compile stage can
|
||||
// adapt the preset to the composition's orientation. Set only when
|
||||
// normalization succeeded — a bad string doesn't need the flag.
|
||||
const outputResolutionAspectAgnostic = outputResolution
|
||||
? isAspectAgnosticResolutionAlias(rawOutputResolution)
|
||||
: undefined;
|
||||
return { variables, outputResolution, outputResolutionAspectAgnostic };
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -210,6 +233,7 @@ function buildRenderJobConfig(input: RenderInput, log: ProducerLogger) {
|
||||
entryFile: input.entryFile,
|
||||
variables: input.variables,
|
||||
outputResolution: input.outputResolution,
|
||||
outputResolutionAspectAgnostic: input.outputResolutionAspectAgnostic,
|
||||
videoFrameFormat: input.videoFrameFormat,
|
||||
logger: log,
|
||||
};
|
||||
|
||||
@@ -128,6 +128,12 @@ export interface DistributedRenderConfig {
|
||||
videoFrameFormat?: VideoFrameFormat;
|
||||
/** Output resolution preset; engages Chrome `deviceScaleFactor` supersampling. */
|
||||
outputResolution?: CanvasResolution;
|
||||
/**
|
||||
* True when `outputResolution` was normalized from an aspect-agnostic alias
|
||||
* (`1080p`, `hd`, `4k`, `uhd`) — the compile stage re-targets the preset
|
||||
* to the composition's orientation.
|
||||
*/
|
||||
outputResolutionAspectAgnostic?: boolean;
|
||||
|
||||
/**
|
||||
* Frames per chunk. When explicitly set, that value is used and
|
||||
@@ -762,6 +768,7 @@ export async function plan(
|
||||
bitrate: config.bitrate,
|
||||
videoFrameFormat: config.videoFrameFormat,
|
||||
outputResolution: config.outputResolution,
|
||||
outputResolutionAspectAgnostic: config.outputResolutionAspectAgnostic,
|
||||
// HDR is banned in distributed mode. force-sdr keeps the
|
||||
// extract / encoder paths off the HDR branches entirely.
|
||||
hdrMode: config.hdrMode ?? "force-sdr",
|
||||
|
||||
@@ -99,6 +99,7 @@ export interface SyntheticRenderJobInput {
|
||||
bitrate?: string;
|
||||
videoFrameFormat?: VideoFrameFormat;
|
||||
outputResolution?: RenderConfig["outputResolution"];
|
||||
outputResolutionAspectAgnostic?: RenderConfig["outputResolutionAspectAgnostic"];
|
||||
hdrMode: RenderConfig["hdrMode"];
|
||||
strictness?: RenderConfig["strictness"];
|
||||
entryFile: string;
|
||||
@@ -120,6 +121,7 @@ export function buildSyntheticRenderJob(input: SyntheticRenderJobInput): RenderJ
|
||||
videoBitrate: input.bitrate,
|
||||
videoFrameFormat: input.videoFrameFormat,
|
||||
outputResolution: input.outputResolution,
|
||||
outputResolutionAspectAgnostic: input.outputResolutionAspectAgnostic,
|
||||
// Distributed mode hard-pins to software GPU. The plan-time validator
|
||||
// refuses to fan out otherwise.
|
||||
useGpu: false,
|
||||
|
||||
@@ -22,7 +22,12 @@ import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { tmpdir } from "node:os";
|
||||
import type { EngineConfig } from "@hyperframes/engine";
|
||||
import { runCompileStage, type CompileStageInput } from "./compileStage.js";
|
||||
import type { CanvasResolution } from "@hyperframes/core";
|
||||
import {
|
||||
runCompileStage,
|
||||
type CompileStageInput,
|
||||
type CompileStageResult,
|
||||
} from "./compileStage.js";
|
||||
import type { RenderJob } from "../../renderOrchestrator.js";
|
||||
|
||||
const noopLog = {
|
||||
@@ -70,12 +75,13 @@ function createCfg(overrides: Partial<EngineConfig> = {}): EngineConfig {
|
||||
};
|
||||
}
|
||||
|
||||
function createJob(): RenderJob {
|
||||
function createJob(overrides: Partial<RenderJob["config"]> = {}): RenderJob {
|
||||
return {
|
||||
id: "test-job",
|
||||
config: {
|
||||
fps: { num: 30, den: 1 },
|
||||
quality: "standard",
|
||||
...overrides,
|
||||
},
|
||||
status: "queued",
|
||||
progress: 0,
|
||||
@@ -106,6 +112,20 @@ const IFRAME_HTML = `<!doctype html>
|
||||
</body>
|
||||
</html>`;
|
||||
|
||||
// Portrait/square/landscape fixture template — same shape as PLAIN_HTML but
|
||||
// with caller-supplied composition dimensions. Consumed by the aspect-agnostic
|
||||
// re-map tests below (and reachable from any sibling describe block, unlike a
|
||||
// helper scoped inside one describe).
|
||||
const orientedHtml = (w: number, h: number): string => `<!doctype html>
|
||||
<html>
|
||||
<head><meta charset="utf-8"></head>
|
||||
<body>
|
||||
<div data-composition-id="root" data-width="${w}" data-height="${h}" data-duration="1">
|
||||
<p>oriented composition</p>
|
||||
</div>
|
||||
</body>
|
||||
</html>`;
|
||||
|
||||
interface CompileFixture {
|
||||
workDir: string;
|
||||
htmlPath: string;
|
||||
@@ -211,3 +231,153 @@ describe("runCompileStage — forceScreenshot snapshot", () => {
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Tests for the aspect-agnostic --resolution re-map in `runCompileStage`.
|
||||
*
|
||||
* Field signal ts=1784176662 (darwin/arm64, CLI 0.7.59):
|
||||
* "--resolution 1080p rejects a 1080x1920 portrait comp — render at native."
|
||||
*
|
||||
* `--resolution 1080p` / `hd` / `4k` / `uhd` name a resolution *tier* without
|
||||
* pinning an orientation. `normalizeResolutionFlag` maps them all to a
|
||||
* landscape preset (backwards compat), and the CLI/server layers then flag
|
||||
* the raw input as aspect-agnostic on `RenderConfig`. The compile stage
|
||||
* consults that flag before `resolveDeviceScaleFactor` — if the composition's
|
||||
* orientation differs from the preset's, the preset is re-targeted to the
|
||||
* matching sibling in the same tier (HD ↔ HD, 4K ↔ 4K). Explicit
|
||||
* orientation-bearing presets stay strict.
|
||||
*/
|
||||
describe("runCompileStage — aspect-agnostic --resolution re-map", () => {
|
||||
let fixture: CompileFixture | null = null;
|
||||
|
||||
afterEach(() => {
|
||||
fixture?.cleanup();
|
||||
fixture = null;
|
||||
});
|
||||
|
||||
async function runResolutionCase(input: {
|
||||
compWidth: number;
|
||||
compHeight: number;
|
||||
outputResolution: CanvasResolution;
|
||||
aspectAgnostic: boolean;
|
||||
}): Promise<CompileStageResult> {
|
||||
fixture = setupFixture(orientedHtml(input.compWidth, input.compHeight));
|
||||
const projectDir = join(fixture.workDir, "project");
|
||||
const cfg = createCfg();
|
||||
const stageInput: CompileStageInput = {
|
||||
projectDir,
|
||||
workDir: fixture.workDir,
|
||||
htmlPath: fixture.htmlPath,
|
||||
entryFile: "index.html",
|
||||
job: createJob({
|
||||
outputResolution: input.outputResolution,
|
||||
outputResolutionAspectAgnostic: input.aspectAgnostic,
|
||||
}),
|
||||
cfg,
|
||||
needsAlpha: false,
|
||||
log: noopLog,
|
||||
assertNotAborted: () => {},
|
||||
};
|
||||
return runCompileStage(stageInput);
|
||||
}
|
||||
|
||||
// ─── Positive branches: aspect-agnostic auto-flip ──────────────────────
|
||||
|
||||
it("landscape composition + aspect-agnostic `landscape` preset → no re-map, DPR=1", async () => {
|
||||
// Sanity: the flag was ambiguous but the composition IS landscape, so
|
||||
// the preset already matches — nothing to flip.
|
||||
const result = await runResolutionCase({
|
||||
compWidth: 1920,
|
||||
compHeight: 1080,
|
||||
outputResolution: "landscape",
|
||||
aspectAgnostic: true,
|
||||
});
|
||||
expect(result.deviceScaleFactor).toBe(1);
|
||||
expect(result.outputWidth).toBe(1920);
|
||||
expect(result.outputHeight).toBe(1080);
|
||||
});
|
||||
|
||||
it("portrait composition + aspect-agnostic `landscape` preset → re-maps to portrait, DPR=1 (field signal scenario)", async () => {
|
||||
// The reporter's exact case: --resolution 1080p (normalized landscape)
|
||||
// on a 1080×1920 portrait comp. Previously threw "aspect ratio does not
|
||||
// match"; now re-maps to portrait (1080×1920) and DPR resolves to 1.
|
||||
const result = await runResolutionCase({
|
||||
compWidth: 1080,
|
||||
compHeight: 1920,
|
||||
outputResolution: "landscape",
|
||||
aspectAgnostic: true,
|
||||
});
|
||||
expect(result.deviceScaleFactor).toBe(1);
|
||||
expect(result.outputWidth).toBe(1080);
|
||||
expect(result.outputHeight).toBe(1920);
|
||||
});
|
||||
|
||||
it("square composition + aspect-agnostic `landscape` preset → re-maps to square, DPR=1", async () => {
|
||||
// --resolution 1080p on a 1080×1080 square comp → renders at 1080×1080.
|
||||
const result = await runResolutionCase({
|
||||
compWidth: 1080,
|
||||
compHeight: 1080,
|
||||
outputResolution: "landscape",
|
||||
aspectAgnostic: true,
|
||||
});
|
||||
expect(result.deviceScaleFactor).toBe(1);
|
||||
expect(result.outputWidth).toBe(1080);
|
||||
expect(result.outputHeight).toBe(1080);
|
||||
});
|
||||
|
||||
it("portrait composition + aspect-agnostic `landscape-4k` preset → re-maps to portrait-4k, preserves 4K tier", async () => {
|
||||
// `--resolution 4k` (normalized landscape-4k) on a portrait comp: the
|
||||
// re-map picks portrait-4k (same tier) rather than downgrading to
|
||||
// portrait (HD). 1080×1920 comp × 2 = 2160×3840 (portrait-4k).
|
||||
const result = await runResolutionCase({
|
||||
compWidth: 1080,
|
||||
compHeight: 1920,
|
||||
outputResolution: "landscape-4k",
|
||||
aspectAgnostic: true,
|
||||
});
|
||||
expect(result.deviceScaleFactor).toBe(2);
|
||||
expect(result.outputWidth).toBe(2160);
|
||||
expect(result.outputHeight).toBe(3840);
|
||||
});
|
||||
|
||||
it("square composition + aspect-agnostic `landscape-4k` preset → re-maps to square-4k, preserves 4K tier", async () => {
|
||||
const result = await runResolutionCase({
|
||||
compWidth: 1080,
|
||||
compHeight: 1080,
|
||||
outputResolution: "landscape-4k",
|
||||
aspectAgnostic: true,
|
||||
});
|
||||
expect(result.deviceScaleFactor).toBe(2);
|
||||
expect(result.outputWidth).toBe(2160);
|
||||
expect(result.outputHeight).toBe(2160);
|
||||
});
|
||||
|
||||
// ─── Negative branch: explicit preset stays strict ─────────────────────
|
||||
|
||||
it("portrait composition + explicit `landscape` preset (NOT aspect-agnostic) still throws aspect-mismatch", async () => {
|
||||
// The user typed `--resolution landscape` explicitly, not an alias.
|
||||
// Their stated intent is landscape orientation, which the renderer
|
||||
// cannot produce from a portrait composition (deviceScaleFactor can't
|
||||
// change aspect). Keep the actionable error rather than silently
|
||||
// swapping orientation.
|
||||
await expect(
|
||||
runResolutionCase({
|
||||
compWidth: 1080,
|
||||
compHeight: 1920,
|
||||
outputResolution: "landscape",
|
||||
aspectAgnostic: false,
|
||||
}),
|
||||
).rejects.toThrow(/aspect ratio|--resolution portrait/i);
|
||||
});
|
||||
|
||||
it("portrait composition + explicit `landscape-4k` preset (NOT aspect-agnostic) still throws", async () => {
|
||||
await expect(
|
||||
runResolutionCase({
|
||||
compWidth: 1080,
|
||||
compHeight: 1920,
|
||||
outputResolution: "landscape-4k",
|
||||
aspectAgnostic: false,
|
||||
}),
|
||||
).rejects.toThrow(/aspect ratio|--resolution portrait/i);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -37,6 +37,7 @@
|
||||
|
||||
import { join } from "node:path";
|
||||
import type { EngineConfig } from "@hyperframes/engine";
|
||||
import { suggestMatchingPreset } from "@hyperframes/core";
|
||||
import type { CompiledComposition } from "../../htmlCompiler.js";
|
||||
import { compileForRender } from "../../htmlCompiler.js";
|
||||
import type { ProducerLogger } from "../../../logger.js";
|
||||
@@ -273,10 +274,35 @@ export async function runCompileStage(input: CompileStageInput): Promise<Compile
|
||||
height: compiled.height,
|
||||
};
|
||||
const { width, height } = composition;
|
||||
// Aspect-agnostic aliases (`--resolution 1080p` / `hd` / `4k` / `uhd`) all
|
||||
// normalize to a landscape preset up-front (see `normalizeResolutionFlag`),
|
||||
// which was historically fine because 16:9 was the only shipped orientation.
|
||||
// Once portrait + square presets landed, that early normalization started
|
||||
// rejecting portrait/square compositions with a cryptic "aspect ratio does
|
||||
// not match" from `resolveDeviceScaleFactor` — a common enough hit that a
|
||||
// field report (CLI 0.7.59) surfaced it. When the flag was aspect-agnostic,
|
||||
// re-target the preset to the sibling that matches the composition's
|
||||
// orientation while preserving the tier (HD vs 4K). Explicit
|
||||
// orientation-bearing presets stay strict — a `--resolution portrait` on a
|
||||
// landscape composition still errors, honoring the user's stated intent.
|
||||
const requestedResolution = job.config.outputResolution;
|
||||
let effectiveResolution = requestedResolution;
|
||||
if (requestedResolution && job.config.outputResolutionAspectAgnostic) {
|
||||
const flipped = suggestMatchingPreset(width, height, requestedResolution);
|
||||
if (flipped && flipped !== requestedResolution) {
|
||||
log.info("Adapted aspect-agnostic --resolution to composition orientation", {
|
||||
compositionWidth: width,
|
||||
compositionHeight: height,
|
||||
requestedResolution,
|
||||
effectiveResolution: flipped,
|
||||
});
|
||||
effectiveResolution = flipped;
|
||||
}
|
||||
}
|
||||
const deviceScaleFactor = resolveDeviceScaleFactor({
|
||||
compositionWidth: width,
|
||||
compositionHeight: height,
|
||||
outputResolution: job.config.outputResolution,
|
||||
outputResolution: effectiveResolution,
|
||||
hdrRequested: job.config.hdrMode === "force-hdr",
|
||||
alphaRequested: needsAlpha,
|
||||
});
|
||||
@@ -286,7 +312,7 @@ export async function runCompileStage(input: CompileStageInput): Promise<Compile
|
||||
log.info("Supersampling composition via deviceScaleFactor", {
|
||||
compositionWidth: width,
|
||||
compositionHeight: height,
|
||||
outputResolution: job.config.outputResolution,
|
||||
outputResolution: effectiveResolution,
|
||||
outputWidth,
|
||||
outputHeight,
|
||||
deviceScaleFactor,
|
||||
|
||||
@@ -344,6 +344,21 @@ export interface RenderConfig {
|
||||
* HDR constraints.
|
||||
*/
|
||||
outputResolution?: CanvasResolution;
|
||||
/**
|
||||
* True when `outputResolution` was normalized from an aspect-agnostic alias
|
||||
* (`1080p`, `hd`, `4k`, `uhd`) rather than a preset that names its own
|
||||
* orientation (`landscape`, `portrait`, `1080p-portrait`, …). Set by the
|
||||
* CLI + server layers via `isAspectAgnosticResolutionAlias(rawInput)` at
|
||||
* flag/body parse time.
|
||||
*
|
||||
* When true, the compile stage adapts the preset to the composition's
|
||||
* orientation before calling `resolveDeviceScaleFactor` — a portrait
|
||||
* 1080×1920 composition with `--resolution 1080p` (normalized to
|
||||
* `landscape`) is re-mapped to `portrait`, honoring the user's intent
|
||||
* ("render at 1080p") without forcing them to know the aspect-suffixed
|
||||
* alias (`1080p-portrait`). Explicit orientation presets stay strict.
|
||||
*/
|
||||
outputResolutionAspectAgnostic?: boolean;
|
||||
}
|
||||
|
||||
export interface RenderPerfSummary {
|
||||
|
||||
Reference in New Issue
Block a user