fix(renderer): prevent stale SwiftShader layers

[miguel-heygen]

What

Prevents a transformed element from flashing twice for a few frames when a GSAP yoyo animation reverses during CPU/SwiftShader rendering.

The browser state was correct, but Chromium's GPU compositor could briefly retain the element's previous transformed layer. The stale layer was already present in the raw captured JPEG, before FFmpeg encoded the video.

How

• Add --disable-gpu-compositing only when using SwiftShader with BeginFrame capture.
• Keep BeginFrame itself, WebGL, hardware-GPU rendering, and screenshot capture unchanged.
• Add a 25 fps visual regression that reproduces the reversal at the affected frame boundary.
• Let regression fixtures explicitly select BeginFrame so this path stays covered.

Why

Switching all CPU renders to screenshots hid the problem but made rendering slower. Routing only compositing through Chrome's software path removes the stale layer while preserving deterministic BeginFrame capture.

On the reduced reproduction:

• screenshot capture: ~41s
• previous BeginFrame path: ~28s, visible duplicate
• fixed BeginFrame path: ~21–29s, no duplicate

Upstream Chromium tracker and removal condition: issue 535256667.

Verification

• Original project ZIP: clean at the previously failing 10.12–10.20s window with 1 and 6 workers
• Standalone sanitized Chromium repro: control duplicates frames 253–255; adding only --disable-gpu-compositing is clean
• Regression proof: fails without the fix at 10.1376s (PSNR 29.47), passes 100/100 checkpoints with the fix
• WebGL BeginFrame smoke: rendered the expected non-blank canvas color
• bun test packages/engine/src/services/browserManager.test.ts — 28 passed
• Engine + Producer typecheck — passed
• Full workspace build — passed
• Full Engine suite — 1079 passed; 2 unrelated VFR tests require a newer FFmpeg than the host's 4.2 (-fps_mode unsupported)
• Fallow --gate all — passed (default new-only attribution could not create its temporary worktree on this busy checkout)

[james-russo-rames-d-jusso]

Reviewed at 54a3ef20. Traced the workaround gate + envelope carriers (browserGpuMode, PRODUCER_FORCE_SCREENSHOT, captureMode), the concrete-resolved-GPU clamp interaction, and the fixture's opt-out path. Also confirmed the unit-test 3-way matrix and the fixture PSNR threshold vs the reported failure.

Blockers

(none)

Concerns

🟠 Workaround has no documented removal path in code. The PR body notes "I'll be filing the [upstream Chromium] issue separately" — the workaround is inherently temporary (goes away when the SwiftShader compositor bug is fixed upstream), but neither the code comment at browserManager.ts:686-696 nor the PR description cites a tracker URL. Recommend either dropping a TODO(crbug/PENDING) inside the comment and updating it in a follow-up once the issue is filed, OR filing the upstream now and landing the PR with the link inline. The workaround will otherwise silently outlive its need — nobody knows they can delete these lines. Same treatment for the regression fixture's meta.json description ideally (so the removal path also unblocks reverting the extra fixture). Non-blocking, but worth surfacing before merge.

Nits

(none)

Questions

(none)

Green notes

🟢 Workaround scope is minimal and correctly gated. browserManager.ts:686-696 adds --disable-gpu-compositing only when BOTH options.captureMode !== "screenshot" (BeginFrame path) AND browserGpuMode === "software" (SwiftShader). Hardware-GPU renders and screenshot capture stay byte-identical to pre-PR. The +484 line count is dominated by the 410-line self-contained regression fixture — the actual production code delta is +11 lines in browserManager.ts + 24 lines of harness plumbing in regression-harness.ts.

🟢 Unit test pins the 3-way orthogonal matrix. browserManager.test.ts:31-46 covers all three cells that matter:

• software + beginframe → flag present ✓
• software + screenshot → flag absent ✓ (screenshot path unchanged)
• hardware + beginframe → flag absent ✓ (hardware GPU unchanged)

This directly refutes a scope-creep regression — if a future edit accidentally lifted the browserGpuMode === "software" gate, the hardwareBeginFrame assertion catches it.

🟢 Envelope-carrier trace is clean for the "auto" GPU-mode case. The gate keys on the concrete resolved GPU mode, not the pre-resolve string: frameCapture.ts:985 sets requestedGpuMode, :986 calls resolveBrowserGpuMode(...) to concretize, and :1014 passes browserGpuMode: resolvedGpuMode into buildChromeArgs. So browserGpuMode: "auto" that probes to software at runtime correctly hits the --disable-gpu-compositing branch — no gap for the same combination the invariant is meant to prevent.

🟢 Regression fixture SHAPE is correct — fails without the fix, passes with. PR body reports PSNR 29.47 at 10.1376s without the fix vs 100/100 checkpoints with it, and meta.json pins minPsnr: 30, maxFrameFailures: 0. PSNR is a broad proxy, but for this defect (a duplicated transformed layer painting a ghost badge in ~4 sequential frames) the pixel-diff spike at the reversal boundary is exactly what PSNR catches. The 25 fps + 10.6s duration + captureMode: "beginframe" pinning reproduces the exact frame boundary Miguel debugged.

