feat(sdk): image-alpha hit-test phase 1 (WS-G)

[vanceingalls]

WS-G — image-alpha hit-test (phase 1)

Part of the AI Studio (Pacific) SDK integration. Independent branch off main.

Context

WS-A1 already gives element-level hit-testing + opacity-at-playhead. This adds image-alpha fall-through: a click on a transparent pixel of an <img> / transparent PNG should miss that image and hit the layer behind it.

What this does

• elementAtPoint now uses elementsFromPoint (full z-stack) instead of elementFromPoint, so a transparent-image hit falls through to the element behind it.
• Pure mapPointToImagePixel — maps client coords → natural-pixel coords, handling object-fit: cover/contain/fill and object-position; returns null for points outside the image box.
• Pure alphaIsOpaque(imageData, threshold) — tests the alpha channel of a sampled pixel.
• imageAlphaOpaqueAt(img, x, y, win) — impure shell: draws the image to an OffscreenCanvas once (cached by currentSrc), samples alpha at the mapped pixel. A SecurityError / canvas taint falls back to treating the pixel as opaque (fail-safe).
• _imgCanvasCache exported for test teardown.

Files (2 changed)

• packages/sdk/src/adapters/iframe.ts (+637)
• packages/sdk/src/adapters/iframe.test.ts — tests for the pure functions + z-stack behavior

Gates

• bun run build ✅ · bun test iframe.test.ts 51/51 ✅
• bunx oxlint 0/0 ✅ · bunx oxfmt --check ✅
• fallow --gate new-only ✅ (3 inherited duplications excluded; one complexity site suppressed with fallow-ignore-next-line)

Review fixes (addressed in code)

The gaps flagged in review are now fixed (not just documented):

• CSS rotation/skew — the image or any ancestor carrying a rotation/skew now fails safe to opaque (hasRotationOrSkew via DOMMatrix) rather than sampling the wrong pixel off the axis-aligned getBoundingClientRect. Full transform-inverse mapping remains phase 2; pure translate/scale map correctly and are unaffected.
• Canvas-taint visibility — a cross-origin/tainted canvas now emits a once-per-src console.warn before the opaque fall-back, so the failure is no longer silent.
• Cache-key dimension drift — the offscreen-canvas cache is now keyed on src + naturalWidth x naturalHeight, so a srcset/responsive re-render of the same URL gets a fresh canvas.
• Memory guard — images above a pixel budget skip alpha-testing (opaque) to bound OffscreenCanvas memory.
• Tests: removed the duplicate a=0 threshold case; added a transparent-over-transparent-over-div fallthrough. iframe.test.ts 59/59.

Still phase 2 (not blocking)

• Full transform-inverse pixel mapping for rotated/skewed images (currently fail-safe-to-opaque).
• Per-pixel alpha for arbitrary elements via drawElement rasterization (needs a perf spike).
• Byte-budget canvas cache (vs the current entry-count cap) and a getImageData GPU-sync perf characterization.
• background-image CSS transparent overlays.

🤖 Generated with Claude Code

[vanceingalls]

• feat(sdk): image-alpha hit-test phase 1 (WS-G) #1574 👈 (View in Graphite)
• main

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

[miga-heygen]

Review — feat(sdk): image-alpha hit-test phase 1 (WS-G)

Nice work, Vance. The decomposition here is chef's kiss — splitting the problem into alphaIsOpaque (pure predicate), mapPointToImagePixel (pure coordinate mapping), parseObjectPosition (pure CSS parser), and imageAlphaOpaqueAt (impure shell with canvas caching) is exactly the right layering. Every error path falls back to opaque, which is the correct fail-safe for a hit-test: you never silently drop a click. The z-stack iteration via elementsFromPoint is a clean upgrade from the single-element elementFromPoint, and the hitStack() fallback for hosts that lack the API is a thoughtful touch.

Test coverage is thorough — 51 tests covering all object-fit modes, object-position keyword/percentage/px parsing, z-stack fallthrough with opaque/transparent/tainted canvas behaviors, and the WS-A1 regression guard. Solid.

Findings

1. () => true passed to resolveNearestHfElement needs an invariant comment (nit)

const result = resolveNearestHfElement(candidate, () => true);

This is correct — isOpacityVisible already walked the entire ancestor chain from the candidate to the root, and the hf-element is necessarily an ancestor of the candidate, so it's already been validated. But a reviewer hitting this line cold will immediately think "wait, did we just skip the opacity check?" A one-line comment explaining the invariant would prevent that false alarm on every future review:

