fix: stabilize windows cursor redraws and settle deadlines (#4389)

* fix: prevent cursor jumps during synchronized redraws

refs #4303

* fix: settle cursor positions across delayed windows redraws

refs #4303

* fix(windows): hold jump-shaped cursor moves through the max window

* fix: bound windows cursor settling and repaint at its deadline

refs #4303

---------

Co-authored-by: Ogulcan Celik <ogulcancelik@gmail.com>
Co-authored-by: JJLiebig <jj@liebig.gg>
Co-authored-by: Axel Karlsson <axelalbertkarlsson@outlook.com>

1 files changed, 201 insertions, 24 deletions

diff --git a/src/pane/cursor.rs b/src/pane/cursor.rs
index c72f697a..13ac61f0 100644
--- a/src/pane/cursor.rs
+++ b/src/pane/cursor.rs
@@ -90,55 +90,59 @@ pub(crate) struct CursorPositionSettleState {
     settled: Option<TerminalCursorState>,
     candidate: Option<TerminalCursorState>,
     pending_since: Option<Instant>,
+    candidate_since: Option<Instant>,
+    /// True when this candidate jumped away from the settled caret (a different
+    /// row, or a large same-row column move).
+    ///
+    /// Those are the shape of a redraw parking the cursor on a temporary cell,
+    /// so they are held for the max window before being shown. Ordinary caret
+    /// steps are small and same-row, and settle on the normal window.
+    candidate_jump: bool,
 }
 
 impl CursorPositionSettleState {
     pub(crate) fn observe(&mut self, current: Option<TerminalCursorState>, now: Instant) {
+        // A candidate that stayed quiet for its hold window is real, so preserve
+        // it before considering the first (possibly temporary) position of a
+        // later redraw.
+        if let (Some(candidate), Some(since)) = (self.candidate, self.candidate_since) {
+            if now.duration_since(since) >= self.candidate_hold() {
+                self.settle(Some(candidate));
+            }
+        }
         let Some(current) = current else {
-            self.settled = None;
-            self.candidate = None;
-            self.pending_since = None;
+            self.settle(None);
             return;
         };
         if !current.visible {
-            self.settled = Some(current);
-            self.candidate = None;
-            self.pending_since = None;
+            self.settle(Some(current));
             return;
         }
         let Some(settled) = self.settled else {
-            self.settled = Some(current);
-            self.candidate = None;
-            self.pending_since = None;
+            self.settle(Some(current));
             return;
         };
         if same_cursor_position(settled, current) && settled.visible {
-            self.settled = Some(current);
-            self.candidate = None;
-            self.pending_since = None;
+            self.settle(Some(current));
             return;
         }
 
         let Some(candidate) = self.candidate else {
             self.candidate = Some(current);
             self.pending_since = Some(now);
+            self.candidate_since = Some(now);
+            self.candidate_jump = is_jump(settled, current);
             return;
         };
 
         let pending_since = self.pending_since.unwrap_or(now);
         if now.duration_since(pending_since) >= CURSOR_POSITION_MAX_HOLD {
-            self.settled = Some(current);
-            self.candidate = None;
-            self.pending_since = None;
-        } else if same_cursor_position(candidate, current) {
-            if now.duration_since(pending_since) >= CURSOR_POSITION_SETTLE {
-                self.settled = Some(current);
-                self.candidate = None;
-                self.pending_since = None;
-            } else {
-                self.candidate = Some(current);
-            }
+            self.settle(Some(current));
         } else {
+            if !same_cursor_position(candidate, current) {
+                self.candidate_since = Some(now);
+                self.candidate_jump = is_jump(settled, current);
+            }
             self.candidate = Some(current);
         }
     }
@@ -152,8 +156,14 @@ impl CursorPositionSettleState {
         let Some(candidate) = self.candidate else {
             return Some(current);
         };
+        let candidate_since = self.candidate_since.unwrap_or(now);
         let pending_since = self.pending_since.unwrap_or(now);
-        if now.duration_since(pending_since) >= CURSOR_POSITION_SETTLE {
+        // A jump-shaped candidate is treated as a redraw park until proven
+        // otherwise, so it waits for the max window. An ordinary caret step only
+        // waits the normal settle window; the max hold bounds either case.
+        if now.duration_since(candidate_since) >= self.candidate_hold()
+            || now.duration_since(pending_since) >= CURSOR_POSITION_MAX_HOLD
+        {
             return Some(TerminalCursorState {
                 visible: current.visible && candidate.visible,
                 shape: current.shape,
@@ -176,12 +186,37 @@ impl CursorPositionSettleState {
     pub(crate) fn pending(&self) -> bool {
         self.candidate.is_some()
     }
+
+    pub(crate) fn render_delay(&self) -> Option<Duration> {
+        self.pending().then(|| self.candidate_hold())
+    }
+
+    fn candidate_hold(&self) -> Duration {
+        if self.candidate_jump {
+            CURSOR_POSITION_MAX_HOLD
+        } else {
+            CURSOR_POSITION_SETTLE
+        }
+    }
+
+    fn settle(&mut self, cursor: Option<TerminalCursorState>) {
+        *self = Self {
+            settled: cursor,
+            ..Self::default()
+        };
+    }
 }
 
 fn same_cursor_position(left: TerminalCursorState, right: TerminalCursorState) -> bool {
     left.x == right.x && left.y == right.y
 }
 
+/// A cursor move that changes row, or jumps more than a couple of columns, is
+/// the shape of a redraw parking the cursor rather than an ordinary caret step.
+fn is_jump(settled: TerminalCursorState, current: TerminalCursorState) -> bool {
+    current.y != settled.y || current.x.abs_diff(settled.x) > 2
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -227,11 +262,60 @@ mod tests {
     }
 
     #[test]
+    fn cursor_settle_keeps_previous_caret_during_next_system_conpty_redraw() {
+        let now = Instant::now();
+        let mut settle = CursorPositionSettleState::default();
+        let caret = cursor(2, 12, true, 0);
+        let typed_caret = cursor(3, 12, true, 0);
+        let repair = cursor(0, 10, true, 0);
+        settle.observe(Some(caret), now);
+        settle.observe(Some(typed_caret), now + Duration::from_millis(1));
+
+        // System ConPTY closes the next frame at the repair cell, then emits
+        // the caret restoration separately about 10 ms later.
+        settle.observe(Some(repair), now + Duration::from_millis(160));
+        assert_eq!(
+            settle.reported_cursor(Some(repair), now + Duration::from_millis(161)),
+            Some(typed_caret)
+        );
+        settle.observe(Some(typed_caret), now + Duration::from_millis(170));
+        assert_eq!(
+            settle.reported_cursor(Some(typed_caret), now + Duration::from_millis(171)),
+            Some(typed_caret)
+        );
+    }
+
+    #[test]
+    fn cursor_settle_restarts_quiet_window_when_candidate_moves() {
+        let now = Instant::now();
+        let mut settle = CursorPositionSettleState::default();
+        let initial = cursor(1, 0, true, 0);
+        let latest = cursor(3, 0, true, 0);
+        settle.observe(Some(initial), now);
+        settle.observe(Some(cursor(2, 0, true, 0)), now + Duration::from_millis(1));
+        settle.observe(Some(latest), now + Duration::from_millis(19));
+        assert_eq!(
+            settle.reported_cursor(Some(latest), now + Duration::from_millis(22)),
+            Some(initial)
+        );
+        assert_eq!(
+            settle.reported_cursor(Some(latest), now + Duration::from_millis(39)),
+            Some(latest)
+        );
+    }
+
+    #[test]
     fn cursor_settle_caps_continuous_position_changes_from_first_pending_time() {
         let now = Instant::now();
         let mut settle = CursorPositionSettleState::default();
         settle.observe(Some(cursor(1, 0, true, 0)), now);
         settle.observe(Some(cursor(2, 0, true, 0)), now + Duration::from_millis(1));
+        for ms in (11..=91).step_by(10) {
+            settle.observe(
+                Some(cursor(ms as u16, 0, true, 0)),
+                now + Duration::from_millis(ms),
+            );
+        }
         settle.observe(
             Some(cursor(3, 0, true, 0)),
             now + CURSOR_POSITION_MAX_HOLD + Duration::from_millis(1),
@@ -248,6 +332,57 @@ mod tests {
     }
 
     #[test]
+    fn cursor_settle_caps_hold_even_without_another_observation() {
+        let now = Instant::now();
+        for step in [10, 30] {
+            let mut settle = CursorPositionSettleState::default();
+            settle.observe(Some(cursor(0, 0, true, 0)), now);
+            for ms in (1..=91).step_by(step) {
+                settle.observe(
+                    Some(cursor(ms as u16, 1, true, 0)),
+                    now + Duration::from_millis(ms),
+                );
+            }
+            assert_eq!(
+                settle.reported_cursor(
+                    Some(cursor(91, 1, true, 0)),
+                    now + Duration::from_millis(101)
+                ),
+                Some(cursor(91, 1, true, 0))
+            );
+        }
+    }
+
+    #[test]
+    fn cursor_settle_repeated_position_does_not_restart_quiet_window() {
+        let now = Instant::now();
+        let mut settle = CursorPositionSettleState::default();
+        let next = cursor(2, 0, true, 0);
+        settle.observe(Some(cursor(1, 0, true, 0)), now);
+        settle.observe(Some(next), now + Duration::from_millis(1));
+        settle.observe(Some(next), now + Duration::from_millis(19));
+        assert_eq!(
+            settle.reported_cursor(Some(next), now + Duration::from_millis(21)),
+            Some(next)
+        );
+    }
+
+    #[test]
+    fn cursor_settle_repeated_jump_position_does_not_starve() {
+        let now = Instant::now();
+        let mut settle = CursorPositionSettleState::default();
+        let next = cursor(2, 1, true, 0);
+        settle.observe(Some(cursor(2, 0, true, 0)), now);
+        for ms in [1, 31, 61, 91] {
+            settle.observe(Some(next), now + Duration::from_millis(ms));
+        }
+        assert_eq!(
+            settle.reported_cursor(Some(next), now + Duration::from_millis(101)),
+            Some(next)
+        );
+    }
+
+    #[test]
     fn cursor_settle_keeps_render_read_pure() {
         let now = Instant::now();
         let mut settle = CursorPositionSettleState::default();
@@ -312,4 +447,46 @@ mod tests {
             Some(cursor(1, 0, false, 0))
         );
     }
+
+    #[test]
+    fn cursor_settle_does_not_publish_a_jump_before_the_max_hold() {
+        let now = Instant::now();
+        let mut settle = CursorPositionSettleState::default();
+        let caret = cursor(2, 26, true, 0);
+        let park = cursor(0, 25, true, 0);
+        settle.observe(Some(caret), now);
+
+        // A transition redraw parks the cursor on a cell above the composer and
+        // restores the caret in a later write, more than one settle window away.
+        // The park must not be published in the meantime.
+        settle.observe(Some(park), now + Duration::from_millis(1));
+        assert_eq!(
+            settle.reported_cursor(
+                Some(park),
+                now + CURSOR_POSITION_SETTLE + Duration::from_millis(1)
+            ),
+            Some(caret)
+        );
+        assert_eq!(
+            settle.reported_cursor(Some(park), now + Duration::from_millis(70)),
+            Some(caret)
+        );
+
+        // An ordinary same-row caret step still settles on the normal window.
+        settle.observe(
+            Some(cursor(3, 26, true, 0)),
+            now + Duration::from_millis(80),
+        );
+        settle.observe(
+            Some(cursor(4, 26, true, 0)),
+            now + Duration::from_millis(81),
+        );
+        assert_eq!(
+            settle.reported_cursor(
+                Some(cursor(4, 26, true, 0)),
+                now + CURSOR_POSITION_SETTLE + Duration::from_millis(82)
+            ),
+            Some(cursor(4, 26, true, 0))
+        );
+    }
 }