Merge pull request 'fix(remote): smooth copy-mode scrolling over remote links' (#19) from perf/remote-scroll-smoothness into main
Reviewed-on: https://git.bnema.dev/bnema/vev/pulls/19
8 files · +429 -20
M docs/performance.md +12 -0
| @@ -226,6 +226,18 @@ 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 | + | 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. | |
| 236 | + | `TestCopyScrollOverDelayedAcks` replays a flick and a sustained scroll over | |
| 237 | + | 0–400 ms acknowledgement delays with windows of 1 and 8 frames; it asserts | |
| 238 | + | that no frame jumps more than 9 rows and that every requested row is shown, | |
| 239 | + | and logs the rows moved per frame. | |
| 240 | + | ||
| 229 | 241 | Mouse animation is tested separately with injected clocks: first response, | |
| 230 | 242 | 16 ms pacing, burst accumulation, deceleration, reversal, cancellation, and the | |
| 231 | 243 | 120 ms tail deadline. The rendering benchmark deliberately bypasses those timers | |
| @@ -226,6 +226,18 @@ 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 | + | 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. | |||
| 236 | + | `TestCopyScrollOverDelayedAcks` replays a flick and a sustained scroll over | |||
| 237 | + | 0–400 ms acknowledgement delays with windows of 1 and 8 frames; it asserts | |||
| 238 | + | that no frame jumps more than 9 rows and that every requested row is shown, | |||
| 239 | + | and logs the rows moved per frame. | |||
| 240 | + | ||||
| 229 | Mouse animation is tested separately with injected clocks: first response, | 241 | Mouse animation is tested separately with injected clocks: first response, | ||
| 230 | 16 ms pacing, burst accumulation, deceleration, reversal, cancellation, and the | 242 | 16 ms pacing, burst accumulation, deceleration, reversal, cancellation, and the | ||
| 231 | 120 ms tail deadline. The rendering benchmark deliberately bypasses those timers | 243 | 120 ms tail deadline. The rendering benchmark deliberately bypasses those timers | ||
M docs/protocol.md +4 -1
| @@ -158,7 +158,10 @@ base. `ValidateOutput` enforces the epoch/base/new chain | |||
| 158 | 158 | - At most `MaxOutputWindow` (8) states may be in flight per attachment | |
| 159 | 159 | (`daemon.go:maxUnackedOutputStates`); the daemon negotiates the window | |
| 160 | 160 | down from the transport capabilities (`Hello.MaxOutputInFlight` claim | |
| 161 | - | capped by the transport ceiling; datagram carriage reports 1). | |
| 161 | + | capped by the transport ceiling; datagram carriage reports 1). A client | |
| 162 | + | claims its carriage's preferred window, or `MaxOutputWindow` when the | |
| 163 | + | carriage states none, so output pipelines instead of waiting a round trip | |
| 164 | + | per frame. | |
| 162 | 165 | - The client coalesces ACKs (`client.go:cumulativeAckQueue`) and keeps | |
| 163 | 166 | applying + acknowledging daemon frames even while a picker lease owns | |
| 164 | 167 | the terminal (see below) — admission without physical write. | |
| @@ -158,7 +158,10 @@ base. `ValidateOutput` enforces the epoch/base/new chain | |||||
| 158 | - At most `MaxOutputWindow` (8) states may be in flight per attachment | 158 | - At most `MaxOutputWindow` (8) states may be in flight per attachment | ||
| 159 | (`daemon.go:maxUnackedOutputStates`); the daemon negotiates the window | 159 | (`daemon.go:maxUnackedOutputStates`); the daemon negotiates the window | ||
| 160 | down from the transport capabilities (`Hello.MaxOutputInFlight` claim | 160 | down from the transport capabilities (`Hello.MaxOutputInFlight` claim | ||
| 161 | - | capped by the transport ceiling; datagram carriage reports 1). | 161 | + | capped by the transport ceiling; datagram carriage reports 1). A client |
| 162 | + | claims its carriage's preferred window, or `MaxOutputWindow` when the | |||
| 163 | + | carriage states none, so output pipelines instead of waiting a round trip | |||
| 164 | + | per frame. | |||
| 162 | - The client coalesces ACKs (`client.go:cumulativeAckQueue`) and keeps | 165 | - The client coalesces ACKs (`client.go:cumulativeAckQueue`) and keeps | ||
| 163 | applying + acknowledging daemon frames even while a picker lease owns | 166 | applying + acknowledging daemon frames even while a picker lease owns | ||
| 164 | the terminal (see below) — admission without physical write. | 167 | the terminal (see below) — admission without physical write. | ||
M internal/adapters/brokeripc/pipe_test.go +33 -0
| @@ -6,6 +6,8 @@ import ( | |||
| 6 | 6 | "io" | |
| 7 | 7 | "testing" | |
| 8 | 8 | ||
| 9 | + | "github.com/bnema/vev/internal/adapters/brokerwire" | |
| 10 | + | "github.com/bnema/vev/internal/protocol" | |
| 9 | 11 | "github.com/stretchr/testify/require" | |
| 10 | 12 | ) | |
| 11 | 13 | ||
| @@ -106,3 +108,34 @@ func TestStreamPipeCloseWithPreservesQueuedDataOnOrderlyClose(t *testing.T) { | |||
| 106 | 108 | }) | |
| 107 | 109 | } | |
| 108 | 110 | } | |
| 111 | + | ||
| 112 | + | // TestStreamPipeHoldsAFullOutputWindow pins the client pipe headroom for the | |
| 113 | + | // session output window. A client claims protocol.MaxOutputWindow unacked | |
| 114 | + | // Output frames; with a terminal that is not draining, all of them may sit in | |
| 115 | + | // this pipe at once, and overflow fails the whole stream | |
| 116 | + | // (ErrStreamBackpressure). The frame size is a typical text repaint: a | |
| 117 | + | // near-full page of a 500x140 terminal measured about 70 KB. Uncompressed diff | |
| 118 | + | // frames with per-cell truecolor SGR can be far larger (estimated near 1 MB at | |
| 119 | + | // 300x80), and eight of those can still overflow the default 4 MiB bound. | |
| 120 | + | func TestStreamPipeHoldsAFullOutputWindow(t *testing.T) { | |
| 121 | + | const frameBytes = 72 << 10 | |
| 122 | + | chunk := int(brokerwire.MaxStreamChunkBytes) | |
| 123 | + | p := newStreamPipe(0, 0, func([]byte) error { return nil }) | |
| 124 | + | payload := bytes.Repeat([]byte{'x'}, frameBytes) | |
| 125 | + | for frame := range protocol.MaxOutputWindow { | |
| 126 | + | for data := payload; len(data) > 0; { | |
| 127 | + | n := min(len(data), chunk) | |
| 128 | + | require.NoError(t, p.deliver(data[:n]), "frame %d overflowed the client pipe", frame) | |
| 129 | + | data = data[n:] | |
| 130 | + | } | |
| 131 | + | } | |
| 132 | + | } | |
| 133 | + | ||
| 134 | + | // TestClientStreamPrefersTheFullOutputWindow pins what a real broker-routed | |
| 135 | + | // attachment reports: its carriage is a reliable stream, not a datagram link, | |
| 136 | + | // so the client claims the full output window and frames pipeline. | |
| 137 | + | func TestClientStreamPrefersTheFullOutputWindow(t *testing.T) { | |
| 138 | + | st, err := newClientStream(&client{}, 1) | |
| 139 | + | require.NoError(t, err) | |
| 140 | + | require.Equal(t, uint8(protocol.MaxOutputWindow), st.Capabilities().PreferredOutputWindow) | |
| 141 | + | } | |
| @@ -6,6 +6,8 @@ import ( | |||||
| 6 | "io" | 6 | "io" | ||
| 7 | "testing" | 7 | "testing" | ||
| 8 | 8 | ||||
| 9 | + | "github.com/bnema/vev/internal/adapters/brokerwire" | |||
| 10 | + | "github.com/bnema/vev/internal/protocol" | |||
| 9 | "github.com/stretchr/testify/require" | 11 | "github.com/stretchr/testify/require" | ||
| 10 | ) | 12 | ) | ||
| 11 | 13 | ||||
| @@ -106,3 +108,34 @@ func TestStreamPipeCloseWithPreservesQueuedDataOnOrderlyClose(t *testing.T) { | |||||
| 106 | }) | 108 | }) | ||
| 107 | } | 109 | } | ||
| 108 | } | 110 | } | ||
| 111 | + | ||||
| 112 | + | // TestStreamPipeHoldsAFullOutputWindow pins the client pipe headroom for the | |||
| 113 | + | // session output window. A client claims protocol.MaxOutputWindow unacked | |||
| 114 | + | // Output frames; with a terminal that is not draining, all of them may sit in | |||
| 115 | + | // this pipe at once, and overflow fails the whole stream | |||
| 116 | + | // (ErrStreamBackpressure). The frame size is a typical text repaint: a | |||
| 117 | + | // near-full page of a 500x140 terminal measured about 70 KB. Uncompressed diff | |||
| 118 | + | // frames with per-cell truecolor SGR can be far larger (estimated near 1 MB at | |||
| 119 | + | // 300x80), and eight of those can still overflow the default 4 MiB bound. | |||
| 120 | + | func TestStreamPipeHoldsAFullOutputWindow(t *testing.T) { | |||
| 121 | + | const frameBytes = 72 << 10 | |||
| 122 | + | chunk := int(brokerwire.MaxStreamChunkBytes) | |||
| 123 | + | p := newStreamPipe(0, 0, func([]byte) error { return nil }) | |||
| 124 | + | payload := bytes.Repeat([]byte{'x'}, frameBytes) | |||
| 125 | + | for frame := range protocol.MaxOutputWindow { | |||
| 126 | + | for data := payload; len(data) > 0; { | |||
| 127 | + | n := min(len(data), chunk) | |||
| 128 | + | require.NoError(t, p.deliver(data[:n]), "frame %d overflowed the client pipe", frame) | |||
| 129 | + | data = data[n:] | |||
| 130 | + | } | |||
| 131 | + | } | |||
| 132 | + | } | |||
| 133 | + | ||||
| 134 | + | // TestClientStreamPrefersTheFullOutputWindow pins what a real broker-routed | |||
| 135 | + | // attachment reports: its carriage is a reliable stream, not a datagram link, | |||
| 136 | + | // so the client claims the full output window and frames pipeline. | |||
| 137 | + | func TestClientStreamPrefersTheFullOutputWindow(t *testing.T) { | |||
| 138 | + | st, err := newClientStream(&client{}, 1) | |||
| 139 | + | require.NoError(t, err) | |||
| 140 | + | require.Equal(t, uint8(protocol.MaxOutputWindow), st.Capabilities().PreferredOutputWindow) | |||
| 141 | + | } | |||
M internal/usecase/client/runtime_primitives.go +8 -3
| @@ -6,6 +6,7 @@ import ( | |||
| 6 | 6 | "sync" | |
| 7 | 7 | ||
| 8 | 8 | "github.com/bnema/vev/internal/ports" | |
| 9 | + | "github.com/bnema/vev/internal/protocol" | |
| 9 | 10 | ) | |
| 10 | 11 | ||
| 11 | 12 | // ProtocolError is a session- or protocol-level failure reported before attach. | |
| @@ -29,11 +30,15 @@ func newClientID() [16]byte { | |||
| 29 | 30 | return id | |
| 30 | 31 | } | |
| 31 | 32 | ||
| 33 | + | // requestedOutputWindow is the number of unacknowledged Output states this | |
| 34 | + | // client lets the daemon keep in flight: what the carriage prefers, or the | |
| 35 | + | // protocol maximum when it states no preference. A window of one makes every | |
| 36 | + | // frame wait a full round trip, which stalls scrolling on any remote link. | |
| 32 | 37 | func requestedOutputWindow(connection ports.ClientConnection) uint8 { | |
| 33 | - | if windowed, ok := connection.(interface{ OutputWindow() uint8 }); ok { | |
| 34 | - | return windowed.OutputWindow() | |
| 38 | + | if window := connection.Capabilities().PreferredOutputWindow; window != 0 { | |
| 39 | + | return window | |
| 35 | 40 | } | |
| 36 | - | return 1 | |
| 41 | + | return protocol.MaxOutputWindow | |
| 37 | 42 | } | |
| 38 | 43 | ||
| 39 | 44 | type foregroundSendLease struct { | |
| @@ -6,6 +6,7 @@ import ( | |||||
| 6 | "sync" | 6 | "sync" | ||
| 7 | 7 | ||||
| 8 | "github.com/bnema/vev/internal/ports" | 8 | "github.com/bnema/vev/internal/ports" | ||
| 9 | + | "github.com/bnema/vev/internal/protocol" | |||
| 9 | ) | 10 | ) | ||
| 10 | 11 | ||||
| 11 | // ProtocolError is a session- or protocol-level failure reported before attach. | 12 | // ProtocolError is a session- or protocol-level failure reported before attach. | ||
| @@ -29,11 +30,15 @@ func newClientID() [16]byte { | |||||
| 29 | return id | 30 | return id | ||
| 30 | } | 31 | } | ||
| 31 | 32 | ||||
| 33 | + | // requestedOutputWindow is the number of unacknowledged Output states this | |||
| 34 | + | // client lets the daemon keep in flight: what the carriage prefers, or the | |||
| 35 | + | // protocol maximum when it states no preference. A window of one makes every | |||
| 36 | + | // frame wait a full round trip, which stalls scrolling on any remote link. | |||
| 32 | func requestedOutputWindow(connection ports.ClientConnection) uint8 { | 37 | func requestedOutputWindow(connection ports.ClientConnection) uint8 { | ||
| 33 | - | if windowed, ok := connection.(interface{ OutputWindow() uint8 }); ok { | 38 | + | if window := connection.Capabilities().PreferredOutputWindow; window != 0 { |
| 34 | - | return windowed.OutputWindow() | 39 | + | return window |
| 35 | } | 40 | } | ||
| 36 | - | return 1 | 41 | + | return protocol.MaxOutputWindow |
| 37 | } | 42 | } | ||
| 38 | 43 | ||||
| 39 | type foregroundSendLease struct { | 44 | type foregroundSendLease struct { | ||
A internal/usecase/client/runtime_primitives_test.go +46 -0
| @@ -0,0 +1,46 @@ | |||
| 1 | + | package client | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "testing" | |
| 5 | + | ||
| 6 | + | "github.com/bnema/vev/internal/protocol" | |
| 7 | + | "github.com/stretchr/testify/require" | |
| 8 | + | ) | |
| 9 | + | ||
| 10 | + | // windowTestStream reports fixed connection capabilities. | |
| 11 | + | type windowTestStream struct { | |
| 12 | + | *sessionTestStream | |
| 13 | + | window uint8 | |
| 14 | + | } | |
| 15 | + | ||
| 16 | + | func (s windowTestStream) Capabilities() protocol.ConnectionCapabilities { | |
| 17 | + | return protocol.ConnectionCapabilities{PreferredOutputWindow: s.window} | |
| 18 | + | } | |
| 19 | + | ||
| 20 | + | func TestRequestedOutputWindow(t *testing.T) { | |
| 21 | + | tests := []struct { | |
| 22 | + | name string | |
| 23 | + | preferred uint8 | |
| 24 | + | want uint8 | |
| 25 | + | }{ | |
| 26 | + | {name: "no carriage preference pipelines the full window", preferred: 0, want: protocol.MaxOutputWindow}, | |
| 27 | + | {name: "a carriage that prefers one frame keeps it", preferred: 1, want: 1}, | |
| 28 | + | {name: "a carriage preference is kept as declared", preferred: 4, want: 4}, | |
| 29 | + | } | |
| 30 | + | for _, tt := range tests { | |
| 31 | + | t.Run(tt.name, func(t *testing.T) { | |
| 32 | + | stream := windowTestStream{sessionTestStream: newSessionTestStream(), window: tt.preferred} | |
| 33 | + | require.Equal(t, tt.want, requestedOutputWindow(stream)) | |
| 34 | + | }) | |
| 35 | + | } | |
| 36 | + | } | |
| 37 | + | ||
| 38 | + | // TestHelloRequestsPipelinedOutput pins the fallback window an attachment | |
| 39 | + | // Hello claims when its carriage states no preference. A window of one would | |
| 40 | + | // make every remote frame wait a round trip. | |
| 41 | + | func TestHelloRequestsPipelinedOutput(t *testing.T) { | |
| 42 | + | worker, err := newSessionAttachmentWorker(sessionAttachmentConfig{Request: sessionTestRequest(false), SessionEnvironment: SessionEnvironment{Provenance: SessionEnvironmentRemote}}) | |
| 43 | + | require.NoError(t, err) | |
| 44 | + | hello := worker.hello(newSessionTestStream()) | |
| 45 | + | require.Equal(t, uint8(protocol.MaxOutputWindow), hello.MaxOutputInFlight) | |
| 46 | + | } | |
| @@ -0,0 +1,46 @@ | |||||
| 1 | + | package client | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "testing" | |||
| 5 | + | ||||
| 6 | + | "github.com/bnema/vev/internal/protocol" | |||
| 7 | + | "github.com/stretchr/testify/require" | |||
| 8 | + | ) | |||
| 9 | + | ||||
| 10 | + | // windowTestStream reports fixed connection capabilities. | |||
| 11 | + | type windowTestStream struct { | |||
| 12 | + | *sessionTestStream | |||
| 13 | + | window uint8 | |||
| 14 | + | } | |||
| 15 | + | ||||
| 16 | + | func (s windowTestStream) Capabilities() protocol.ConnectionCapabilities { | |||
| 17 | + | return protocol.ConnectionCapabilities{PreferredOutputWindow: s.window} | |||
| 18 | + | } | |||
| 19 | + | ||||
| 20 | + | func TestRequestedOutputWindow(t *testing.T) { | |||
| 21 | + | tests := []struct { | |||
| 22 | + | name string | |||
| 23 | + | preferred uint8 | |||
| 24 | + | want uint8 | |||
| 25 | + | }{ | |||
| 26 | + | {name: "no carriage preference pipelines the full window", preferred: 0, want: protocol.MaxOutputWindow}, | |||
| 27 | + | {name: "a carriage that prefers one frame keeps it", preferred: 1, want: 1}, | |||
| 28 | + | {name: "a carriage preference is kept as declared", preferred: 4, want: 4}, | |||
| 29 | + | } | |||
| 30 | + | for _, tt := range tests { | |||
| 31 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 32 | + | stream := windowTestStream{sessionTestStream: newSessionTestStream(), window: tt.preferred} | |||
| 33 | + | require.Equal(t, tt.want, requestedOutputWindow(stream)) | |||
| 34 | + | }) | |||
| 35 | + | } | |||
| 36 | + | } | |||
| 37 | + | ||||
| 38 | + | // TestHelloRequestsPipelinedOutput pins the fallback window an attachment | |||
| 39 | + | // Hello claims when its carriage states no preference. A window of one would | |||
| 40 | + | // make every remote frame wait a round trip. | |||
| 41 | + | func TestHelloRequestsPipelinedOutput(t *testing.T) { | |||
| 42 | + | worker, err := newSessionAttachmentWorker(sessionAttachmentConfig{Request: sessionTestRequest(false), SessionEnvironment: SessionEnvironment{Provenance: SessionEnvironmentRemote}}) | |||
| 43 | + | require.NoError(t, err) | |||
| 44 | + | hello := worker.hello(newSessionTestStream()) | |||
| 45 | + | require.Equal(t, uint8(protocol.MaxOutputWindow), hello.MaxOutputInFlight) | |||
| 46 | + | } | |||
M internal/usecase/daemon/copy_scroll_animation.go +49 -16
| @@ -9,6 +9,14 @@ import ( | |||
| 9 | 9 | const ( | |
| 10 | 10 | copyScrollFrame = 16 * time.Millisecond | |
| 11 | 11 | copyScrollTail = 120 * time.Millisecond | |
| 12 | + | // copyScrollMaxStep bounds each frame drained after the animation was held | |
| 13 | + | // on a full output window, so the backlog that built up during the stall | |
| 14 | + | // resumes as motion rather than one catch-up jump. | |
| 15 | + | copyScrollMaxStep = 9 | |
| 16 | + | // copyScrollMaxHeldFrames ends an animation held on a full output window | |
| 17 | + | // for about one second: the link is effectively gone, and scrolling on its | |
| 18 | + | // own once acknowledgements finally return would surprise the user. | |
| 19 | + | copyScrollMaxHeldFrames = int(time.Second / copyScrollFrame) | |
| 12 | 20 | ) | |
| 13 | 21 | ||
| 14 | 22 | // copyMu owns the animation and its single cancellable overlay-input timer. | |
| @@ -17,6 +25,10 @@ type copyScrollAnimation struct { | |||
| 17 | 25 | remaining int | |
| 18 | 26 | lastInput time.Time | |
| 19 | 27 | timer pendingByteTimer | |
| 28 | + | // heldFrames counts consecutive frames held on a full output window. | |
| 29 | + | heldFrames int | |
| 30 | + | // recovering caps the steps that drain the backlog left by a hold. | |
| 31 | + | recovering bool | |
| 20 | 32 | } | |
| 21 | 33 | ||
| 22 | 34 | func (rt *overlayRuntime) stopCopyScrollLocked() { | |
| @@ -74,22 +86,43 @@ func (d *Daemon) advanceCopyScrollLocked(sess *session, ac *attachedClient) (boo | |||
| 74 | 86 | if remaining == 0 { | |
| 75 | 87 | return false, false | |
| 76 | 88 | } | |
| 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 | |
| 89 | + | // While the client has a full window of unacknowledged frames, moving the | |
| 90 | + | // viewport would only pile rows into one catch-up jump when the window | |
| 91 | + | // reopens. Hold the animation and keep polling at frame pace instead. | |
| 92 | + | changed, exit := false, false | |
| 93 | + | if ac.output.atCapacity() { | |
| 94 | + | motion.heldFrames++ | |
| 95 | + | motion.recovering = true | |
| 96 | + | if motion.heldFrames > copyScrollMaxHeldFrames { | |
| 97 | + | rt.stopCopyScrollLocked() | |
| 98 | + | return false, false | |
| 99 | + | } | |
| 100 | + | } else { | |
| 101 | + | motion.heldFrames = 0 | |
| 102 | + | magnitude := remaining | |
| 103 | + | if magnitude < 0 { | |
| 104 | + | magnitude = -magnitude | |
| 105 | + | } | |
| 106 | + | step := (magnitude + 3) / 4 | |
| 107 | + | if d.clock.Now().Sub(motion.lastInput) >= copyScrollTail { | |
| 108 | + | step = magnitude | |
| 109 | + | } | |
| 110 | + | if motion.recovering { | |
| 111 | + | step = min(step, copyScrollMaxStep) | |
| 112 | + | } | |
| 113 | + | if remaining < 0 { | |
| 114 | + | step = -step | |
| 115 | + | } | |
| 116 | + | motion.remaining -= step | |
| 117 | + | changed, exit = rt.moveCopyWheelLocked(step) | |
| 118 | + | if !changed || exit { | |
| 119 | + | rt.stopCopyScrollLocked() | |
| 120 | + | return changed, exit | |
| 121 | + | } | |
| 122 | + | if motion.remaining == 0 { | |
| 123 | + | // The held backlog is drained: the next scroll eases freely again. | |
| 124 | + | motion.recovering = false | |
| 125 | + | } | |
| 93 | 126 | } | |
| 94 | 127 | if motion.remaining != 0 { | |
| 95 | 128 | mode := rt.copyMode | |
| @@ -9,6 +9,14 @@ import ( | |||||
| 9 | const ( | 9 | const ( | ||
| 10 | copyScrollFrame = 16 * time.Millisecond | 10 | copyScrollFrame = 16 * time.Millisecond | ||
| 11 | copyScrollTail = 120 * time.Millisecond | 11 | copyScrollTail = 120 * time.Millisecond | ||
| 12 | + | // copyScrollMaxStep bounds each frame drained after the animation was held | |||
| 13 | + | // on a full output window, so the backlog that built up during the stall | |||
| 14 | + | // resumes as motion rather than one catch-up jump. | |||
| 15 | + | copyScrollMaxStep = 9 | |||
| 16 | + | // copyScrollMaxHeldFrames ends an animation held on a full output window | |||
| 17 | + | // for about one second: the link is effectively gone, and scrolling on its | |||
| 18 | + | // own once acknowledgements finally return would surprise the user. | |||
| 19 | + | copyScrollMaxHeldFrames = int(time.Second / copyScrollFrame) | |||
| 12 | ) | 20 | ) | ||
| 13 | 21 | ||||
| 14 | // copyMu owns the animation and its single cancellable overlay-input timer. | 22 | // copyMu owns the animation and its single cancellable overlay-input timer. | ||
| @@ -17,6 +25,10 @@ type copyScrollAnimation struct { | |||||
| 17 | remaining int | 25 | remaining int | ||
| 18 | lastInput time.Time | 26 | lastInput time.Time | ||
| 19 | timer pendingByteTimer | 27 | timer pendingByteTimer | ||
| 28 | + | // heldFrames counts consecutive frames held on a full output window. | |||
| 29 | + | heldFrames int | |||
| 30 | + | // recovering caps the steps that drain the backlog left by a hold. | |||
| 31 | + | recovering bool | |||
| 20 | } | 32 | } | ||
| 21 | 33 | ||||
| 22 | func (rt *overlayRuntime) stopCopyScrollLocked() { | 34 | func (rt *overlayRuntime) stopCopyScrollLocked() { | ||
| @@ -74,22 +86,43 @@ func (d *Daemon) advanceCopyScrollLocked(sess *session, ac *attachedClient) (boo | |||||
| 74 | if remaining == 0 { | 86 | if remaining == 0 { | ||
| 75 | return false, false | 87 | return false, false | ||
| 76 | } | 88 | } | ||
| 77 | - | magnitude := remaining | 89 | + | // While the client has a full window of unacknowledged frames, moving the |
| 78 | - | if magnitude < 0 { | 90 | + | // viewport would only pile rows into one catch-up jump when the window |
| 79 | - | magnitude = -magnitude | 91 | + | // reopens. Hold the animation and keep polling at frame pace instead. |
| 80 | - | } | 92 | + | changed, exit := false, false |
| 81 | - | step := (magnitude + 3) / 4 | 93 | + | if ac.output.atCapacity() { |
| 82 | - | if d.clock.Now().Sub(motion.lastInput) >= copyScrollTail { | 94 | + | motion.heldFrames++ |
| 83 | - | step = magnitude | 95 | + | motion.recovering = true |
| 84 | - | } | 96 | + | if motion.heldFrames > copyScrollMaxHeldFrames { |
| 85 | - | if remaining < 0 { | 97 | + | rt.stopCopyScrollLocked() |
| 86 | - | step = -step | 98 | + | return false, false |
| 87 | - | } | 99 | + | } |
| 88 | - | motion.remaining -= step | 100 | + | } else { |
| 89 | - | changed, exit := rt.moveCopyWheelLocked(step) | 101 | + | motion.heldFrames = 0 |
| 90 | - | if !changed || exit { | 102 | + | magnitude := remaining |
| 91 | - | rt.stopCopyScrollLocked() | 103 | + | if magnitude < 0 { |
| 92 | - | return changed, exit | 104 | + | magnitude = -magnitude |
| 105 | + | } | |||
| 106 | + | step := (magnitude + 3) / 4 | |||
| 107 | + | if d.clock.Now().Sub(motion.lastInput) >= copyScrollTail { | |||
| 108 | + | step = magnitude | |||
| 109 | + | } | |||
| 110 | + | if motion.recovering { | |||
| 111 | + | step = min(step, copyScrollMaxStep) | |||
| 112 | + | } | |||
| 113 | + | if remaining < 0 { | |||
| 114 | + | step = -step | |||
| 115 | + | } | |||
| 116 | + | motion.remaining -= step | |||
| 117 | + | changed, exit = rt.moveCopyWheelLocked(step) | |||
| 118 | + | if !changed || exit { | |||
| 119 | + | rt.stopCopyScrollLocked() | |||
| 120 | + | return changed, exit | |||
| 121 | + | } | |||
| 122 | + | if motion.remaining == 0 { | |||
| 123 | + | // The held backlog is drained: the next scroll eases freely again. | |||
| 124 | + | motion.recovering = false | |||
| 125 | + | } | |||
| 93 | } | 126 | } | ||
| 94 | if motion.remaining != 0 { | 127 | if motion.remaining != 0 { | ||
| 95 | mode := rt.copyMode | 128 | mode := rt.copyMode | ||
M internal/usecase/daemon/copy_scroll_animation_test.go +71 -0
| @@ -133,3 +133,74 @@ func TestCopyScrollReducedMotionAndKeyboardCancellation(t *testing.T) { | |||
| 133 | 133 | t.Fatal("keyboard navigation must cancel inertia") | |
| 134 | 134 | } | |
| 135 | 135 | } | |
| 136 | + | ||
| 137 | + | // TestCopyScrollAnimationFullOutputWindow covers a remote link whose output | |
| 138 | + | // window is full: the animation holds still, drains the backlog in capped | |
| 139 | + | // steps once the window reopens, eases freely again once that backlog is | |
| 140 | + | // drained, and gives up after about a second of hold. | |
| 141 | + | func TestCopyScrollAnimationFullOutputWindow(t *testing.T) { | |
| 142 | + | tests := []struct { | |
| 143 | + | name string | |
| 144 | + | heldFrames int | |
| 145 | + | reopen 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 | |
| 151 | + | }{ | |
| 152 | + | {name: "full window holds the viewport", heldFrames: 1, wantMoved: 0, 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}, | |
| 155 | + | {name: "long hold abandons the scroll", heldFrames: copyScrollMaxHeldFrames + 1, wantMoved: 0, wantActive: false}, | |
| 156 | + | } | |
| 157 | + | for _, tt := range tests { | |
| 158 | + | t.Run(tt.name, func(t *testing.T) { | |
| 159 | + | f := newPerformanceFixture(t, performanceConfig{size: domain.Size{Cols: 80, Rows: 24}, panes: 1, historyRows: 200}) | |
| 160 | + | f.ac.output.setWindow(1) | |
| 161 | + | f.d.enterCopyMode(f.sess, f.ac) | |
| 162 | + | clock := newCoordinatorMockClock(t, 2*copyScrollMaxHeldFrames) | |
| 163 | + | f.d.clock = clock.clock | |
| 164 | + | require.True(t, f.ac.output.atCapacity(), "entering copy mode fills a one-frame window") | |
| 165 | + | rt := f.ac.overlays | |
| 166 | + | rt.copyMu.Lock() | |
| 167 | + | defer rt.copyMu.Unlock() | |
| 168 | + | start := rt.copyMode.ViewportTop | |
| 169 | + | // Past the tail deadline the whole backlog is due at once. | |
| 170 | + | rt.copyScroll.remaining = -60 | |
| 171 | + | rt.copyScroll.lastInput = time.Time{}.Add(-copyScrollTail) | |
| 172 | + | for range tt.heldFrames { | |
| 173 | + | // Each frame is driven directly, as its timer callback would: | |
| 174 | + | // retire the previous frame timer before advancing. | |
| 175 | + | rt.copyScroll.timer.stop() | |
| 176 | + | changed, exit := f.d.advanceCopyScrollLocked(f.sess, f.ac) | |
| 177 | + | require.False(t, changed || exit, "a held frame must not move or exit") | |
| 178 | + | } | |
| 179 | + | if tt.reopen { | |
| 180 | + | f.ac.ackOutputState(f.ac.output.currentEpoch(), f.ac.output.next) | |
| 181 | + | rt.copyScroll.timer.stop() | |
| 182 | + | changed, exit := f.d.advanceCopyScrollLocked(f.sess, f.ac) | |
| 183 | + | require.True(t, changed) | |
| 184 | + | require.False(t, exit) | |
| 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 | + | } | |
| 200 | + | require.Equal(t, tt.wantMoved, start-rt.copyMode.ViewportTop) | |
| 201 | + | require.Equal(t, tt.wantActive, rt.copyScroll.remaining != 0) | |
| 202 | + | require.Equal(t, tt.wantActive, rt.copyScroll.timer.timer != nil, "the frame timer stays armed only while the scroll is active") | |
| 203 | + | rt.copyScroll.timer.stop() | |
| 204 | + | }) | |
| 205 | + | } | |
| 206 | + | } | |
| @@ -133,3 +133,74 @@ func TestCopyScrollReducedMotionAndKeyboardCancellation(t *testing.T) { | |||||
| 133 | t.Fatal("keyboard navigation must cancel inertia") | 133 | t.Fatal("keyboard navigation must cancel inertia") | ||
| 134 | } | 134 | } | ||
| 135 | } | 135 | } | ||
| 136 | + | ||||
| 137 | + | // TestCopyScrollAnimationFullOutputWindow covers a remote link whose output | |||
| 138 | + | // window is full: the animation holds still, drains the backlog in capped | |||
| 139 | + | // steps once the window reopens, eases freely again once that backlog is | |||
| 140 | + | // drained, and gives up after about a second of hold. | |||
| 141 | + | func TestCopyScrollAnimationFullOutputWindow(t *testing.T) { | |||
| 142 | + | tests := []struct { | |||
| 143 | + | name string | |||
| 144 | + | heldFrames int | |||
| 145 | + | reopen 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 | |||
| 151 | + | }{ | |||
| 152 | + | {name: "full window holds the viewport", heldFrames: 1, wantMoved: 0, 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}, | |||
| 155 | + | {name: "long hold abandons the scroll", heldFrames: copyScrollMaxHeldFrames + 1, wantMoved: 0, wantActive: false}, | |||
| 156 | + | } | |||
| 157 | + | for _, tt := range tests { | |||
| 158 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 159 | + | f := newPerformanceFixture(t, performanceConfig{size: domain.Size{Cols: 80, Rows: 24}, panes: 1, historyRows: 200}) | |||
| 160 | + | f.ac.output.setWindow(1) | |||
| 161 | + | f.d.enterCopyMode(f.sess, f.ac) | |||
| 162 | + | clock := newCoordinatorMockClock(t, 2*copyScrollMaxHeldFrames) | |||
| 163 | + | f.d.clock = clock.clock | |||
| 164 | + | require.True(t, f.ac.output.atCapacity(), "entering copy mode fills a one-frame window") | |||
| 165 | + | rt := f.ac.overlays | |||
| 166 | + | rt.copyMu.Lock() | |||
| 167 | + | defer rt.copyMu.Unlock() | |||
| 168 | + | start := rt.copyMode.ViewportTop | |||
| 169 | + | // Past the tail deadline the whole backlog is due at once. | |||
| 170 | + | rt.copyScroll.remaining = -60 | |||
| 171 | + | rt.copyScroll.lastInput = time.Time{}.Add(-copyScrollTail) | |||
| 172 | + | for range tt.heldFrames { | |||
| 173 | + | // Each frame is driven directly, as its timer callback would: | |||
| 174 | + | // retire the previous frame timer before advancing. | |||
| 175 | + | rt.copyScroll.timer.stop() | |||
| 176 | + | changed, exit := f.d.advanceCopyScrollLocked(f.sess, f.ac) | |||
| 177 | + | require.False(t, changed || exit, "a held frame must not move or exit") | |||
| 178 | + | } | |||
| 179 | + | if tt.reopen { | |||
| 180 | + | f.ac.ackOutputState(f.ac.output.currentEpoch(), f.ac.output.next) | |||
| 181 | + | rt.copyScroll.timer.stop() | |||
| 182 | + | changed, exit := f.d.advanceCopyScrollLocked(f.sess, f.ac) | |||
| 183 | + | require.True(t, changed) | |||
| 184 | + | require.False(t, exit) | |||
| 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 | + | } | |||
| 200 | + | require.Equal(t, tt.wantMoved, start-rt.copyMode.ViewportTop) | |||
| 201 | + | require.Equal(t, tt.wantActive, rt.copyScroll.remaining != 0) | |||
| 202 | + | require.Equal(t, tt.wantActive, rt.copyScroll.timer.timer != nil, "the frame timer stays armed only while the scroll is active") | |||
| 203 | + | rt.copyScroll.timer.stop() | |||
| 204 | + | }) | |||
| 205 | + | } | |||
| 206 | + | } | |||
A internal/usecase/daemon/copy_scroll_remote_test.go +206 -0
| @@ -0,0 +1,206 @@ | |||
| 1 | + | package daemon | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "fmt" | |
| 5 | + | "strings" | |
| 6 | + | "testing" | |
| 7 | + | "time" | |
| 8 | + | ||
| 9 | + | "github.com/bnema/vev/internal/domain" | |
| 10 | + | "github.com/bnema/vev/internal/protocol" | |
| 11 | + | ) | |
| 12 | + | ||
| 13 | + | // TestCopyScrollOverDelayedAcks replays a wheel scroll on a link whose | |
| 14 | + | // acknowledgements arrive rtt late, for several output windows. One tick is one | |
| 15 | + | // animation frame (copyScrollFrame). The log shows, per tick, the rows the | |
| 16 | + | // viewport moved: "-" is a stall, a value above one frame's worth is a catch-up | |
| 17 | + | // jump. The test asserts that no frame jumps further than copyScrollMaxStep, | |
| 18 | + | // that every requested row is eventually shown, and that the window the daemon | |
| 19 | + | // is allowed to fill bounds the stalls. | |
| 20 | + | func TestCopyScrollOverDelayedAcks(t *testing.T) { | |
| 21 | + | // flick: 12 notches at once. sustained: one notch per tick for 30 ticks. | |
| 22 | + | scenarios := []struct { | |
| 23 | + | name string | |
| 24 | + | upfront, perTick int | |
| 25 | + | }{{"flick", 12, 0}, {"sustained", 1, 1}} | |
| 26 | + | rtts := []time.Duration{0, 50 * time.Millisecond, 100 * time.Millisecond, 200 * time.Millisecond, 400 * time.Millisecond} | |
| 27 | + | for _, scenario := range scenarios { | |
| 28 | + | for _, window := range []uint8{1, protocol.MaxOutputWindow} { | |
| 29 | + | for _, rtt := range rtts { | |
| 30 | + | t.Run(fmt.Sprintf("%s/window%d/%v", scenario.name, window, rtt), func(t *testing.T) { | |
| 31 | + | res := replayCopyScroll(t, scenario.upfront, scenario.perTick, window, rtt) | |
| 32 | + | t.Logf("rows per tick (- = stall): %s", res.log) | |
| 33 | + | if res.maxJump > copyScrollMaxStep { | |
| 34 | + | t.Errorf("one frame jumped %d rows, want at most %d", res.maxJump, copyScrollMaxStep) | |
| 35 | + | } | |
| 36 | + | if res.scrolled != res.requested { | |
| 37 | + | t.Fatalf("scrolled %d rows, want %d", res.scrolled, res.requested) | |
| 38 | + | } | |
| 39 | + | }) | |
| 40 | + | } | |
| 41 | + | } | |
| 42 | + | } | |
| 43 | + | } | |
| 44 | + | ||
| 45 | + | // TestCopyScrollWindowBoundsStalls pins the reason the output window matters: | |
| 46 | + | // on a 100 ms link a one-frame window leaves the viewport still most ticks, | |
| 47 | + | // while the default window keeps it moving. | |
| 48 | + | func TestCopyScrollWindowBoundsStalls(t *testing.T) { | |
| 49 | + | rtt := 100 * time.Millisecond | |
| 50 | + | narrow := replayCopyScroll(t, 1, 1, 1, rtt) | |
| 51 | + | wide := replayCopyScroll(t, 1, 1, protocol.MaxOutputWindow, rtt) | |
| 52 | + | if wide.stalls >= narrow.stalls { | |
| 53 | + | t.Fatalf("window %d stalled %d ticks, window 1 stalled %d: a wider window must stall less", protocol.MaxOutputWindow, wide.stalls, narrow.stalls) | |
| 54 | + | } | |
| 55 | + | } | |
| 56 | + | ||
| 57 | + | type copyScrollReplay struct { | |
| 58 | + | log string | |
| 59 | + | maxJump, stalls int | |
| 60 | + | scrolled, requested int | |
| 61 | + | } | |
| 62 | + | ||
| 63 | + | // replayCopyScroll feeds upfront notches at once, then perTick notches per tick | |
| 64 | + | // for 30 ticks, acknowledging each emitted frame rtt later. A fake clock drives | |
| 65 | + | // the animation and every step waits for its exact expected effect (a frame | |
| 66 | + | // emitted, or the frame timer re-armed while the window is full), so the | |
| 67 | + | // timeouts below only guard against hangs and never decide a result. | |
| 68 | + | func replayCopyScroll(t *testing.T, upfront, perTick int, window uint8, rtt time.Duration) copyScrollReplay { | |
| 69 | + | t.Helper() | |
| 70 | + | f := newPerformanceFixture(t, performanceConfig{size: domain.Size{Cols: 80, Rows: 24}, panes: 1, historyRows: 400}) | |
| 71 | + | f.ac.output.setWindow(window) | |
| 72 | + | f.d.enterCopyMode(f.sess, f.ac) | |
| 73 | + | f.d.copyWheel(f.sess, f.ac, -150) | |
| 74 | + | f.ac.ackOutputState(f.ac.output.currentEpoch(), f.ac.output.next) | |
| 75 | + | clock := newCoordinatorMockClock(t, 256) | |
| 76 | + | f.d.clock = clock.clock | |
| 77 | + | rt := f.ac.overlays | |
| 78 | + | ||
| 79 | + | type ack struct { | |
| 80 | + | at int | |
| 81 | + | epoch, seen uint64 | |
| 82 | + | } | |
| 83 | + | type emitted struct{ epoch, state uint64 } | |
| 84 | + | var ( | |
| 85 | + | acks []ack | |
| 86 | + | tick int | |
| 87 | + | frameTimer *coordinatorMockTimer | |
| 88 | + | // emit runs under sendMu inside the render, so the epoch and state | |
| 89 | + | // it reports are exactly those of the frame sent. | |
| 90 | + | sent = make(chan emitted, 256) | |
| 91 | + | ) | |
| 92 | + | f.ac.renderStages.emit = func() { sent <- emitted{f.ac.output.currentEpoch(), f.ac.output.next} } | |
| 93 | + | ticksPerRTT := int((rtt + copyScrollFrame - 1) / copyScrollFrame) | |
| 94 | + | ||
| 95 | + | viewport := func() int { | |
| 96 | + | rt.copyMu.Lock() | |
| 97 | + | defer rt.copyMu.Unlock() | |
| 98 | + | return rt.copyMode.ViewportTop | |
| 99 | + | } | |
| 100 | + | pending := func() int { | |
| 101 | + | rt.copyMu.Lock() | |
| 102 | + | defer rt.copyMu.Unlock() | |
| 103 | + | return rt.copyScroll.remaining | |
| 104 | + | } | |
| 105 | + | // collectTimer keeps the newest armed animation frame timer, if any. | |
| 106 | + | collectTimer := func() { | |
| 107 | + | for { | |
| 108 | + | select { | |
| 109 | + | case tm := <-clock.timers: | |
| 110 | + | if tm.duration == copyScrollFrame { | |
| 111 | + | frameTimer = tm | |
| 112 | + | } | |
| 113 | + | default: | |
| 114 | + | return | |
| 115 | + | } | |
| 116 | + | } | |
| 117 | + | } | |
| 118 | + | frames := 0 | |
| 119 | + | awaitFrame := func() { | |
| 120 | + | select { | |
| 121 | + | case e := <-sent: | |
| 122 | + | acks = append(acks, ack{at: tick + ticksPerRTT, epoch: e.epoch, seen: e.state}) | |
| 123 | + | frames++ | |
| 124 | + | case <-time.After(5 * time.Second): | |
| 125 | + | t.Fatal("expected a scroll frame, none was emitted") | |
| 126 | + | } | |
| 127 | + | } | |
| 128 | + | awaitTimer := func() { | |
| 129 | + | deadline := time.After(5 * time.Second) | |
| 130 | + | for frameTimer == nil { | |
| 131 | + | select { | |
| 132 | + | case tm := <-clock.timers: | |
| 133 | + | if tm.duration == copyScrollFrame { | |
| 134 | + | frameTimer = tm | |
| 135 | + | } | |
| 136 | + | case <-deadline: | |
| 137 | + | t.Fatal("held animation did not re-arm its frame timer") | |
| 138 | + | } | |
| 139 | + | } | |
| 140 | + | } | |
| 141 | + | fed := 0 | |
| 142 | + | feed := func(n int) { | |
| 143 | + | for range n { | |
| 144 | + | idle, full := frameTimer == nil, f.ac.output.atCapacity() | |
| 145 | + | f.d.smoothCopyWheel(f.sess, f.ac, -3) | |
| 146 | + | fed++ | |
| 147 | + | if idle && !full { | |
| 148 | + | awaitFrame() | |
| 149 | + | } | |
| 150 | + | collectTimer() | |
| 151 | + | } | |
| 152 | + | } | |
| 153 | + | fire := func() { | |
| 154 | + | if frameTimer == nil { | |
| 155 | + | return | |
| 156 | + | } | |
| 157 | + | full := f.ac.output.atCapacity() | |
| 158 | + | timer := frameTimer | |
| 159 | + | frameTimer = nil | |
| 160 | + | timer.ch <- time.Time{} | |
| 161 | + | if full { | |
| 162 | + | awaitTimer() | |
| 163 | + | return | |
| 164 | + | } | |
| 165 | + | awaitFrame() | |
| 166 | + | collectTimer() | |
| 167 | + | } | |
| 168 | + | ||
| 169 | + | var res copyScrollReplay | |
| 170 | + | var log []string | |
| 171 | + | start := viewport() | |
| 172 | + | last := start | |
| 173 | + | record := func() { | |
| 174 | + | if frames == 0 { | |
| 175 | + | log = append(log, "-") | |
| 176 | + | res.stalls++ | |
| 177 | + | return | |
| 178 | + | } | |
| 179 | + | top := viewport() | |
| 180 | + | log = append(log, fmt.Sprint(last-top)) | |
| 181 | + | res.maxJump = max(res.maxJump, last-top) | |
| 182 | + | last = top | |
| 183 | + | frames = 0 | |
| 184 | + | } | |
| 185 | + | ||
| 186 | + | feed(upfront) | |
| 187 | + | record() | |
| 188 | + | for ; tick < 1200 && (pending() != 0 || len(acks) != 0 || tick < 30 && perTick > 0); tick++ { | |
| 189 | + | if tick < 30 { | |
| 190 | + | feed(perTick) | |
| 191 | + | } | |
| 192 | + | for len(acks) > 0 && acks[0].at <= tick { | |
| 193 | + | f.ac.ackOutputState(acks[0].epoch, acks[0].seen) | |
| 194 | + | if rc := f.sess.renderCoordinator(); rc != nil { | |
| 195 | + | rc.notifyAck() | |
| 196 | + | } | |
| 197 | + | acks = acks[1:] | |
| 198 | + | } | |
| 199 | + | fire() | |
| 200 | + | record() | |
| 201 | + | } | |
| 202 | + | res.log = strings.Join(log, " ") | |
| 203 | + | res.scrolled = start - viewport() | |
| 204 | + | res.requested = 3 * fed | |
| 205 | + | return res | |
| 206 | + | } | |
| @@ -0,0 +1,206 @@ | |||||
| 1 | + | package daemon | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "fmt" | |||
| 5 | + | "strings" | |||
| 6 | + | "testing" | |||
| 7 | + | "time" | |||
| 8 | + | ||||
| 9 | + | "github.com/bnema/vev/internal/domain" | |||
| 10 | + | "github.com/bnema/vev/internal/protocol" | |||
| 11 | + | ) | |||
| 12 | + | ||||
| 13 | + | // TestCopyScrollOverDelayedAcks replays a wheel scroll on a link whose | |||
| 14 | + | // acknowledgements arrive rtt late, for several output windows. One tick is one | |||
| 15 | + | // animation frame (copyScrollFrame). The log shows, per tick, the rows the | |||
| 16 | + | // viewport moved: "-" is a stall, a value above one frame's worth is a catch-up | |||
| 17 | + | // jump. The test asserts that no frame jumps further than copyScrollMaxStep, | |||
| 18 | + | // that every requested row is eventually shown, and that the window the daemon | |||
| 19 | + | // is allowed to fill bounds the stalls. | |||
| 20 | + | func TestCopyScrollOverDelayedAcks(t *testing.T) { | |||
| 21 | + | // flick: 12 notches at once. sustained: one notch per tick for 30 ticks. | |||
| 22 | + | scenarios := []struct { | |||
| 23 | + | name string | |||
| 24 | + | upfront, perTick int | |||
| 25 | + | }{{"flick", 12, 0}, {"sustained", 1, 1}} | |||
| 26 | + | rtts := []time.Duration{0, 50 * time.Millisecond, 100 * time.Millisecond, 200 * time.Millisecond, 400 * time.Millisecond} | |||
| 27 | + | for _, scenario := range scenarios { | |||
| 28 | + | for _, window := range []uint8{1, protocol.MaxOutputWindow} { | |||
| 29 | + | for _, rtt := range rtts { | |||
| 30 | + | t.Run(fmt.Sprintf("%s/window%d/%v", scenario.name, window, rtt), func(t *testing.T) { | |||
| 31 | + | res := replayCopyScroll(t, scenario.upfront, scenario.perTick, window, rtt) | |||
| 32 | + | t.Logf("rows per tick (- = stall): %s", res.log) | |||
| 33 | + | if res.maxJump > copyScrollMaxStep { | |||
| 34 | + | t.Errorf("one frame jumped %d rows, want at most %d", res.maxJump, copyScrollMaxStep) | |||
| 35 | + | } | |||
| 36 | + | if res.scrolled != res.requested { | |||
| 37 | + | t.Fatalf("scrolled %d rows, want %d", res.scrolled, res.requested) | |||
| 38 | + | } | |||
| 39 | + | }) | |||
| 40 | + | } | |||
| 41 | + | } | |||
| 42 | + | } | |||
| 43 | + | } | |||
| 44 | + | ||||
| 45 | + | // TestCopyScrollWindowBoundsStalls pins the reason the output window matters: | |||
| 46 | + | // on a 100 ms link a one-frame window leaves the viewport still most ticks, | |||
| 47 | + | // while the default window keeps it moving. | |||
| 48 | + | func TestCopyScrollWindowBoundsStalls(t *testing.T) { | |||
| 49 | + | rtt := 100 * time.Millisecond | |||
| 50 | + | narrow := replayCopyScroll(t, 1, 1, 1, rtt) | |||
| 51 | + | wide := replayCopyScroll(t, 1, 1, protocol.MaxOutputWindow, rtt) | |||
| 52 | + | if wide.stalls >= narrow.stalls { | |||
| 53 | + | t.Fatalf("window %d stalled %d ticks, window 1 stalled %d: a wider window must stall less", protocol.MaxOutputWindow, wide.stalls, narrow.stalls) | |||
| 54 | + | } | |||
| 55 | + | } | |||
| 56 | + | ||||
| 57 | + | type copyScrollReplay struct { | |||
| 58 | + | log string | |||
| 59 | + | maxJump, stalls int | |||
| 60 | + | scrolled, requested int | |||
| 61 | + | } | |||
| 62 | + | ||||
| 63 | + | // replayCopyScroll feeds upfront notches at once, then perTick notches per tick | |||
| 64 | + | // for 30 ticks, acknowledging each emitted frame rtt later. A fake clock drives | |||
| 65 | + | // the animation and every step waits for its exact expected effect (a frame | |||
| 66 | + | // emitted, or the frame timer re-armed while the window is full), so the | |||
| 67 | + | // timeouts below only guard against hangs and never decide a result. | |||
| 68 | + | func replayCopyScroll(t *testing.T, upfront, perTick int, window uint8, rtt time.Duration) copyScrollReplay { | |||
| 69 | + | t.Helper() | |||
| 70 | + | f := newPerformanceFixture(t, performanceConfig{size: domain.Size{Cols: 80, Rows: 24}, panes: 1, historyRows: 400}) | |||
| 71 | + | f.ac.output.setWindow(window) | |||
| 72 | + | f.d.enterCopyMode(f.sess, f.ac) | |||
| 73 | + | f.d.copyWheel(f.sess, f.ac, -150) | |||
| 74 | + | f.ac.ackOutputState(f.ac.output.currentEpoch(), f.ac.output.next) | |||
| 75 | + | clock := newCoordinatorMockClock(t, 256) | |||
| 76 | + | f.d.clock = clock.clock | |||
| 77 | + | rt := f.ac.overlays | |||
| 78 | + | ||||
| 79 | + | type ack struct { | |||
| 80 | + | at int | |||
| 81 | + | epoch, seen uint64 | |||
| 82 | + | } | |||
| 83 | + | type emitted struct{ epoch, state uint64 } | |||
| 84 | + | var ( | |||
| 85 | + | acks []ack | |||
| 86 | + | tick int | |||
| 87 | + | frameTimer *coordinatorMockTimer | |||
| 88 | + | // emit runs under sendMu inside the render, so the epoch and state | |||
| 89 | + | // it reports are exactly those of the frame sent. | |||
| 90 | + | sent = make(chan emitted, 256) | |||
| 91 | + | ) | |||
| 92 | + | f.ac.renderStages.emit = func() { sent <- emitted{f.ac.output.currentEpoch(), f.ac.output.next} } | |||
| 93 | + | ticksPerRTT := int((rtt + copyScrollFrame - 1) / copyScrollFrame) | |||
| 94 | + | ||||
| 95 | + | viewport := func() int { | |||
| 96 | + | rt.copyMu.Lock() | |||
| 97 | + | defer rt.copyMu.Unlock() | |||
| 98 | + | return rt.copyMode.ViewportTop | |||
| 99 | + | } | |||
| 100 | + | pending := func() int { | |||
| 101 | + | rt.copyMu.Lock() | |||
| 102 | + | defer rt.copyMu.Unlock() | |||
| 103 | + | return rt.copyScroll.remaining | |||
| 104 | + | } | |||
| 105 | + | // collectTimer keeps the newest armed animation frame timer, if any. | |||
| 106 | + | collectTimer := func() { | |||
| 107 | + | for { | |||
| 108 | + | select { | |||
| 109 | + | case tm := <-clock.timers: | |||
| 110 | + | if tm.duration == copyScrollFrame { | |||
| 111 | + | frameTimer = tm | |||
| 112 | + | } | |||
| 113 | + | default: | |||
| 114 | + | return | |||
| 115 | + | } | |||
| 116 | + | } | |||
| 117 | + | } | |||
| 118 | + | frames := 0 | |||
| 119 | + | awaitFrame := func() { | |||
| 120 | + | select { | |||
| 121 | + | case e := <-sent: | |||
| 122 | + | acks = append(acks, ack{at: tick + ticksPerRTT, epoch: e.epoch, seen: e.state}) | |||
| 123 | + | frames++ | |||
| 124 | + | case <-time.After(5 * time.Second): | |||
| 125 | + | t.Fatal("expected a scroll frame, none was emitted") | |||
| 126 | + | } | |||
| 127 | + | } | |||
| 128 | + | awaitTimer := func() { | |||
| 129 | + | deadline := time.After(5 * time.Second) | |||
| 130 | + | for frameTimer == nil { | |||
| 131 | + | select { | |||
| 132 | + | case tm := <-clock.timers: | |||
| 133 | + | if tm.duration == copyScrollFrame { | |||
| 134 | + | frameTimer = tm | |||
| 135 | + | } | |||
| 136 | + | case <-deadline: | |||
| 137 | + | t.Fatal("held animation did not re-arm its frame timer") | |||
| 138 | + | } | |||
| 139 | + | } | |||
| 140 | + | } | |||
| 141 | + | fed := 0 | |||
| 142 | + | feed := func(n int) { | |||
| 143 | + | for range n { | |||
| 144 | + | idle, full := frameTimer == nil, f.ac.output.atCapacity() | |||
| 145 | + | f.d.smoothCopyWheel(f.sess, f.ac, -3) | |||
| 146 | + | fed++ | |||
| 147 | + | if idle && !full { | |||
| 148 | + | awaitFrame() | |||
| 149 | + | } | |||
| 150 | + | collectTimer() | |||
| 151 | + | } | |||
| 152 | + | } | |||
| 153 | + | fire := func() { | |||
| 154 | + | if frameTimer == nil { | |||
| 155 | + | return | |||
| 156 | + | } | |||
| 157 | + | full := f.ac.output.atCapacity() | |||
| 158 | + | timer := frameTimer | |||
| 159 | + | frameTimer = nil | |||
| 160 | + | timer.ch <- time.Time{} | |||
| 161 | + | if full { | |||
| 162 | + | awaitTimer() | |||
| 163 | + | return | |||
| 164 | + | } | |||
| 165 | + | awaitFrame() | |||
| 166 | + | collectTimer() | |||
| 167 | + | } | |||
| 168 | + | ||||
| 169 | + | var res copyScrollReplay | |||
| 170 | + | var log []string | |||
| 171 | + | start := viewport() | |||
| 172 | + | last := start | |||
| 173 | + | record := func() { | |||
| 174 | + | if frames == 0 { | |||
| 175 | + | log = append(log, "-") | |||
| 176 | + | res.stalls++ | |||
| 177 | + | return | |||
| 178 | + | } | |||
| 179 | + | top := viewport() | |||
| 180 | + | log = append(log, fmt.Sprint(last-top)) | |||
| 181 | + | res.maxJump = max(res.maxJump, last-top) | |||
| 182 | + | last = top | |||
| 183 | + | frames = 0 | |||
| 184 | + | } | |||
| 185 | + | ||||
| 186 | + | feed(upfront) | |||
| 187 | + | record() | |||
| 188 | + | for ; tick < 1200 && (pending() != 0 || len(acks) != 0 || tick < 30 && perTick > 0); tick++ { | |||
| 189 | + | if tick < 30 { | |||
| 190 | + | feed(perTick) | |||
| 191 | + | } | |||
| 192 | + | for len(acks) > 0 && acks[0].at <= tick { | |||
| 193 | + | f.ac.ackOutputState(acks[0].epoch, acks[0].seen) | |||
| 194 | + | if rc := f.sess.renderCoordinator(); rc != nil { | |||
| 195 | + | rc.notifyAck() | |||
| 196 | + | } | |||
| 197 | + | acks = acks[1:] | |||
| 198 | + | } | |||
| 199 | + | fire() | |||
| 200 | + | record() | |||
| 201 | + | } | |||
| 202 | + | res.log = strings.Join(log, " ") | |||
| 203 | + | res.scrolled = start - viewport() | |||
| 204 | + | res.requested = 3 * fed | |||
| 205 | + | return res | |||
| 206 | + | } | |||