// Candidate already passed isOpacityVisible (full ancestor walk); hf node
// is on that chain, so no second walk is needed.
const result = resolveNearestHfElement(candidate, () => true);

2. "ponytail" comment should be a standard TODO marker (nit)

/** ponytail: FIFO eviction, upgrade to LRU if cache hit-rate matters. */

Unless "ponytail" is a project-wide convention that greps expect, this should be a standard // TODO(phase-2): or // STRETCH: so it surfaces in automated sweep tooling.

3. Canvas memory pressure — worth acknowledging (non-blocking observation)

The cache stores full-resolution OffscreenCanvas objects keyed by currentSrc, capped at 64 entries via FIFO eviction. For typical web images (~1200x800), that's ~3.8 MB per entry, so ~240 MB at capacity — noticeable but manageable. The degenerate case with large images (4000x3000 = ~46 MB each, 64 * 46 MB ≈ 3 GB) is unlikely but theoretically reachable.

For Phase 1 static images this is fine. For Phase 2, consider either:

• Capping by total byte budget instead of entry count, or
• Adding a max-resolution guard that skips alpha testing and falls back to opaque for images where naturalWidth * naturalHeight exceeds a threshold (e.g., 16 megapixels).

4. Redundant getContext("2d") per hit-test (micro-optimization, non-blocking)

The 2D context is obtained once during cache creation (for drawImage) and then again per getImageData call. Browsers return the same context object, so this is correct, but on a hot path (every pointer event), you could avoid the repeated lookup by storing the context alongside the canvas in the cache entry, or by caching { canvas, ctx } instead of just OffscreenCanvas | null.

5. Missing test: stacked transparent images (non-blocking, good Phase 2 addition)

The z-stack tests cover transparent-image-over-div, but not transparent-image-over-transparent-image-over-div. That case exercises the loop iterating past two consecutive fallthroughs and would be a valuable regression test. Not a blocker for Phase 1.

Summary

No bugs found. The architecture is well-reasoned, the fail-safe properties are correct, and the test coverage is strong. The two nits (invariant comment on () => true, standardize the "ponytail" marker) are quick fixes. Everything else is Phase 2 fodder.

LGTM — ready for stamp.

— Miga

[james-russo-rames-d-jusso]

HEAD reviewed: 823e9fbf42540c58b2715d4fe5d291f6f7210fd8

Verdict: Solid phase-1 foundation. Pure helpers are well-factored, test coverage on the pure shell is the highlight (51/51 covering fill/cover/contain/none, keyword-pair normalization, letterbox, taint fallback). A handful of non-blocking concerns around silent-fallback observability, CSS-transform geometry, and cache-key/natural-dim coupling — phase 2 / follow-up territory, not merge-blockers.

Concerns

• iframe.ts:711-718 (canvas-taint fallback is silent). Cross-origin images are a known failure mode for hit-test correctness — fallback-to-opaque is the right behavior, but there is zero signal when it fires. A customer iframe loading uncorsed third-party images will silently lose alpha hit-test, and product will hear about it via "hit-test feels wrong" rather than a dashboard. Suggest a single console.warn (rate-limited per src) or wiring through a onTaint?: (src) => void hook so callers can opt into telemetry. Per feedback_observability_pr_failure_path_coverage.md — the PR's own goal (image-alpha fall-through) silently fails on this path with no visibility.

• iframe.ts:328-340 (no CSS-transform handling on the image). getBoundingClientRect() returns the axis-aligned visual bounding box of the rendered element. If the image (or any ancestor) carries transform: rotate/skew/matrix, the rect grows to encompass the rotated content but the box-to-natural mapping still treats the box as axis-aligned — the alpha sample reads the wrong pixel. GSAP-driven studio scenes routinely apply rotate/scale transforms, which is the dominant Pacific render path. Not a phase-1 blocker if scoped out — but the PR body doesn't list this under "Deferred", and given the milestone is SDK integration with the studio renderer, it's worth either (a) documenting it explicitly as a known phase-1 limitation alongside background-image / animated gif, or (b) confirming the WS-A1 hit-test caller never selects rotated images.

• iframe.ts:265-267 (cache key collides on naturalWidth/Height drift). _imgCanvasCache keys on currentSrc only. Two iframes (or one iframe under viewport changes that swap srcset resolutions) can request the same URL at different natural sizes — the canvas in cache locks to the first observer's dims, and subsequent imageAlphaOpaqueAt calls map points using the current image's naturalWidth/Height and read with possibly out-of-range coords (the clamp in mapPointToImagePixel masks this rather than fixing it). Suggest keying on ${src}@${naturalWidth}x${naturalHeight} so srcset-driven re-renders get fresh canvases. Low-probability but real on responsive layouts.

