detached 5e847fd

fix(daemon): cap copy-scroll frame step and hold animation while output window is full

commit
5e847fd861f85521a60bd7863ac0f1c1878d554a
author
brice · · 2026-10-08 09:38 UTC
committer
brice · · 2026-10-08 09:57 UTC
parent
63791bf
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
4 files · +183 -18
M docs/performance.md +7 -0
@@ -226,6 +226,13 @@ The unadorned live-pane cache stays separate from the displayed copy viewport.
226226 Neither committed frame is mutated while preparing a candidate; failed output
227227 cannot publish its viewport metadata. Copy exit refreshes live content.
228228
229+The animation moves at most 9 rows per 16 ms frame, and waits while the
230+attachment's output window is full of unacknowledged frames. On a slow remote
231+link the scroll pauses and resumes with the same step sizes instead of
232+catching up in one jump. `TestCopyScrollOverDelayedAcks` replays a flick and a
233+sustained scroll over 0–400 ms acknowledgement delays and logs the rows moved
234+per frame.
235+
229236 Mouse animation is tested separately with injected clocks: first response,
230237 16 ms pacing, burst accumulation, deceleration, reversal, cancellation, and the
231238 120 ms tail deadline. The rendering benchmark deliberately bypasses those timers
@@ -226,6 +226,13 @@ The unadorned live-pane cache stays separate from the displayed copy viewport.
226 Neither committed frame is mutated while preparing a candidate; failed output226 Neither committed frame is mutated while preparing a candidate; failed output
227 cannot publish its viewport metadata. Copy exit refreshes live content.227 cannot publish its viewport metadata. Copy exit refreshes live content.
228 228
229+The animation moves at most 9 rows per 16 ms frame, and waits while the
230+attachment's output window is full of unacknowledged frames. On a slow remote
231+link the scroll pauses and resumes with the same step sizes instead of
232+catching up in one jump. `TestCopyScrollOverDelayedAcks` replays a flick and a
233+sustained scroll over 0–400 ms acknowledgement delays and logs the rows moved
234+per frame.
235+
229 Mouse animation is tested separately with injected clocks: first response,236 Mouse animation is tested separately with injected clocks: first response,
230 16 ms pacing, burst accumulation, deceleration, reversal, cancellation, and the237 16 ms pacing, burst accumulation, deceleration, reversal, cancellation, and the
231 120 ms tail deadline. The rendering benchmark deliberately bypasses those timers238 120 ms tail deadline. The rendering benchmark deliberately bypasses those timers
M internal/usecase/daemon/copy_scroll_animation.go +26 -16
@@ -9,6 +9,9 @@ import (
99 const (
1010 copyScrollFrame = 16 * time.Millisecond
1111 copyScrollTail = 120 * time.Millisecond
12+ // copyScrollMaxStep bounds one animation frame so a long backlog (a stalled
13+ // remote link, a big flick) is drained over several frames, not one jump.
14+ copyScrollMaxStep = 9
1215 )
1316
1417 // copyMu owns the animation and its single cancellable overlay-input timer.
@@ -74,22 +77,29 @@ func (d *Daemon) advanceCopyScrollLocked(sess *session, ac *attachedClient) (boo
7477 if remaining == 0 {
7578 return false, false
7679 }
77- magnitude := remaining
78- if magnitude < 0 {
79- magnitude = -magnitude
80- }
81- step := (magnitude + 3) / 4
82- if d.clock.Now().Sub(motion.lastInput) >= copyScrollTail {
83- step = magnitude
84- }
85- if remaining < 0 {
86- step = -step
87- }
88- motion.remaining -= step
89- changed, exit := rt.moveCopyWheelLocked(step)
90- if !changed || exit {
91- rt.stopCopyScrollLocked()
92- return changed, exit
80+ // While the client has a full window of unacknowledged frames, moving the
81+ // viewport would only pile rows into one catch-up jump when the window
82+ // reopens. Hold the animation and keep polling at frame pace instead.
83+ changed, exit := false, false
84+ if ac.output == nil || !ac.output.atCapacity() {
85+ magnitude := remaining
86+ if magnitude < 0 {
87+ magnitude = -magnitude
88+ }
89+ step := (magnitude + 3) / 4
90+ if d.clock.Now().Sub(motion.lastInput) >= copyScrollTail {
91+ step = magnitude
92+ }
93+ step = min(step, copyScrollMaxStep)
94+ if remaining < 0 {
95+ step = -step
96+ }
97+ motion.remaining -= step
98+ changed, exit = rt.moveCopyWheelLocked(step)
99+ if !changed || exit {
100+ rt.stopCopyScrollLocked()
101+ return changed, exit
102+ }
93103 }
94104 if motion.remaining != 0 {
95105 mode := rt.copyMode
@@ -9,6 +9,9 @@ import (
9 const (9 const (
10 copyScrollFrame = 16 * time.Millisecond10 copyScrollFrame = 16 * time.Millisecond
11 copyScrollTail = 120 * time.Millisecond11 copyScrollTail = 120 * time.Millisecond
12+ // copyScrollMaxStep bounds one animation frame so a long backlog (a stalled
13+ // remote link, a big flick) is drained over several frames, not one jump.
14+ copyScrollMaxStep = 9
12 )15 )
13 16
14 // copyMu owns the animation and its single cancellable overlay-input timer.17 // copyMu owns the animation and its single cancellable overlay-input timer.
@@ -74,22 +77,29 @@ func (d *Daemon) advanceCopyScrollLocked(sess *session, ac *attachedClient) (boo
74 if remaining == 0 {77 if remaining == 0 {
75 return false, false78 return false, false
76 }79 }
77- magnitude := remaining80+ // While the client has a full window of unacknowledged frames, moving the
78- if magnitude < 0 {81+ // viewport would only pile rows into one catch-up jump when the window
79- magnitude = -magnitude82+ // reopens. Hold the animation and keep polling at frame pace instead.
80- }83+ changed, exit := false, false
81- step := (magnitude + 3) / 484+ if ac.output == nil || !ac.output.atCapacity() {
82- if d.clock.Now().Sub(motion.lastInput) >= copyScrollTail {85+ magnitude := remaining
83- step = magnitude86+ if magnitude < 0 {
84- }87+ magnitude = -magnitude
85- if remaining < 0 {88+ }
86- step = -step89+ step := (magnitude + 3) / 4
87- }90+ if d.clock.Now().Sub(motion.lastInput) >= copyScrollTail {
88- motion.remaining -= step91+ step = magnitude
89- changed, exit := rt.moveCopyWheelLocked(step)92+ }
90- if !changed || exit {93+ step = min(step, copyScrollMaxStep)
91- rt.stopCopyScrollLocked()94+ if remaining < 0 {
92- return changed, exit95+ step = -step
96+ }
97+ motion.remaining -= step
98+ changed, exit = rt.moveCopyWheelLocked(step)
99+ if !changed || exit {
100+ rt.stopCopyScrollLocked()
101+ return changed, exit
102+ }
93 }103 }
94 if motion.remaining != 0 {104 if motion.remaining != 0 {
95 mode := rt.copyMode105 mode := rt.copyMode
M internal/usecase/daemon/copy_scroll_animation_test.go +16 -2
@@ -98,12 +98,26 @@ func TestCopyScrollAnimationTailDeadline(t *testing.T) {
9898 start := rt.copyMode.ViewportTop
9999 rt.copyScroll.remaining = -60
100100 rt.copyScroll.lastInput = time.Time{}.Add(-copyScrollTail)
101+ // Past the tail deadline the whole backlog is due, but one frame still
102+ // moves at most copyScrollMaxStep rows so a stalled link cannot cause a jump.
101103 changed, exit := f.d.advanceCopyScrollLocked(f.sess, f.ac)
102104 require.True(t, changed)
103105 require.False(t, exit)
104- require.Equal(t, start-60, rt.copyMode.ViewportTop)
106+ require.Equal(t, start-copyScrollMaxStep, rt.copyMode.ViewportTop)
107+ require.Equal(t, -(60 - copyScrollMaxStep), rt.copyScroll.remaining)
108+ rt.copyMu.Unlock()
109+ for frame := 2; frame <= 7; frame++ {
110+ timer := awaitCoordinatorScheduledTimer(t, clock)
111+ want := start - min(frame*copyScrollMaxStep, 60)
112+ timer.ch <- time.Time{}
113+ require.Eventually(t, func() bool {
114+ rt.copyMu.Lock()
115+ defer rt.copyMu.Unlock()
116+ return rt.copyMode.ViewportTop == want
117+ }, time.Second, time.Millisecond, "frame %d", frame)
118+ }
119+ rt.copyMu.Lock()
105120 require.Zero(t, rt.copyScroll.remaining)
106- require.Nil(t, rt.copyScroll.timer.timer)
107121 rt.copyMu.Unlock()
108122 }
109123
@@ -98,12 +98,26 @@ func TestCopyScrollAnimationTailDeadline(t *testing.T) {
98 start := rt.copyMode.ViewportTop98 start := rt.copyMode.ViewportTop
99 rt.copyScroll.remaining = -6099 rt.copyScroll.remaining = -60
100 rt.copyScroll.lastInput = time.Time{}.Add(-copyScrollTail)100 rt.copyScroll.lastInput = time.Time{}.Add(-copyScrollTail)
101+ // Past the tail deadline the whole backlog is due, but one frame still
102+ // moves at most copyScrollMaxStep rows so a stalled link cannot cause a jump.
101 changed, exit := f.d.advanceCopyScrollLocked(f.sess, f.ac)103 changed, exit := f.d.advanceCopyScrollLocked(f.sess, f.ac)
102 require.True(t, changed)104 require.True(t, changed)
103 require.False(t, exit)105 require.False(t, exit)
104- require.Equal(t, start-60, rt.copyMode.ViewportTop)106+ require.Equal(t, start-copyScrollMaxStep, rt.copyMode.ViewportTop)
107+ require.Equal(t, -(60 - copyScrollMaxStep), rt.copyScroll.remaining)
108+ rt.copyMu.Unlock()
109+ for frame := 2; frame <= 7; frame++ {
110+ timer := awaitCoordinatorScheduledTimer(t, clock)
111+ want := start - min(frame*copyScrollMaxStep, 60)
112+ timer.ch <- time.Time{}
113+ require.Eventually(t, func() bool {
114+ rt.copyMu.Lock()
115+ defer rt.copyMu.Unlock()
116+ return rt.copyMode.ViewportTop == want
117+ }, time.Second, time.Millisecond, "frame %d", frame)
118+ }
119+ rt.copyMu.Lock()
105 require.Zero(t, rt.copyScroll.remaining)120 require.Zero(t, rt.copyScroll.remaining)
106- require.Nil(t, rt.copyScroll.timer.timer)
107 rt.copyMu.Unlock()121 rt.copyMu.Unlock()
108 }122 }
109 123
A internal/usecase/daemon/copy_scroll_remote_test.go +134 -0
@@ -0,0 +1,134 @@
1+package daemon
2+
3+import (
4+ "fmt"
5+ "strings"
6+ "testing"
7+ "time"
8+
9+ "github.com/bnema/vev/internal/domain"
10+)
11+
12+// TestCopyScrollOverDelayedAcks measures how a wheel flick looks on a link
13+// whose acknowledgements arrive rtt late. One tick is one animation frame
14+// (copyScrollFrame). It logs, per tick, the rows jumped by the frame the client
15+// would see, so stalls (0) and catch-up jumps (>1) are visible.
16+func TestCopyScrollOverDelayedAcks(t *testing.T) {
17+ // flick: 12 notches at once. sustained: one notch per tick for 30 ticks.
18+ for _, scenario := range []struct {
19+ name string
20+ upfront, perTick int
21+ }{{"flick", 12, 0}, {"sustained", 1, 1}} {
22+ for _, rtt := range []time.Duration{0, 50 * time.Millisecond, 100 * time.Millisecond, 200 * time.Millisecond, 400 * time.Millisecond} {
23+ t.Run(scenario.name+"/"+rtt.String(), func(t *testing.T) {
24+ f := newPerformanceFixture(t, performanceConfig{size: domain.Size{Cols: 80, Rows: 24}, panes: 1, historyRows: 400})
25+ f.d.enterCopyMode(f.sess, f.ac)
26+ f.d.copyWheel(f.sess, f.ac, -150)
27+ f.ac.ackOutputState(f.ac.output.currentEpoch(), f.ac.output.next)
28+ clock := newCoordinatorMockClock(t, 64)
29+ f.d.clock = clock.clock
30+ rt := f.ac.overlays
31+
32+ type ack struct {
33+ at int
34+ epoch, seen uint64
35+ }
36+ var (
37+ acks []ack
38+ tick int
39+ emitted = make(chan struct{}, 64)
40+ )
41+ // emit runs under sendMu: only signal here, read ACK coordinates later.
42+ f.ac.renderStages.emit = func() { emitted <- struct{}{} }
43+ drainTimers := func() *coordinatorMockTimer {
44+ var frame *coordinatorMockTimer
45+ for {
46+ select {
47+ case tm := <-clock.timers:
48+ if tm.duration == copyScrollFrame {
49+ frame = tm
50+ }
51+ default:
52+ return frame
53+ }
54+ }
55+ }
56+ viewport := func() int {
57+ rt.copyMu.Lock()
58+ defer rt.copyMu.Unlock()
59+ return rt.copyMode.ViewportTop
60+ }
61+ pending := func() int {
62+ rt.copyMu.Lock()
63+ defer rt.copyMu.Unlock()
64+ return rt.copyScroll.remaining
65+ }
66+ delivered := func() bool {
67+ select {
68+ case <-emitted:
69+ f.ac.sendMu.Lock()
70+ epoch, state := f.ac.output.currentEpoch(), f.ac.output.next
71+ f.ac.sendMu.Unlock()
72+ acks = append(acks, ack{at: tick + int((rtt+copyScrollFrame-1)/copyScrollFrame), epoch: epoch, seen: state})
73+ return true
74+ case <-time.After(15 * time.Millisecond):
75+ return false
76+ }
77+ }
78+
79+ start := viewport()
80+ last := start
81+ maxJump := 0
82+ var jumps []string
83+ // A fast flick: 12 notches of 3 rows arrive in the first tick.
84+ fed := 0
85+ feed := func(n int) {
86+ for range n {
87+ f.d.smoothCopyWheel(f.sess, f.ac, -3)
88+ fed++
89+ }
90+ }
91+ feed(scenario.upfront)
92+ shown := func() {
93+ if delivered() {
94+ top := viewport()
95+ jumps = append(jumps, fmt.Sprint(last-top))
96+ maxJump = max(maxJump, last-top)
97+ last = top
98+ } else {
99+ jumps = append(jumps, "-")
100+ }
101+ }
102+ shown()
103+ for ; tick < 80 && (pending() != 0 || len(acks) != 0 || (scenario.perTick > 0 && tick < 30)); tick++ {
104+ if tick < 30 {
105+ feed(scenario.perTick)
106+ }
107+ for len(acks) > 0 && acks[0].at <= tick {
108+ f.ac.ackOutputState(acks[0].epoch, acks[0].seen)
109+ if rc := f.sess.renderCoordinator(); rc != nil {
110+ rc.notifyAck()
111+ }
112+ acks = acks[1:]
113+ }
114+ // With a full window no timer is armed: the server is stalled
115+ // until an ACK arrives, so keep ticking to let it release.
116+ if frame := drainTimers(); frame != nil {
117+ frame.ch <- time.Time{}
118+ }
119+ shown()
120+ }
121+ t.Logf("%s rtt=%-6v rows per tick (- = nothing sent): %s", scenario.name, rtt, strings.Join(jumps, " "))
122+ // A slow link may delay frames but must not make the picture jump
123+ // further than one animation frame allows.
124+ if maxJump > copyScrollMaxStep {
125+ t.Errorf("one frame jumped %d rows, want at most %d", maxJump, copyScrollMaxStep)
126+ }
127+ // Whatever the link, every requested row is eventually shown.
128+ if got, want := start-viewport(), 3*fed; got != want {
129+ t.Fatalf("scrolled %d rows, want %d", got, want)
130+ }
131+ })
132+ }
133+ }
134+}
@@ -0,0 +1,134 @@
1+package daemon
2+
3+import (
4+ "fmt"
5+ "strings"
6+ "testing"
7+ "time"
8+
9+ "github.com/bnema/vev/internal/domain"
10+)
11+
12+// TestCopyScrollOverDelayedAcks measures how a wheel flick looks on a link
13+// whose acknowledgements arrive rtt late. One tick is one animation frame
14+// (copyScrollFrame). It logs, per tick, the rows jumped by the frame the client
15+// would see, so stalls (0) and catch-up jumps (>1) are visible.
16+func TestCopyScrollOverDelayedAcks(t *testing.T) {
17+ // flick: 12 notches at once. sustained: one notch per tick for 30 ticks.
18+ for _, scenario := range []struct {
19+ name string
20+ upfront, perTick int
21+ }{{"flick", 12, 0}, {"sustained", 1, 1}} {
22+ for _, rtt := range []time.Duration{0, 50 * time.Millisecond, 100 * time.Millisecond, 200 * time.Millisecond, 400 * time.Millisecond} {
23+ t.Run(scenario.name+"/"+rtt.String(), func(t *testing.T) {
24+ f := newPerformanceFixture(t, performanceConfig{size: domain.Size{Cols: 80, Rows: 24}, panes: 1, historyRows: 400})
25+ f.d.enterCopyMode(f.sess, f.ac)
26+ f.d.copyWheel(f.sess, f.ac, -150)
27+ f.ac.ackOutputState(f.ac.output.currentEpoch(), f.ac.output.next)
28+ clock := newCoordinatorMockClock(t, 64)
29+ f.d.clock = clock.clock
30+ rt := f.ac.overlays
31+
32+ type ack struct {
33+ at int
34+ epoch, seen uint64
35+ }
36+ var (
37+ acks []ack
38+ tick int
39+ emitted = make(chan struct{}, 64)
40+ )
41+ // emit runs under sendMu: only signal here, read ACK coordinates later.
42+ f.ac.renderStages.emit = func() { emitted <- struct{}{} }
43+ drainTimers := func() *coordinatorMockTimer {
44+ var frame *coordinatorMockTimer
45+ for {
46+ select {
47+ case tm := <-clock.timers:
48+ if tm.duration == copyScrollFrame {
49+ frame = tm
50+ }
51+ default:
52+ return frame
53+ }
54+ }
55+ }
56+ viewport := func() int {
57+ rt.copyMu.Lock()
58+ defer rt.copyMu.Unlock()
59+ return rt.copyMode.ViewportTop
60+ }
61+ pending := func() int {
62+ rt.copyMu.Lock()
63+ defer rt.copyMu.Unlock()
64+ return rt.copyScroll.remaining
65+ }
66+ delivered := func() bool {
67+ select {
68+ case <-emitted:
69+ f.ac.sendMu.Lock()
70+ epoch, state := f.ac.output.currentEpoch(), f.ac.output.next
71+ f.ac.sendMu.Unlock()
72+ acks = append(acks, ack{at: tick + int((rtt+copyScrollFrame-1)/copyScrollFrame), epoch: epoch, seen: state})
73+ return true
74+ case <-time.After(15 * time.Millisecond):
75+ return false
76+ }
77+ }
78+
79+ start := viewport()
80+ last := start
81+ maxJump := 0
82+ var jumps []string
83+ // A fast flick: 12 notches of 3 rows arrive in the first tick.
84+ fed := 0
85+ feed := func(n int) {
86+ for range n {
87+ f.d.smoothCopyWheel(f.sess, f.ac, -3)
88+ fed++
89+ }
90+ }
91+ feed(scenario.upfront)
92+ shown := func() {
93+ if delivered() {
94+ top := viewport()
95+ jumps = append(jumps, fmt.Sprint(last-top))
96+ maxJump = max(maxJump, last-top)
97+ last = top
98+ } else {
99+ jumps = append(jumps, "-")
100+ }
101+ }
102+ shown()
103+ for ; tick < 80 && (pending() != 0 || len(acks) != 0 || (scenario.perTick > 0 && tick < 30)); tick++ {
104+ if tick < 30 {
105+ feed(scenario.perTick)
106+ }
107+ for len(acks) > 0 && acks[0].at <= tick {
108+ f.ac.ackOutputState(acks[0].epoch, acks[0].seen)
109+ if rc := f.sess.renderCoordinator(); rc != nil {
110+ rc.notifyAck()
111+ }
112+ acks = acks[1:]
113+ }
114+ // With a full window no timer is armed: the server is stalled
115+ // until an ACK arrives, so keep ticking to let it release.
116+ if frame := drainTimers(); frame != nil {
117+ frame.ch <- time.Time{}
118+ }
119+ shown()
120+ }
121+ t.Logf("%s rtt=%-6v rows per tick (- = nothing sent): %s", scenario.name, rtt, strings.Join(jumps, " "))
122+ // A slow link may delay frames but must not make the picture jump
123+ // further than one animation frame allows.
124+ if maxJump > copyScrollMaxStep {
125+ t.Errorf("one frame jumped %d rows, want at most %d", maxJump, copyScrollMaxStep)
126+ }
127+ // Whatever the link, every requested row is eventually shown.
128+ if got, want := start-viewport(), 3*fed; got != want {
129+ t.Fatalf("scrolled %d rows, want %d", got, want)
130+ }
131+ })
132+ }
133+ }
134+}

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 12

Tags 11

↑↓ moveenter openesc close