fix(engine): densify drawelement self-verify with parallel worker count#2250
Conversation
miga-heygen
left a comment
There was a problem hiding this comment.
#2250 — densify drawelement self-verify with parallel worker count
Verdict: LGTM 🟢
Multi-worker DE capture: N workers share the verify sample grid but each drains ~1/N, thinning coverage exactly when GPU-pressure damage risk peaks. Fix: resolveParallelDeVerifySamples raises the sample count (4 base + 2 per extra worker, clamped to 8). Passed through CaptureOptions.deVerifySamples → session → captureDeVerificationFrames, where env HF_DE_VERIFY still overrides.
Priority chain is clean: explicit HF_DE_VERIFY → session.options.deVerifySamples → default 4. No SSOT issue — each level is a single fallback. Tests cover densification formula, clamp, single-worker passthrough, and caller-set passthrough.
Review by Miga
miguel-heygen
left a comment
There was a problem hiding this comment.
Reviewed current head a2a6812. Worker-count sample densification is bounded 4..8, caller/env overrides remain authoritative, and tests cover distribution/clamping/single-worker behavior. Required functional checks are green; Graphite mergeability is pending only. APPROVE.
james-russo-rames-d-jusso
left a comment
There was a problem hiding this comment.
Reviewed at a2a6810.
Clean. resolveParallelDeVerifySamples(4 base + 2/worker, clamped 8) densifies exactly where multi-worker risk peaks, single-worker is a no-op, and a caller-set value passes through untouched. Threaded via CaptureOptions.deVerifySamples with HF_DE_VERIFY still winning inside the session — the precedence order matches the natural authoring intuition (env > coordinator-computed > default).
Reference identity preserved when unchanged (deVerifySamples === captureOptions.deVerifySamples ? captureOptions : { ...captureOptions, deVerifySamples }) — nice.
Live 3-worker canary landing at the expected [45, 90, 135, 180, 225, 270, 315, 342] (8-sample grid at 360 frames) confirms the wiring end-to-end.
LGTM from my side.
jrusso1020
left a comment
There was a problem hiding this comment.
Approving per the channel stamp-flow: Rames-D and Magi both cleared, CI clean at a2a6810.
4c02c8f to
ddcd380
Compare
a2a6810 to
1b95af8
Compare
The base branch was changed.

What
Densifies the drawElement self-verify sample grid with parallel worker count.
resolveParallelDeVerifySamples(): 4 base samples + 2 per extra worker, clamped to 8 (the verify path's max). A caller-set value passes through untouched.Why
Each parallel worker arms the same shared sample grid but drains only ~1/N of it — N concurrent hardware-GPU browsers are exactly where compositor-tile damage shows up (the same wild black-slab report driving PR2249). Thinner effective per-worker coverage exactly when risk is highest.
Validation
4 new unit tests (
resolveParallelDeVerifySamples) covering densification, the 8-sample clamp, single-worker no-op, and caller-value passthrough. Full engine test suite (47 tests) green. Live canary render (12s/360-frame comp, 3 workers) shows the grid land at 8 samples[45, 90, 135, 180, 225, 270, 315, 342]— correctly densified from the base 4.🤖 Generated with Claude Code