Skip to content

Commit c12b262

Browse files
committed
fix(cli): keep the color grading GPU warning out of --strict results
Whether a host has a GPU says nothing about the composition, so the warning stays on stderr and no longer fails check --strict on GPU-less CI. Also trims the warning text and doc comment.
1 parent b203d61 commit c12b262

4 files changed

Lines changed: 22 additions & 56 deletions

File tree

‎packages/cli/src/browser/gpuPolicy.colorGradingStall.test.ts‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
1+
import { beforeEach, describe, expect, it, vi } from "vitest";
22

33
const mocks = vi.hoisted(() => ({
44
ensureBrowser: vi.fn(),
@@ -13,22 +13,25 @@ vi.mock("@hyperframes/engine", () => ({
1313
resolveBrowserGpuMode: mocks.resolveBrowserGpuMode,
1414
}));
1515

16-
import { detectColorGradingGpuStallRisk } from "./gpuPolicy.js";
16+
import { compositionUsesColorGrading, detectColorGradingGpuStallRisk } from "./gpuPolicy.js";
1717

1818
const GRADED_HTML =
1919
'<div data-composition-id="main"><img data-color-grading=\'{"adjust":{"saturation":-1}}\' src="a.jpg" /></div>';
2020
const UNGRADED_HTML = '<div data-composition-id="main"><img src="a.jpg" /></div>';
2121

22+
describe("compositionUsesColorGrading", () => {
23+
it("detects data-color-grading on any element, not just the composition root", () => {
24+
expect(compositionUsesColorGrading(GRADED_HTML)).toBe(true);
25+
expect(compositionUsesColorGrading(UNGRADED_HTML)).toBe(false);
26+
});
27+
});
28+
2229
describe("detectColorGradingGpuStallRisk", () => {
2330
beforeEach(() => {
2431
vi.resetAllMocks();
2532
mocks.ensureBrowser.mockResolvedValue({ executablePath: "/chrome", source: "cache" });
2633
});
2734

28-
afterEach(() => {
29-
vi.resetAllMocks();
30-
});
31-
3235
it("warns when the composition uses color grading and no hardware GPU is found", async () => {
3336
mocks.resolveBrowserGpuMode.mockResolvedValue("software");
3437
const warning = await detectColorGradingGpuStallRisk(GRADED_HTML, "auto");
Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, expect, it } from "vitest";
2-
import { compositionUsesColorGrading, resolveLocalBrowserGpuMode } from "./gpuPolicy.js";
2+
import { resolveLocalBrowserGpuMode } from "./gpuPolicy.js";
33

44
// compositionRequiresWebGpu and assertWebGpuAdapterAvailable are implemented
55
// in @hyperframes/engine (browserManager.ts) and only re-exported here — see
@@ -12,15 +12,4 @@ describe("local browser GPU policy", () => {
1212
expect(resolveLocalBrowserGpuMode(true, "software")).toBe("hardware");
1313
expect(resolveLocalBrowserGpuMode(false, "hardware")).toBe("software");
1414
});
15-
16-
it("detects data-color-grading on any element, not just the composition root", () => {
17-
expect(
18-
compositionUsesColorGrading(
19-
'<div data-composition-id="main"><img data-color-grading=\'{"adjust":{"saturation":-1}}\' src="a.jpg" /></div>',
20-
),
21-
).toBe(true);
22-
expect(
23-
compositionUsesColorGrading('<div data-composition-id="main"><img src="a.jpg" /></div>'),
24-
).toBe(false);
25-
});
2615
});

‎packages/cli/src/browser/gpuPolicy.ts‎

Lines changed: 11 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -27,37 +27,22 @@ export async function resolveCaptureBrowserGpuMode(
2727
// existing CLI imports don't need to change their module path.
2828
export { compositionRequiresWebGpu, assertWebGpuAdapterAvailable } from "@hyperframes/engine";
2929

30+
const COLOR_GRADING_ATTR_RE = new RegExp(`\\s${HF_COLOR_GRADING_ATTR}[\\s=>]`, "i");
31+
3032
export function compositionUsesColorGrading(html: string): boolean {
31-
const escapedAttr = HF_COLOR_GRADING_ATTR.replace(/[-/\\^$*+?.()|[\]{}]/g, "\\$&");
32-
return new RegExp(`\\s${escapedAttr}(?:\\s|=|>)`, "i").test(html);
33+
return COLOR_GRADING_ATTR_RE.test(html);
3334
}
3435

3536
const COLOR_GRADING_GPU_STALL_WARNING =
36-
`This composition uses ${HF_COLOR_GRADING_ATTR}, but no hardware GPU was detected — ` +
37-
"the browser will render on the SwiftShader/software WebGL fallback. Color grading's " +
38-
"per-element canvas readback has no fast path under software WebGL: it has been measured " +
39-
"at roughly 40x slower than an ungraded composition, which is easily enough to exceed the " +
40-
"navigation/render-ready timeout, or to make check/render painfully slow even once past it. " +
41-
"If this run is unexpectedly slow or times out, try a much larger --timeout, run on a host " +
42-
"with a real GPU, or preprocess to monochrome derivatives with grading intensity 0 before " +
43-
"capturing with --browser-gpu to skip the expensive per-frame grading pass entirely.";
37+
`This composition uses ${HF_COLOR_GRADING_ATTR}, but no hardware GPU was detected, so the ` +
38+
"browser renders with software WebGL (SwiftShader). Color grading's per-frame canvas readback " +
39+
"is far slower there and can exceed the navigation timeout. If this run is slow or " +
40+
"times out, raise --timeout or run on a host with a real GPU.";
4441

4542
/**
46-
* Preflight for a known SwiftShader limitation (not a hyperframes bug): a
47-
* per-element color-grading canvas pays a synchronous GPU-stall readback cost
48-
* that software WebGL has no fast path for, ~40x slower than an ungraded
49-
* composition in measured practice. `requestedMode: "software"` is a
50-
* deliberate, already-informed choice and is not warned about; `"auto"` /
51-
* `"hardware"` both expect speed, so a silent fallback to software there is
52-
* exactly the surprise this call is meant to catch before capture starts.
53-
* Reuses `resolveCaptureBrowserGpuMode`'s cached probe (forcing `"auto"` to
54-
* get the ground-truth answer even when the caller requested `"hardware"`,
55-
* which always reports back `"hardware"` verbatim) — resolved against the
56-
* same `ensureBrowser()` executable path a subsequent real launch will use,
57-
* so this doesn't seed the shared cache with a different browser's probe.
58-
* Best-effort: any probe failure here is treated as "nothing to warn about"
59-
* rather than failing the caller — the real launch will surface a genuine
60-
* browser problem on its own.
43+
* Warns before capture when a graded composition will run on software WebGL. An explicit
44+
* "software" request is a deliberate choice and stays silent; "hardware" is reported verbatim
45+
* by the engine, so probe "auto" for the real answer. Shares the engine's cached probe.
6146
*/
6247
export async function detectColorGradingGpuStallRisk(
6348
html: string,
@@ -70,6 +55,7 @@ export async function detectColorGradingGpuStallRisk(
7055
const actualMode = await resolveCaptureBrowserGpuMode("auto", browser.executablePath);
7156
return actualMode === "software" ? COLOR_GRADING_GPU_STALL_WARNING : null;
7257
} catch {
58+
// Best-effort: the real launch surfaces a genuine browser failure.
7359
return null;
7460
}
7561
}

‎packages/cli/src/utils/checkBrowser.ts‎

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -158,11 +158,7 @@ export async function runBrowserCheck(
158158
const html = await bundleWithLocalizedFonts(project.dir);
159159
await preResolveHostileMediaProxies(project.dir, html, options.autoProxy);
160160
const requestedGpuMode = options.browserGpuMode ?? resolveCliChromeGpuMode();
161-
// Printed eagerly (not just recorded as a finding) because the risk this
162-
// flags is a navigation timeout — if it fires, `runBrowserCheck` throws
163-
// before ever returning a report, so a finding pushed to `drafts` would be
164-
// discarded along with the whole in-flight result (see runCheckPipeline's
165-
// catch, which replaces browser with emptyBrowserResult() on that path).
161+
// stderr, not a report finding: a timeout discards the report, and a host fact must not fail --strict.
166162
const colorGradingGpuWarning = await detectColorGradingGpuStallRisk(html, requestedGpuMode);
167163
if (colorGradingGpuWarning) console.warn(`\n[hyperframes] ${colorGradingGpuWarning}`);
168164
const server = await serveStaticProjectHtml(
@@ -173,14 +169,6 @@ export async function runBrowserCheck(
173169
options.autoProxy,
174170
);
175171
const drafts: RuntimeDraft[] = [];
176-
if (colorGradingGpuWarning) {
177-
drafts.push({
178-
code: "color_grading_gpu_stall_risk",
179-
severity: "warning",
180-
message: colorGradingGpuWarning,
181-
time: 0,
182-
});
183-
}
184172
let currentTime = 0;
185173
let chromeBrowser: import("puppeteer-core").Browser | undefined;
186174

0 commit comments

Comments
 (0)