🟢 Fixture correctly opts out of the concrete-GPU-clamp so it can exercise BeginFrame+software. regression-harness.ts:1057-1082 sets PRODUCER_FORCE_SCREENSHOT="false" when captureMode === "beginframe" — that value is the literal opt-out the config layer honors at config.ts:661 (explicitForceScreenshotOptOut), which also flows through to applyConcreteGpuScreenshotClamp via forceScreenshotExplicitlyOptedOut. Without this opt-out, the software → screenshot clamp in resolveConfig would silently reroute the fixture to the screenshot path and the BeginFrame-only regression would never fire. The env save/restore around createRenderJob matches the pre-existing useFast pattern and is safe under serial suite execution.

🟢 meta.json validation is symmetric with existing fields. The new captureMode?: "screenshot" | "beginframe" field in TestMetadata has a matching validator branch at regression-harness.ts:399-407 — same shape as experimentalFastCapture above it. Consistent with the surrounding field conventions.

🟢 A/B perf claim is directional and preserves BeginFrame's win. PR body: screenshot ~41s / previous BeginFrame ~28s w/ duplicate / fixed BeginFrame ~21–29s no duplicate. So the workaround retains ~30% of the BeginFrame perf win over screenshot while eliminating the artifact — better than the alternative of "switching all CPU renders to screenshots" which was the prior workaround-consideration.

