feat(core): let a vfx ref read another host's output - #4808
vanceingalls wants to merge 2 commits into
Conversation
A ref that resolves to an element with data-vfx-chain now uploads that host's finished .hf-vfx-out as u_src2 instead of capturing the host a second time, so a ref layer that runs its own self-capture kernel no longer needs a second layoutsubtree canvas (the nest that hangs drawElementImage, #4405). Behavior change: a ref that named a chain host used to read the host's own .hf-vfx-src (the layer before its effects); it now reads the host's output. data-vfx-ref-visible is a no-op on a host, and a host's own source is never visible. A ref to a non-host wrapper is unchanged. Hosts are ordered by their refs at init so a host paints after the hosts it reads (engine path), and the preview path waits on the same edges. A ref cycle, or a ref to a host that failed to register, drops the referencing chain loudly. A referenced host outside its window reads as empty. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
miguel-heygen
left a comment
There was a problem hiding this comment.
Reviewed 5bf82ca56d13f8dccae240a6e8c8e952972fa42e incrementally on #4804. Registry dependency ordering, cycle/missing-host removal and per-context output uploads work in the authored controls.
Blocker — a referenced host hidden by an ancestor still supplies its stale frame (packages/core/src/runtime/vfx.ts:999). isPaintableHost(ref.host) checks the host’s own computed display, which remains block under a display:none parent. The referenced host then has zero layout size and does not repaint, but its preserved output canvas still has its previous dimensions/pixels, so this condition uploads that old output. This breaks the stated transparent-hidden-source rule for nested/timed composition layers.
I reproduced it on the freshly built runtime in real Chrome: paint a visible referenced host, then hide its parent and seek again. The referenced host reports display:block, offsetWidth:0; the receiving matte’s alpha stays 255, rather than becoming 0. Setting the host itself to hidden is a successful negative control (alpha 0). A separate actual-runtime unit witness also records the stale canvas upload. Check effective ancestor visibility/current layout before accepting a host output; cover the live→ancestor-hidden transition in both paint paths.
67 authored runtime cases and all 29 authored browser cases pass. The two independent witnesses fail on the newly covered ancestor-hidden case; they are preserved separately from the source tree. Browser bundle SHA256 cdb1bcb5f878afae7ad114e879d5a08622d8d15aa3e1561c941411700b0cdc10. Cached Vitest3.2.4/Chrome152 were reused, with pinned checkout parser source for the build; no installs or external services. No exporter-side or original AE-machine hang validation claimed.
CI’s optional Comments check still fails on the new 14-line block (base limit12); its actual log identifies the comment-length ratchet. Other current checks pass. No prior review findings at refresh. Read the four-file incremental diff, dependency linking/preview/engine ordering and resource cleanup plus the behavior-change docs; author-owned exporter compatibility remains unverified. No merge or retarget action. The clean disposable worktree is retiring after this stacked review.
— Magi
Verdict: REQUEST CHANGES
Reasoning: Processed-output binding works for normal hosts, but ancestor-hidden references retain stale pixels instead of the promised transparent output.
| gl.activeTexture(gl.TEXTURE0); | ||
| gl.bindTexture(gl.TEXTURE_2D, ref.texture); | ||
| const out = ref.entry?.out; | ||
| if (out && out.width > 0 && out.height > 0 && isPaintableHost(ref.host)) { |
There was a problem hiding this comment.
Blocker: under a display:none ancestor, this host still computes display:block, but has offsetWidth0 and skips repainting. Its preserved out canvas retains the last frame, so this condition uploads stale pixels. Fresh-runtime Chrome live→ancestor-hidden witness leaves receiving alpha255 (expected0); own-hidden control correctly gives0. Require effective visibility/current layout before reading host output and cover this transition.
…comments A referenced host hidden by an ancestor's display:none keeps its own style at display:block, but it has no box and never repaints, while its preserved .hf-vfx-out still holds the last frame. isPaintableHost let the receiving host upload that stale frame, so a matte stayed opaque after its source was hidden. The upload now requires the host to be on screen: visible, with a layout box, and displayed through every ancestor. Otherwise it reads the 1x1 transparent texture, the documented hidden-source rule. Also trims the two new comment blocks over the 12-line comment-ratchet limit. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
Pushed
|
Stacked on #4804. Base is
fix/4405-vfx-nested-capture; retarget tomainonce #4804 merges.What
A vfx
refparam (map/matte) that names a host running its owndata-vfx-chainnow binds that host's finished.hf-vfx-outtexture directly.Why
Item 1 of #4405. A ref to a layer that runs its own kernel could only be expressed by wrapping it in a second
layoutsubtreecapture canvas, which nests two capture canvases and hangsdrawElementImage. Reading the host's output needs no second capture, so nothing nests.How
.hf-vfx-outcanvas into its own texture withtexImage2D(preserveDrawingBuffer: truekeeps it readable).initVfxlinks host refs after every host registers and orders the registry so a host comes after every host it reads. The engine path and the inline paint path walk that order. In the preview path a referencing host waits for the referenced host's capture and paint, and always takes the deferred path even if it captures nothing itself.reportVfxErrorand their GL resources released. The placement loop only places a host once its dependencies are placed, so it cannot loop.data-vfx-ref-visibledoes nothing for a host ref: the host already paints itself. A host outside its window reads as a 1x1 transparent texture, matching the existing hidden-source rule.canvas.hf-vfx-src) is unchanged, includingdata-vfx-ref-visible.Behavior change
Host binding is automatic, not opt-in. A
refthat names an element withdata-vfx-chainused to read that element's own.hf-vfx-src(its raw layer). It now reads the host's processed output. Anything that relied on the raw reading (for example retro-waveLogo Animlayer 5, thedata-vfx-ref-visible-on-a-chain-host shape) must reference a separate wrapper around the raw layer instead;docs/guides/vfx-chain.mdxsays so.This also supersedes part of #4804's M3 change. With host refs, a chain host is never captured through the ref path, so the "one canvas reached both ways gets two opposite
visibleflags" case can no longer occur.resolveCaptureSourceis back tovisible: false, and only the wrapper-ref path reads the attribute. The M3 test is replaced with one that pins the new behavior.Test plan
src/runtime: 65 files, 1736 tests pass;tscclean.vfx.test.tshas 67 tests.vfxDeterminism.test.ts: all 29 browser tests pass on a rebuilt bundle.#a(luma matte) names host#b;#b's raw pixels are opaque white and its output carries alpha strips, and#a's alpha follows the strips.data-vfx-ref-visibleshape is covered.#aprecedes#bin the DOM on a cold frame, so#areads#b's output only if the registry order is right.visibleread (5), host source readingvisible(2), skipping hosts that have their own capture canvas (10), treating every ref as a host (15). In the browser suite, removing the registry sort fails only the ordering test; turning the host branch off and removing the upload fail all three new browser tests.wrapAsVfxRefSourceand ref-layer emission need checking against the new default), and the original hang on the AE-export machine (this change avoids the nest by construction).afterAllbrowser.close()timeout showed up once in a filtered run; the file passed cleanly on rerun.Part of #4405 (item 1).
🤖 Generated with Claude Code