Repository navigation
fix(core): a channel rebuilt after a late first frame carries on from the screen - #9637
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
PR Analysis Report🔎 Preview ·
|
fbaec3e to
df98ca7
Compare
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
The animator fix itself holds up: the six new and changed cases fail against the base animator and pass here, and the change restores the no-jump retarget that spec:AST-094 FR6 requires. Before approval, please correct the description and the commit message: on this branch and on main no component uses the animator yet, so the Switch snap-back, its P14 reproduction and the Switch.interaction.browser.spec.ts command belong to #9636, not to this change. Say that the animator drives no shipped component yet, as #9401 did, and write beginGroup where the description says findGroup; the title and changeset can stay as they are.
[Automated review]
|
|
||
| /** | ||
| * When a waiting curve's call had its first frame, which put the curve on | ||
| * screen. Null for a curve that does not wait or whose frame has not run. |
There was a problem hiding this comment.
Non-blocking: shownSince returns undefined, not null. Suggest: "Undefined for a curve that does not wait, or whose call has had no first frame."
… the screen After a call's first frame came late and left its start unread, the animator records that frame's time per channel and reads the curve from it. A rebase, a set or a retarget that starts now could still rebuild one of those channels: the rebuilt animation joined the waiting call and started at the next commit from its first keyframe, so the screen stepped back and replayed the curve. The animator drives no shipped component yet. Such a rebuild now carries the channel on. Its curves leave the waiting call and take a start of their own, dated from that frame as the browser's start would date them, and the rebuilt animation starts now from the value on screen. A rebase builds its new spring from that value at the rebase's own time; another curve keeps its course. A call that starts while such a channel is on screen counts as continuing motion, so it starts now instead of waiting for its own commit. A curve on screen also ends where that frame dates it, so a call that waits past its own time's end keeps it. A waiting call left with no channel starts at that frame, which its handle reports. The call's other channels keep waiting for the browser's start, which still decides for them. A call that joins the waiting call by its clock still rebuilds the channel from its first keyframe at the next commit.
df98ca7 to
baa77fe
Compare
|
Fixed in baa77fe: the description and the commit message now say the animator drives no shipped component yet, as #9401 did. The Switch snap-back, its P14 reproduction and the |
There was a problem hiding this comment.
The carry-on fix holds up: the six new and changed animator cases fail against the base and pass here, the Chromium animator browser spec passes, and the change restores the no-jump retarget that spec:AST-094 FR6 requires. The description and commit message now match what the change reproduces. Two optional comment suggestions are inline, one noting that attach() of a new node takes the same branch without a test; it looks ready once it leaves draft.
[Automated review]
| } | ||
| const moving = properties.filter(p => segments.has(p)); | ||
| if (callGroup == null) { | ||
| // A call that starts now rebuilds a channel the compositor has moving |
There was a problem hiding this comment.
Non-blocking: this branch runs for every rebuild with no call group, so attach() of a new node takes it too (line 3382); the comment names only a call that starts now. Suggest: "A rebuild outside a call from rest can meet a channel that moves on screen since a first frame with no known start. That channel carries on from the frame, with its own start." A unit case for the attach path would pin it.
| // A curve still waiting for the browser's start stays in its call: | ||
| // it starts with that call's commit. | ||
| // it starts with that call's commit. A curve whose call had its first | ||
| // frame is on screen already, so it leaves the call (leaveWaitingCall). |
There was a problem hiding this comment.
Non-blocking: "(leaveWaitingCall)" repeats the name of the function this block calls below. Suggest: "A curve whose call had its first frame is on screen already, so it leaves the call."
cixzhang
added
the
ci:run
User impact
A follow-up to #9401 (merged) in the touch presentations' animator, which is internal and drives no shipped component yet, so the changeset is
[chore]. When a call's first frame comes late and leaves its start unread, #9401 reads the call from that frame. A rebase, a set or a retarget that starts now could still rebuild one of that call's channels in that window: the rebuilt animation then started from its first keyframe at the next commit, so the screen stepped back and replayed the curve.Problem and solution fit
rebase()builds its new spring from the value on screen at its own time, shifted by its delta, and starts it then. Another curve, such as a coast, keeps its course, dated from that frame.set()of another property rebuilds the channel, and the carried curve continues.startedreports that time.start: 'commit'), which no shipped caller passes.Expected behavior and authority
spec:AST-094FR6, as #9255 and #9401 hold it: a retarget made before the page can read a call's start begins where the screen shows the element, and the screen never steps back.Smallest restoration
Within the animator: a new
leaveWaitingCalltakes a curve on screen out of its waiting call, re-dated from that call's first frame, and starts a call left with no channel at that frame;rebuild, for a call that starts now, andrebaseuse it;beginGrouptreats such a channel as moving; andendOfdates a waiting curve's end from that frame forsegmentAt,rebuild's settled curves andbeginGroup. Nothing else changes: the first-frame recording, the reads of #9401, the explicit-clock join,resolveGroupand the start of a call from rest are as in #9401.Evidence
animator.test.tschange from fix(core): a retarget after a late first frame continues from the screen #9401's restart to carrying on, and three are new; each fails onmainwith fix(core): a retarget after a late first frame continues from the screen #9401 (4a7a6f2) and passes here:{x: 300}, a first frame at 1070 with the start unread, andrebase({x: -16})at 1170. The rebuilt animation starts at 1170 from the value on screen less 16; the read, the inline value and a zero presented offset agree; a frame later the read and a retarget follow the rebuilt animation.set({y: 10}); x keeps its value and velocity.animate({y: 50}); the call starts at 1170, x carries on and y starts from rest.startedresolves at 1070.set({y: 10})30 ms past the end counted from the call's own time keeps x on its curve, short of its target.Status: this head differs from df98ca7 only in one comment. On this head the animator's unit file (215), Prettier and strict lint pass. On df98ca7, on
mainwith #9401, the type check and the unit suites of the motion, Switch and Slider modules (915) pass. On the same commit on #9401's last head (fbaec3e): build, type check,test:ui(17,809),check:repo,ASTRYX_STRICT_LINT=1 pnpm lint(0 errors),check:changesetsand changeset coverage pass; the node suites pass but for one CLI test (doctor --built) that timed out at 30 s under load and passes 3 of 3 alone; the animator browser spec passes ×3 in Chromium pinned to 2 cores (54 of 54) and ×10 under the CI WebKit job's command (140 passed, 40 skipped by design).Scope
Testing