From ba5168293f8d0db11ff2801814aacd7b8497df94 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Mon, 13 Jul 2026 21:06:07 +0000 Subject: [PATCH] fix(engine): software-GPU browsers imply screenshot capture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When `browserGpuMode === "software"`, set `forceScreenshot = true` in `resolveConfig`. Explicit opt-outs (`PRODUCER_FORCE_SCREENSHOT=false` or `overrides.forceScreenshot === false`) are honored. This is defense-in-depth on top of the existing platform gates: 1. Linux + software (SwiftShader host) skips BeginFrame, avoiding the compositor stall on shader-heavy frames under CPU raster (same motivation as the closed PR #822). 2. `renderOrchestrator`'s reported `captureMode` field is derived from `cfg.forceScreenshot ? "screenshot" : "beginframe"` — without this clamp it misreports `"beginframe"` for the actual screenshot capture on darwin + software. 3. Any new BeginFrame or drawElement entry point that forgets to gate on GPU mode still routes to screenshot here. Does NOT fix SwiftShader-on-darwin text-rasterization artifacts (an ANGLE-SwiftShader issue on macOS text — the fix there is to use `--browser-gpu`, which routes to `--use-angle=metal`). --- packages/engine/src/config.test.ts | 43 ++++++++++++++++++++++++++++++ packages/engine/src/config.ts | 35 ++++++++++++++++++++++++ 2 files changed, 78 insertions(+) diff --git a/packages/engine/src/config.test.ts b/packages/engine/src/config.test.ts index e61504ffc..93088fca7 100644 --- a/packages/engine/src/config.test.ts +++ b/packages/engine/src/config.test.ts @@ -296,6 +296,49 @@ describe("resolveConfig", () => { }); }); + describe("forceScreenshot (software-GPU clamp)", () => { + it("forces screenshot capture when browserGpuMode resolves to software", () => { + setEnv("PRODUCER_BROWSER_GPU_MODE", "software"); + unsetEnv("PRODUCER_FORCE_SCREENSHOT"); + const config = resolveConfig(); + expect(config.forceScreenshot).toBe(true); + }); + + it("leaves forceScreenshot alone on hardware GPU (default off)", () => { + setEnv("PRODUCER_BROWSER_GPU_MODE", "hardware"); + unsetEnv("PRODUCER_FORCE_SCREENSHOT"); + const config = resolveConfig(); + expect(config.forceScreenshot).toBe(false); + }); + + it("does not force screenshot on auto (auto probes to hardware on real GPUs)", () => { + setEnv("PRODUCER_BROWSER_GPU_MODE", "auto"); + unsetEnv("PRODUCER_FORCE_SCREENSHOT"); + const config = resolveConfig(); + expect(config.forceScreenshot).toBe(false); + }); + + it("explicit env opt-out (PRODUCER_FORCE_SCREENSHOT=false) is honored on software", () => { + setEnv("PRODUCER_BROWSER_GPU_MODE", "software"); + setEnv("PRODUCER_FORCE_SCREENSHOT", "false"); + const config = resolveConfig(); + expect(config.forceScreenshot).toBe(false); + }); + + it("explicit programmatic opt-out is honored on software", () => { + setEnv("PRODUCER_BROWSER_GPU_MODE", "software"); + unsetEnv("PRODUCER_FORCE_SCREENSHOT"); + const config = resolveConfig({ forceScreenshot: false }); + expect(config.forceScreenshot).toBe(false); + }); + + it("caller override forceScreenshot=true stays true regardless of GPU mode", () => { + setEnv("PRODUCER_BROWSER_GPU_MODE", "hardware"); + const config = resolveConfig({ forceScreenshot: true }); + expect(config.forceScreenshot).toBe(true); + }); + }); + describe("lowMemoryMode", () => { it("forces on for truthy PRODUCER_LOW_MEMORY_MODE values", () => { setEnv("PRODUCER_LOW_MEMORY_MODE", "true"); diff --git a/packages/engine/src/config.ts b/packages/engine/src/config.ts index c675818f5..1a974f4ac 100644 --- a/packages/engine/src/config.ts +++ b/packages/engine/src/config.ts @@ -554,6 +554,41 @@ export function resolveConfig(overrides?: Partial): EngineConfig { merged.useDrawElement = false; } + // Software GPU implies screenshot capture. + // + // Two existing platform gates already do most of the work: `browserManager` + // only launches BeginFrame on Linux + chrome-headless-shell + !forceScreenshot, + // and the DE clamp above turns off `useDrawElement` on non-(darwin + + // non-software) hosts. Setting `forceScreenshot` here layers defense-in-depth + // on top: + // + // 1. Linux + software (SwiftShader host): kicks the browser off BeginFrame, + // which stalls the compositor on shader-heavy frames under CPU raster + // (same motivation as the closed PR #822). + // 2. Observability truth: `renderOrchestrator`'s reported `captureMode` + // field is derived from `cfg.forceScreenshot ? "screenshot" : "beginframe"` + // — without this clamp it misreports `"beginframe"` for the actual + // screenshot capture on darwin + software. + // 3. Future-proofing: any new BeginFrame or drawElement entry point that + // forgets to gate on GPU mode still routes to screenshot here. + // + // Note this does NOT eliminate SwiftShader-on-darwin text-rasterization + // artifacts (an ANGLE-SwiftShader issue on macOS text — the fix there is to + // use `--browser-gpu`, which routes to `--use-angle=metal`). It only makes + // routing consistent + observability accurate. + // + // Explicit opt-out (env or programmatic override) is honored so BeginFrame- + // on-software debugging remains possible. + const explicitForceScreenshotOptOut = + env("PRODUCER_FORCE_SCREENSHOT") === "false" || overrides?.forceScreenshot === false; + if ( + merged.browserGpuMode === "software" && + !merged.forceScreenshot && + !explicitForceScreenshotOptOut + ) { + merged.forceScreenshot = true; + } + // drawElement capture and page-side shader compositing are mutually // incompatible capture strategies (drawElement reads paint records directly // and bypasses the page-side prepare→composite→resolve protocol). When