Repository navigation
feat(core): block capture motion blur on html-in-canvas defects - #5971
vanceingalls wants to merge 1 commit into
Conversation
61a8a54 to
8bb0fe5
Compare
fd62b9e to
13842a1
Compare
miguel-heygen
left a comment
miguel-heygen
left a comment
There was a problem hiding this comment.
Delta review at full head13842a175e0315b9cbff6dab34932fbd51f0adb7, relative to8bb0fe5e.
Audited runtime probe→CLI collection before grid→finding/fixHint propagation, warning deduplication and first-completed-frame engine diagnostic. Probe scans without capture/wrap and restores the current time on its successful path. Unsupported browser/no movers stay informational; a disabled requested effect becomes a warning.
Nonblocking: checkMotionBlur.ts:23–24 suggests moving WebGL/WebGPU to a sub-composition without opt-in. The preceding PR checks all canvases in the same document, so an ordinary inlined sub-composition still blocks the root. Clarify a separate/flattened render or keep the drop-blur alternative.
The inherited #5875 pose and #5967 timeout findings remain; this delta approval does not clear the complete stack. Saved/fresh prior human reviews and inline comments were empty. Exact-head hosted checks pass/skip; the private focused core run passed124 authored tests. Inspected child regression logs execute3 visual/audio cases, not every new unit test. Browser-version screenshots, HDR output, worker parity, colour and performance measurements were not independently rerun. No provider call, merge or release.
— Magi (Home)
Verdict: APPROVE
Reasoning: No additional blocking defect found in this change relative to its stack parent; the foundation fixes remain required.
There was a problem hiding this comment.
Nonblocking: the accelerated-canvas guard from #5970 scans root.ownerDocument, so moving this effect to an ordinary inlined sub-composition still disables blur on the root. Please specify a separately rendered/flattened effect, or retain the drop-blur alternative; the current hint implies an in-document move is sufficient.
8bb0fe5 to
4e120e5
Compare
13842a1 to
2d4f676
Compare
vanceingalls
changed the title
4e120e5 to
793f8a3
Compare
2d4f676 to
92598cf
Compare
793f8a3 to
d8d7200
Compare
92598cf to
a03da4f
Compare
miguel-heygen
left a comment
miguel-heygen
left a comment
There was a problem hiding this comment.
Reviewed exact head a03da4f7d8af648e4dd020030ea1f844333ab876 and composed tip d5e218c7565b433df887c803fa4ad5de20c44b09.
Both html-in-canvas forms and VFX/Studio consumers are accounted for; nested/import/adopted stylesheet support is explicit. One detector gap remains at packages/core/src/htmlInCanvasDefects.ts:234–244.
P2 blocker: candidate discovery only includes inline backface declarations or selectors from CSSStyleRules that literally declare hidden. A normal DOM face whose backface-visibility:hidden is supplied by @Keyframes (or WAAPI) has neither, so its computed hidden/3D state is never read. Exact own-head control: equivalent inline face detected; animated-only face missed. At final tip both timeline scan and facesNow remain false, and with another ordinary mover the real runtime stays on and wraps the root in CANVAS. This bypasses the stated plain-frame safeguard for the known mirrored-backface capture defect. Include animation-derived candidates or a conservative guard and test scan/backstop with that case.
Witness uses normal DOM .face{animation:turn 1s both} with hidden backface/rotateY in keyframes, no inline backface or ordinary selector declaration. Computed style/Canvas boundaries are controlled; this proves detector/runtime policy, not a newly measured Chrome pixel failure. The known mirrored capture behavior is the PR's own premise.
196 authored core tests and44 extra WAAPI/CLI tests pass locally; own-head independent positive passes/animated negative fails, and final-runtime negative fails. init.blurHold could not load due cached generated parser export. Exact-head child regression executes3 generic cases; broad unit/pixel execution is not established by its skipped checks. Existing reviews/inline comments read; none covers this new witness.
— Magi / desktop-review
Verdict: REQUEST CHANGES
Reasoning: An ordinary animated hidden 3D face escapes both the defect scan and the later backstop, allowing the capture path that the guard is intended to reject.
There was a problem hiding this comment.
P2 blocker: candidate discovery only includes inline backface declarations or selectors from CSSStyleRules that literally declare hidden. A normal DOM face whose backface-visibility:hidden is supplied by @Keyframes (or WAAPI) has neither, so its computed hidden/3D state is never read. Exact own-head control: equivalent inline face detected; animated-only face missed. At final tip both timeline scan and facesNow remain false, and with another ordinary mover the real runtime stays on and wraps the root in CANVAS. This bypasses the stated plain-frame safeguard for the known mirrored-backface capture defect. Include animation-derived candidates or a conservative guard and test scan/backstop with that case.
html-in-canvas captures a few things wrongly, and a wrong blurred frame is worse than a sharp one. Add a defect list the runtime checks before it wraps a composition: a WebGL or WebGPU canvas (captured frozen), backface-hidden faces in a 3D context (drawn mirrored), an iframe (reloaded by the wrap and not recoverable), a transform or opacity below 1 on the root (ignored by the capture) and a canvas that is already a capture canvas (nesting hangs the renderer). Any hit turns blur off for the whole composition and the runtime says which one. The backface scan reads rules nested inside a style rule (CSS nesting), sheets pulled in with @import and adopted style sheets. Shadow DOM stays a documented limit. A face whose backface-visibility: hidden arrives through an animation counts too: the scan also takes the elements of running CSS or WAAPI animations that key it, and elements naming a @Keyframes rule that does, as candidates. Support Chrome 157's html-in-canvas API alongside the older one: a canvas marked content="drawable" with a drawable attribute on captured children replaces layoutsubtree. Blur, the VFX runtime and Studio's browser check detect both through the new @hyperframes/core/html-in-canvas subpath. Folded review fixes: R14, R9 (the iframe defect entry). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
a03da4f to
3d6c41b
Compare
miguel-heygen
left a comment
miguel-heygen
left a comment
There was a problem hiding this comment.
Reviewed exact head 3d6c41b95fe38297b778db01f73eed91a2bac20b against its actual parent d8d72006e0d837d70a7d501dd65473bd4af2a8a3 and the previously reviewed tree.
The omitted animated hidden-face candidate is now found from running CSS/WAAPI keyframes and from named stylesheet keyframes. Root containment and flat-face rejection remain. My earlier exact-source witness fails on a03da4f and passes on this repair; the inline positive remains green.
Independent verification: own-head and composed-tip synthetic computed-style controls 4/4 pass; predecessor witness remains red (inline positive passes). Exact-source authored detector suites run locally:31 own-head +47 tip tests pass, including WAAPI containment, named/multiple animations and cached-candidate retention. The computed animation-name seam is explicit because jsdom does not expand animation shorthand. These are deterministic detector controls, not fresh Chrome pixel/performance proof.
Current exact-head check snapshot has no failed or pending check. Read completed regression gate logs and executed #5991 preflight lint/format output; stacked broad-suite skips and aggregate gate success do not prove execution of the new pixel/unit suites. Author Chrome152 experiments, 26ms scan cost and package-budget margin remain attributed.
Known sampling limit remains: a short WAAPI-only face absent at every scan cannot enter the cached set. This bounded sampling residual differs from the repaired omission of ordinary animation-derived candidates; no universal detection guarantee is claimed. No new source blocker found in this successor composition. No browser/app/model call, merge or release.
— Magi / desktop-review
Verdict: APPROVE
Reasoning: The earlier animated-face candidate blocker is repaired and retained through the restacked composition; no additional blocking change was found.
This branch has not been deployed
2 participants
What
htmlInCanvasDefects.ts). Before the runtime wraps a composition it checks for things html-in-canvas captures wrongly; a hit turns blur off for the whole composition and the runtime says which one. In this PR: a nested capture canvas (nesting hangs the renderer), a WebGL / WebGPU canvas in the root (captured frozen; Chrome 157 fixed it),backface-visibility: hiddeninside a 3D context (drawn mirrored), an<iframe>in the root (the wrap moves the root in the document and reloads the iframe, which cannot be undone), a transform on the root (ignored by the capture) and opacity below 1 on the root. Some entries apply only to a range of Chrome versions.&resolved against the parent), sheets pulled in with@import(theirstyleSheet, read once each, depth capped, unreadable cross-origin sheets skipped), and adopted style sheets. Before, a nested or@imported rule that computesbackface-visibility: hiddenwas missed and blur stayed on with a mirrored face.backface-visibility: hiddenfrom an animation is a candidate too. The scan also takes the targets of running CSS and WAAPI animations (document.getAnimations(), keyframes that keybackfaceVisibility) and, for an animation not instantiated yet (a delay, a class added later), any element whose computedanimation-namenames a@keyframesrule that sets it. Candidates are confirmed exactly like inline and stylesheet ones: computed hidden plus a 3D context. Before, an@keyframes- orelement.animate()-only flip face had neither an inline declaration nor a rule declaring it, was never read, and with another ordinary mover the runtime stayed on and wrapped the root (reviewer finding on this PR). OnegetAnimations()pass per scan; the keyframes walk reuses the sheet walk above and the per-element read only happens when some@keyframesrule sets the property. Measured on a 5,000-element root (Chrome 152, scan time): 26 ms before and after; with an unused@keyframesrule that sets the property, 30 ms.<canvas layoutsubtree>with<canvas content="drawable">plus adrawableattribute on captured children. The new@hyperframes/core/html-in-canvassubpath detects and handles both forms; blur, the VFX runtime and Studio's browser check use it. Studio'suseRenderQueuenow asks it whether the browser can blur.Why
Part 6 of 9 in the capture motion blur stack: #5875 → #5967 → #5968 → #5969 → #5970 → #5971 → #5990 → #5991 → #5992. It builds on #5970.
Stack goal: opt-in capture motion blur for compositions, in Studio preview and in renders, drawn with Chrome's html-in-canvas. A wrong blurred frame is worse than a sharp one, so blur steps aside rather than draw it.
Notes
backface-visibility: hiddenused as a performance hack (no 3D context) is deliberately not blocked. The same holds when the hack arrives through an animation.scale/rotate/translatecounts as a root transform, a rootmix-blend-modeother thannormalis a defect, and the iframe entry covers<object>,<embed>and defined custom elements (individualrotateaxes and ztranslatecount as 3D for the backface check).data-motion-bluron a sub-composition is ignored.Before
k3-promo at 9.1 s, no motion blur:
After
Same frame with capture motion blur (360° shutter): the pins, the text and the box smear along their paths.
Testing
htmlInCanvasDefects.test.ts: every defect, with Chrome versions on both sides of each version bound, the iframe on every version, the nested-rule /&/@import/ adopted-sheet cases (a self-importing sheet is read once; an unreadable one is skipped; a chain past the depth cap stops), and the first-match order. The animation-derived candidates: a face hidden by@keyframes(named, in a list of names, or not matching any name), a face a running animation keys tohidden(a non-hidden keyframe ignored, a target outside the root ignored).htmlInCanvas.test.ts: form detection and marking for both thelayoutsubtreeanddrawableforms;vfx.test.ts(the VFX runtime uses the helper); runtime tests for blur staying off on each defect, including an iframe present at init and one added later; StudiouseRenderQueueMotionBlur.test.tsx.bun run build,tsc --noEmitin core and studio, and the touched packages' tests pass; comment ratchet against feat(core,engine): track accelerated canvases before any page script runs #5970's branch passes; no package size budget changes (core stays under the budget feat(core,engine): track accelerated canvases before any page script runs #5970 set).🤖 Generated with Claude Code