fix(cli): fold canvas pixel hashes into the frozen-sweep fingerprint#2253
Conversation
4dab6e7 to
dec7c23
Compare
2e69883 to
17db454
Compare
miga-heygen
left a comment
There was a problem hiding this comment.
#2253 — fold canvas pixel hashes into the frozen-sweep fingerprint
Verdict: LGTM 🟢
Fixes false-positive sweep_static for WebGL compositions where motion is pixel-only (no DOM elements move). canvasPixelHash downsamples each visible canvas to 8x8 and folds a simple hash (hash * 31 + data[i]) into the geometry fingerprint as c:${hash}.
Key safety properties: tainted/zero-sized/unreadable canvases hash to "x" (constant), so they can't introduce false NEGATIVES for DOM-motion comps — they're no worse than the current code where canvas content is invisible to the fingerprint. The try/catch around getImageData handles cross-origin tainted canvases gracefully.
Review by Miga
miguel-heygen
left a comment
There was a problem hiding this comment.
Reviewed current head 17db454. Canvas hashing is bounded to an 8x8 readback, visibility-filtered, and fail-silent for tainted/unreadable canvases without weakening DOM-motion detection. Required checks are green. APPROVE.
james-russo-rames-d-jusso
left a comment
There was a problem hiding this comment.
Reviewed at 17db454.
Right diagnosis — a canvas repainting without any element moving is exactly the class the geometry+opacity fingerprint was blind to, and folding a downsampled pixel hash into the same per-sample signature is the least-invasive way to close it. try/catch around drawImage covers tainted/zero-sized/unreadable canvases with a constant hash — no worse than pre-PR for those cases, no new false negative for DOM-motion comps.
One thing worth thinking about before landing:
Question — WebGL preserveDrawingBuffer: false and readback stability. ctx.drawImage(canvas, 0, 0, 8, 8) on a WebGL canvas reads the current composited pixels. Chrome guarantees this for WebGL contexts created with preserveDrawingBuffer: true (or read within the same event-loop turn as the GL draw call). For preserveDrawingBuffer: false (the default), the browser MAY clear the drawing buffer between composites — subsequent drawImage reads can return blank or the last-composited-frame depending on state.
For a WebGL comp that renders every frame (your positive-repro case), each sample is fresh — hashes differ across seek samples where the app draws different content, hashes match where it doesn't. Works.
For a WebGL comp that draws ONCE and stops (splash/still-frame): the front-buffer state at sample N vs N+1 could differ purely because of buffer preservation, not because pixels changed. Risk case: same static canvas across two samples produces hash A then hash "x" (or A then constant-black), and the fingerprint diff false-positives motion → misses the sweep_static detection it was meant to catch.
Vance validated the positive (pure-canvas motion detected) and one negative (DOM-motion still detected). The specific "static WebGL canvas, single draw, subsequent seeks" negative isn't in the validation notes. Worth spot-checking that one before rollout, since misclassifying static-WebGL as motion would flip the sign of the fix for that authoring pattern. off.getContext("2d").drawImage(webglCanvas, ...) is likely stable across the same event-loop turn as the seek → screenshot cycle, but the seek→sample gap may cross a composite boundary.
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 17db454.
17db454 to
a9d320d
Compare
dec7c23 to
9001fee
Compare
a9d320d to
7f47b36
Compare
9001fee to
27cd996
Compare
7f47b36 to
2935a46
Compare
2935a46 to
3338a6f
Compare
The base branch was changed.

What
Folds a canvas-pixel hash into the frozen-sweep audit fingerprint (
__hyperframesLayoutGeometryinlayout-audit.browser.js). Each visible<canvas>is downsampled to 8x8 and its pixels hashed into the per-sample signature; tainted/zero-sized/unreadable canvases hash to a constant.Why
Wild report:
strict checkreportedsweep_staticfor a seek-correct, pixel-only canvas animation (native 4K WebGL PNG-sequence render) — a transparent moving DOM sentinel was needed to make it pass. The frozen-sweep guard's fingerprint was geometry+opacity of DOM elements only, blind to a canvas repainting without any element moving.Validation
Built a minimal pure-canvas-motion fixture (zero DOM motion, only a canvas repaint) and confirmed: WITHOUT the fix,
checkreportssweep_static(bug reproduced); WITH the fix, it doesn't. Also confirmed no new false positive on a DOM-motion fixture. CLI build clean.🤖 Generated with Claude Code