perf(engine): faster shader transitions via page-side WebGL compositing

[vanceingalls]

Summary

Moves shader transition blending from the Node-side layered pipeline into Chrome's WebGL, using a two-phase drawElementImage capture protocol. Default-on for SDR shader-transition renders — no flag needed.

Performance (15 scenes, 14 shader transitions, 28s @ 30fps, Mac)

Workers	Published CLI (v0.6.6)	This branch	Speedup
1	135s	20.9s	6.5×
6	139s	6.9s	20.1×

Published CLI gets slower with more workers on shader transitions (coordination overhead dominates). Page-side compositing scales linearly with workers.

• Native fidelity: uses Chromium's drawElementImage (same capture path as preview mode)
• Auto-disables when unsafe: HDR, alpha output, video content, beginFrame mode

How it works

1. Seek phase (page-side): GSAP seek + clone scenes into visible layoutsubtree staging canvases + set pending flag
2. Paint force (engine-side): micro Page.captureScreenshot (1×1 clip) forces browser compositor to paint staging canvas children
3. Composite phase (page-side): drawElementImage reads paint records → WebGL shader blend → GL overlay displayed
4. Screenshot: engine captures the composited result in one opaque RGB frame

Why two phases?

Under virtual time, requestAnimationFrame is shimmed and the browser compositor doesn't paint new elements. drawElementImage requires a cached paint record, which only exists after a compositor pass. The micro-screenshot forces that pass without needing rAF.

Gating

Auto-enabled when all conditions met:

• compiled.hasShaderTransitions
• !hasHdrContent (HDR needs per-layer alpha compositing in Node)
• !needsAlpha (WebM/MOV/PNG-sequence need transparent captures)
• composition.videos.length === 0 (cloneNode loses video playback state)
• captureMode !== "beginframe" (CDP micro-screenshot conflicts with beginFrame compositor)

Use --no-page-side-compositing to force the Node-side layered path.

Changes by package

engine

• frameCapture.ts: two-phase protocol — detect pending flag after seek, micro-screenshot paint force, call resolve
• config.ts: enablePageSideCompositing defaults to true

shader-transitions

• engineModePageComposite.ts: full rewrite — two-phase drawElementImage compositor replacing the original broken drawElementImage attempt and the intermediate html2canvas approach
• capture.ts: export stabilizeTransformedBoxShadows

producer

• renderOrchestrator.ts: unified gating predicate (usePageSideCompositingForTransitions) gates both stub injection and layered-path bypass; addPreHeadScript for probe-created fileServer; !needsAlpha and videos.length === 0 exclusions
• fileServer.ts: addPreHeadScript method on FileServerHandle for post-creation stub injection

cli

• render.ts: flag default flipped to true; --no-page-side-compositing to disable
• dockerRunArgs.ts: forwards --no-page-side-compositing when disabled

Test plan

• Unit tests pass (shader-transitions: 10, engine: 594, cli: 309)
• Render shader-perf fixture (15 scenes, 14 transitions) — correct output at 1 and 6 workers
• Benchmarked against published v0.6.6 via npx hyperframes (both 1 and 6 workers)
• Static frames identical to baseline
• Transition frames show correct shader blending (domain-warp, glitch, etc.)
• --no-page-side-compositing falls back to layered path
• Docker flag forwarding tested (2 assertions in dockerRunArgs.test.ts)
• CI integration tests
• Test with video-containing composition (should auto-disable)

🤖 Generated with Claude Code

[vanceingalls]

• perf(engine): faster shader transitions via page-side WebGL compositing #832 👈 (View in Graphite)
• fix(cli): scan puppeteer cache for chrome-headless-shell; warn on system-chrome fallback #821
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[miguel-heygen]

Review: Page-side compositing for shader transitions

Why it's 6.5x faster

The architectural change is elegant. Before: each transition frame required TWO transparent-alpha screenshots (one per scene layer), pixel data transfer from Chrome → Node, color space conversion (rgb48le/rgba8), then CPU-bound GLSL shader blending in a Node worker_threads pool. Two round-trips through the screenshot pipeline plus CPU pixel math per frame.

After: a WebGL canvas overlay inside Chrome captures both scenes via drawElementImage, uploads them as textures, and runs the GLSL fragment shader on Chrome's GPU — all in-process. The engine takes ONE opaque RGB screenshot of the already-composited result. This eliminates: (1) two per-layer transparent screenshots, (2) Node-side color space conversion, (3) the entire Node-side shader-blend worker pool. The GPU work happens where the GPU already is.

One issue to resolve

