fix(daemon): lift the copy-scroll step cap once a held backlog is drained
- commit
- 4fda349ea807e5cefe76fd1ab39bed6875ae95aa
- author
- brice · · 2026-10-08 10:38 UTC
- parent
- f3b7322
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +33 -9
M docs/performance.md +7 -6
| @@ -226,12 +226,13 @@ The unadorned live-pane cache stays separate from the displayed copy viewport. | |||
| 226 | 226 | Neither committed frame is mutated while preparing a candidate; failed output | |
| 227 | 227 | cannot publish its viewport metadata. Copy exit refreshes live content. | |
| 228 | 228 | ||
| 229 | - | Remote clients request the full output window (`MaxOutputWindow`, 8 frames), | |
| 230 | - | so the animation keeps moving while acknowledgements are in flight; it stays | |
| 231 | - | smooth up to about 100 ms of round trip. When the window is full the animation | |
| 232 | - | holds still instead of piling rows into one frame, then drains the backlog at | |
| 233 | - | no more than 9 rows per frame once acknowledgements return. A hold longer than | |
| 234 | - | about one second abandons the scroll. Local scrolling is unchanged. | |
| 229 | + | Clients request the full output window (`MaxOutputWindow`, 8 frames), so the | |
| 230 | + | animation keeps moving while acknowledgements are in flight; on a remote link | |
| 231 | + | it stays smooth up to about 100 ms of round trip. When the window is full the | |
| 232 | + | animation holds still instead of piling rows into one frame, then drains the | |
| 233 | + | backlog it held at no more than 9 rows per frame; the next scroll eases | |
| 234 | + | normally again. A hold longer than about one second abandons the scroll. | |
| 235 | + | Local scroll easing is unchanged, since a local window never fills. | |
| 235 | 236 | `TestCopyScrollOverDelayedAcks` replays a flick and a sustained scroll over | |
| 236 | 237 | 0–400 ms acknowledgement delays with windows of 1 and 8 frames; it asserts | |
| 237 | 238 | that no frame jumps more than 9 rows and that every requested row is shown, | |
| @@ -226,12 +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 output | 226 | 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 | - | Remote clients request the full output window (`MaxOutputWindow`, 8 frames), | 229 | + | Clients request the full output window (`MaxOutputWindow`, 8 frames), so the |
| 230 | - | so the animation keeps moving while acknowledgements are in flight; it stays | 230 | + | animation keeps moving while acknowledgements are in flight; on a remote link |
| 231 | - | smooth up to about 100 ms of round trip. When the window is full the animation | 231 | + | it stays smooth up to about 100 ms of round trip. When the window is full the |
| 232 | - | holds still instead of piling rows into one frame, then drains the backlog at | 232 | + | animation holds still instead of piling rows into one frame, then drains the |
| 233 | - | no more than 9 rows per frame once acknowledgements return. A hold longer than | 233 | + | backlog it held at no more than 9 rows per frame; the next scroll eases |
| 234 | - | about one second abandons the scroll. Local scrolling is unchanged. | 234 | + | normally again. A hold longer than about one second abandons the scroll. |
| 235 | + | Local scroll easing is unchanged, since a local window never fills. | |||
| 235 | `TestCopyScrollOverDelayedAcks` replays a flick and a sustained scroll over | 236 | `TestCopyScrollOverDelayedAcks` replays a flick and a sustained scroll over | ||
| 236 | 0–400 ms acknowledgement delays with windows of 1 and 8 frames; it asserts | 237 | 0–400 ms acknowledgement delays with windows of 1 and 8 frames; it asserts | ||
| 237 | that no frame jumps more than 9 rows and that every requested row is shown, | 238 | that no frame jumps more than 9 rows and that every requested row is shown, | ||
M internal/usecase/daemon/copy_scroll_animation.go +4 -0
| @@ -119,6 +119,10 @@ func (d *Daemon) advanceCopyScrollLocked(sess *session, ac *attachedClient) (boo | |||
| 119 | 119 | rt.stopCopyScrollLocked() | |
| 120 | 120 | return changed, exit | |
| 121 | 121 | } | |
| 122 | + | if motion.remaining == 0 { | |
| 123 | + | // The held backlog is drained: the next scroll eases freely again. | |
| 124 | + | motion.recovering = false | |
| 125 | + | } | |
| 122 | 126 | } | |
| 123 | 127 | if motion.remaining != 0 { | |
| 124 | 128 | mode := rt.copyMode | |
| @@ -119,6 +119,10 @@ func (d *Daemon) advanceCopyScrollLocked(sess *session, ac *attachedClient) (boo | |||||
| 119 | rt.stopCopyScrollLocked() | 119 | rt.stopCopyScrollLocked() | ||
| 120 | return changed, exit | 120 | return changed, exit | ||
| 121 | } | 121 | } | ||
| 122 | + | if motion.remaining == 0 { | |||
| 123 | + | // The held backlog is drained: the next scroll eases freely again. | |||
| 124 | + | motion.recovering = false | |||
| 125 | + | } | |||
| 122 | } | 126 | } | ||
| 123 | if motion.remaining != 0 { | 127 | if motion.remaining != 0 { | ||
| 124 | mode := rt.copyMode | 128 | mode := rt.copyMode | ||
M internal/usecase/daemon/copy_scroll_animation_test.go +22 -3
| @@ -136,17 +136,22 @@ func TestCopyScrollReducedMotionAndKeyboardCancellation(t *testing.T) { | |||
| 136 | 136 | ||
| 137 | 137 | // TestCopyScrollAnimationFullOutputWindow covers a remote link whose output | |
| 138 | 138 | // window is full: the animation holds still, drains the backlog in capped | |
| 139 | - | // steps once the window reopens, and gives up after about a second of hold. | |
| 139 | + | // steps once the window reopens, eases freely again once that backlog is | |
| 140 | + | // drained, and gives up after about a second of hold. | |
| 140 | 141 | func TestCopyScrollAnimationFullOutputWindow(t *testing.T) { | |
| 141 | 142 | tests := []struct { | |
| 142 | 143 | name string | |
| 143 | 144 | heldFrames int | |
| 144 | 145 | reopen bool | |
| 145 | - | wantMoved int | |
| 146 | - | wantActive bool | |
| 146 | + | // drainThenFlick drains the held backlog, then starts a new 60-row | |
| 147 | + | // flick past the tail deadline; wantMoved counts only that flick. | |
| 148 | + | drainThenFlick bool | |
| 149 | + | wantMoved int | |
| 150 | + | wantActive bool | |
| 147 | 151 | }{ | |
| 148 | 152 | {name: "full window holds the viewport", heldFrames: 1, wantMoved: 0, wantActive: true}, | |
| 149 | 153 | {name: "reopened window drains in capped steps", heldFrames: 1, reopen: true, wantMoved: copyScrollMaxStep, wantActive: true}, | |
| 154 | + | {name: "drained backlog lifts the cap for the next flick", heldFrames: 1, reopen: true, drainThenFlick: true, wantMoved: 60, wantActive: false}, | |
| 150 | 155 | {name: "long hold abandons the scroll", heldFrames: copyScrollMaxHeldFrames + 1, wantMoved: 0, wantActive: false}, | |
| 151 | 156 | } | |
| 152 | 157 | for _, tt := range tests { | |
| @@ -178,6 +183,20 @@ func TestCopyScrollAnimationFullOutputWindow(t *testing.T) { | |||
| 178 | 183 | require.True(t, changed) | |
| 179 | 184 | require.False(t, exit) | |
| 180 | 185 | } | |
| 186 | + | if tt.drainThenFlick { | |
| 187 | + | for rt.copyScroll.remaining != 0 { | |
| 188 | + | f.ac.ackOutputState(f.ac.output.currentEpoch(), f.ac.output.next) | |
| 189 | + | rt.copyScroll.timer.stop() | |
| 190 | + | changed, exit := f.d.advanceCopyScrollLocked(f.sess, f.ac) | |
| 191 | + | require.True(t, changed) | |
| 192 | + | require.False(t, exit) | |
| 193 | + | } | |
| 194 | + | start = rt.copyMode.ViewportTop | |
| 195 | + | rt.copyScroll.remaining = -60 | |
| 196 | + | changed, exit := f.d.advanceCopyScrollLocked(f.sess, f.ac) | |
| 197 | + | require.True(t, changed) | |
| 198 | + | require.False(t, exit) | |
| 199 | + | } | |
| 181 | 200 | require.Equal(t, tt.wantMoved, start-rt.copyMode.ViewportTop) | |
| 182 | 201 | require.Equal(t, tt.wantActive, rt.copyScroll.remaining != 0) | |
| 183 | 202 | require.Equal(t, tt.wantActive, rt.copyScroll.timer.timer != nil, "the frame timer stays armed only while the scroll is active") | |
| @@ -136,17 +136,22 @@ func TestCopyScrollReducedMotionAndKeyboardCancellation(t *testing.T) { | |||||
| 136 | 136 | ||||
| 137 | // TestCopyScrollAnimationFullOutputWindow covers a remote link whose output | 137 | // TestCopyScrollAnimationFullOutputWindow covers a remote link whose output | ||
| 138 | // window is full: the animation holds still, drains the backlog in capped | 138 | // window is full: the animation holds still, drains the backlog in capped | ||
| 139 | - | // steps once the window reopens, and gives up after about a second of hold. | 139 | + | // steps once the window reopens, eases freely again once that backlog is |
| 140 | + | // drained, and gives up after about a second of hold. | |||
| 140 | func TestCopyScrollAnimationFullOutputWindow(t *testing.T) { | 141 | func TestCopyScrollAnimationFullOutputWindow(t *testing.T) { | ||
| 141 | tests := []struct { | 142 | tests := []struct { | ||
| 142 | name string | 143 | name string | ||
| 143 | heldFrames int | 144 | heldFrames int | ||
| 144 | reopen bool | 145 | reopen bool | ||
| 145 | - | wantMoved int | 146 | + | // drainThenFlick drains the held backlog, then starts a new 60-row |
| 146 | - | wantActive bool | 147 | + | // flick past the tail deadline; wantMoved counts only that flick. |
| 148 | + | drainThenFlick bool | |||
| 149 | + | wantMoved int | |||
| 150 | + | wantActive bool | |||
| 147 | }{ | 151 | }{ | ||
| 148 | {name: "full window holds the viewport", heldFrames: 1, wantMoved: 0, wantActive: true}, | 152 | {name: "full window holds the viewport", heldFrames: 1, wantMoved: 0, wantActive: true}, | ||
| 149 | {name: "reopened window drains in capped steps", heldFrames: 1, reopen: true, wantMoved: copyScrollMaxStep, wantActive: true}, | 153 | {name: "reopened window drains in capped steps", heldFrames: 1, reopen: true, wantMoved: copyScrollMaxStep, wantActive: true}, | ||
| 154 | + | {name: "drained backlog lifts the cap for the next flick", heldFrames: 1, reopen: true, drainThenFlick: true, wantMoved: 60, wantActive: false}, | |||
| 150 | {name: "long hold abandons the scroll", heldFrames: copyScrollMaxHeldFrames + 1, wantMoved: 0, wantActive: false}, | 155 | {name: "long hold abandons the scroll", heldFrames: copyScrollMaxHeldFrames + 1, wantMoved: 0, wantActive: false}, | ||
| 151 | } | 156 | } | ||
| 152 | for _, tt := range tests { | 157 | for _, tt := range tests { | ||
| @@ -178,6 +183,20 @@ func TestCopyScrollAnimationFullOutputWindow(t *testing.T) { | |||||
| 178 | require.True(t, changed) | 183 | require.True(t, changed) | ||
| 179 | require.False(t, exit) | 184 | require.False(t, exit) | ||
| 180 | } | 185 | } | ||
| 186 | + | if tt.drainThenFlick { | |||
| 187 | + | for rt.copyScroll.remaining != 0 { | |||
| 188 | + | f.ac.ackOutputState(f.ac.output.currentEpoch(), f.ac.output.next) | |||
| 189 | + | rt.copyScroll.timer.stop() | |||
| 190 | + | changed, exit := f.d.advanceCopyScrollLocked(f.sess, f.ac) | |||
| 191 | + | require.True(t, changed) | |||
| 192 | + | require.False(t, exit) | |||
| 193 | + | } | |||
| 194 | + | start = rt.copyMode.ViewportTop | |||
| 195 | + | rt.copyScroll.remaining = -60 | |||
| 196 | + | changed, exit := f.d.advanceCopyScrollLocked(f.sess, f.ac) | |||
| 197 | + | require.True(t, changed) | |||
| 198 | + | require.False(t, exit) | |||
| 199 | + | } | |||
| 181 | require.Equal(t, tt.wantMoved, start-rt.copyMode.ViewportTop) | 200 | require.Equal(t, tt.wantMoved, start-rt.copyMode.ViewportTop) | ||
| 182 | require.Equal(t, tt.wantActive, rt.copyScroll.remaining != 0) | 201 | require.Equal(t, tt.wantActive, rt.copyScroll.remaining != 0) | ||
| 183 | require.Equal(t, tt.wantActive, rt.copyScroll.timer.timer != nil, "the frame timer stays armed only while the scroll is active") | 202 | require.Equal(t, tt.wantActive, rt.copyScroll.timer.timer != nil, "the frame timer stays armed only while the scroll is active") | ||