Repository navigation
feat(core): capture motion blur pose, discovery and compositor - #5875
Conversation
Edit accuracy: accurate 271 (base branch 271), smooth 164 of thoseThe gate passes. Quarantined, measured but not gated (0) Unstable (2)
|
5a9ee9c to
5e05a35
Compare
b945446 to
f78f8a6
Compare
miguel-heygen
left a comment
There was a problem hiding this comment.
Reviewed at full head f78f8a6, bottom of the six-PR stack. No prior human reviews or inline findings in the saved and fresh snapshots.
P2 blocker: pose.ts:196–205 omits CSS individual translate/rotate/scale although discover.ts explicitly selects that motion. An individual translate mover is discovered, but x=120→180 returns pose.e=0 at both times and zero sweep; the equivalent matrix control returns e=150. Those computed properties are separate from computed transform. Compose them in CSS order with transform origin/ancestors, or exclude unsupported motion consistently. This head has no caller yet; #5967 activates the same unchanged pose code, so repair the foundation before shipping the stack.
Reviewed source contracts and the independent exact-module control; synthetic layout/Canvas, not a Chrome pixel proof. The private six-suite run passed124 authored tests and two additional controls reproduced the gaps. Current123 hosted checks pass/skip, and the inspected original core log executes the new19 pure tests. No render/performance/provider call, merge or release.
— Magi (Home)
Verdict: REQUEST CHANGES
Reasoning: Ordinary motion selected by discovery is omitted from the pose calculation once the next PR enables it.
| if (cached) return staticOwn.get(n) ?? null; | ||
| const cs = getComputedStyle(n); | ||
| let own: Affine | null = null; | ||
| const t = parseTransform(cs.transform); |
There was a problem hiding this comment.
P2: Read the individual transform properties as well. Discovery explicitly includes translate/rotate/scale, but these remain separate computed properties and are not serialized into cs.transform. With transform:none and translate:120px 0→180px 0, the exact reader returns e=0 for both poses, so the discovered mover has zero sweep and stays sharp. The equivalent matrix control returns e=150. Compose the individual transforms in CSS order (including origin/ancestor handling), and add individual-property versus matrix controls; alternatively consistently exclude unsupported movers. Dormant here, but this unchanged code becomes live in #5967.
f78f8a6 to
c654769
Compare
c654769 to
e809a44
Compare
Add the three pure modules capture motion blur is built from: pose (an element's box as an affine in composition space, and the poses between two times), discover (find the elements that move fast enough for a smear to show) and composite (redraw each mover along its path into one frame). Nothing calls them yet; the runtime that wires them up lands in the next change. The pose reads the individual translate, rotate and scale properties in CSS order, adds the offsetParent border back and subtracts ancestor scroll, so it matches getBoundingClientRect for a box centered with translate: -50% -50%. Folded review fixes: R3 (pose half; the scan's 3D skip for individual transforms lands with the mover scan). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
e809a44 to
af29b40
Compare
miguel-heygen
left a comment
There was a problem hiding this comment.
Reviewed exact head af29b40ef15eb11e1c347b1feed2a62e6b398ae0 relative to 852f6f9f6386fa88b434504233449a9babf995dc.
The individual-transform blocker is repaired: pose.ts composes translate/rotate/scale with the transform/origin chain. The independent exact-module individual translate120→180 control now reads pose.e120→180 with nonzero sweep; the equivalent matrix control remains150. This pose production blob is unchanged through the tested stack tip. Original exact-head hosted runtime log executes23 pose tests (core4834pass/1skip overall).
Independent revision-bound review of the nine-PR composition. Earlier approvals/requests covered older heads. Current saved head/review/inline/check snapshot has no fresh duplicate finding and no failing/pending latest check. Stacked broad-suite skips are not executed tests.
Audited: Pose/discovery/compositor: individual transform order, origins/ancestors, scroll/offset geometry, crop/sweep and alpha composition.
Limits: Composed local focused run:196 authored core tests pass; separate WAAPI/CLI44 pass. Synthetic geometry/canvas controls, not fresh browser pixel/performance proof. No app/model/provider call, source fix, merge or release.
— Magi / desktop-review
Verdict: APPROVE
Reasoning: No additional blocker in this own delta. The later #5971 defect must be repaired before landing the complete stack.
What
Adds the three pure modules capture motion blur is built from, under
packages/core/src/runtime/captureMotionBlur/:pose.ts: an element's box as a 2D affine in composition space (layout offsets plus every CSS transform up the ancestor chain), and the in-between poses a shutter sweeps through. It reads the individualtranslate,rotateandscaleproperties in CSS order (translate percentages against the border box), adds the offsetParent's border back and subtracts ancestor scroll, so the pose matchesgetBoundingClientRectfor a box centered withtranslate: -50% -50%, a turned box or a box in a scrolled container.discover.ts: finds the elements that move fast enough for a smear to show (inline-style changes and CSS / WAAPI animations reaching 2 px per frame, outside any 3D context, outermost of nested candidates only). An element on anoffset-path, or animated throughoffset-distance, is no mover: the pose has no offset-path term, so blurring it would only paint the element twice with no smear.composite.ts: the per-frame draw. It crops each mover from a capture of the scene and redraws it at every sample pose into a float buffer, additive at weight 1/n.Nothing calls them yet, so this PR changes no behaviour.
Why
Part 1 of 9 in the capture motion blur stack: #5875 → #5967 → #5968 → #5969 → #5970 → #5971 → #5990 → #5991 → #5992.
The whole stack delivers opt-in motion blur for compositions, drawn with Chrome's html-in-canvas (
drawElementImage), in Studio preview and in renders. A composition turns it on withdata-motion-blur="on"on its root; nothing blurs without that attribute. This first PR is the math and drawing with no wiring, so the later PRs can be read against modules that are already reviewed.Notes
rotateaxes and z translates (individualIs3d, exported here) is wired into discovery in feat(producer,studio): wire capture motion blur into renders and Studio #5969.zoom, aposition: fixedelement, and content inside<foreignObject>.scaleX = 0blends as a rotation; sub-pixelleft/topsteps land in whole pixels (offsetLeftis rounded); a fast child inside a parent that moved earlier is not blurred (only the outermost candidate is kept); SVG elements are not scanned.Testing
Unit tests next to each module: affine multiply / invert / interpolate, the sweep taps and the individual
translate/rotate/scaleproperties (each compared with its equivalent matrix transform, thetranslate: 120pxand180pxsweep included), a bordered offset parent and a scrolled container (pose.test.ts); mover discovery thresholds and nesting, and that anoffset-distanceanimation or anoffset-pathelement is no mover (discover.test.ts); the compositor's draw calls against a recording canvas fake (composite.test.ts).bun run build,tsc --noEmitin core and the core runtime tests pass. Comment ratchet againstorigin/mainpasses; no package size budget changes.🤖 Generated with Claude Code