Default mismatch: PR description says "default OFF" (opt-in), but the code sets default: true in the CLI flag and enablePageSideCompositing: true in DEFAULT_CONFIG. These need to agree — if ON is intended, existing CI fixtures may need PSNR-based pinning since f32 (WebGL) vs f64 (Node) precision means output is not bit-identical.

Notes (non-blocking)

• composition.videos.length === 0 gate silently disables page-side compositing for any composition with embedded video. Worth documenting.
• No PSNR-pinned visual regression test yet (documented as follow-up) — acceptable given the opt-in framing, less so if it defaults ON.
• Error handling and fallback are solid — probes for drawElementImage + WebGL, falls back to opacity-flip with warning.

[miguel-heygen]

Description updated — default ON matches the code now. LGTM.

The base branch was changed.

[miguel-heygen]

Re-reviewed with the new commit (a56ef01d). The three-phase protocol for video support is architecturally sound — moving video frame injection before cloneNode so clones contain the injected <img> replacements is the right fix. The img.decode() awaiting on data-URI images in clones is a good catch for stale paint cache.

One blocking issue — lint rule checks wrong attribute:

The video_missing_timing_attrs rule checks for data-end:

const dataEnd = readAttr(tag.raw, "data-end");

But data-end is computed by the timing compiler from data-start + data-duration — it never exists in source HTML. Authors write data-start + data-duration (optional for video, defaults to media duration). Every other lint rule in media.ts references data-duration, not data-end.

This rule will false-positive on every valid video element:

<!-- Valid source HTML — has data-start + data-duration, no data-end -->
<video id="hero" src="clip.mp4" data-start="0" data-duration="5" muted></video>
<!-- ↑ lint error: "missing data-end" — false positive -->

Fix: check data-duration instead of data-end. And since data-duration is optional for video (defaults to media duration per the spec), consider making it a warning when both data-start and data-duration are missing, not an error when just data-end is missing.

The test cases also use data-end="5" — should be data-duration="5" to match the actual authoring contract.

Rest of the commit looks good:

• Removing the composition.videos.length === 0 gate is correct now that three-phase handles video
• Removing the opacity toggling on source scenes is cleaner — GL canvas overlay covers them
• gsap.set type addition is fine

[jrusso1020]

Architectural design is sound — Miguel's prior review dismissed once the default-on/off framing was reconciled, and the perf math holds up (the two-screenshot + Node-side blend → one-screenshot + Chrome-GPU blend swap is exactly what removes the 20× coordination overhead at higher worker counts). Most of my notes here are about gates that need confirmation before merge.

Audited

• frameCapture.ts three-phase protocol — clean: seek + pending-flag check folded into one round-trip; before-capture hook runs first so injected video <img> replacements are picked up by the page-side compositor's cloneNode; the 1×1 Page.captureScreenshot paint-force is gated on hasPendingComposite && session.captureMode !== "beginframe". Reading sequence is correct for the documented invariants. ✓
• config.ts — enablePageSideCompositing: true default, HF_PAGE_SIDE_COMPOSITING env override. Matches the description's "default ON for SDR shader-transition renders." ✓
• Determinism comment in config docstring — explicit "page-side WebGL is f32, not f64. Byte-equality fixture pins are NOT compatible with this path; the new path's correctness pin is PSNR-based." That's exactly the right thing to call out, and matches the regression-fixture pattern. ✓
• Capture-mode auto-disable — frameCapture.ts: session.captureMode !== "beginframe" is the right pivot. ✓
• manager.ts puppeteer cache version ordering — the lexicographic .sort().reverse() → numeric compareVersionDirsDescending fix is genuinely useful (linux-99.x was sorting ahead of linux-148.x because '9' > '1') and the comment is sharp. But it's orthogonal to the perf work — see scope note below.

Worth confirming before merge — body-vs-diff mismatch on the video gate

The PR body lists the auto-disable conditions:

"Auto-disables when unsafe: HDR, alpha output, video content, beginFrame mode"

and later in the gating section:

"composition.videos.length === 0 (cloneNode loses video playback state)"

But the usePageSideCompositingForTransitions predicate in renderOrchestrator.ts:1752-1762 is:

cfg.enablePageSideCompositing &&
compiled.hasShaderTransitions &&
!hasHdrContent &&
!isPngSequence &&
!needsAlpha;

— I don't see composition.videos.length === 0 in there. Three of the four body-promised gates are present (HDR via !hasHdrContent, alpha via !needsAlpha, beginFrame via the frameCapture.ts site), but the video-count gate is missing from this predicate. Two possibilities:

