Repository navigation
fix(core): no frame shows where a touch motion is going, and a call can start on a presented frame - #8340
fix(core): no frame shows where a touch motion is going, and a call can start on a presented frame#8340vjeux wants to merge 1674 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
b769f83 to
6037617
Compare
ba93cf7 to
078a635
Compare
e9217a8 to
57cc8d7
Compare
There was a problem hiding this comment.
Changes requested: the animator work this branch adds (the hand-off of a waiting presented call, the after-block start in WebKit and the held-animation finished) fits spec:AST-094 FR5 and FR6, but the branch now merges the whole open #8360 stack, 27,058 authored lines of which 316 are this change. Please land #8360 first, or rebase once it lands, so this diff is only the animator work, and bring the gesture code it carries in line with the current specs wherever that code lands:
- Gestures must attach for every pointer (
spec:AST-094FR43, FR19), butGestureHostattaches and listens only under a coarse primary pointer (GestureHost.ts:11-28,:1403-1425). - Nothing touch-related may remain after the last instance unmounts (FR24), but the derived
touch-actionrules are never removed (touchActionRule.ts:36-45). architecture:react-component-runtimeINV13 builds every module outside the recognizer family fromcreate…factories, butPressTrackeris a class (PressTracker.ts:179).- The
@astryxdesign/buildchangeset should name the legacy form's newtreeshakeCompensation: truedefault.
[Automated review]
| this.isClaimingInstalled = isClaiming; | ||
| } | ||
|
|
||
| private shouldListen(): boolean { |
There was a problem hiding this comment.
Current spec:AST-094 FR43 says gestures attach for every pointer, a mouse, a pen and a finger, not only a coarse primary pointer, and architecture:react-component-runtime INV13 says the host attaches with no touch-only query. This gate, with the header at lines 11-28, leaves every recognizer silent under a fine pointer. FR24's fine-pointer budget (listen on the element, add document listeners only during a press) is the shape to keep instead.
| /** | ||
| * Per document or shadow root: the values it has a rule for, their rules' | ||
| * text, and where that text goes (an adopted sheet, or a style element where | ||
| * constructable sheets are missing). Rules are never removed: there are only |
There was a problem hiding this comment.
spec:AST-094 FR24 says nothing touch-related remains after the last instance unmounts (architecture:react-component-runtime INV5, INV7). Rules that are never removed leave the adopted sheet and its touch-action rules in the document or shadow root after the last recognizer lets go; remove a root's rules with its last marked element.
| return tap.numberOfTapsRequired === 1 && tap.numberOfTouchesRequired === 1; | ||
| } | ||
|
|
||
| export class PressTracker implements TouchConsumer { |
There was a problem hiding this comment.
architecture:react-component-runtime INV13 keeps classes to the recognizer family (its base, the arena and the host) and builds every other module from create… factories; its Verification row fails a class outside the recognizer family. A createPressTracker factory would meet it.
| '@astryxdesign/build': patch | ||
| --- | ||
|
|
||
| [fix] `astryxStylex` resolves `defineConsts` values read from another file. With the `{stylexOptions}` form (the one Storybook uses), a file that read only constants from a `.stylex.ts` module lost that import once StyleX compiled it, so the constants never reached the CSS: a constant used as a media-query key failed the build ("Invalid empty selector") and one used as a value shipped as a custom property nothing declares. In either form, a constant defined on the other side of the layer split, or in an Astryx package (compiled with the library class-name prefix) and read from an app file (compiled with its own), stayed unresolved. |
There was a problem hiding this comment.
This release note does not say that the legacy astryxStylex({stylexOptions}) form now compiles with treeshakeCompensation: true (packages/build/src/vite.ts:675). A caller of the released plugin should learn that the default moved and that false keeps the old compile.
|
Thanks.
|
There was a problem hiding this comment.
Changes requested: the animator work this pull request adds (the hand-off of a waiting presented call, the after-block start in WebKit and the held-animation finished) fits spec:AST-094 FR5 and FR6, but the diff still carries the whole open #8360 stack, 27,508 authored lines around a 316-line change. Please land #8360 first, or rebase onto main once it lands, so this pull request shows only its own change.
- Wherever the gesture code lands, the host must attach for every pointer as
spec:AST-094FR43 andarchitecture:react-component-runtimeINV13 require; today it takes input only under a coarse primary pointer, outside action groups (GestureHost.ts:911-921). PressTrackershould be built from acreate…factory, since INV13 keeps classes to the recognizer family (PressTracker.ts:179).
[Automated review]
| * coarse, or with `perEventPointer` (FR19; `spec:AST-057`). | ||
| */ | ||
| private isActive(recognizer: GestureRecognizer): boolean { | ||
| return this.isCoarse || this.perEventRecognizers.has(recognizer); |
There was a problem hiding this comment.
Current spec:AST-094 FR43 says gestures attach for every pointer, a mouse, a pen and a finger, not only a coarse primary pointer, and architecture:react-component-runtime INV13 says the host attaches with no touch-only query. This gate (with isConsumerActive below and the header at lines 11-28) leaves every recognizer outside an action group and without perEventPointer silent under a fine pointer. FR24's element-scoped, press-only listening is the budget a fine pointer gets instead.
| return tap.numberOfTapsRequired === 1 && tap.numberOfTouchesRequired === 1; | ||
| } | ||
|
|
||
| export class PressTracker implements TouchConsumer { |
There was a problem hiding this comment.
architecture:react-component-runtime INV13 keeps classes to the recognizer family (its base, the arena and the host) and builds every other module from create… factories, naming only the velocity tracker as a known gap; its Verification row fails a class outside the recognizer family. A createPressTracker factory would meet it.
…esture host's subscriptions The gesture host subscribes no click: the one cancelling click subscriber is the click guard's, added at a window's first arm and kept. The release note, the stream's header and the gestures README listed click with the host's touchend and contextmenu; they now name the guard's own.
…task ended at once, however late the first read WebKit under load from other tasks: the call's task ends at once, another task holds the main thread, and the browser commits the animation after it, dated at the call. Nothing in the call's task delayed it, but the first read still moved its start to the read, about 95 ms late on a loaded box, so whatever chains on its end (the second half of a Reduce Motion cross-fade, a cancel fading on the previous fade's end) ran that much late and the cross-fade missed design:reduced-motion DR2's 200 ms. Fails now: the start moves to 1100.
e7b0676 moved a call's start from rest to its first read whenever WebKit had dated it more than two frames before that read. Under load from other tasks the first read comes late with nothing in the call's task delaying it, so the start moved about 95 ms on a loaded box, and whatever chained on the call's end ran that late: the second half of a Reduce Motion cross-fade, so SearchField's cancel missed DR2's 200 ms. The call's task now records when it ended, at its first microtask (or a read in those microtasks is that time), and only a call whose own task ran past the call by more than two frames, the block e7b0676 fixed, moves its start; a read other tasks delay keeps the browser's start.
…s its start past the block Core Animation dates an animation at the commit, when the calling task ends, however short the block before it. A 20 ms block in the call's task, with WebKit's start dated at the call, left the start at the call: the two-frame bound skipped it. Fails now.
… task, with no threshold Core Animation dates an animation at the commit, when the calling task ends, whatever the block's length. The call's task already records when it ended, at its first microtask, so a start the browser dated before that (WebKit's last frame time before the task) now moves to it: a task that ended at once leaves the start at the call, a blocked one moves it past the block, however short, and a read other tasks delay changes nothing. The two-frame bound, which told a read's own frame from a block before the task's end was known, goes, and the start no longer moves to the read.
…ame window Both 'a dip held before its first frame lands once the hold is …' proofs waited for no running animation, then one frame and a timeout, assuming the next step starts within a frame. The dip lands on a timer due on the call's own clock, though, and the proofs passed only because the evaluate round trips covered the gap: in WebKit on a loaded box the window closed first and the proof read the element mid-dip (opacity 0, translate none; reproduced 1 in 4 with the round trips removed). They now wait on the call's own finished, then on the browser's own animations still running or waiting for their first frame, and read nothing from the animator.
…ment at once and settles then A Reduce Motion dip whose landing runs more than its whole fade back in late (a hold released long after the dip was due) played a 1 ms opacity animation, which WebKit keeps pending until its next frame, and settled the call on a 1 ms timer: the call's finished could resolve while the element still showed opacity 0. The animator's notes say the element appears at once there. Fails now: the landing plays one opacity animation and the call has not settled.
…element at once and settles A Reduce Motion landing that runs at least the whole fade back in late had nothing left of the cross-fade, but still played a 1 ms opacity animation and settled the call 1 ms later. WebKit keeps that animation pending until its next frame, so the call's finished could resolve with the element still at opacity 0. Such a landing now writes the end opacity at once (handing a borrowed one back to the stylesheet), plays nothing, and settles the call then, as the animator's notes say.
There was a problem hiding this comment.
Changes requested: the animator work this pull request adds (the end-of-task start in WebKit, the hand-off of a waiting presented call and the held-animation finished) fits spec:AST-094 FR5 and FR6, but the diff still carries the whole open #8360 stack, 27,546 authored lines around a 354-line change. Please land #8360 first, or rebase onto main once it lands, so this pull request shows only its own change.
- Wherever the gesture code lands, the host must attach for every pointer as
spec:AST-094FR43,architecture:react-component-runtimeINV13 andarchitecture:interaction-modalityINV11 require; today it takes input, hover included, only under a coarse primary pointer, outside action groups (GestureHost.ts:911-921,:1459). PressTrackershould be built from acreate…factory, since INV13 keeps classes to the recognizer family (PressTracker.ts:179).
[Automated review]
…few ms, and one task's calls start together Browser start times are frame times, while a task's end is a live clock, so on every call they differ by up to a frame plus the task's own length. Moving every start from rest to its task's end shifted each one by that length, about 5 ms in a component's effect after a tap, and the shift added up along a chained sequence: SearchField's Reduce Motion cancel measured 204-207 ms against design:reduced-motion DR2's 200. The first case fails now (the start moves to 1010). The second keeps one motion's parts together when they are separate calls in one task: a block measured per call would split them by a frame.
…n on more than two frames, and all the task's calls move together fc42d64 moved every start from rest to the end of the call's task. Browser start times are frame times while that end is a live clock, so on every call they differ by up to a frame plus the task's own length: each start slid by that length, and a chained sequence added it up, about 5 ms per link in SearchField. The rule from e6876ca is back, with its bound now named for that quantization: only a task that ran on more than LATE_START_MS (two frames) past its first call blocked anything, and its calls' starts move to their first read. The block is now measured per task, not per call, and every call of a blocked task takes the same read, so one motion's parts that are separate calls in one task (a component's effect) start together instead of splitting by a frame. The 20 ms case keeps the browser's start, as no clock here tells it from quantization.
… judges its first frame on that frame's time 'a spring started right before a block plays in full after it' waited a fixed 300 ms of wall clock after the block, then read the screencast. In the full interaction run on a loaded box no moving frame had reached the screencast by then, and the proof failed on an empty read. It now waits until the page shows the box a quarter of the way along and three more frames reach the screencast. Its bound changes form, not strength: the curve's own start (the handle's started) must come after the block, and the first moving frame may be no further along than a curve started a frame before the block ended, at that frame's own time, which is the old 30 px for a frame right after the block and still holds when a loaded machine sends frames sparsely. A curve dated at the call is 107 px in.
Takes meta#8360's head: the held-dip proofs wait on the call's finished and a late landing shows the element at once (88a1866), and the spring-before-a-block proof waits for the motion (d0f5fd5). In the animator spec the proof keeps meta#8360's waits and bounds and runs in WebKit too, as this pull request's start-after-block makes it pass there. # Conflicts: # packages/core/src/motion/__tests__/animator.interaction.browser.spec.ts
…tor's notes drop an undefined law The animator spec's header cited 'law M1' for compositor-only motion and useAnimator.ts cited 'law M4' for an animator that never re-renders, and no document defines either. The spec now cites spec:AST-094 FR5, which keeps touch motion to transform, translate, scale, rotate and opacity; useAnimator's sentence says what it means without the citation.
There was a problem hiding this comment.
Changes requested: the animator's narrower WebKit start fits the touch-presentations spec, but this branch still carries the whole open #8360 stack (27,654 authored lines, 462 of them this change's own), whose gesture host attaches only under a coarse primary pointer and whose PressTracker is a class, against spec:AST-094 FR43 and architecture:react-component-runtime INV13.
- Land #8360 with those two conformed, or rebase once it lands, so this diff is only the animator change.
- A
start: 'presented'call is measured from its call time rather than the frame callback that creates its animations (animator.ts:1978,:2150), so in WebKit whether it keeps its clock depends on where in the frame it was called; please measure from that callback and add a WebKit-dated presented test.
[Automated review]
| @@ -1446,6 +1974,11 @@ export function createAnimator( | |||
| return; | |||
| } | |||
| pendingGroups.add(group); | |||
| group.fresh = true; | |||
| group.task = markTask(group.provisional, now); | |||
There was a problem hiding this comment.
This marks the task with group.provisional, which for a start: 'presented' call is its call time, two frames before this frame callback creates its animations (:1935); startAfterBlock compares the browser's start against the same time (:2150). In WebKit on a 60 Hz screen, a presented call made at 1001 whose frame callback ends at 1036 passes the 34 ms task test (35 > 34), so a first read that another task delays to 1140 moves its start to 1140, while the same call made at 1003 keeps 1034. When the two frames span more than 34 ms, :2150 keeps WebKit's frame-dated start even after a block in this callback. Dating the mark and that comparison from this callback would make a presented call follow the rule the header states.
…ide lands when its retire runs first Reported as a WebKit hang of the held-dip proof under an 8-worker run: the proof's wait for the call's finished never settled. Instrumented, the page kept drawing frames (about 60 a second) while the element sat at opacity 0 where it started, with no animation left: the held fade out had been finished from outside, then retired (cancelled) by its timer before the frame that reads the call's start, so the call never started and its landing was never armed. This case orders the retire timer before that frame and fails on this head: the move never lands.
… planned once A Switch's press plays the same two springs from the same values on every press, and on a phone at 6x CPU throttling planning their keyframes again is the largest part of the press's task: 33-66 ms on a fresh page, 11-14 ms warm. The animator now counts the tracks it plans (plannedTrackCount, internal). Fails now: two identical moves plan twice.
…e keyframes of an identical one A control's press plays the same springs from the same values every time, and planning their keyframes was the press's largest cost on a slow phone. A track is now planned from 0 at its start, so the same curves give the same keyframes whenever they play, and a spring segment names what fixes its shape (its spring, from, to, velocity and length). A track whose curves all name theirs keys a cache on those, each curve's offset, the values of its channel's other properties, its length and the device pixel size, and reuses the keyframes and span planned for it before; 256 plans are kept, the least recently used going first. Curves that name nothing (a played curve, an additive retarget) are planned each time, as before.
… that cannot come [skip ci] The retire timer cancels a finished animation once the main thread runs past its curve. When a sampler finished a call's animations before the browser started them, a loaded page could run that timer before the frame that reads the call's start: the cancel left only idle animations, the start was never read, and the call's Reduce Motion landing was never armed, so the element stayed hidden where it started and the call never settled. The retire now reads such a call's start from the finished animation before cancelling it, and with nothing left to wait on the call keeps its own clock, so its landing still runs.
There was a problem hiding this comment.
Changes requested: the newly merged late-landing fix and the animator's own start-rule work fit the touch-presentations and reduced-motion specs, but this branch still carries the whole open #8360 stack (27,768 authored lines, 471 of them this change's own), whose gesture host attaches only under a coarse primary pointer and whose PressTracker is a class, against spec:AST-094 FR43 and architecture:react-component-runtime INV13.
- Land #8360 with those two conformed, or rebase once it lands, so this diff is only the animator change.
- A
start: 'presented'call is still measured from its call time rather than the frame callback that creates its animations (animator.ts:1978,:2150); please measure from that callback, add a WebKit-dated presented test, and update the motion README's note on WebKit's start (packages/core/src/motion/README.md:129-132).
[Automated review]
| delays the motion instead of skipping into it (WebKit starts it at the | ||
| frame before the block). Everything the call starts shares that start |
There was a problem hiding this comment.
This note still says WebKit starts a call from rest at the frame before a block. At this head a block of more than 34 ms in the call's own task moves WebKit's start past it (animator.ts:2141-2173, pinned by animator.test.ts:531), so the module's own reference misdescribes the rule. Please state the 34 ms rule here, as the header at animator.ts:79-86 does.
…in its frame the call was made A presented call creates its animations in its second frame's callback, but its block was measured from the call: a call made at the start of its frame looked like a task of over two frames, and a late first read (other tasks holding the main thread until the next frame) moved its WebKit start there. Fails now: the start moves from 1033 to 1090. A guard: a block in the frame that creates the animations still moves their start past it.
…animations [skip ci] A presented call creates its animations in its second frame's callback, but the block that moves a WebKit start was measured from the call, so a call made at the start of its frame could look like a task of over two frames, and a late first read moved its start. The task mark now opens when a call's animations are created, and 'a start well after the call' is measured from that mark too; a call that is not presented creates its animations in its own task, so nothing changes for it. The motion README's note on WebKit's start says what the animator does now.
There was a problem hiding this comment.
Changes requested: measuring a presented call's block from the frame that creates its animations fits the touch-presentations spec, but this branch still carries the whole open #8360 stack (28,184 authored lines, 857 of them this change's own), whose gesture host attaches only under a coarse primary pointer and whose PressTracker is a class, against spec:AST-094 FR43 and architecture:react-component-runtime INV13.
- Land #8360 with those two conformed, or rebase once it lands, so this diff is only the animator change; the keyframe reuse would fit a pull request of its own.
- The new guard case creates its animations 33 ms after its call, so it passes with or without the new “start well after” check (
packages/core/src/motion/animator.ts:2256); a case whose creating frame comes more than 34 ms after the call, as after a dropped frame, would pin it.
[Automated review]
| } | ||
| }); | ||
|
|
||
| it('presented: a block in the frame that creates the animations moves their start past it', async () => { |
There was a problem hiding this comment.
This guard creates its animations at 1033, 33 ms after the call at 1000, so it passes with the old start > group.provisional + LATE_START_MS check at animator.ts:2256 as well as with the new one, and no case pins the frame-span half of this push's fix. With the creating frame more than 34 ms after the call (a second frame at 1050, one dropped while the call waits), the call-time check keeps WebKit's 1050 and this case would fail on it.
Depends on #8360, which carries #8076. Review only this PR's change: vjeux/astryx@3fb31dd...11c3536, vjeux/astryx@339a352...26ecfe7, vjeux/astryx@85f95b0...0e0946c and vjeux/astryx@1042d61...9610ad3; the merges take #8360's head and main.
Why
On iOS, an alert closed right after opening flashed for two frames where it was going, and a motion started while the page was busy could lose its first frames or, in Safari, start late. On the simulator a search field cancelled mid-activation stepped back or turned hard in 7 of 8 runs.
What led to it
This comes from making Astryx's touch components behave as native iOS ones do.
spec:AST-094FR5 asks motion to start at its commit and play whole, delayed rather than skipped or flashing when the main thread or compositor is busy.Options and tradeoffs
Why this choice
The animator owns every touch motion, so it is the one place that sees the commit. On the simulator, the additive reversal turned the search field smoothly in 7 of 8 runs, against 1 of 8 when replacing.
What changes
opacity: 0stays visible.start: 'presented'andstart: 'commit'let a motion start when it reaches the screen; in Safari a start from rest moves past a block of over two frames (browser start times are frame times) in the task that created it, a presented call's second frame included, so the motion is delayed instead of skipping, and that task's calls move together.additive: truefor reversals;handle.curve(property)for companion animations.finishedwaits while something holds its animation (paused or sought back) instead of settling on a timer.How to check it
pnpm test:interaction animator.interaction.browser.spec.tsin Chromium and WebKit: frames before the compositor has the curve, and after a blocked main thread, show the motion from its start.Evidence