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>

2 files changed, 321 insertions, 43 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))
+        );
+    }
 }
diff --git a/src/pane/terminal.rs b/src/pane/terminal.rs
index 4d7f07b1..c30ef415 100644
--- a/src/pane/terminal.rs
+++ b/src/pane/terminal.rs
@@ -21,7 +21,9 @@ mod migration_tests;
 #[cfg(windows)]
 mod windows_recent_fallback;
 
-use super::cursor::{CursorPositionSettleState, DecscusrTracker, CURSOR_POSITION_SETTLE};
+#[cfg(test)]
+use super::cursor::CURSOR_POSITION_SETTLE;
+use super::cursor::{CursorPositionSettleState, DecscusrTracker};
 use super::{
     input::{
         ghostty_key_event_from_terminal_key, ghostty_mouse_encoder_for_terminal,
@@ -1445,7 +1447,8 @@ impl GhosttyPaneTerminal {
             .terminal
             .mode_get(crate::ghostty::MODE_SYNCHRONIZED_OUTPUT)
             .unwrap_or(false);
-        if CURSOR_POSITION_SETTLE_ENABLED {
+        // Intermediate synchronized-frame positions must not become settled cursors.
+        if CURSOR_POSITION_SETTLE_ENABLED && !synchronized_output {
             let cursor_started = crate::render_prof::timer();
             let cursor_after_write = current_cursor_state(&mut core);
             crate::render_prof::duration_since("pty.cursor_state_update", cursor_started);
@@ -1463,7 +1466,7 @@ impl GhosttyPaneTerminal {
         let render_delay = render_delay_after_pty_write(
             synchronized_output,
             has_kitty_graphics_sequence,
-            cursor_position_settle_pending(&core),
+            core.cursor_settle_state.render_delay(),
             CURSOR_POSITION_SETTLE_ENABLED,
         );
         if request_render {
@@ -2476,10 +2479,6 @@ fn encoded_key_preserves_event_kind(
         })
 }
 
-fn cursor_position_settle_pending(core: &GhosttyPaneCore) -> bool {
-    core.cursor_settle_state.pending()
-}
-
 fn effective_cursor_state(
     core: &mut GhosttyPaneCore,
     current: Option<TerminalCursorState>,
@@ -2494,17 +2493,16 @@ fn effective_cursor_state(
 fn render_delay_after_pty_write(
     synchronized_output: bool,
     has_kitty_graphics_sequence: bool,
-    cursor_position_settle_pending: bool,
+    cursor_position_settle_delay: Option<Duration>,
     cursor_position_settle_enabled: bool,
 ) -> Option<Duration> {
     if synchronized_output {
         None
-    } else if has_kitty_graphics_sequence {
-        Some(KITTY_GRAPHICS_REDRAW_SETTLE)
-    } else if cursor_position_settle_enabled && cursor_position_settle_pending {
-        Some(CURSOR_POSITION_SETTLE)
     } else {
-        None
+        let cursor_delay = cursor_position_settle_enabled
+            .then_some(cursor_position_settle_delay)
+            .flatten();
+        cursor_delay.max(has_kitty_graphics_sequence.then_some(KITTY_GRAPHICS_REDRAW_SETTLE))
     }
 }
 
@@ -4374,12 +4372,109 @@ mod tests {
 
         let result = pane.process_pty_bytes(pane_id, 0, b"\x1b[6;21H", &tx);
 
-        assert_eq!(result.render_delay, Some(CURSOR_POSITION_SETTLE));
+        assert_eq!(result.render_delay, Some(Duration::from_millis(100)));
         assert_eq!(
             pane.cursor_state()
                 .map(|cursor| (cursor.x, cursor.y, cursor.visible)),
             Some((1, 0, true))
         );
+        // If output stops here, the scheduled repaint must be late enough to
+        // publish this cursor without relying on an unrelated later redraw.
+        let mut core = pane.core.lock().unwrap();
+        let current = current_cursor_state(&mut core);
+        assert_eq!(
+            core.cursor_settle_state
+                .reported_cursor(current, Instant::now() + result.render_delay.unwrap()),
+            current
+        );
+    }
+
+    #[test]
+    #[cfg(windows)]
+    fn cursor_settle_ignores_intermediate_synchronized_frame_positions() {
+        let (tx, _rx) = mpsc::channel(4);
+        let terminal = crate::ghostty::Terminal::new(80, 24, 0).unwrap();
+        let pane = GhosttyPaneTerminal::new(terminal, tx.clone()).unwrap();
+        let pane_id = PaneId::from_raw(1);
+        pane.process_pty_bytes(pane_id, 0, b"\x1b[15;4H", &tx);
+
+        let previous = TerminalCursorState {
+            x: 3,
+            y: 14,
+            visible: true,
+            shape: 0,
+        };
+        {
+            let mut core = pane.core.lock().unwrap();
+            let now = Instant::now();
+            core.cursor_settle_state = CursorPositionSettleState::default();
+            core.cursor_settle_state.observe(
+                Some(TerminalCursorState { x: 2, ..previous }),
+                now - Duration::from_millis(300),
+            );
+            // Seed a pending hold whose deadline has passed, without wall-clock sleeps.
+            core.cursor_settle_state
+                .observe(Some(previous), now - Duration::from_millis(200));
+        }
+
+        for bytes in [
+            b"\x1b[?2026h\x1b[15;4Hx\x1b[13;1H".as_slice(),
+            b"\x1b[0 q\x1b[13;1H \x1b[15;5H",
+            b"\x1b[?25h",
+        ] {
+            let result = pane.process_pty_bytes(pane_id, 0, bytes, &tx);
+            assert!(!result.request_render);
+            assert_eq!(result.render_delay, None);
+            assert_eq!(pane.cursor_state(), Some(previous));
+            assert!(pane.core.lock().unwrap().cursor_settle_state.pending());
+        }
+
+        let result = pane.process_pty_bytes(pane_id, 0, b"\x1b[?2026l", &tx);
+        assert!(result.request_render);
+        assert_eq!(result.render_delay, Some(CURSOR_POSITION_SETTLE));
+        assert!(pane.core.lock().unwrap().cursor_settle_state.pending());
+        assert_eq!(pane.cursor_state(), Some(previous));
+
+        // ConPTY may restore the real caret after the synchronized frame closes.
+        let mut core = pane.core.lock().unwrap();
+        let current = current_cursor_state(&mut core);
+        assert_eq!(
+            core.cursor_settle_state
+                .reported_cursor(current, Instant::now() + CURSOR_POSITION_SETTLE),
+            Some(TerminalCursorState { x: 4, ..previous })
+        );
+    }
+
+    #[test]
+    #[cfg(windows)]
+    fn cursor_settle_preserves_final_visibility_and_shape_across_split_sync_sequences() {
+        let (tx, _rx) = mpsc::channel(4);
+        let terminal = crate::ghostty::Terminal::new(80, 24, 0).unwrap();
+        let pane = GhosttyPaneTerminal::new(terminal, tx.clone()).unwrap();
+        let pane_id = PaneId::from_raw(1);
+        pane.process_pty_bytes(pane_id, 0, b"\x1b[15;4H", &tx);
+
+        for bytes in [
+            b"\x1b[?202".as_slice(),
+            b"6h\x1b[13;1H",
+            b"\x1b[6 q\x1b[15;5H\x1b[?25l\x1b[?20",
+        ] {
+            pane.process_pty_bytes(pane_id, 0, bytes, &tx);
+            assert!(!pane.core.lock().unwrap().cursor_settle_state.pending());
+        }
+
+        let result = pane.process_pty_bytes(pane_id, 0, b"26l", &tx);
+        assert!(result.request_render);
+        assert_eq!(result.render_delay, None);
+        assert_eq!(
+            pane.cursor_state(),
+            Some(TerminalCursorState {
+                x: 4,
+                y: 14,
+                visible: false,
+                shape: 6,
+            })
+        );
     }
 
     #[test]
@@ -4403,19 +4498,25 @@ mod tests {
 
     #[test]
     fn cursor_settle_policy_controls_render_delay() {
+        let delay = Some(CURSOR_POSITION_SETTLE);
         assert_eq!(
-            render_delay_after_pty_write(false, false, true, true),
-            Some(CURSOR_POSITION_SETTLE)
+            render_delay_after_pty_write(false, false, delay, true),
+            delay
         );
         assert_eq!(
-            render_delay_after_pty_write(false, false, true, false),
+            render_delay_after_pty_write(false, false, delay, false),
             None
         );
         assert_eq!(
-            render_delay_after_pty_write(false, true, true, false),
+            render_delay_after_pty_write(false, true, delay, false),
             Some(KITTY_GRAPHICS_REDRAW_SETTLE)
         );
-        assert_eq!(render_delay_after_pty_write(true, false, true, true), None);
+        assert_eq!(render_delay_after_pty_write(true, false, delay, true), None);
+        let jump_delay = Some(Duration::from_millis(100));
+        assert_eq!(
+            render_delay_after_pty_write(false, true, jump_delay, true),
+            jump_delay
+        );
     }
 
     #[test]