Repository navigation
chore(core): the animator takes start: 'next-frame' for a call made in an input event's own task - #9261
chore(core): the animator takes start: 'next-frame' for a call made in an input event's own task#9261vjeux wants to merge 14 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
c4466e3 to
695ec9b
Compare
PR Analysis Report
Generated by PR Enrichment workflow | View current CI run |
There was a problem hiding this comment.
Changes requested for one small fix. start: 'next-frame' is set on whatever start group beginGroup returns, so a call with an explicit at that joins an earlier call still waiting for its start switches that earlier call to the next-frame rule, although the option's doc says it is ignored with at (animator.ts:247; set at :3201 and :3333). Guard the flag with at == null for both animate and play and add a unit case for the join. Separately, the description quotes spec:AST-094 FR6 for the start rule, but FR6 is "Motion is interruptible"; architecture:react-component-runtime INV13 (an internal chosen start) with FR5 is the governing text.
[Automated review]
| if (group != null && animateOptions.start === 'next-frame') { | ||
| group.nextFrame = true; | ||
| } |
There was a problem hiding this comment.
With an explicit at no later than a call from rest still waiting on these channels, beginGroup returns that earlier call's group (line 1797), so this marks a call that never asked for 'next-frame'. The option's doc says it is ignored with at (line 247). Suggest group != null && animateOptions.at == null && animateOptions.start === 'next-frame', the same guard for playOptions at line 3333, and a unit case where a {at, start: 'next-frame'} call joins a waiting call.
There was a problem hiding this comment.
Fixed in f2795a9: the next-frame flag is set only when the call has no at, in both animate and play. A new unit case has a {at, start: 'next-frame'} call join a waiting call and leave it on the browser's start after a block; it was red before the guard.
meta#8360's change on the internal touch foundation: one click guard shared by the menu press and the gesture host (its click listener stays once the menu press arms it, as its record has it, and lasts only while armed for the gesture host, FR24), the host as the one touch owner (finger scroll, page holds, attachments, the press tracker hook and a held control's kept touch, the long press's single source, presented hit testing on iOS WebKit), its cases in the gesture browser proof (Chromium and WebKit) and the README. It carries meta#8074, meta#8270, meta#8293, meta#8315, meta#8320 and meta#8356, and folds in meta#8358 (the long press's tolerance comment and an unused import). Rebased onto main after section 2 as one commit; the touch-action rule came with meta#8071. Where fingers come from their pointer events, a claimed finger's touchend is cancelled from its element (FR21), under a fine primary pointer wherever the browser sends Touch Events and inside an open shadow root too; at most eight such lifts wait for their touchend, and each goes once taken or after 500 ms, kept across the host's uninstall, which can run in the microtask after the pointerup, before the touchend that comes later in the same task (FR24). An armed press's page listeners go once the arena has decided its last touch, a tap that waits on a double tap included, and after its last detach the host holds no listener, note, frame or timer (FR24). The animator's busy proof is judged against the harness's compositor-only control (meta#8906), with a main-thread mutant as its negative, and the spring's curve is placed against the control's own clock, so the late frames a loaded runner sends fail nothing. With the host serving every pointer (meta#8071), this change's own pieces follow it: a press tracker takes a mouse press as a press, painting at once and braking no scroll (FR11); a finger's or a pen's hold that does something keeps text unselected under any primary pointer, while a mouse keeps its released behavior (FR41); and `perEventPointer`, from before FR43, changes nothing. These come from meta#8665, which keeps only the undecided rule for text selection during a mouse or pen drag. The hover recognizer and the host's hover routing land with hover's first user. Co-authored-by: Cindy Zhang <cindyxz@meta.com>
…finger landed On iOS WebKit the host resolves moving content at touch-down and asks the engine what is under the finger. When a busy main thread hands it the touch later, the content has moved on: the host now asks at the point where what was under the finger at the touch's timestamp is drawn, so a tap on a moving sheet's Done still clicks Done (spec:AST-094 FR11). A presented source reports its offset at a given time for this. The presented browser proof drives the sheet with Core's animator and a critically damped spring, up into place and down off the screen over the scrim: a tap on the drawn Done clicks it once, and a drag on the drawn sheet catches it where it is drawn, in both directions.
…the finger (FR11, FR19); INV13 names the shared input listener Where the engine hit-tests an element a compositor animation moves at the motion's start (iOS WebKit), a touch on that content goes to what was drawn under the finger at touch-down, and a tap there activates that element with one click while the browser's own click is cancelled or swallowed (FR11). While such content around a recognizer that claims moving touches is moving, the document may carry a non-passive touchmove listener that cancels only owned moves and comes off when the motion ends; while it is on, a native scroll waits for a free main thread (FR19). The gesture host, press feedback, page hold, finger-scroll watch and click guard hear document-wide input through one capture-phase listener per event type on the window (architecture:react-component-runtime INV13).
…view fixes A touch the host handles late now also leaves alone content that slid under the finger after it landed: the touch goes to what was beneath it then (spec:AST-094 FR11). The input-stream changeset says what moved: the gesture host's listeners, and the press feedback's scroll and visibility listeners, since its pointer listeners were already on the window's capture phase. The coarse-pointer gate reads TOUCH_QUERY from touchQueries.ts instead of spelling it again. The WebKit interaction job's scope covers every module the gesture host now bundles: hooks/isRtlElement and utils' flatTree, hapticSwitch, inputStream, clickGuard, selectionLock and coarsePointer. The scope test's example of a util outside it moves from clickGuard, which the gesture fixture now bundles, to focusReturn. The presented proof checks that the sheet moves on a spring (its speed changes along the animator's curve) and no longer compares the drawn box with the animator's offset: under a 3-CPU cap the two disagree by 3 to 27 px in about half the frames whatever reference is used, while the tap and drag cases go through the host's real path.
…e origin most frames agree with Under the CI lane's WebKit profile (four CPUs, one worker), "a spring keeps advancing every frame through a 1 s main-thread block" failed in two ways, each about once in a few hundred runs, and a failing run's frames show each cause: - The spring drifted 138.6 ms against the control. The control's clock took its origin from the first usable frame alone, reading that frame's direction from the next usable one: the frame sat at x 210 on its way out, the next usable one at x 210 after the turn, and the tie read as "back". The clock now lives in controlClock.ts and takes the origin most frames agree with. On that run's own frames it reads 6.5 ms (bound 12), and its unit case for that layout fails on the old reading (150.7 ms astray). - The spring "moved on 61 of the 62 frames the control moved". At the block's end it was in its tail, under two of the cast's 1 px steps a frame, where a frame delivered early shows no step and the next catches up two. The harness now reads such a tail at its own pace (busyWindowProof), and the proof judges the whole block: on that run's own frames the earlier proof fails it and this one passes. No bound changes.
… (architecture:public-component-api INV23) Main's lint rule now keeps every private data attribute in its owner module. This change had moved `data-astryx-haptic-switch` from the haptics module to utils/hapticSwitch.ts, so the click guard could skip the hidden switch without importing haptics (architecture:react-component-runtime INV13). The name now lives in privateAttributes/hapticSwitch.stylex.ts, modelled on segmentDisabled.stylex.ts: it exports markHapticSwitch, which the haptics module uses on its hidden switch, and isHapticSwitch, which the click guard reads. utils/hapticSwitch.ts and the deprecated gestures/hapticSwitch.ts re-export go, the haptics module's baseline entry goes with its raw name, and the WebKit interaction job's scope covers the new module. No behavior changes.
…trl-wheel (spec:AST-094 FR43) meta#8676's reviewed change, rebuilt as one commit on meta#8360's head: a wheel gesture drives the recognizers that allow its scroll type as a contact moving with the content, a pan along its own axis once every scroller between it and the page is at its edge, and a ctrl-wheel drives a pinch. A screen-edge pan stays a finger's.
…ain input reads as a mouse wheel, and a pan can take a swipe as one step (spec:AST-094 FR43) The round-4 trackpad behaviors, as a draft for the owner to try each one: - wheelSegmenter.ts splits the wheel stream into gestures and reads a trackpad's lift: once a run of shrinking deltas has lost a fifth of its size, momentum began. A pan the host follows ends there at the fingers' travel, released at their last move with their speed, and the momentum after it never moves the pan. Fingers that stop release still, 180 ms (vertical) or 220 ms (horizontal) after their last wheel. - Input the deltas cannot place reads as a mouse wheel: a delta a hair off a whole pixel, a scroll with no legacy wheelDelta (an engine that sends none), a legacy wheelDelta that is not three times the delta, or a 120 tick over a 40 px delta. - A pan created with wheelStep takes a trackpad swipe as one step: a short flick once the swipe travels 24 px, a slow swipe's travel carried across pauses, its slow tail absorbed, and one undo back to rest. A mouse wheel's gesture and a pinch keep their 100 ms end. In Chromium the harness's wheels arrive as notches (wheelDeltaY -120 whatever the delta), which now read as a mouse wheel, so the two trackpad-only browser cases run in WebKit, whose driver sends a trackpad's precise deltas. The lift's release is stamped at the fingers' last move: stamped at the lift, three held wheels later, it read as a stop and lost the fling.
…speed until the splitter decides, so it never stalls before a lift (spec:AST-094 FR43) The splitter reads a lift only after four shrinking deltas, so a pan that held them stood still for that stretch (about 64 to 130 ms) and then let go at the fingers' speed: a stop, then a go. Through the undecided stretch the pan now moves on from where it last was at the fingers' last speed, never past what that speed projects for the time since. At the lift it keeps that travel and lets go there, still moving at that speed, and nothing more moves. When the fingers carry on, their moves absorb what the stretch showed past their travel or short of it, half a move at a time, so the pan never steps back and ends on their travel; fingers that stop without a lift end on their travel where it runs ahead.
…t, and never a mouse wheel (spec:AST-094 FR43, DEC-24) The merged record decides it: a trackpad's two-finger scroll is direct manipulation, so a pan follows it where its record allows it; a mouse wheel's notches are a scrolling control, so a pan takes none, and a scroll without the trackpad signature, Shift+wheel included, counts as a wheel's. - A pan's allowedScrollTypes takes only 'continuous', and none by default, as UIKit's allowedScrollTypesMask: the sheet, the pager and swipe rows opt in. The host never feeds a wheel's notches to a pan, whatever a caller passes. ScrollType keeps 'discrete' for the pinch, which takes a ctrl-wheel from a trackpad or a mouse. - A scroll with Shift held reads as a mouse wheel's. - wheelStep goes: DEC-24 rejects a discrete step per trackpad swipe for rows and paging, and nothing used it. - The splitter's verification streams: ten swipes that each start in the last one's momentum, ten whose momentum runs out, and a slow drag with 150 ms pauses count ten, ten and one gestures.
The public animator (spec:AST-107 FR3, FR6): createAnimator(element) over the internal animator, with animate, decay, set, get, getVelocity, stop and destroy, from the package root; and useAnimator(ref), which takes the element's ref like the gesture hooks, so a gesture hook can share the element, and returns the animator. After every commit the hook follows the element behind the ref. The internal factory is renamed createMotionAnimator, so no internal factory shares a public factory's name. Docs, a changeset, and playing-cards templates in React and plain JavaScript, with their registrations.
…s (spec:AST-107 FR1, FR6)
The public pan: `createPanGesture(element, {axis?, onPan}) → {destroy}` and
`usePanGesture(ref, {axis?, onPan})` over it, reporting
`{state, translation, velocity}` as a drag begins, changes, ends or is
cancelled, under a finger, a mouse and a pen; the translation leaves out the
travel the drag needed to begin, so an element that follows it does not
jump. Docs pages for both, with a playing-card example in plain JavaScript
and in React, the route lock and the sandbox's routes.
…element on snap points
`createDragToSnapPoints(element, {axis, snapPoints, flickThreshold?,
onSettle}) → {settleTo, destroy}` and `useDragToSnapPoints(ref, …)` over it
(spec:AST-107 FR2, FR6): a drag moves the element along its axis under every
pointer, and the release settles it where Core's one release policy says
(the nearest point; the next one past the flick speed, 1008 px/s by default;
back to where the drag started when the release moves back toward it), on
the settle spring from the release velocity. Under Reduce Motion a touch
release cross-fades to its point and any other lands at once. Docs pages and
playing-card examples for both, the route lock and the sandbox's routes.
…n an input event's own task A caller handling an input event (a key, a click, a pointer, a touch) runs in that event's own task, never in a frame callback, so the first frame whose callbacks run after its call is the one that commits the call's animations, however long the task ran. The animator cannot tell that from a call a page's own frame callback makes before a busy task, whose browser start is right, so the caller says so with the internal option `start: 'next-frame'`; a call from rest made with it starts at that frame, and a later listener of the same event that holds the main thread delays the motion instead of letting it skip into its curve. - Chromium, which tells the page the start a frame or more after the compositor takes it: such a call starts at its first frame, as a call whose first frame comes within LATE_START_MS already does, and also when an unrelated task delayed that frame. - WebKit, which dates the start at its last frame time before the task: the task's end is read at the call's first read, not at its first microtask, which runs before a later listener's block, so the dated start moves past the block. The first read stands for the task's end, so a first frame that merely comes late (more than LATE_START_MS after the call, with no block) moves the dated start to that frame too: nothing was painted before it, so the motion is delayed by that slow frame instead of skipping into its curve. A call without the option, a call with an explicit start, and a call that continues motion keep the rule they had. Internal only: the public animator does not take it.
695ec9b to
f2795a9
Compare
|
On the description: fixed. It now cites |
There was a problem hiding this comment.
Approval waits on an engineering owner's decision about three spec edits this branch carries from #8360: the new tap rule in spec:AST-094 FR11, the document-level non-passive touchmove listener in FR19, and the single window capture-phase listener in architecture:react-component-runtime INV13. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, which the author may do if they are an engineering owner. Two changes are needed either way:
- Rebase onto
mainso this pull request holds only the animator commit, which applies cleanly there with its animator tests passing; at this head the diff is about 26,000 lines from five other open pull requests and adds six public APIs the description says it does not. - Have
usePanGestureanduseDragToSnapPointscalluseAttachmentinstead of copying its attach-and-release lifecycle.
The at guard on start: 'next-frame' resolves the earlier finding.
[Automated review]
| }); | ||
| // One pan for the node behind `ref` with this axis: re-created when either | ||
| // changes, after a commit that replaced the node. | ||
| useIsomorphicLayoutEffect(() => { |
There was a problem hiding this comment.
This Effect and the unmount one below re-implement useAttachment (packages/core/src/gestures/useAttachment.ts:68), which useGestureRecognizer and usePressTracker already call. useAttachment(ref, element => { …create the pan…; return () => { next.isForwarding = false; next.gesture.destroy(); }; }, axis) keeps one shared lifecycle.
| // creates the drag again through the factory and does nothing more | ||
| // (spec:AST-107 FR6). Destroying the old drag leaves the element where it | ||
| // is, a settle in flight included, and the new one starts there. | ||
| useIsomorphicLayoutEffect(() => { |
There was a problem hiding this comment.
Same lifecycle as useAttachment (packages/core/src/gestures/useAttachment.ts:68): attach on the node behind ref, re-create on a key change, destroy at unmount. Calling it with axis, the serialized points and flickThreshold as the key, and storing the snap in a ref for settleTo, removes the copy.
Depends on #8658 (on #8655, #8653, #8676 and #8360). This pull request's own commit: vjeux/astryx@8ec02b0...f2795a9
Status: draft. CI is running at this head. Already passed locally at this head: unit tests, lint, tsc,
check:repo, and the animator's interaction proofs in Chromium (1 and 4 workers) and in WebKit (10 repeats).User impact
A touch component that starts a motion from rest when someone presses a key, clicks, or taps should play that motion from its start, even if another listener of the same event holds the page. Today, when a later listener holds the main thread, the first frame after the block shows the motion about 100 ms into its curve, and the next frame continues from there: the start is skipped. A swipe row closed by Escape while a page's own keydown handler runs for 800 ms is one example.
No component on main uses this animator yet. Its first caller is the swipe row rebuild, #8690, stacked directly on this pull request, and it merges in that chain's order.
Problem and solution fit
LATE_START_MSof the call.Expected behavior and authority
spec:AST-094FR5: touch motion keeps advancing at display rate through a main-thread block.architecture:react-component-runtimeINV13 lists: a call from rest starts at the frame that commits it, so a block before that frame delays the motion instead of skipping into its curve.Smallest restoration
The animator's internal
startoption gains a third value beside'commit'and'presented':start: 'next-frame'.LATE_START_MSalready does.LATE_START_MSafter the call, with no block.A call without the option, a call with an explicit
at, and a call that continues motion keep the rule they had. The option is ignored withat, as the otherstartvalues are: anatcan join an earlier waiting call's start, and that call keeps its own rule.Why an option and not the event: the start policy already lives in
start, so readers find the reason at the call site beside the other start rules, and no Event object has to travel through a component's settle path. The caller states the one fact it knows: it runs in its input event's own task.The option is internal: the public animator does not take it, so the public API does not change.
Evidence
atwins;animator.test.ts. The three that start at the next frame are red without this change. The sixth checks that an explicitatjoining an earlier waiting call leaves that call's rule alone.Scope
Testing
pnpm exec vitest run packages/core/src/motion/animator.test.tspackages/core/src/motion/__tests__/animator.interaction.browser.spec.ts).