1. The gate exists at a different layer — e.g., engineModePageComposite.ts probes <video> elements at page-side init and bails out (no __hf_page_composite_pending flag → orchestrator's stub injection is a no-op). If so, please confirm — the orchestrator-level early-out would still be cleaner because it would skip injecting the stub script entirely.
2. The gate isn't there yet — cloneNode on a playing <video> produces a paused clone at the source's currentTime, and drawElementImage would read that paused frame. Page-side compositing on a video-bearing composition would silently freeze the video plane on every transition frame. The test plan's unchecked "Test with video-containing composition (should auto-disable)" row suggests this hasn't been verified yet.

If it's #2, the body's "auto-disables when unsafe: video content" claim is a Rule-3 mismatch and worth a one-line addition: && composition.videos.length === 0 (or the equivalent — compiled.hasVideos if that derived flag exists).

Required-check failures gating merge

1. CLI smoke (required) — failure (job 76221755878). Couldn't pull the log, but the diff adds a new --page-side-compositing CLI flag + the new scripts/page-side-compositing-smoke/run.mjs invokes the bundled CLI. If packages/cli/dist/cli.js hasn't been rebuilt to include the flag yet, the smoke script's assertCliCanary would fail. Worth checking whether the CI's CLI smoke job is running this new smoke script or the standard one.

2. CodeQL — failure + advanced-security bot review surfaced one alert: "Insecure creation of file in the os temp dir" at scripts/page-side-compositing-smoke/run.mjs. The script hardcodes /tmp/hf-page-side-smoke/ as WORK_DIR — predictable path in shared /tmp can allow symlink-attack pre-creation. Low-risk for a local smoke test, but easy to clear: replace the hardcoded /tmp/hf-page-side-smoke with mkdtempSync(path.join(os.tmpdir(), "hf-page-side-smoke-")). CodeQL alert closes automatically once the predictable path is gone.

3. Analyze (python) — failure — likely a CodeQL Python-analyzer workflow issue (this PR has no Python). Probably the same root as the CodeQL failure above; check the workflow log.

Non-blocking notes

• Scope — manager.ts puppeteer-cache version ordering (+~60 LoC) and the system-Chrome warn-string nudge are genuinely good fixes but unrelated to page-side compositing. Reviewable as-is, but a separate PR would have a cleaner revert surface if someone needs to roll back the perf work without losing the cache-ordering fix.

• frameCapture.ts:hasPendingComposite read order — the second page.evaluate (resolve) doesn't check cancelled/teardown state. If the render job is aborted between the micro-screenshot and the resolve call, the compositor stays in pending state for the next frame. Probably benign because the next seek + prepare resets it, but worth a one-line if (!session.active) return guard if session.active exists in this scope.

• The 1×1 clip paint-force — semantically the same trick the engine uses for Page.captureScreenshot elsewhere; just noting it overlaps slightly with the always-on-clip change from hf#838 (scale: 1 explicit clip changes Chrome compositor's geometry resolution path). The two changes coexist fine but if one regresses, the other shares the failure mode.

• drawElementImage paint-record requirement — the body explains "drawElementImage requires a cached paint record, which only exists after a compositor pass". The micro-screenshot is the forcing function. Worth noting in frameCapture.ts:711-728 that this isn't a bandaid — it's the documented protocol. The inline comment is good; consider promoting to a doc-comment block since it's the non-obvious linchpin.

Once the video gate is confirmed (or added) and CI failures are sorted, happy to flip to APPROVE.

— Review by Rames Jusso (pr-review)

[jrusso1020]

Self-correction on the video-gate flag.

Magi's review noted "Removing the composition.videos.length === 0 gate is correct now that three-phase handles video" — and that's right. The three-phase protocol runs onBeforeCapture (video frame injection) BEFORE the page-side prepare() does cloneNode, so the clone picks up the injected <img> replacement (a static frame from the video) instead of a <video> clone that would freeze at the source's currentTime. The architecture replaces the need for the gate.

So my prior "missing video gate" framing was wrong — the gate is intentionally absent because the architectural change makes it unnecessary. What's left is a stale PR body: the body still lists composition.videos.length === 0 and "auto-disables when unsafe: video content" among the gates, but those describe an older version of the design. Worth a body update so reviewers (and future readers) aren't pointed at a check that no longer needs to exist.

My other flags stand:

• CLI smoke (required) failure — gates merge regardless
• CodeQL "Insecure temporary file" on scripts/page-side-compositing-smoke/run.mjs — quick mkdtempSync fix
• Magi's video_missing_timing_attrs blocker (lint rule checks data-end instead of data-duration — would false-positive every valid video element) is the substantive blocker

Once those three are addressed + the body's gate list is brought in sync with the three-phase architecture, happy to APPROVE.

— Self-correction by Rames Jusso (pr-review)

[jrusso1020]

Re-approving on dbac2e7c. Magi's blocker is resolved, and the response is more complete than I expected.

Rather than fixing the buggy lint rule, you removed it entirely and replaced it with a different architectural shape:

Audited — the new commits

• media.ts -28 / media.test.ts -42 — the video_missing_timing_attrs rule (Magi's blocker) is gone. The whole "warn on missing timing attrs" angle is replaced by the next two changes. ✓
• timingCompiler.ts +7 / timingCompiler.test.ts +26 — compile-time auto-injection of data-start="0" when missing, plus a data-hf-auto-start sentinel to mark which videos got auto-injection. Three new tests pin: (a) auto-inject when missing, (b) sentinel on auto-inject, (c) NO sentinel when author provides data-start. Clean three-way distinction. ✓ This is the right shape — the compiler resolves the missing-attr problem at the source instead of warning about it.
• htmlCompiler.ts:discoverVideoVisibilityFromTimeline +103 — runtime video visibility probing: seeks GSAP timeline at 0.1s intervals to find when each data-hf-auto-start video's parent .scene has opacity > 0, then binary-searches the boundaries to 1/60s frame precision. Suppresses events on seek (tl.totalTime(t, true)), resets to 0 before returning. ✓ Good cleanup discipline. Engine then writes the discovered window back to composition.videos[i].start/end.
• probeStage.ts +26 — wires the visibility discovery into the probe stage; needsBrowser now also fires on hasAutoStartVideos so renders that previously skipped the probe (known duration, fully-resolved compositions) now run it when auto-injected timing is in play. Sensible — the alternative is rendering with the auto-injected start=0, duration=composition.duration defaults which would write video frames where the scene isn't visible.

Body claim verification

• CodeQL now reports neutral (was failure on a56ef01). The temp-file alert on scripts/page-side-compositing-smoke/run.mjs likely became stale-but-still-open rather than newly-failing — worth a quick glance at the security tab to confirm the alert is dismissed/resolved before merge, but no longer build-gating.
• CLI smoke (required) still in-progress on this commit; will gate merge if it fails again.
• Body still mentions the composition.videos.length === 0 gate (in the "Gating" section) and "auto-disables when unsafe: video content." Stale text — the gate was intentionally removed when the three-phase protocol with pre-clone video frame injection landed. Worth a body update so future readers aren't pointed at a check that no longer needs to exist.
• Test plan — still has "Test with video-containing composition (should auto-disable)" unchecked. With the new auto-injection + visibility-discovery path, the test should be re-framed as "Render a composition with videos and shader transitions — auto-injected timing windows discovered correctly, output preserves video frames at scene-visibility boundaries."

Non-blocking notes on the new runtime-discovery code

1. videoEl.closest(".scene") || videoEl assumes a CSS class convention (htmlCompiler.ts:1196). If a composition uses different scene wrapping (div.layer, section.slide, etc.), the discovery falls back to the video element itself — which typically inherits opacity from its parent stacking context, so it may still produce the right answer. But if <video> has opacity: 1 while parent has opacity: 0, computed opacity on the <video> is unaffected — getComputedStyle(videoEl).opacity returns the video's own opacity, not the effective stacked opacity. Worth verifying compositions that don't use .scene still discover correctly, or use getBoundingClientRect() + IntersectionObserver-style visibility instead. Probably fine for current HF authoring conventions; brittle to future composition shapes.

2. hasAutoStartVideos = compiled.html.includes("data-hf-auto-start") (probeStage.ts) — substring check could false-positive on string literals containing that token in the compiled HTML. A regex against attribute syntax (e.g., /data-hf-auto-start[=>\s]/) would be more robust. Minor.

3. Probe perf cost — ~10Hz sampling means ~280 seekTl + getComputedStyle calls per video on a 28-second composition. At ~5ms per round-trip across the CDP boundary, that's ~1.4s added to the probe stage per video. For multi-video compositions, multiplicative. Acceptable given the discovery benefit, but worth a perf note in the comment or scaling SAMPLE_STEP with compositionDuration for very long renders.

4. Edge: video in a scene that's never visible — firstVisible === null → continue, no window written, video keeps its auto-injected start=0 default. Likely results in the engine rendering frames for a video the user can't see. Probably benign (the engine just generates output that gets composited at opacity: 0) but worth a log warning so authors notice their scene structure is busted.

Once the body's composition.videos.length === 0 gate text is updated (or removed) and CLI smoke comes back green, this is fully clear.

— Re-review by Rames Jusso (pr-review)

[miguel-heygen]

Re-reviewed on dbac2e7c. My original blocker (lint rule checking data-end instead of data-duration) is fully resolved — you went further than just fixing the attribute name:

1. Removed the lint rule entirely — correct call. The problem was architectural, not a lint gap.
2. Compile-time auto-injection — timing compiler injects data-start="0" + data-hf-auto-start sentinel when videos lack data-start. This is the right layer to solve it.
3. Runtime visibility discovery — discoverVideoVisibilityFromTimeline seeks the GSAP timeline at 10Hz to find when each auto-start video's parent scene is visible, then binary-searches to 1/60s. Clever approach.
4. Probe trigger — needsBrowser now fires on hasAutoStartVideos so the browser probe runs even when duration is known.

Rames' non-blocking notes are worth tracking (.scene class assumption, substring check, scaling SAMPLE_STEP with duration) but none are blocking.

CLI smoke green. Three-phase compositor protocol + video support + auto-start pipeline all land together cleanly.

[jrusso1020]

Re-approving on a995e304. All my prior flags now resolved + CI fully green.

The two new commits address the CodeQL temp-file finding I'd flagged: WORK_DIR = mkdtempSync(join(tmpdir(), "hf-page-side-smoke-")) is exactly the pattern I'd suggested — randomized temp dir per invocation, no symlink-attack window. Doc comment updated to use the <tmpdir> placeholder.

CI

• All required checks now green: Test, Typecheck, Lint, Build, CodeQL (was neutral/failure on prior commits → now success), Test: runtime contract, CLI smoke (required), Smoke: global install, Tests on windows-latest, Render on windows-latest, Preview parity, preview-regression, Perf:drift/fps/load/parity/scrub, player-perf, regression, all 8 regression-shards green ✓
• mergeable_state: blocked is reviewer-gate

Recap of resolved items

1. CodeQL "Insecure temporary file" → fixed via mkdtempSync ✓
2. CLI smoke (required) failure on earlier commits → green now ✓
3. Magi's video_missing_timing_attrs lint-rule blocker → resolved via lint-rule removal + compile-time auto-injection + runtime visibility discovery ✓
4. My video-gate "missing" framing → self-corrected (architectural change makes the gate unnecessary) ✓

Clear to merge.

— Re-review by Rames Jusso (pr-review)

[miguel-heygen]

Re-reviewed on a995e304 (2 commits past the prior approvals on dbac2e7c).

New commits since last approval:

• 1cbf3f4e — fixes smoke test to use mkdtempSync instead of a static temp path (prevents collisions on concurrent runs)
• a995e304 — oxfmt formatting on the smoke script

Both are trivial and don't touch the core architecture. The substance reviewed:

Architecture — page-side WebGL compositing (344-line engineModePageComposite.ts):

• Three-phase capture protocol (prepare → micro-screenshot → resolve) is clean. The clone-before-paint approach ensures drawElementImage reads current bitmaps, not stale paint cache entries.
• Seek wrapper installed via setInterval polling for window.__hf.seek (up to 10s) — reasonable given the async bridge initialization order. The opacity-flip timeline still runs underneath, so fallback is automatic if the compositor fails.
• Staging canvases use layoutsubtree + fixed z-index -9998 — positioned off-viewport-layer to avoid capturing in the final screenshot.

Browser cache priority fix (manager.ts):

• Puppeteer cache now checked before HF-managed cache — correct, since HF cache is pinned to a stale Chrome version.
• parseVersionSegments + compareVersionDirsDescending fix the lexicographic sort bug (linux-99 > linux-148 character-wise). Good regression tests covering the exact scenario.
• Note: the same lexicographic bug still exists in engine's resolveHeadlessShellPath (mentioned in the comment). Worth a follow-up to fix there too.

Video timing auto-injection (timingCompiler.ts):

• data-hf-auto-start sentinel cleanly separates author-provided from auto-injected timing, so discoverVideoVisibilityFromTimeline only processes auto-injected videos.
• Binary search for visibility boundaries (1/60s precision) is a smart approach — coarse linear scan + binary refinement keeps the seek count reasonable.

Orchestrator wiring:

• addPreHeadScript injected after HDR check — correct order, since HDR forces layered path regardless.
• needsAlpha and isPngSequence also bypass page-side compositing — all edge cases accounted for.

Smoke test:

• Tests bundled CLI path (not dev path) — validates the full pipeline including tsup bundling. Canary string check ensures the compositor code survived tree-shaking.

Clean, well-architected feature. The two post-approval commits are trivial fixes.