From 7dc1856685d58374e46ee61eb63f084bcbaddc52 Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Sat, 1 Aug 2026 14:27:07 +0000 Subject: [PATCH] Avoid querying terminal size on every TUI redraw (#36482) ## What changed - Carry dimensions on resize events and reuse the cached screen size for ordinary draws. - Refresh terminal geometry after resize settling, process resume, and external program execution. - Pass the resolved size through transcript reflow, viewport updates, and standalone TUI screens so each frame uses consistent dimensions. ## Testing - Cover cached draws, resize-driven rendering, settled-size rechecks, resume refreshes, and alternate-screen size updates. GitOrigin-RevId: 9d83dbab50d7a038bd59b4c66c13abcac69ddc79 --- codex-rs/tui/src/app.rs | 32 ++++--- codex-rs/tui/src/app/event_dispatch.rs | 7 +- codex-rs/tui/src/app/input.rs | 3 +- codex-rs/tui/src/app/resize_reflow.rs | 56 ++++++++---- codex-rs/tui/src/app/tests.rs | 24 ++++- codex-rs/tui/src/app_backtrack.rs | 6 +- codex-rs/tui/src/custom_terminal.rs | 91 ++++++++++++++++++- codex-rs/tui/src/cwd_prompt.rs | 3 +- .../external_agent_config_migration/mod.rs | 3 +- .../external_agent_config_migration/source.rs | 3 +- codex-rs/tui/src/model_migration.rs | 3 +- .../tui/src/onboarding/onboarding_screen.rs | 3 +- codex-rs/tui/src/pager_overlay.rs | 4 +- codex-rs/tui/src/resume_picker.rs | 22 ++--- codex-rs/tui/src/startup_hooks_review.rs | 3 +- codex-rs/tui/src/tui.rs | 63 +++++++------ codex-rs/tui/src/tui/event_stream.rs | 14 ++- codex-rs/tui/src/tui/job_control.rs | 9 +- codex-rs/tui/src/tui/screen_size.rs | 78 ++++++++++++++++ codex-rs/tui/src/tui/screen_size_tests.rs | 65 +++++++++++++ codex-rs/tui/src/update_prompt.rs | 3 +- 21 files changed, 398 insertions(+), 97 deletions(-) create mode 100644 codex-rs/tui/src/tui/screen_size.rs create mode 100644 codex-rs/tui/src/tui/screen_size_tests.rs diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index b201d8f020..5c1a6f9a73 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -174,6 +174,7 @@ use crossterm::event::KeyEventKind; use crossterm::event::KeyModifiers; use ratatui::backend::Backend; use ratatui::layout::Rect; +use ratatui::layout::Size; use ratatui::style::Stylize; use ratatui::text::Line; use ratatui::widgets::Paragraph; @@ -1135,7 +1136,7 @@ See the Codex keymap documentation for supported actions and examples." let tui_events = tui.event_stream(); tokio::pin!(tui_events); - tui.frame_requester().schedule_frame(); + tui.schedule_screen_size_recheck(Duration::ZERO); tracing::info!( duration_ms = %(startup_elapsed_before_app + startup_started_at.elapsed()).as_millis(), bootstrap_ms = %bootstrap_ms, @@ -1289,8 +1290,9 @@ See the Codex keymap documentation for supported actions and examples." app_server: &mut AppServerSession, event: TuiEvent, ) -> Result { - if matches!(event, TuiEvent::Draw | TuiEvent::Resize) { - self.handle_draw_pre_render(tui)?; + let screen_size = tui.screen_size_for_event(&event)?; + if !matches!(&event, TuiEvent::Key(_) | TuiEvent::Paste(_)) { + self.handle_draw_pre_render(tui, screen_size)?; } if self.overlay.is_some() { @@ -1308,9 +1310,9 @@ See the Codex keymap documentation for supported actions and examples." let pasted = pasted.replace("\r", "\n"); self.chat_widget.handle_paste(pasted); } - TuiEvent::Draw | TuiEvent::Resize => { + TuiEvent::Draw | TuiEvent::Resume | TuiEvent::Resize(_) => { if self.backtrack_render_pending { - self.rebuild_transcript_after_backtrack(tui)?; + self.rebuild_transcript_after_backtrack(tui, screen_size.into())?; self.backtrack_render_pending = false; } self.chat_widget.maybe_post_pending_notification(tui); @@ -1318,18 +1320,18 @@ See the Codex keymap documentation for supported actions and examples." .chat_widget .handle_paste_burst_tick(tui.frame_requester()) { + tui.defer_screen_size(screen_size); return Ok(AppRunControl::Continue); } // Allow widgets to process any pending timers before rendering. self.chat_widget.pre_draw_tick(); - let rendered_area = self.render_chat_widget_frame(tui)?; + let rendered_area = self.render_chat_widget_frame(tui, screen_size)?; if self.chat_widget.ambient_pet_image_enabled() { - let terminal_size = tui.terminal.size()?; let ambient_pet_area = Rect::new( /*x*/ 0, /*y*/ 0, - terminal_size.width, - terminal_size.height, + screen_size.width, + screen_size.height, ); if let Err(err) = tui.draw_ambient_pet_image( self.chat_widget @@ -1361,17 +1363,17 @@ See the Codex keymap documentation for supported actions and examples." pub(super) fn show_shutdown_feedback(&mut self, tui: &mut tui::Tui) -> Result<()> { self.disable_ambient_pet_before_shutdown(tui)?; self.chat_widget.show_shutdown_in_progress(); - self.handle_draw_pre_render(tui)?; + let screen_size = tui.terminal.last_known_screen_size; + self.handle_draw_pre_render(tui, screen_size)?; self.chat_widget.pre_draw_tick(); - self.render_chat_widget_frame(tui)?; + self.render_chat_widget_frame(tui, screen_size)?; Ok(()) } - fn render_chat_widget_frame(&mut self, tui: &mut tui::Tui) -> Result { - let width = tui.terminal.size()?.width; - self.with_chat_widget_frame(width, |desired_height, chat_widget| { + fn render_chat_widget_frame(&mut self, tui: &mut tui::Tui, screen_size: Size) -> Result { + self.with_chat_widget_frame(screen_size.width, |desired_height, chat_widget| { let mut rendered_area = Rect::default(); - tui.draw_with_resize_reflow(desired_height, |frame| { + tui.draw_with_resize_reflow(desired_height, screen_size, |frame| { let area = frame.area(); rendered_area = area; chat_widget.render(area, frame.buffer); diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index 626b2001d9..fa6bf71862 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -437,13 +437,14 @@ impl App { AppEvent::CodexOp(op) => { let is_user_turn = matches!(&op, AppCommand::UserTurn { .. }); if is_user_turn { - self.handle_draw_pre_render(tui)?; + let screen_size = tui.terminal.last_known_screen_size; + self.handle_draw_pre_render(tui, screen_size)?; if self.transcript_reflow.has_pending_reflow() { self.transcript_reflow.schedule_immediate(); - self.maybe_run_resize_reflow(tui)?; + self.maybe_run_resize_reflow(tui, screen_size)?; } self.chat_widget.pre_draw_tick(); - self.render_chat_widget_frame(tui)?; + self.render_chat_widget_frame(tui, screen_size)?; } self.chat_widget.prepare_local_op_submission(&op); if let Err(err) = self.submit_active_thread_op(app_server, op).await { diff --git a/codex-rs/tui/src/app/input.rs b/codex-rs/tui/src/app/input.rs index dcd88dfb82..b81b479f3d 100644 --- a/codex-rs/tui/src/app/input.rs +++ b/codex-rs/tui/src/app/input.rs @@ -79,7 +79,8 @@ impl App { } else { self.chat_widget.set_raw_output_mode(enabled); } - if let Err(err) = self.reflow_transcript_now(tui) { + let terminal_width = tui.terminal.last_known_screen_size.into(); + if let Err(err) = self.reflow_transcript_now(tui, terminal_width) { tracing::warn!(error = %err, "failed to reflow transcript after raw output mode toggle"); self.chat_widget .add_error_message(format!("Failed to redraw transcript: {err}")); diff --git a/codex-rs/tui/src/app/resize_reflow.rs b/codex-rs/tui/src/app/resize_reflow.rs index 7d00f7f5ff..266d3aa602 100644 --- a/codex-rs/tui/src/app/resize_reflow.rs +++ b/codex-rs/tui/src/app/resize_reflow.rs @@ -30,6 +30,16 @@ use crate::terminal_hyperlinks::HyperlinkLine; use crate::transcript_reflow::TRANSCRIPT_REFLOW_DEBOUNCE; use crate::tui; +/// Full terminal width before transcript-specific layout reservations. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(super) struct TerminalWidth(u16); + +impl From for TerminalWidth { + fn from(size: ratatui::layout::Size) -> Self { + Self(size.width) + } +} + struct ReflowCellDisplay { lines: Vec, is_stream_continuation: bool, @@ -253,7 +263,8 @@ impl App { pub(super) fn maybe_finish_stream_reflow(&mut self, tui: &mut tui::Tui) -> Result<()> { if self.transcript_reflow.take_stream_finish_reflow_needed() { self.schedule_immediate_resize_reflow(tui); - self.maybe_run_resize_reflow(tui)?; + let screen_size = tui.terminal.last_known_screen_size; + self.maybe_run_resize_reflow(tui, screen_size)?; } else if self.transcript_reflow.pending_is_due(Instant::now()) { tui.frame_requester().schedule_frame(); } @@ -285,7 +296,8 @@ impl App { } self.schedule_immediate_resize_reflow(tui); - self.maybe_run_resize_reflow(tui)?; + let screen_size = tui.terminal.last_known_screen_size; + self.maybe_run_resize_reflow(tui, screen_size)?; if !self.transcript_reflow.has_pending_reflow() { self.transcript_reflow.clear_stream_flags(); } @@ -342,8 +354,11 @@ impl App { self.reset_history_emission_state(); } - pub(super) fn handle_draw_pre_render(&mut self, tui: &mut tui::Tui) -> Result<()> { - let size = tui.terminal.size()?; + pub(super) fn handle_draw_pre_render( + &mut self, + tui: &mut tui::Tui, + size: ratatui::layout::Size, + ) -> Result<()> { let should_rebuild_transcript = self.handle_draw_size_change( size, tui.terminal.last_known_screen_size, @@ -355,7 +370,7 @@ impl App { // rebuild from transcript cells. tui.clear_pending_history_lines(); } - self.maybe_run_resize_reflow(tui)?; + self.maybe_run_resize_reflow(tui, size)?; Ok(()) } @@ -365,7 +380,11 @@ impl App { /// surface. Callers must keep using `HistoryCell` source as the rebuild input; attempting to /// reuse terminal-wrapped output here would preserve exactly the stale wrapping this feature is /// meant to remove. - pub(super) fn maybe_run_resize_reflow(&mut self, tui: &mut tui::Tui) -> Result<()> { + pub(super) fn maybe_run_resize_reflow( + &mut self, + tui: &mut tui::Tui, + screen_size: ratatui::layout::Size, + ) -> Result<()> { let Some(deadline) = self.transcript_reflow.pending_until() else { return Ok(()); }; @@ -390,8 +409,8 @@ impl App { let reflow_ran_during_stream = !self.transcript_cells.is_empty() && self.should_mark_reflow_as_stream_time(); - let width = self.reflow_transcript_now(tui)?; - self.transcript_reflow.mark_reflowed_width(width); + let width = self.reflow_transcript_now(tui, screen_size.into())?; + self.transcript_reflow.mark_reflowed_width(width.0); if reflow_ran_during_stream { self.transcript_reflow.mark_ran_during_stream(); @@ -399,15 +418,17 @@ impl App { // Some terminals settle their final reported width after the repaint that handled the // last resize event. Request one cheap follow-up draw so `handle_draw_pre_render` can // sample that width and schedule a final reflow if needed. - tui.frame_requester() - .schedule_frame_in(TRANSCRIPT_REFLOW_DEBOUNCE); + tui.schedule_screen_size_recheck(TRANSCRIPT_REFLOW_DEBOUNCE); Ok(()) } - pub(super) fn reflow_transcript_now(&mut self, tui: &mut tui::Tui) -> Result { - let terminal_width = tui.terminal.size()?.width; - let width = self.chat_widget.history_wrap_width(terminal_width); + pub(super) fn reflow_transcript_now( + &mut self, + tui: &mut tui::Tui, + terminal_width: TerminalWidth, + ) -> Result { + let width = self.chat_widget.history_wrap_width(terminal_width.0); if self.transcript_cells.is_empty() { // Drop any queued pre-resize/pre-consolidation inserts before rebuilding from cells. tui.clear_pending_history_lines(); @@ -438,9 +459,12 @@ impl App { /// Unlike resize reflow, rollback must clear the terminal even when no cells remain. Otherwise /// the cancelled user prompt stays visible in scrollback despite being removed from the source /// transcript. - pub(super) fn rebuild_transcript_after_backtrack(&mut self, tui: &mut tui::Tui) -> Result<()> { - let terminal_width = tui.terminal.size()?.width; - let width = self.chat_widget.history_wrap_width(terminal_width); + pub(super) fn rebuild_transcript_after_backtrack( + &mut self, + tui: &mut tui::Tui, + terminal_width: TerminalWidth, + ) -> Result<()> { + let width = self.chat_widget.history_wrap_width(terminal_width.0); let reflowed_lines = if self.transcript_cells.is_empty() { self.reset_history_emission_state(); Vec::new() diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index c319cc35de..1ef8c4bc2c 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -5357,7 +5357,8 @@ async fn required_stream_reflow_during_capped_initial_replay_survives_transcript assert!(app.initial_history_replay_buffer.is_none()); assert!(app.transcript_reflow.has_pending_reflow()); - app.maybe_run_resize_reflow(&mut tui)?; + let screen_size = tui.terminal.last_known_screen_size; + app.maybe_run_resize_reflow(&mut tui, screen_size)?; assert!(app.transcript_reflow.has_pending_reflow()); app.close_transcript_overlay(&mut tui); @@ -5422,6 +5423,27 @@ async fn height_shrink_schedules_resize_reflow() { assert!(app.transcript_reflow.has_pending_reflow()); } +#[tokio::test] +async fn resizing_empty_transcript_schedules_settled_size_recheck() { + let (mut app, _rx, _op_rx) = make_test_app_with_channels().await; + let mut tui = crate::tui::test_support::make_test_tui().expect("test tui"); + let frame_requester = crate::tui::FrameRequester::test_dummy(); + let initial_size = ratatui::layout::Size::new(/*width*/ 80, /*height*/ 24); + let resized_size = ratatui::layout::Size::new(/*width*/ 100, /*height*/ 24); + + assert!(!app.handle_draw_size_change(initial_size, initial_size, &frame_requester)); + tui.screen_size_for_event(&TuiEvent::Resize(resized_size)) + .expect("resolve resize event"); + tui.terminal.resize(resized_size).expect("apply event size"); + assert!(app.handle_draw_size_change(resized_size, initial_size, &frame_requester)); + tokio::time::sleep(crate::transcript_reflow::TRANSCRIPT_REFLOW_DEBOUNCE).await; + assert_eq!( + tui.screen_size_for_event(&TuiEvent::Draw) + .expect("resolve settled size"), + initial_size + ); +} + fn test_turn(turn_id: &str, status: TurnStatus, items: Vec) -> Turn { Turn { id: turn_id.to_string(), diff --git a/codex-rs/tui/src/app_backtrack.rs b/codex-rs/tui/src/app_backtrack.rs index f4114a3f90..609e3e6b1e 100644 --- a/codex-rs/tui/src/app_backtrack.rs +++ b/codex-rs/tui/src/app_backtrack.rs @@ -341,8 +341,10 @@ impl App { /// source of truth for the active cell and its cache invalidation key, and because `App` owns /// overlay lifecycle and frame scheduling for animations. fn overlay_forward_event(&mut self, tui: &mut tui::Tui, event: TuiEvent) -> Result<()> { - if matches!(&event, TuiEvent::Draw | TuiEvent::Resize) - && let Some(Overlay::Transcript(t)) = &mut self.overlay + if matches!( + &event, + TuiEvent::Draw | TuiEvent::Resume | TuiEvent::Resize(_) + ) && let Some(Overlay::Transcript(t)) = &mut self.overlay { let active_key = self.chat_widget.active_cell_transcript_key(); let chat_widget = &self.chat_widget; diff --git a/codex-rs/tui/src/custom_terminal.rs b/codex-rs/tui/src/custom_terminal.rs index d7d0cf36e5..480d1a8635 100644 --- a/codex-rs/tui/src/custom_terminal.rs +++ b/codex-rs/tui/src/custom_terminal.rs @@ -345,8 +345,17 @@ where where F: FnOnce(&mut Frame), { - self.try_draw(|frame| { - render_callback(frame); + let screen_size = self.size()?; + self.draw_with_size(screen_size, render_callback) + } + + /// Draws a single frame using a screen size already obtained by the caller. + pub(crate) fn draw_with_size(&mut self, screen_size: Size, render: F) -> io::Result<()> + where + F: FnOnce(&mut Frame), + { + self.try_draw_with_size(screen_size, |frame| { + render(frame); io::Result::Ok(()) }) } @@ -391,10 +400,18 @@ where F: FnOnce(&mut Frame) -> Result<(), E>, E: Into, { - // Autoresize - otherwise we get glitches if shrinking or potential desync between widgets - // and the terminal (if growing), which may OOB. - self.autoresize()?; + let screen_size = self.size()?; + self.try_draw_with_size(screen_size, render_callback) + } + fn try_draw_with_size(&mut self, screen_size: Size, render_callback: F) -> io::Result<()> + where + F: FnOnce(&mut Frame) -> Result<(), E>, + E: Into, + { + if screen_size != self.last_known_screen_size { + self.resize(screen_size)?; + } let mut frame = self.get_frame(); render_callback(&mut frame).map_err(Into::into)?; @@ -792,6 +809,7 @@ mod tests { use super::*; use std::num::NonZeroU16; + use insta::assert_snapshot; use pretty_assertions::assert_eq; use ratatui::backend::WindowSize; use ratatui::layout::Rect; @@ -806,6 +824,7 @@ mod tests { output: Vec, size: Size, cursor: Position, + size_call_count: std::cell::Cell, } impl CaptureBackend { @@ -814,6 +833,7 @@ mod tests { output: Vec::new(), size: Size { width, height }, cursor: Position { x: 0, y: 0 }, + size_call_count: std::cell::Cell::new(/*value*/ 0), } } @@ -889,6 +909,8 @@ mod tests { } fn size(&self) -> io::Result { + self.size_call_count + .set(self.size_call_count.get().saturating_add(/*rhs*/ 1)); Ok(self.size) } @@ -904,6 +926,65 @@ mod tests { } } + #[test] + fn ordinary_redraws_with_known_size_do_not_query_backend_size() { + let mut terminal = + Terminal::with_options(CaptureBackend::new(/*width*/ 80, /*height*/ 24)) + .expect("terminal"); + let screen_size = terminal.last_known_screen_size; + + for _ in 0..3 { + terminal.draw_with_size(screen_size, |_| {}).expect("draw"); + } + + assert_eq!(terminal.backend().size_call_count.get(), 1); + } + + #[test] + fn resize_draw_applies_event_dimensions_without_querying_backend_size() { + let mut terminal = + Terminal::with_options(CaptureBackend::new(/*width*/ 12, /*height*/ 4)) + .expect("terminal"); + let mut snapshots = Vec::new(); + + for width in [12, 8] { + let size = Size::new(width, /*height*/ 4); + let area = Rect::new(/*x*/ 0, /*y*/ 0, size.width, size.height); + terminal.set_viewport_area(area); + terminal + .draw_with_size(size, |frame| { + Paragraph::new("alpha beta") + .wrap(Wrap { trim: false }) + .render(area, frame.buffer_mut()); + }) + .expect("draw resized frame"); + + let rendered = (0..size.height) + .map(|y| { + (0..size.width) + .map(|x| terminal.previous_buffer()[(x, y)].symbol()) + .collect::() + .trim_end() + .to_string() + }) + .collect::>() + .join("\n"); + snapshots.push(rendered.trim_end().to_string()); + } + + assert_eq!(terminal.backend().size_call_count.get(), 1); + assert_eq!( + terminal.last_known_screen_size, + Size::new(/*width*/ 8, /*height*/ 4) + ); + assert_snapshot!(snapshots.join("\n\n"), @r" + alpha beta + + alpha + beta + "); + } + #[test] fn diff_buffers_does_not_emit_clear_to_end_for_full_width_row() { let area = Rect::new(0, 0, 3, 2); diff --git a/codex-rs/tui/src/cwd_prompt.rs b/codex-rs/tui/src/cwd_prompt.rs index 3af493a14e..1083a99ae1 100644 --- a/codex-rs/tui/src/cwd_prompt.rs +++ b/codex-rs/tui/src/cwd_prompt.rs @@ -129,10 +129,11 @@ pub(crate) async fn run_cwd_selection_prompt( while !screen.is_done() { if let Some(event) = events.next().await { + tui.screen_size_for_event(&event)?; match event { TuiEvent::Key(key_event) => screen.handle_key(key_event), TuiEvent::Paste(_) => {} - TuiEvent::Draw | TuiEvent::Resize => { + TuiEvent::Draw | TuiEvent::Resume | TuiEvent::Resize(_) => { tui.draw(u16::MAX, |frame| { frame.render_widget_ref(&screen, frame.area()); })?; diff --git a/codex-rs/tui/src/external_agent_config_migration/mod.rs b/codex-rs/tui/src/external_agent_config_migration/mod.rs index ff6c344b8d..41d1f1de5f 100644 --- a/codex-rs/tui/src/external_agent_config_migration/mod.rs +++ b/codex-rs/tui/src/external_agent_config_migration/mod.rs @@ -101,10 +101,11 @@ pub(crate) async fn run_external_agent_config_migration_prompt( while !screen.is_done() { if let Some(event) = events.next().await { + let _ = tui.screen_size_for_event(&event); match event { TuiEvent::Key(key_event) => screen.handle_key(key_event), TuiEvent::Paste(_) => {} - TuiEvent::Draw | TuiEvent::Resize => { + TuiEvent::Draw | TuiEvent::Resume | TuiEvent::Resize(_) => { let _ = tui.draw(u16::MAX, |frame| { frame.render_widget_ref(&screen, frame.area()); }); diff --git a/codex-rs/tui/src/external_agent_config_migration/source.rs b/codex-rs/tui/src/external_agent_config_migration/source.rs index 92e64a2bf3..12197c8fbc 100644 --- a/codex-rs/tui/src/external_agent_config_migration/source.rs +++ b/codex-rs/tui/src/external_agent_config_migration/source.rs @@ -58,10 +58,11 @@ pub(crate) async fn run_external_agent_config_source_prompt( while !screen.is_done() { if let Some(event) = events.next().await { + let _ = tui.screen_size_for_event(&event); match event { TuiEvent::Key(key_event) => screen.handle_key(key_event), TuiEvent::Paste(_) => {} - TuiEvent::Draw | TuiEvent::Resize => { + TuiEvent::Draw | TuiEvent::Resume | TuiEvent::Resize(_) => { let _ = tui.draw(u16::MAX, |frame| { frame.render_widget_ref(&screen, frame.area()); }); diff --git a/codex-rs/tui/src/model_migration.rs b/codex-rs/tui/src/model_migration.rs index c307abb78f..afaa2245f5 100644 --- a/codex-rs/tui/src/model_migration.rs +++ b/codex-rs/tui/src/model_migration.rs @@ -150,10 +150,11 @@ pub(crate) async fn run_model_migration_prompt( while !screen.is_done() { if let Some(event) = events.next().await { + let _ = alt.tui.screen_size_for_event(&event); match event { TuiEvent::Key(key_event) => screen.handle_key(key_event), TuiEvent::Paste(_) => {} - TuiEvent::Draw | TuiEvent::Resize => { + TuiEvent::Draw | TuiEvent::Resume | TuiEvent::Resize(_) => { let _ = alt.tui.draw(u16::MAX, |frame| { frame.render_widget_ref(&screen, frame.area()); }); diff --git a/codex-rs/tui/src/onboarding/onboarding_screen.rs b/codex-rs/tui/src/onboarding/onboarding_screen.rs index 0236d06894..af151409b6 100644 --- a/codex-rs/tui/src/onboarding/onboarding_screen.rs +++ b/codex-rs/tui/src/onboarding/onboarding_screen.rs @@ -495,6 +495,7 @@ pub(crate) async fn run_onboarding_app( tokio::select! { event = tui_events.next() => { if let Some(event) = event { + tui.screen_size_for_event(&event)?; match event { TuiEvent::Key(key_event) => { onboarding_screen.handle_key_event(key_event); @@ -509,7 +510,7 @@ pub(crate) async fn run_onboarding_app( TuiEvent::Paste(text) => { onboarding_screen.handle_paste(text); } - TuiEvent::Draw | TuiEvent::Resize => { + TuiEvent::Draw | TuiEvent::Resume | TuiEvent::Resize(_) => { if !did_full_clear_after_success && onboarding_screen.steps.iter().any(|step| { if let Step::Auth(w) = step { diff --git a/codex-rs/tui/src/pager_overlay.rs b/codex-rs/tui/src/pager_overlay.rs index 8920bc2715..b29fe1e760 100644 --- a/codex-rs/tui/src/pager_overlay.rs +++ b/codex-rs/tui/src/pager_overlay.rs @@ -799,7 +799,7 @@ impl TranscriptOverlay { } other => self.view.handle_key_event(tui, other), }, - TuiEvent::Draw | TuiEvent::Resize => { + TuiEvent::Draw | TuiEvent::Resume | TuiEvent::Resize(_) => { tui.draw(u16::MAX, |frame| { self.render(frame.area(), frame.buffer); })?; @@ -897,7 +897,7 @@ impl StaticOverlay { } other => self.view.handle_key_event(tui, other), }, - TuiEvent::Draw | TuiEvent::Resize => { + TuiEvent::Draw | TuiEvent::Resume | TuiEvent::Resize(_) => { tui.draw(u16::MAX, |frame| { self.render(frame.area(), frame.buffer); })?; diff --git a/codex-rs/tui/src/resume_picker.rs b/codex-rs/tui/src/resume_picker.rs index 1131ce9509..2203929988 100644 --- a/codex-rs/tui/src/resume_picker.rs +++ b/codex-rs/tui/src/resume_picker.rs @@ -50,6 +50,7 @@ use crossterm::event::KeyModifiers; use ratatui::layout::Constraint; use ratatui::layout::Layout; use ratatui::layout::Rect; +use ratatui::layout::Size; use ratatui::style::Color; use ratatui::style::Style; use ratatui::style::Styled as _; @@ -480,6 +481,7 @@ async fn run_session_picker_with_loader( loop { tokio::select! { Some(ev) = tui_events.next() => { + let screen_size = alt.tui.screen_size_for_event(&ev)?; if state.overlay.is_some() { state.handle_overlay_event(alt.tui, ev)?; continue; @@ -496,14 +498,13 @@ async fn run_session_picker_with_loader( TuiEvent::Paste(pasted) => { state.handle_paste(pasted); } - TuiEvent::Draw | TuiEvent::Resize => { - if let Ok(size) = alt.tui.terminal.size() { - let list_height = - size.height.saturating_sub(PICKER_CHROME_HEIGHT) as usize; - state.update_viewport(list_height, list_viewport_width(size.width)); - state.ensure_minimum_rows_for_view(list_height); - } - draw_picker(alt.tui, &state)?; + TuiEvent::Draw | TuiEvent::Resume | TuiEvent::Resize(_) => { + let list_width = list_viewport_width(screen_size.width); + let list_height = + usize::from(screen_size.height.saturating_sub(PICKER_CHROME_HEIGHT)); + state.update_viewport(list_height, list_width); + state.ensure_minimum_rows_for_view(list_height); + draw_picker(alt.tui, &state, screen_size)?; if state.note_transcript_loading_frame_drawn() { state.open_pending_transcript_if_ready(); } @@ -1876,10 +1877,9 @@ fn parse_timestamp_str(ts: &str) -> Option> { .ok() } -fn draw_picker(tui: &mut Tui, state: &PickerState) -> std::io::Result<()> { +fn draw_picker(tui: &mut Tui, state: &PickerState, screen_size: Size) -> std::io::Result<()> { // Render full-screen overlay - let height = tui.terminal.size()?.height; - tui.draw(height, |frame| { + tui.draw(screen_size.height, |frame| { let area = frame.area(); let [header, _header_gap, search, _search_gap, list, footer] = Layout::vertical([ Constraint::Length(1), diff --git a/codex-rs/tui/src/startup_hooks_review.rs b/codex-rs/tui/src/startup_hooks_review.rs index f53947e15f..3cec4d0cb9 100644 --- a/codex-rs/tui/src/startup_hooks_review.rs +++ b/codex-rs/tui/src/startup_hooks_review.rs @@ -107,6 +107,7 @@ async fn run_startup_hooks_review_app( let Some(event) = tui_events.next().await else { return Ok(StartupHooksReviewOutcome::Continue); }; + tui.screen_size_for_event(&event)?; match event { TuiEvent::Key(key_event) => { if matches!(key_event.kind, KeyEventKind::Press | KeyEventKind::Repeat) { @@ -167,7 +168,7 @@ async fn run_startup_hooks_review_app( } } TuiEvent::Paste(_) => {} - TuiEvent::Draw | TuiEvent::Resize => draw_view(tui, &view)?, + TuiEvent::Draw | TuiEvent::Resume | TuiEvent::Resize(_) => draw_view(tui, &view)?, } } } diff --git a/codex-rs/tui/src/tui.rs b/codex-rs/tui/src/tui.rs index fb0e10dd25..51a8b17a5a 100644 --- a/codex-rs/tui/src/tui.rs +++ b/codex-rs/tui/src/tui.rs @@ -35,6 +35,7 @@ use ratatui::crossterm::terminal::enable_raw_mode; use ratatui::layout::Offset; use ratatui::layout::Position; use ratatui::layout::Rect; +use ratatui::layout::Size; use ratatui::text::Line; use tokio::sync::broadcast; use tokio_stream::Stream; @@ -52,6 +53,7 @@ use crate::tui::event_stream::EventBroker; use crate::tui::event_stream::TuiEventStream; #[cfg(unix)] use crate::tui::job_control::SuspendContext; +use crate::tui::screen_size::ScreenSizePolicy; use codex_config::types::NotificationCondition; use codex_config::types::NotificationMethod; @@ -61,6 +63,7 @@ mod frame_requester; #[cfg(unix)] mod job_control; mod keyboard_modes; +mod screen_size; mod terminal_stderr; #[cfg(test)] pub(crate) mod test_support; @@ -548,13 +551,18 @@ pub enum TuiEvent { Key(KeyEvent), /// A bracketed paste payload normalized by the app layer before it reaches the composer. Paste(String), - /// A terminal size notification that should be handled as resize-sensitive draw work. + /// A terminal size notification and its reported dimensions. /// /// Resize is separate from `Draw` so the app can run feature-gated pre-render logic without /// changing the default draw path for scheduled frames. - Resize, + Resize(Size), /// A scheduled repaint that does not necessarily correspond to a terminal size change. Draw, + /// The first repaint after returning from process suspension. + /// + /// The app refreshes terminal geometry for this draw because resize events are not delivered + /// while the process is suspended. + Resume, } pub struct Tui { @@ -563,6 +571,7 @@ pub struct Tui { event_broker: Arc, pub(crate) terminal: Terminal, pending_history_lines: Vec, + screen_size: ScreenSizePolicy, ambient_pet_image_state: crate::pets::PetImageRenderState, pet_picker_preview_image_state: crate::pets::PetImageRenderState, alt_saved_viewport: Option, @@ -620,6 +629,7 @@ impl Tui { event_broker: Arc::new(EventBroker::new()), terminal, pending_history_lines: vec![], + screen_size: ScreenSizePolicy::default(), ambient_pet_image_state: crate::pets::PetImageRenderState::default(), pet_picker_preview_image_state: crate::pets::PetImageRenderState::default(), alt_saved_viewport: None, @@ -715,6 +725,7 @@ impl Tui { } self.resume_events(); + self.schedule_screen_size_recheck(Duration::ZERO); output } @@ -775,6 +786,7 @@ impl Tui { let _ = execute!(self.terminal.backend_mut(), EnableAlternateScroll); if let Ok(size) = self.terminal.size() { self.alt_saved_viewport = Some(self.terminal.viewport_area); + self.terminal.resize(size)?; self.terminal.set_viewport_area(ratatui::layout::Rect::new( 0, 0, @@ -848,29 +860,29 @@ impl Tui { fn update_inline_viewport_for_resize_reflow( terminal: &mut Terminal, height: u16, + screen_size: Size, ) -> Result { - let size = terminal.size()?; - let terminal_height_shrank = size.height < terminal.last_known_screen_size.height; - let terminal_height_grew = size.height > terminal.last_known_screen_size.height; + let terminal_height_shrank = screen_size.height < terminal.last_known_screen_size.height; + let terminal_height_grew = screen_size.height > terminal.last_known_screen_size.height; let viewport_was_bottom_aligned = terminal.viewport_area.bottom() == terminal.last_known_screen_size.height; let previous_area = terminal.viewport_area; let mut area = terminal.viewport_area; - area.height = height.min(size.height); - area.width = size.width; + area.height = height.min(screen_size.height); + area.width = screen_size.width; let mut needs_full_repaint = false; - if area.bottom() > size.height { - let scroll_by = area.bottom() - size.height; + if area.bottom() > screen_size.height { + let scroll_by = area.bottom() - screen_size.height; if !terminal_height_shrank { terminal .backend_mut() .scroll_region_up(0..area.top(), scroll_by)?; } - area.y = size.height - area.height; + area.y = screen_size.height - area.height; } else if terminal_height_grew && viewport_was_bottom_aligned { - area.y = size.height - area.height; + area.y = screen_size.height - area.height; } if area != terminal.viewport_area { @@ -915,6 +927,7 @@ impl Tui { height: u16, draw_fn: impl FnOnce(&mut custom_terminal::Frame), ) -> Result<()> { + let screen_size = self.take_event_screen_size()?; // If we are resuming from ^Z, we need to prepare the resume action now so we can apply it // in the synchronized update. #[cfg(unix)] @@ -924,14 +937,14 @@ impl Tui { // Precompute any viewport updates that need a cursor-position query before entering // the synchronized update, to avoid racing with the event reader. - let mut pending_viewport_area = self.pending_viewport_area()?; + let mut pending_viewport_area = self.pending_viewport_area(screen_size)?; ensure_virtual_terminal_processing()?; stdout().sync_update(|_| { #[cfg(unix)] if let Some(prepared) = prepared_resume.take() { - prepared.apply(&mut self.terminal)?; + prepared.apply(&mut self.terminal, screen_size)?; } let terminal = &mut self.terminal; @@ -940,17 +953,15 @@ impl Tui { terminal.clear()?; } - let size = terminal.size()?; - let mut area = terminal.viewport_area; - area.height = height.min(size.height); - area.width = size.width; + area.height = height.min(screen_size.height); + area.width = screen_size.width; // If the viewport has expanded, scroll everything else up to make room. - if area.bottom() > size.height { + if area.bottom() > screen_size.height { terminal .backend_mut() - .scroll_region_up(0..area.top(), area.bottom() - size.height)?; - area.y = size.height - area.height; + .scroll_region_up(0..area.top(), area.bottom() - screen_size.height)?; + area.y = screen_size.height - area.height; } if area != terminal.viewport_area { // On startup, the old viewport can still be empty. Clear from the @@ -979,7 +990,7 @@ impl Tui { self.suspend_context.set_cursor_y(inline_area_bottom); } - terminal.draw(|frame| { + terminal.draw_with_size(screen_size, |frame| { draw_fn(frame); }) })? @@ -1049,6 +1060,7 @@ impl Tui { pub fn draw_with_resize_reflow( &mut self, height: u16, + screen_size: Size, draw_fn: impl FnOnce(&mut custom_terminal::Frame), ) -> Result<()> { // If we are resuming from ^Z, we need to prepare the resume action now so we can apply it @@ -1063,12 +1075,12 @@ impl Tui { stdout().sync_update(|_| { #[cfg(unix)] if let Some(prepared) = prepared_resume.take() { - prepared.apply(&mut self.terminal)?; + prepared.apply(&mut self.terminal, screen_size)?; } let terminal = &mut self.terminal; let needs_full_repaint = - Self::update_inline_viewport_for_resize_reflow(terminal, height)?; + Self::update_inline_viewport_for_resize_reflow(terminal, height, screen_size)?; Self::flush_pending_history_lines( terminal, &mut self.pending_history_lines, @@ -1093,15 +1105,14 @@ impl Tui { self.suspend_context.set_cursor_y(inline_area_bottom); } - terminal.draw(|frame| { + terminal.draw_with_size(screen_size, |frame| { draw_fn(frame); }) })? } - fn pending_viewport_area(&mut self) -> Result> { + fn pending_viewport_area(&mut self, screen_size: Size) -> Result> { let terminal = &mut self.terminal; - let screen_size = terminal.size()?; let last_known_screen_size = terminal.last_known_screen_size; if screen_size != last_known_screen_size && let Ok(cursor_pos) = terminal.get_cursor_position() diff --git a/codex-rs/tui/src/tui/event_stream.rs b/codex-rs/tui/src/tui/event_stream.rs index da719463f3..da972c87e7 100644 --- a/codex-rs/tui/src/tui/event_stream.rs +++ b/codex-rs/tui/src/tui/event_stream.rs @@ -263,11 +263,13 @@ impl TuiEventStream { "failed to suspend TUI process" ); } - return Some(TuiEvent::Draw); + return Some(TuiEvent::Resume); } Some(TuiEvent::Key(key_event)) } - Event::Resize(_, _) => Some(TuiEvent::Resize), + Event::Resize(width, height) => { + Some(TuiEvent::Resize(ratatui::layout::Size { width, height })) + } Event::Paste(pasted) => Some(TuiEvent::Paste(pasted)), Event::FocusGained => { self.terminal_focused.store(true, Ordering::Relaxed); @@ -513,7 +515,13 @@ mod tests { handle.send(Ok(Event::Resize(80, 24))); let next = stream.next().await; - assert!(matches!(next, Some(TuiEvent::Resize))); + assert!(matches!( + next, + Some(TuiEvent::Resize(ratatui::layout::Size { + width: 80, + height: 24 + })) + )); } #[tokio::test(flavor = "current_thread")] diff --git a/codex-rs/tui/src/tui/job_control.rs b/codex-rs/tui/src/tui/job_control.rs index bc46a0a895..2db1fc04f5 100644 --- a/codex-rs/tui/src/tui/job_control.rs +++ b/codex-rs/tui/src/tui/job_control.rs @@ -14,6 +14,7 @@ use crossterm::terminal::EnterAlternateScreen; use crossterm::terminal::LeaveAlternateScreen; use ratatui::crossterm::execute; use ratatui::layout::Rect; +use ratatui::layout::Size; use crate::key_hint; @@ -178,7 +179,7 @@ pub(crate) enum PreparedResumeAction { } impl PreparedResumeAction { - pub(crate) fn apply(self, terminal: &mut Terminal) -> Result<()> { + pub(crate) fn apply(self, terminal: &mut Terminal, screen_size: Size) -> Result<()> { match self { PreparedResumeAction::RealignViewport(area) => { terminal.set_viewport_area(area); @@ -187,10 +188,8 @@ impl PreparedResumeAction { execute!(terminal.backend_mut(), EnterAlternateScreen)?; // Enable "alternate scroll" so terminals may translate wheel to arrows execute!(terminal.backend_mut(), EnableAlternateScroll)?; - if let Ok(size) = terminal.size() { - terminal.set_viewport_area(Rect::new(0, 0, size.width, size.height)); - terminal.clear()?; - } + terminal.set_viewport_area(Rect::from(screen_size)); + terminal.clear()?; } } Ok(()) diff --git a/codex-rs/tui/src/tui/screen_size.rs b/codex-rs/tui/src/tui/screen_size.rs new file mode 100644 index 0000000000..c08709cc77 --- /dev/null +++ b/codex-rs/tui/src/tui/screen_size.rs @@ -0,0 +1,78 @@ +//! Terminal geometry policy shared by the main app and standalone screens. + +use std::io; +use std::time::Duration; +use std::time::Instant; + +use ratatui::layout::Size; + +use super::Tui; +use super::TuiEvent; +use crate::transcript_reflow::TRANSCRIPT_REFLOW_DEBOUNCE; + +#[derive(Default)] +pub(super) struct ScreenSizePolicy { + pending_recheck_at: Option, + pending_draw_size: Option, + deferred_size: Option, +} + +impl Tui { + /// Resolve event geometry while avoiding backend queries on ordinary repaint frames. + pub(crate) fn screen_size_for_event(&mut self, event: &TuiEvent) -> io::Result { + if matches!(event, TuiEvent::Resize(_)) { + self.schedule_screen_size_recheck(TRANSCRIPT_REFLOW_DEBOUNCE); + } else if matches!(event, TuiEvent::Draw) + && let Some(deadline) = self.screen_size.pending_recheck_at + { + let now = Instant::now(); + if now < deadline { + self.frame_requester.schedule_frame_in(deadline - now); + } else { + return self.screen_size_for_event(&TuiEvent::Resume); + } + } + + let cached = self.terminal.last_known_screen_size; + let size = match event { + TuiEvent::Resize(size) => { + self.screen_size.deferred_size = None; + *size + } + TuiEvent::Resume => { + self.screen_size.pending_recheck_at = None; + self.screen_size.deferred_size = None; + self.terminal.size()? + } + TuiEvent::Draw => self.screen_size.deferred_size.take().unwrap_or(cached), + TuiEvent::Key(_) | TuiEvent::Paste(_) => cached, + }; + self.screen_size.pending_draw_size = + (!matches!(event, TuiEvent::Key(_) | TuiEvent::Paste(_))).then_some(size); + Ok(size) + } + + /// Schedule one backend sample after a resize, reflow, or external program. + pub(crate) fn schedule_screen_size_recheck(&mut self, delay: Duration) { + self.screen_size.pending_recheck_at = Some(Instant::now() + delay); + self.frame_requester.schedule_frame_in(delay); + } + + pub(crate) fn defer_screen_size(&mut self, size: Size) { + self.screen_size.deferred_size = Some(size); + } + + pub(super) fn take_event_screen_size(&mut self) -> io::Result { + if let Some(size) = self.screen_size.pending_draw_size.take() { + return Ok(size); + } + let size = self.terminal.size()?; + self.screen_size.pending_recheck_at = None; + self.screen_size.deferred_size = None; + Ok(size) + } +} + +#[cfg(test)] +#[path = "screen_size_tests.rs"] +mod tests; diff --git a/codex-rs/tui/src/tui/screen_size_tests.rs b/codex-rs/tui/src/tui/screen_size_tests.rs new file mode 100644 index 0000000000..248f69b0d1 --- /dev/null +++ b/codex-rs/tui/src/tui/screen_size_tests.rs @@ -0,0 +1,65 @@ +use std::time::Duration; + +use pretty_assertions::assert_eq; +use ratatui::layout::Size; + +use crate::tui::TuiEvent; + +#[tokio::test] +async fn draw_size_policy_refreshes_only_after_resume() { + let mut tui = crate::tui::test_support::make_test_tui().expect("test tui"); + let cached = Size::new(/*width*/ 120, /*height*/ 40); + let resumed = tui.terminal.size().expect("backend size"); + tui.terminal.last_known_screen_size = cached; + let resized = Size::new(/*width*/ 100, /*height*/ 30); + for (event, expected) in [ + (TuiEvent::Draw, cached), + (TuiEvent::Resume, resumed), + (TuiEvent::Resize(resized), resized), + (TuiEvent::Paste(String::new()), cached), + (TuiEvent::Draw, resized), + (TuiEvent::Paste(String::new()), cached), + ] { + assert_eq!(tui.screen_size_for_event(&event).expect("size"), expected); + if matches!(event, TuiEvent::Resize(_)) { + tui.defer_screen_size(resized); + } + } + assert_eq!(tui.take_event_screen_size().expect("size"), resumed); + assert!(tui.screen_size.pending_recheck_at.is_none()); +} + +#[tokio::test] +async fn standalone_resize_draw_rechecks_settled_screen_size_once() { + let mut tui = crate::tui::test_support::make_test_tui().expect("test tui"); + let resize_size = Size::new(/*width*/ 120, /*height*/ 40); + + tui.screen_size_for_event(&TuiEvent::Resize(resize_size)) + .expect("resolve resize"); + tui.terminal.resize(resize_size).expect("apply resize"); + assert_eq!( + tui.screen_size_for_event(&TuiEvent::Draw) + .expect("resolve early draw"), + resize_size + ); + + tui.schedule_screen_size_recheck(Duration::ZERO); + assert_eq!( + tui.screen_size_for_event(&TuiEvent::Draw) + .expect("resolve settled draw"), + tui.terminal.size().expect("terminal size") + ); + assert!(tui.screen_size.pending_recheck_at.is_none()); +} + +#[tokio::test] +async fn entering_alternate_screen_updates_cached_screen_size() { + let mut tui = crate::tui::test_support::make_test_tui().expect("test tui"); + let screen_size = tui.terminal.size().expect("terminal size"); + tui.terminal.last_known_screen_size = Size::new(/*width*/ 120, /*height*/ 40); + + tui.enter_alt_screen().expect("enter alternate screen"); + + assert_eq!(tui.terminal.last_known_screen_size, screen_size); + tui.leave_alt_screen().expect("leave alternate screen"); +} diff --git a/codex-rs/tui/src/update_prompt.rs b/codex-rs/tui/src/update_prompt.rs index a4e0f38ffe..de8bfb0dca 100644 --- a/codex-rs/tui/src/update_prompt.rs +++ b/codex-rs/tui/src/update_prompt.rs @@ -55,10 +55,11 @@ pub(crate) async fn run_update_prompt_if_needed( while !screen.is_done() { if let Some(event) = events.next().await { + tui.screen_size_for_event(&event)?; match event { TuiEvent::Key(key_event) => screen.handle_key(key_event), TuiEvent::Paste(_) => {} - TuiEvent::Draw | TuiEvent::Resize => { + TuiEvent::Draw | TuiEvent::Resume | TuiEvent::Resize(_) => { tui.draw(u16::MAX, |frame| { frame.render_widget_ref(&screen, frame.area()); })?;