• iframe.ts:285-286 (cache invalidation strategy is documented but not load-tested). FIFO with cap 64, eviction on insert. For a studio session with many distinct images (large composition, scroll-heavy thumbnail panel inside the iframe), FIFO churns the most-recently-loaded entries first. The ponytail: upgrade to LRU comment acknowledges this. Fine for phase 1; flag for phase 2 if hit-rate ever matters.

Questions

• iframe.ts:333-339 (border/padding inset → fall-through on chrome). A click on the image's CSS border or padding region maps to null from mapPointToImagePixel (correctly — those pixels aren't part of the natural image), which makes imageAlphaOpaqueAt return false and the click falls through to the layer behind. Is this the intended UX? The WS-A1 behavior (pre-PR) would have selected the image element for a click anywhere inside its border box. If a user has an <img> with a 4px border and clicks the border, they'll now miss the image entirely. Worth confirming with the Pacific UX expectation — if intentional, a comment on imageAlphaOpaqueAt would help.

• iframe.ts:380-388 (per-pointermove getImageData cost). Drawing is cached (good), but the 1×1 getImageData happens on every pointermove. For images backed by a software pixmap on the CPU this is microseconds; for any path where the OffscreenCanvas ends up GPU-resident (some browsers do this for large images), getImageData forces a GPU→CPU sync. Did you do a quick perf check on a representative scene with 5-10 images? Phase-1 docstring mentions a "perf spike" is gated to phase 2 for full per-pixel rasterization, but the spike-or-not for this phase isn't called out.

Nits

• iframe.test.ts:39 (alpha-threshold semantics). Test "returns false for alpha below default threshold (a=0 < 1)" duplicates "returns false for a fully transparent pixel (a=0)". Both pass but the second one isn't testing what the description says. (nit)

• iframe.ts:303 (fallow-ignore-next-line complexity x2). Two complexity suppressions in mapPointToImagePixel + resolveToken. Both functions are genuinely doing two things — the suppression is fine, but resolveToken could split out keyword vs unit handling at the same call sites you've already commented. Not blocking; pure code health. (nit)

What I didn't verify

• Real-browser behavior of getImageData on OffscreenCanvas after drawImage(<img>) w.r.t. GPU sync cost. Would take a perf trace on Chromium + Safari to characterize.
• Behavior with cross-origin srcset images where currentSrc taint state can change after the canvas is already cached (e.g., a CORS-enabled fallback URL succeeds on retry). The cache stores null permanently on first taint.
• Integration test in studio — the file at packages/core/src/studio-api/helpers/previewAdapter.test.ts doesn't exercise the image-alpha path. The PR body says "tested via a lightweight fake-DOM helper" for the unit shell, and a future integration test mounting a real iframe is owed (WS-A1 follow-on per the existing TODO at the top of iframe.test.ts).

— Rames D Jusso

[miguel-heygen]

LGTM

[james-russo-rames-d-jusso]

R2 — tight delta verification of R1 phase-1 concerns

Reviewed at HEAD 9162ede1adf6a6ef37a4972d191eebadb0f599b0. Delta from R1 (823e9fb) is 2 files (+95/-18 in iframe.ts, +11/-4 in iframe.test.ts).

Per-R1-item resolution

1. ✅ Silent canvas-taint fallback → resolved. iframe.ts:321-331 adds warnTaintOnce(src) with a _warnedTaintSrcs Set to dedupe, and it's called on both failure paths: initial drawImage catch (iframe.ts:447) and getImageData catch (iframe.ts:465). Covers the full failure surface, not just one branch — matches the feedback_observability_pr_failure_path_coverage.md rubric.

2. ⤴️ No CSS-transform handling → alternative-resolved (better than suggested). R1 suggested document-as-limit; Vance went further with hasRotationOrSkew() at iframe.ts:340-353 that walks ancestors with DOMMatrix, checks b/c for rotation/skew, and fails safe to opaque. Pure translate/scale keep b=c=0 so they map correctly — claim verified. Phase-2 limit (full transform-inverse) documented in header at iframe.ts:43-45. Crediting per feedback_open_item_alternative_resolution.md — operational goal (no wrong-pixel sampling on rotated images) is met.

