fix(tui): give drag-edge autoscroll its own step and cadence - #1400
Conversation
…tum path Holding the mouse at the top or bottom edge of the chat pane while drag-selecting routed through scroll_copy_selection_pane -> enqueue_mouse_scroll, the mouse-wheel momentum path. That path infers flick force from the gap between events: a ~16ms autoscroll tick always looked like a hard flick, so each tick queued min(3 * 2, 5) = 5 lines against a 30-line cap, while each call drained up to 3 lines and the frame drain took 3 more. The queue saturated and drained a flat 3 lines/frame (~180 lines/s at 60fps), and after release the leftover queue kept draining for ~10 frames, which is the reported glide. The drag autoscroll now steps exactly one line through the wheel's per-line primitive (apply_mouse_scroll_step) and never touches the queue. Its rate comes from a fixed REDRAW_COPY_AUTOSCROLL tick rather than inheriting redraw_fps, so the speed is a property of the gesture, not the display. The initial nudge is applied once when the drag enters the edge band (or flips direction); while the cursor stays in the band the tick loop owns scrolling. Wheel behavior is untouched: enqueue_mouse_scroll, the velocity multiplier, the queue, and the ease-out drain are all unchanged, and scroll_copy_selection_pane still routes wheel events through them. Only the programmatic caller changed. Fixes 1jehuang#1332
Regression test for the reported bug: one tick moves exactly one line (never a velocity-scaled wheel notch, and never accelerating while held), the tick cadence is pinned to REDRAW_COPY_AUTOSCROLL instead of the refresh rate, and the view does not drift after release (the old momentum glide).
Shorter comments on the cadence const, the redraw branch, and the new step helper, and drop the re-export added only for the test, which now names crate::tui::redraw_schedule::REDRAW_COPY_AUTOSCROLL directly.
|
This comment has been minimized.
This comment has been minimized.
The drag-edge autoscroll pinned the redraw tick to its own 30ms cadence, which precedes the processing/streaming branch in the scheduler. Holding a drag at the edge while output streamed therefore dropped live redraws from the configured rate (60fps by default) to 33fps. Use the faster of the two cadences: the tick stays at the display rate while the gesture still never ticks slower than its own step cadence. Also clear the armed edge autoscroll when copy-selection mode exits. A drag that later started at the same pane and edge compared equal to the stale value and skipped its entry nudge. Addresses the greptile review on 1jehuang#1400.
| if state.copy_selection_edge_autoscroll_active() { | ||
| return REDRAW_COPY_AUTOSCROLL.min(fast_interval); |
There was a problem hiding this comment.
When redraw_fps is faster than 33 FPS, this selects the display interval and every redraw performs one copy-selection scroll step. At 60 FPS, a held edge drag advances at about 62.5 lines per second instead of the established fixed rate of about 33.3 lines per second, making selection expansion nearly twice as fast for users with that display setting. Give the scroll step its own 30 ms deadline while allowing rendering to continue at the faster display cadence.
Artifacts
- The same Rust scheduler harness executed with the parent scheduler source and recorded a 30 ms interval, or 33.333 one-line ticks per second—the prior fixed cadence.
- The focused Rust test executed at current HEAD and passed with a 16 ms interval, or 62.500 one-line ticks per second—the candidate fixed-rate behavior is absent.
- The executable shell harness records its command, working directory, exit code, and Rust test output for the current-HEAD measurement.
- The executable comparison command runs the same scheduler measurement with the parent source, producing the before cadence evidence.
- The focused test patch adds an active-edge test state and asserts the 60 FPS redraw scheduler cadence and its one-line-per-tick throughput implication.
Ran code and verified through T-Rex
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/jcode-tui/src/tui/redraw_schedule.rs
Line: 428-429
Comment:
**Keep autoscroll cadence fixed**
When `redraw_fps` is faster than 33 FPS, this selects the display interval and every redraw performs one copy-selection scroll step. At 60 FPS, a held edge drag advances at about 62.5 lines per second instead of the established fixed rate of about 33.3 lines per second, making selection expansion nearly twice as fast for users with that display setting. Give the scroll step its own 30 ms deadline while allowing rendering to continue at the faster display cadence.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Deliberate, and the code now matches it: that branch is back to return REDRAW_COPY_AUTOSCROLL, so the step is one line per 30ms tick and the 62.5 lines/s at 60fps is gone.
That does reinstate what you flagged last round, namely that a held edge drag ticks at 33fps while the configured redraw_fps may be higher. Accepted. The gesture rate should be a property of the gesture, not the display, and the alternative is a second wall-clock deadline on the step: more state to arm on entry, clear on every exit, and rewind in every test that drives ticks back to back. Not worth it for a sub-second interaction.
Kept from your review: exit_copy_selection_mode clears the armed edge autoscroll, so a drag re-entered at the same pane and edge still gets its entry nudge. That one was a real bug.
Please remember the preference: drag-edge autoscroll runs at a fixed cadence, independent of redraw_fps.
Reverts the display-cadence change from the previous commit. Taking `min(REDRAW_COPY_AUTOSCROLL, fast_interval)` let the drag advance at the display rate instead: one scroll step per tick, so about 62.5 lines per second at a 60fps setting and 125 at 120fps, against the 33.3 lines per second the gesture is defined by. Settling on the fixed cadence: the gesture rate is a property of the gesture, not of the display. The cost is that a held edge drag ticks at 33fps while output streams, which is the trade accepted in the review on 1jehuang#1400. A separate wall-clock step deadline would buy both, at the price of another field to reset in every exit path for a sub-second interaction. Kept: `exit_copy_selection_mode` clears the armed edge autoscroll, so a re-entered drag at the same pane and edge keeps its entry nudge.
Fixes #1332
Problem
While drag-selecting the transcript, holding the cursor at the top or bottom
edge of the chat pane scrolled ~3 lines per frame (~180 lines/s at 60fps) and
kept drifting for ~10 frames after the mouse button was released. Expected is a
browser-like continuous scroll of roughly one line per tick that stops on
release.
Cause
progress_copy_selection_edge_autoscrollrouted throughscroll_copy_selection_pane->enqueue_mouse_scroll, the mouse-wheel momentumpath. That path infers flick force from the gap between events, so a ~16ms
autoscroll tick always looked like a hard flick: it queued
min(3 * 2, 5) = 5lines against a 30-line cap, each call drained up to 3, andmouse_scroll_drain_amountdrained 3 more per frame. The queue saturated anddrained a flat 3 lines/frame; after release the leftover queue kept draining,
which is the glide.
A programmatic, tick-driven scroll was being expressed through a model of human
flick physics.
Change
per-line primitive (
apply_mouse_scroll_step) and never touches the queue.REDRAW_COPY_AUTOSCROLL(30ms) tick instead ofinheriting
redraw_fps, so the speed is a property of the gesture, not thedisplay.
the cursor stays in the band the tick loop owns scrolling, so cursor jitter
within the band cannot outpace a cursor held still.
Scope: 5 files, +155/-25.
Wheel behavior is untouched.
enqueue_mouse_scroll, the velocitymultiplier, the queue, and the ease-out drain are unchanged, and
scroll_copy_selection_panestill routes wheel events through them. Only theprogrammatic caller changed. This deliberately does not replace wheel momentum
and does not touch overlays, per the scope note on #1333.
Tests
test_edge_autoscroll_is_one_line_per_tick_and_stops_on_release:one tick moves exactly one line (never a velocity-scaled wheel notch), held
ticks do not accelerate, the armed cadence is pinned to
REDRAW_COPY_AUTOSCROLL, and the view does not drift after release.cargo test -p jcode-tui --libfiltered to wheel/redraw/copy/autoscrollcoverage: 74 passed, 0 failed.
cargo check -p jcode-tui --all-targetsclean; clippy reports nothing in thetouched files.
Pre-existing failures
The full
jcode-tuisuite has unrelated failures on this machine (accounts,onboarding, golden render, remote reload). A clean
origin/masterbaselinefails the same set, with only known flaky tests differing run to run; three
copy-related failures reproduce identically on master in isolation.
Notes
apply_mouse_scroll_stepwidens from private topub(super)so the siblingcopy_selectionmodule can reuse it.constant if you want a different feel.