🟢 CDN dependency in the fixture (cdn.jsdelivr.net for GSAP, Google Fonts) is precedented. ≥10 sibling fixtures under packages/producer/tests/*/src/index.html use the same CDN pattern (animejs-adapter, d3-adapter, gsap-letters-render-compat, maplibre-adapter, leaflet-adapter, sub-comp-t0, etc.) — the regression harness's network posture already permits this.

What I didn't verify

• Didn't run the fixture end-to-end locally. Structural trace + Miguel's reported 100/100 checkpoint pass is my ground for the "test-passes-with-fix" side; the "test-fails-without-fix" side rests on the PR body's cited PSNR 29.47 datapoint (which I didn't rerun).
• Didn't measure the --disable-gpu-compositing cost on a large real-world composition vs a synthetic one. Miguel's ~21-29s post-fix range vs ~28s pre-fix suggests it's within noise for this fixture, but heavy compositions may pay more.
• Didn't check whether --disable-gpu-compositing interacts with any other Chrome flag the engine already passes on the same path (e.g., --enable-gpu-rasterization — the pre-existing tests at browserManager.test.ts:26 assert it's absent on software, so no conflict there).

Verdict framing

Clean, well-scoped workaround with a targeted regression that provably fires on the exact defect. One concern on the missing upstream-tracker link in-code — the workaround should carry its removal path so it doesn't calcify. LGTM from my side once that's addressed (or explicitly deferred to a follow-up PR that lands the crbug link + comment update once Miguel files upstream).

Note: several required CI checks are still IN_PROGRESS at this SHA (regression shards, Windows render, CodeQL javascript-typescript). Code verdict above is independent of the merge gate — merge only once required checks are green.

— Review by Rames D Jusso

[vanceingalls]

Reviewed at head 54a3ef2000da635b93c03a41c129a78f0276bf38 (+484/-0, 7 files). No prior reviews on the PR at post time. CI: all completed lanes green (Format, Lint, Typecheck, Test, Producer: unit/integration, Fallow audit, SDK: unit+contract+smoke, Studio: load smoke, CLI smoke (required), Smoke: global install, CodeQL x3, preview-regression, player-perf, File size check, Semantic PR title, Build); regression-shards 1-8, Windows render, Tests on windows-latest still IN_PROGRESS — normal HF cadence, not gating. mergeStateStatus=BLOCKED reads as REVIEW_REQUIRED, not a failing required check.

Strengths

• packages/engine/src/services/browserManager.ts:689-711 — the workaround lives in exactly one place: the args builder, gated by the conjunction of captureMode !== "screenshot" and browserGpuMode === "software". Hardware BeginFrame keeps its compositor, screenshot mode keeps its compositor. The band-aid does not tax the healthy paths.
• packages/engine/src/services/browserManager.test.ts:794-811 — affirmative unit-level pin: --disable-gpu-compositing present for { beginframe, software }, absent for { screenshot, software } and { beginframe, hardware }. Any future edit that broadens or narrows the gate breaks the test on the specific offending combination.
• packages/producer/tests/software-beginframe-yoyo-compositor/meta.json + regression-harness.ts:1322,2249-2277 — the fixture pins the affected path explicitly via renderConfig.captureMode: "beginframe", and the harness plumbs that into PRODUCER_FORCE_SCREENSHOT=false for the duration of just that suite (restored in finally). This is the right shape: the fixture opts into the software-BeginFrame lane so a producer-default that ever flipped to screenshots wouldn't silently make the regression stop firing.
• minPsnr: 30 + maxFrameFailures: 0 in the fixture, combined with the PR body's PSNR 29.47 unfixed reading at 10.1376s, means the failure is real (delta 0.53 dB is well outside noise) and zero-tolerance — a single stale-layer frame trips CI. Not aspirational.

Findings

nit — no upstream-issue anchor for the rip-out hook. browserManager.ts:690-697 explains why the flag is added but there's no TODO(crbug/…), no target chromium version, and no HF_DISABLE_SWIFTSHADER_COMPOSITOR_WORKAROUND env escape hatch. You've said you'll file the Chromium issue — once it exists, dropping the issue URL into that comment (or above the push("--disable-gpu-compositing") line) turns the workaround from "band-aid indefinitely" into "band-aid tracked to a fix." Purely a hygiene ask; not gating.

nit — regression fixture is single-worker. meta.json runs workers: 1. The bug you're fixing is compositor-layer, not scheduler-layer, so single-worker is the right minimum repro; but the parallel-worker path (HF_DE_PARALLEL_ROUTER, etc.) has been the source of multiple recent check-vs-render parity gaps in the field-signal channel. A second variant of this fixture with workers: 4 would be cheap insurance that the compositor fix survives the parallel dispatch path. Follow-up worthy, not blocking.

nit — captureMode presence-vs-absence branch stays pre-existing. browserManager.ts:689 treats undefined captureMode as non-screenshot (falls through to BeginFrame flags). That's the pre-existing behavior; this PR doesn't change it. Callers under my read either set captureMode explicitly or expect BeginFrame-only flags. Flagging so it's on the radar next time this branch is touched — not a regression from this PR.

Field-signal cross-reference

None of the 7 documented check-vs-render parity variants in my hyperframes-fastpath memory match this ghost-layer signature. The closest thematic neighbor is ts=1784041295 (left:50% + translateX(-50%) + GSAP scale renders shifted in the MP4 while snapshot is correct), but that's a shift not a duplicate ghost frame, and it isn't yoyo-specific. This PR resolves a new parity class — transform-persistence-on-GSAP-reversal at the SwiftShader compositor layer — not any of the seven previously catalogued.

If ts=1784041295's transform-shift is also rooted in stale compositor layers under SwiftShader (as opposed to a GSAP-tween-numeric issue), this workaround may incidentally help it. Worth revisiting the ts=1784041295 fixture against a build with --disable-gpu-compositing on to confirm/rule-out.

Band-aid bar

Under your own long-standing rule, this PR is on the acceptable side of the bar. Framing as workaround + regression test locking failure at PSNR 29.47 + gate scoped to the single vulnerable combination + upstream Chromium issue explicitly forthcoming + no cross-scaffold duplication. The one weak point (no TODO(crbug/...) anchor + no runtime escape hatch) is a hygiene nit, not a violation of the pattern that turns a workaround into a permanent wedge.

Audited: browserManager.ts (end-to-end), browserManager.test.ts (end-to-end), regression-harness.ts (captureMode plumbing at :1319-1324, :2247-2277; validator at :1593-1601), meta.json (end-to-end).
Trusting: the fixture's src/index.html (410-line GSAP timeline — read summary + your PR-body claim that the reversal reproduces at 10.12s; not stepped through each keyframe), output/output.mp4 + output/compiled.html (git-LFS pointer files — cannot validate binary artifact locally).

Verdict: APPROVE
Reasoning: Workaround at the correct layer, tightly gated to the single vulnerable combination, regression-locked with a zero-tolerance PSNR pin that empirically fails without the fix. Escape-hatch and upstream-anchor asks are follow-up hygiene, not blockers.

— Via

[miguel-heygen]

Upstream-tracker follow-up for the reviewer hygiene note: the sanitized standalone Chromium repro is now verified on chrome-headless-shell 150.0.7871.24 (control reproduces frames 253–255; adding only --disable-gpu-compositing fixes them), and Miguel is filing the Chromium issue with that bundle. Once the crbug URL exists, I’ll add it to the browserManager.ts workaround comment as the explicit removal hook. I’m keeping that URL-only edit out of the current head until there is a real tracker to cite; no placeholder/TODO that can calcify.

[miguel-heygen]

@vanceingalls @james-russo-rames-d-jusso exact-head re-review requested at 9c25e27da.

Delta from your reviewed 54a3ef20 is tracker-only: the browser workaround comment now links Chromium issue https://issues.chromium.org/issues/535256667 as its explicit removal condition, and the regression fixture description names the same issue. No behavior or baseline changed.

Focused verification: browserManager.test.ts 28/28, regression-harness parse 5/5, git diff --check clean; commit hooks passed lint, format, typecheck, and tracked-artifact checks.