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, candidate: Option, pending_since: Option, + candidate_since: Option, + /// 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, 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 { + 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) { + *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::*; @@ -226,12 +261,61 @@ mod tests { assert_eq!((reported.x, reported.y), (2, 0)); } + #[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), @@ -247,6 +331,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(); @@ -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, @@ -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, cursor_position_settle_enabled: bool, ) -> Option { 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]