3. ✅ Cache-key collision → resolved exactly as suggested. iframe.ts:430: `${src}@${img.naturalWidth}x${img.naturalHeight}`. Threaded through both write paths (iframe.ts:439, 443, 448, 466) and the read path (iframe.ts:431). Srcset/responsive re-render gets a fresh canvas.

New findings at R2

1. ⚠️ No test coverage for the three R2 code paths. hasRotationOrSkew(), warnTaintOnce(), and the _MAX_ALPHA_TEST_PIXELS cap are all new logic, but iframe.test.ts only adds one transparent-over-transparent fallthrough case (iframe.test.ts:764-773). The existing tainted test at iframe.test.ts:755-762 asserts the behavioral fallback to opaque but doesn't assert the new console.warn is emitted. Non-blocking — code paths are simple and CI is green — but the R2 hardening will silently regress if any of these is refactored. Suggest a follow-up: stub console.warn for the tainted test and assert call count + once-per-src dedupe, plus a hasRotationOrSkew test with a stubbed getComputedStyle({transform: "matrix(0, 1, -1, 0, 0, 0)"}).

2. ⚠️ _warnedTaintSrcs is unbounded. iframe.ts:319 — the dedupe Set grows monotonically for the lifetime of the SDK module. Long-lived studio sessions cycling through many tainted srcs (template galleries, user uploads) accumulate entries forever. The _imgCanvasCache next to it has a _IMG_CANVAS_CACHE_MAX = 64 LRU cap; this companion Set doesn't. Per-src strings are short, so the memory floor is low — flagging as phase-2 cleanup rather than blocking. Cheap fix: cap at the same 64 or clear alongside _imgCanvasCache.clear().

3. ℹ️ 3D-rotation edge case in hasRotationOrSkew. DOMMatrix.b/.c correspond to the 2D matrix slots; a pure rotateY(90deg) / rotateX(90deg) serialized as matrix3d(...) can leave b/c at 0 while still producing a non-axis-aligned render. Real-world GSAP studio scenes mostly use 2D rotate(...), so this is a tail case — but worth a one-line ponytail comment alongside the phase-2 deferral.

What I didn't verify

• Didn't actually run bun test iframe.test.ts locally; trusted the green SDK gate.
• Didn't trace elementsFromPoint ↔ elementAtPoint interaction across the full WS-A1 contract — only the WS-G delta surface.

Review by Rames D Jusso

[jrusso1020]

LGTM per Rames D Jusso's R2 routing. Verified all 3 R1 phase-1 fixes at HEAD 9162ede1:

• Taint signal (R1 #1 resolved): warnTaintOnce() at iframe.ts:711-713 with once-per-src dedupe via _warnedTaintSrcs Set; fires on both drawImage catch (iframe.ts:837) AND getImageData catch (iframe.ts:855). Full failure-path coverage as RDJ called out.
• CSS-transform handling (R1 #2 resolved better-than-asked): hasRotationOrSkew() at iframe.ts:730 with DOMMatrix ancestor walk, called at iframe.ts:779. R1 only asked for "document as phase-1 limit"; you built the actual detection with fail-safe-to-opaque on rotate/skew, leaving pure translate/scale unaffected. Genuine upgrade over the ask.
• Cache-key collision (R1 #3 resolved): keyed on ${src}@${naturalWidth}x${naturalHeight} shape threaded through all read/write sites — srcset-driven natural-dim changes no longer collide.

Non-blocking observations carried per RDJ (worth a phase-2 follow-up but not gating):

1. No test coverage for the 3 new code paths. The rotation/skew detection, taint warn dedupe, and oversized-cap branches all lack assertions. Cheapest add: stub console.warn and assert dedupe semantics on a tainted-src tight loop. Worth folding alongside the next change to this file.
2. _warnedTaintSrcs unbounded Set. Neighbor _imgCanvasCache has a 64-entry LRU; this companion Set doesn't have parity. Long-lived sessions with many distinct tainted srcs grow it monotonically. Cap with the same FIFO eviction shape as the cache, or accept the unbounded growth as the price of dedupe.
3. hasRotationOrSkew only checks 2D b/c. A matrix3d doing a pure X/Y rotation has b=c=0 in its 2D projection and would slip through. Tail case for GSAP-studio scenes that use 3D transforms; worth a one-line "phase-2 limit" note if Vance wants to document.

CI all-green (Analyze, Build, CLI smoke, CodeQL, Format, Lint, Perf suite — 36 checks pass, 0 fail). Stamp matches RDJ's routing.

Stamp by Rames Jusso (per Rames D Jusso's R2 substance review at HEAD 9162ede1).