From 36de78dd4fa4e2c203ee955f18caf82a6bca7e80 Mon Sep 17 00:00:00 2001 From: Ogulcan Celik Date: Fri, 24 Jul 2026 01:30:05 +0300 Subject: [PATCH] fix: preserve kitty graphics during host repaints refs #1628 --- src/app/mod.rs | 8 +- src/app/runtime.rs | 2 +- src/client/mod.rs | 34 +++++--- src/protocol/render_ansi.rs | 97 ++++++++++++++++------ src/server/clients.rs | 5 +- src/server/headless.rs | 13 +-- src/server/headless/tests/pane_graphics.rs | 48 ++++++++++- src/server/render_stream.rs | 56 ++++++++----- 8 files changed, 192 insertions(+), 71 deletions(-) diff --git a/src/app/mod.rs b/src/app/mod.rs index f6dbe261..0c0a2688 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -862,7 +862,7 @@ impl App { self.terminal_runtimes.assume_handoff_ownership(); } - fn request_full_redraw(&mut self) { + fn request_repaint(&mut self) { self.full_redraw_pending = true; } @@ -1046,10 +1046,10 @@ impl App { let _sync_output = SyncOutputGuard::begin()?; let kitty_graphics_enabled = self.state.kitty_graphics_enabled; if self.full_redraw_pending { - if kitty_graphics_enabled { - crate::kitty_graphics::clear_all_host_graphics()?; + for cell in &mut terminal.current_buffer_mut().content { + cell.set_skip(true); } - terminal.clear()?; + terminal.swap_buffers(); self.full_redraw_pending = false; } let mut cell_size = crate::kitty_graphics::HostCellSize::default(); diff --git a/src/app/runtime.rs b/src/app/runtime.rs index 97054f4b..428cd35b 100644 --- a/src/app/runtime.rs +++ b/src/app/runtime.rs @@ -209,7 +209,7 @@ impl App { crate::raw_input::RawInputEvent::OuterFocusGained => { self.send_outer_focus_event(crate::ghostty::FocusEvent::Gained); if self.state.redraw_on_focus_gained { - self.request_full_redraw(); + self.request_repaint(); } self.state.outer_terminal_focus = Some(true); self.state.mark_active_tab_seen(); diff --git a/src/client/mod.rs b/src/client/mod.rs index b1c363f2..23e331e8 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -87,6 +87,8 @@ struct ClientState { remote_image_paste_key: Option<(crossterm::event::KeyCode, crossterm::event::KeyModifiers)>, /// Whether outer focus gain should force a full host-terminal redraw. redraw_on_focus_gained: bool, + /// Whether the next semantic frame must repaint every cell without clearing the surface. + repaint_pending: bool, /// Whether this client draws the cursor into frame cells instead of using the host cursor. draw_host_cursor: bool, } @@ -220,8 +222,8 @@ fn attach_scroll_action( } impl ClientState { - fn request_full_redraw(&mut self) { - self.blit_encoder = render_ansi::BlitEncoder::new(); + fn request_repaint(&mut self) { + self.repaint_pending = true; } } @@ -1302,6 +1304,7 @@ async fn run_client_loop( #[cfg(unix)] remote_image_paste_key: config.remote_image_paste_key, redraw_on_focus_gained: config.redraw_on_focus_gained, + repaint_pending: false, draw_host_cursor, }; debug!(?negotiated_encoding, "client render encoding active"); @@ -1417,7 +1420,7 @@ async fn run_client_loop( &events, state.redraw_on_focus_gained, ) { - state.request_full_redraw(); + state.request_repaint(); } if crate::raw_input::events_require_host_terminal_theme_query(&events) { query_host_terminal_theme(); @@ -1489,7 +1492,7 @@ async fn run_client_loop( &raw_events, state.redraw_on_focus_gained, ) { - state.request_full_redraw(); + state.request_repaint(); } let msg = ClientMessage::InputEvents { events }; if let Err(e) = write_to_server(&mut write_stream, &msg) { @@ -1516,11 +1519,14 @@ async fn run_client_loop( frame_data }; let encoded = if state.draw_host_cursor { + state.blit_encoder.encode_with_suppressed_visible_cursor( + &frame_data, + state.repaint_pending, + ) + } else { state .blit_encoder - .encode_with_suppressed_visible_cursor(&frame_data, false) - } else { - state.blit_encoder.encode(&frame_data, false) + .encode(&frame_data, state.repaint_pending) }; let mut stdout = io::stdout(); let graphics = if state.kitty_graphics_enabled { @@ -1532,6 +1538,7 @@ async fn run_client_loop( write_encoded_frame_with_graphics(&mut stdout, &encoded.bytes, graphics); let _ = stdout.flush(); state.blit_encoder.commit(frame_data, encoded); + state.repaint_pending = false; } ServerMessage::Terminal(frame) => { if state.kitty_graphics_enabled && contains_kitty_graphics_bytes(&frame.bytes) { @@ -1983,15 +1990,18 @@ fn write_encoded_frame_with_graphics( encoded: &[u8], graphics: &[u8], ) -> io::Result<()> { - writer.write_all(encoded)?; if graphics.is_empty() { - return Ok(()); + return writer.write_all(encoded); } + let insertion = render_ansi::final_sync_output_end(encoded).unwrap_or(encoded.len()); + + writer.write_all(&encoded[..insertion])?; record_received_kitty_graphics(graphics); writer.write_all(b"\x1b7")?; writer.write_all(graphics)?; - writer.write_all(b"\x1b8") + writer.write_all(b"\x1b8")?; + writer.write_all(&encoded[insertion..]) } fn contains_kitty_graphics_bytes(bytes: &[u8]) -> bool { @@ -2360,7 +2370,7 @@ mod tests { } #[test] - fn graphics_bytes_are_written_after_blit_with_saved_cursor() { + fn graphics_bytes_are_written_inside_synchronized_blit_with_saved_cursor() { let mut output = Vec::new(); write_encoded_frame_with_graphics( &mut output, @@ -2371,7 +2381,7 @@ mod tests { assert_eq!( output, - b"\x1b[?2026htext\x1b[?2026lcursor\x1b7graphics\x1b8" + b"\x1b[?2026htext\x1b7graphics\x1b8\x1b[?2026lcursor" ); } diff --git a/src/protocol/render_ansi.rs b/src/protocol/render_ansi.rs index f02ec2bc..6cee4fde 100644 --- a/src/protocol/render_ansi.rs +++ b/src/protocol/render_ansi.rs @@ -34,6 +34,13 @@ use unicode_width::UnicodeWidthStr; use crate::protocol::{underline_style_from_modifier, CellData, FrameData}; const REVERSED_MODIFIER: u16 = 1 << 6; +const SYNC_OUTPUT_END: &[u8] = b"\x1b[?2026l"; + +pub(crate) fn final_sync_output_end(bytes: &[u8]) -> Option { + bytes + .windows(SYNC_OUTPUT_END.len()) + .rposition(|window| window == SYNC_OUTPUT_END) +} /// Bytes produced by a [`BlitEncoder`] for one terminal frame. pub(crate) struct EncodedBlit { @@ -58,44 +65,44 @@ impl BlitEncoder { Self::default() } - pub(crate) fn encode(&self, frame: &FrameData, force_full: bool) -> EncodedBlit { - self.encode_inner(frame, force_full, false) + pub(crate) fn encode(&self, frame: &FrameData, repaint: bool) -> EncodedBlit { + self.encode_inner(frame, repaint, false) } pub(crate) fn encode_with_suppressed_visible_cursor( &self, frame: &FrameData, - force_full: bool, + repaint: bool, ) -> EncodedBlit { - self.encode_inner(frame, force_full, true) + self.encode_inner(frame, repaint, true) } fn encode_inner( &self, frame: &FrameData, - force_full: bool, + repaint: bool, suppress_visible_cursor: bool, ) -> EncodedBlit { - let prev = if force_full { - None - } else { - self.last_frame.as_ref() - }; - let full = force_full + let previous_frame = self.last_frame.as_ref(); + let prev = if repaint { None } else { previous_frame }; + let full = repaint || prev.is_none() || prev.is_some_and(|p| p.width != frame.width || p.height != frame.height); + let clear_before_full_redraw = previous_frame.is_none(); let prof_stats = crate::render_prof::enabled().then(|| compute_prof_blit_stats(frame, prev, full)); let prof_started = crate::render_prof::timer(); let mut bytes = Vec::new(); let mut next_last_visible_cursor = self.last_visible_cursor; let mut next_last_cursor_shape = self.last_cursor_shape; - blit_frame_to_with_cursor_memory( + blit_frame_to_with_cursor_memory_and_clear_policy( &mut bytes, frame, prev, &mut next_last_visible_cursor, &mut next_last_cursor_shape, + repeat_ime_anchor_after_sync(), + clear_before_full_redraw, suppress_visible_cursor, ); if let Some(stats) = prof_stats { @@ -395,8 +402,9 @@ fn blit_frame_to(writer: impl Write, frame: &FrameData, prev: Option<&FrameData> ); } +#[cfg(test)] fn blit_frame_to_with_cursor_memory( - mut writer: impl Write, + writer: impl Write, frame: &FrameData, prev: Option<&FrameData>, last_visible_cursor: &mut Option<(u16, u16)>, @@ -404,7 +412,7 @@ fn blit_frame_to_with_cursor_memory( suppress_visible_cursor: bool, ) { blit_frame_to_with_cursor_memory_and_policy( - &mut writer, + writer, frame, prev, last_visible_cursor, @@ -414,13 +422,36 @@ fn blit_frame_to_with_cursor_memory( ); } +#[cfg(test)] fn blit_frame_to_with_cursor_memory_and_policy( + writer: impl Write, + frame: &FrameData, + prev: Option<&FrameData>, + last_visible_cursor: &mut Option<(u16, u16)>, + last_cursor_shape: &mut u8, + repeat_ime_anchor: bool, + suppress_visible_cursor: bool, +) { + blit_frame_to_with_cursor_memory_and_clear_policy( + writer, + frame, + prev, + last_visible_cursor, + last_cursor_shape, + repeat_ime_anchor, + true, + suppress_visible_cursor, + ); +} + +fn blit_frame_to_with_cursor_memory_and_clear_policy( mut writer: impl Write, frame: &FrameData, prev: Option<&FrameData>, last_visible_cursor: &mut Option<(u16, u16)>, last_cursor_shape: &mut u8, repeat_ime_anchor: bool, + clear_before_full_redraw: bool, suppress_visible_cursor: bool, ) { // On first frame or size change, do a full redraw. @@ -442,8 +473,9 @@ fn blit_frame_to_with_cursor_memory_and_policy( let _ = writer.write_all(b"\x1b]8;;\x1b\\"); if full_redraw { - // Clear the screen and write all cells. - let _ = writer.write_all(b"\x1b[2J\x1b[H"); + if clear_before_full_redraw { + let _ = writer.write_all(b"\x1b[2J"); + } write_all_cells(&mut writer, frame); } else { // Diff-based update: only write changed cells. @@ -1459,19 +1491,34 @@ mod tests { } #[test] - fn blit_frame_size_change_triggers_full_redraw() { + fn encoder_size_change_repaints_without_clearing() { let prev = make_frame(2, 2, vec![make_cell("A", 0, 0, 0); 4]); - let curr = make_frame(3, 2, vec![make_cell("B", 0, 0, 0); 6]); + let mut encoder = BlitEncoder::new(); + let initial = encoder.encode(&prev, false); + encoder.commit(prev, initial); - let mut output = Vec::new(); - blit_frame_to(&mut output, &curr, Some(&prev)); + let encoded = encoder.encode(&curr, false); + assert!(encoded.full); + let output = String::from_utf8(encoded.bytes).unwrap(); - let output_str = String::from_utf8(output).unwrap(); - assert!( - output_str.contains("\x1b[2J"), - "size change should trigger full redraw" - ); + assert!(!output.contains("\x1b[2J")); + assert!(output.bytes().filter(|byte| *byte == b'B').count() >= 6); + } + + #[test] + fn encoder_forced_repaint_writes_all_cells_without_clearing() { + let frame = make_frame(3, 2, vec![make_cell("A", 0, 0, 0); 6]); + let mut encoder = BlitEncoder::new(); + let initial = encoder.encode(&frame, false); + encoder.commit(frame.clone(), initial); + + let encoded = encoder.encode(&frame, true); + assert!(encoded.full); + let output = String::from_utf8(encoded.bytes).unwrap(); + + assert!(!output.contains("\x1b[2J")); + assert!(output.bytes().filter(|byte| *byte == b'A').count() >= 6); } #[test] diff --git a/src/server/clients.rs b/src/server/clients.rs index 3782bd15..c6c617ec 100644 --- a/src/server/clients.rs +++ b/src/server/clients.rs @@ -135,9 +135,8 @@ impl ClientConnection { } } - pub(crate) fn request_full_redraw(&mut self) { - self.render_state.reset_baseline(); - self.graphics_surface_reset_pending = true; + pub(crate) fn request_repaint(&mut self) { + self.render_state.request_repaint(); self.pane_graphics_render_pending = false; } diff --git a/src/server/headless.rs b/src/server/headless.rs index f330fbec..aaa3742a 100644 --- a/src/server/headless.rs +++ b/src/server/headless.rs @@ -971,7 +971,7 @@ impl HeadlessServer { // rendering semantics. Force one fresh frame to every remaining client // even if the next rendered buffer compares equal to its cached frame. for client in self.clients.values_mut() { - client.request_full_redraw(); + client.request_repaint(); } if !start_pending_agent_resumes { self.app.pending_agent_resume_deadline = None; @@ -984,7 +984,7 @@ impl HeadlessServer { .start_pending_agent_resumes(self.app.pending_agent_resume_due(now)) { for client in self.clients.values_mut() { - client.request_full_redraw(); + client.request_repaint(); } } } @@ -2585,7 +2585,7 @@ impl HeadlessServer { ); if let Some(client) = self.clients.get_mut(&client_id) { if host_surface_redraw { - client.request_full_redraw(); + client.request_repaint(); client.defer_full_render(); } else { // Ensure semantic clients receive one post-input frame even if the @@ -2872,7 +2872,7 @@ impl HeadlessServer { width_px: cell_width_px, height_px: cell_height_px, }; - render_state.reset_baseline(); + render_state.request_repaint(); Some(terminal_id.clone()) } else { None @@ -2896,7 +2896,7 @@ impl HeadlessServer { width_px: cell_width_px, height_px: cell_height_px, }; - render_state.reset_baseline(); + render_state.request_repaint(); return true; } if let Some(client) = self.clients.get_mut(&client_id) { @@ -7611,7 +7611,7 @@ next_tab = "" } #[test] - fn outer_focus_gained_forces_terminal_ansi_full_redraw() { + fn outer_focus_gained_repaints_terminal_ansi_without_clearing() { let mut server = test_headless_server(); let (client_tx, _client_control_rx, client_rx) = test_client_writer(); @@ -7644,6 +7644,7 @@ next_tab = "" ServerMessage::Terminal(frame) => { assert_eq!(frame.seq, 2); assert!(frame.full); + assert!(!frame.bytes.windows(4).any(|bytes| bytes == b"\x1b[2J")); } other => panic!("expected terminal frame, got {other:?}"), } diff --git a/src/server/headless/tests/pane_graphics.rs b/src/server/headless/tests/pane_graphics.rs index 627ffc78..1eba9816 100644 --- a/src/server/headless/tests/pane_graphics.rs +++ b/src/server/headless/tests/pane_graphics.rs @@ -85,6 +85,52 @@ fn stream_set_message( ) } +#[tokio::test] +async fn focus_repaint_preserves_uploaded_graphics() { + let (mut server, client_rx, pane_id) = retained_test_server(b"aaaa"); + let (client_2_writer, _client_2_control_rx, client_2_rx) = test_client_writer(); + server.clients.insert( + 2, + ClientConnection::new( + (80, 24), + crate::kitty_graphics::HostCellSize { + width_px: 10, + height_px: 20, + }, + crate::terminal_theme::TerminalTheme::default(), + Some(false), + 0, + RenderEncoding::SemanticFrame, + Some(client_2_writer), + ), + ); + set_graphics_layer(&mut server, pane_id, vec![1, 2, 3]); + let initial = enable_graphics_and_render(&mut server, &client_rx); + let initial_graphics = String::from_utf8_lossy(&initial.graphics); + assert!(initial_graphics.contains("a=t")); + assert!(initial_graphics.contains("a=p")); + let client_2_initial = read_server_frame( + client_2_rx + .recv_timeout(Duration::from_millis(100)) + .expect("second client initial frame"), + ); + assert!(String::from_utf8_lossy(&client_2_initial.graphics).contains("a=t")); + + assert!(server.handle_server_event(ServerEvent::ClientInput { + client_id: 2, + data: b"\x1b[I".to_vec(), + })); + assert_eq!(server.foreground_client_id, Some(2)); + server.render_and_stream(); + + let focused = read_server_frame( + client_2_rx + .recv_timeout(Duration::from_millis(100)) + .expect("focus redraw"), + ); + assert!(focused.graphics.is_empty()); +} + #[tokio::test] async fn retained_update_sends_only_graphics_message() { let (mut server, client_rx, pane_id) = retained_test_server(b"aaaa"); @@ -148,7 +194,7 @@ async fn retained_update_does_not_downgrade_pending_full_render() { let _ = enable_graphics_and_render(&mut server, &client_rx); fill_render_lane(&server); let client = server.clients.get_mut(&1).unwrap(); - client.request_full_redraw(); + client.request_repaint(); server.render_and_stream(); assert_eq!( server.clients.get(&1).unwrap().deferred_render(), diff --git a/src/server/render_stream.rs b/src/server/render_stream.rs index 8858bb6b..f1f6c428 100644 --- a/src/server/render_stream.rs +++ b/src/server/render_stream.rs @@ -14,7 +14,11 @@ pub(crate) enum ClientRenderState { /// Semantic clients compare full frame data and skip identical frames. Semantic { last_frame: Option }, /// Terminal-ANSI clients keep a terminal diff encoder and sequence number. - TerminalAnsi { blit_encoder: BlitEncoder, seq: u64 }, + TerminalAnsi { + blit_encoder: BlitEncoder, + seq: u64, + repaint_pending: bool, + }, } impl ClientRenderState { @@ -24,6 +28,7 @@ impl ClientRenderState { RenderEncoding::TerminalAnsi => Self::TerminalAnsi { blit_encoder: BlitEncoder::new(), seq: 0, + repaint_pending: false, }, } } @@ -31,7 +36,23 @@ impl ClientRenderState { pub(crate) fn reset_baseline(&mut self) { match self { Self::Semantic { last_frame } => *last_frame = None, - Self::TerminalAnsi { blit_encoder, .. } => *blit_encoder = BlitEncoder::new(), + Self::TerminalAnsi { + blit_encoder, + repaint_pending, + .. + } => { + *blit_encoder = BlitEncoder::new(); + *repaint_pending = false; + } + } + } + + pub(crate) fn request_repaint(&mut self) { + match self { + Self::Semantic { last_frame } => *last_frame = None, + Self::TerminalAnsi { + repaint_pending, .. + } => *repaint_pending = true, } } @@ -53,12 +74,16 @@ impl ClientRenderState { message: ServerMessage::Frame(frame), }) } - Self::TerminalAnsi { blit_encoder, seq } => { - if blit_encoder.is_current(&frame) { + Self::TerminalAnsi { + blit_encoder, + seq, + repaint_pending, + } => { + if !*repaint_pending && blit_encoder.is_current(&frame) { crate::render_prof::event("prepare_frame.ansi.skip_current"); return None; } - let mut encoded = blit_encoder.encode(&frame, false); + let mut encoded = blit_encoder.encode(&frame, *repaint_pending); crate::render_prof::event("prepare_frame.ansi.changed"); crate::render_prof::counter("prepare_frame.ansi.bytes", encoded.bytes.len() as u64); if encoded.full { @@ -102,7 +127,11 @@ impl ClientRenderState { }, ) => *last_frame = Some(frame), ( - Self::TerminalAnsi { blit_encoder, seq }, + Self::TerminalAnsi { + blit_encoder, + seq, + repaint_pending, + }, PreparedRender::TerminalAnsi { frame, encoded: Some(encoded), @@ -111,6 +140,7 @@ impl ClientRenderState { ) => { blit_encoder.commit(frame, encoded); *seq += 1; + *repaint_pending = false; } _ => {} } @@ -125,30 +155,18 @@ impl ClientRenderState { } } -const SYNC_OUTPUT_END: &[u8] = b"\x1b[?2026l"; - fn insert_graphics_before_sync_end(encoded: &mut Vec, graphics: &[u8]) { if graphics.is_empty() { return; } - if let Some(sync_end) = rfind_subslice(encoded, SYNC_OUTPUT_END) { + if let Some(sync_end) = crate::protocol::render_ansi::final_sync_output_end(encoded) { encoded.splice(sync_end..sync_end, graphics.iter().copied()); } else { encoded.extend_from_slice(graphics); } } -fn rfind_subslice(haystack: &[u8], needle: &[u8]) -> Option { - if needle.is_empty() || needle.len() > haystack.len() { - return None; - } - - haystack - .windows(needle.len()) - .rposition(|window| window == needle) -} - /// A prepared client render message plus any baseline state needed after send. pub(crate) enum PreparedRender { Semantic {