From 20cd61e2a464684f8d1ff8d83d7ec95d5fe52d72 Mon Sep 17 00:00:00 2001 From: Jeremy Rose <172423086+nornagon-openai@users.noreply.github.com> Date: Thu, 14 Aug 2025 16:59:47 -0400 Subject: [PATCH] use a central animation loop (#2268) instead of each shimmer needing to have its own animation thread, have render_ref schedule a new frame if it wants one and coalesce to the earliest next frame. this also makes the animations frame-timing-independent, based on start time instead of frame count. --- codex-rs/tui/src/app.rs | 86 +++++++++++------- codex-rs/tui/src/app_event.rs | 5 ++ codex-rs/tui/src/onboarding/auth.rs | 23 ++--- codex-rs/tui/src/shimmer.rs | 48 ++++------- codex-rs/tui/src/status_indicator_widget.rs | 96 +++------------------ 5 files changed, 97 insertions(+), 161 deletions(-) diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index a6ced654..2bef9fb9 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -30,6 +30,7 @@ use std::sync::mpsc::Receiver; use std::sync::mpsc::channel; use std::thread; use std::time::Duration; +use std::time::Instant; /// Time window for debouncing redraw requests. const REDRAW_DEBOUNCE: Duration = Duration::from_millis(1); @@ -59,15 +60,16 @@ pub(crate) struct App<'a> { file_search: FileSearchManager, - /// True when a redraw has been scheduled but not yet executed. - pending_redraw: Arc, - pending_history_lines: Vec>, enhanced_keys_supported: bool, /// Controls the animation thread that sends CommitTick events. commit_anim_running: Arc, + + /// Channel to schedule one-shot animation frames; coalesced by a single + /// scheduler thread. + frame_schedule_tx: std::sync::mpsc::Sender, } /// Aggregate parameters needed to create a `ChatWidget`, as creation may be @@ -91,7 +93,6 @@ impl App<'_> { let (app_event_tx, app_event_rx) = channel(); let app_event_tx = AppEventSender::new(app_event_tx); - let pending_redraw = Arc::new(AtomicBool::new(false)); let enhanced_keys_supported = supports_keyboard_enhancement().unwrap_or(false); @@ -169,6 +170,47 @@ impl App<'_> { }; let file_search = FileSearchManager::new(config.cwd.clone(), app_event_tx.clone()); + + // Spawn a single scheduler thread that coalesces both debounced redraw + // requests and animation frame requests, and emits a single Redraw event + // at the earliest requested time. + let (frame_tx, frame_rx) = channel::(); + { + let app_event_tx = app_event_tx.clone(); + std::thread::spawn(move || { + use std::sync::mpsc::RecvTimeoutError; + let mut next_deadline: Option = None; + loop { + if next_deadline.is_none() { + match frame_rx.recv() { + Ok(deadline) => next_deadline = Some(deadline), + Err(_) => break, + } + } + + #[allow(clippy::expect_used)] + let deadline = next_deadline.expect("deadline set"); + let now = Instant::now(); + let timeout = if deadline > now { + deadline - now + } else { + Duration::from_millis(0) + }; + + match frame_rx.recv_timeout(timeout) { + Ok(new_deadline) => { + next_deadline = + Some(next_deadline.map_or(new_deadline, |d| d.min(new_deadline))); + } + Err(RecvTimeoutError::Timeout) => { + app_event_tx.send(AppEvent::Redraw); + next_deadline = None; + } + Err(RecvTimeoutError::Disconnected) => break, + } + } + }); + } Self { server: conversation_manager, app_event_tx, @@ -177,38 +219,19 @@ impl App<'_> { app_state, config, file_search, - pending_redraw, enhanced_keys_supported, commit_anim_running: Arc::new(AtomicBool::new(false)), + frame_schedule_tx: frame_tx, } } - /// Schedule a redraw if one is not already pending. - #[allow(clippy::unwrap_used)] - fn schedule_redraw(&self) { - // Attempt to set the flag to `true`. If it was already `true`, another - // redraw is already pending so we can return early. - if self - .pending_redraw - .compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed) - .is_err() - { - return; - } - - let tx = self.app_event_tx.clone(); - let pending_redraw = self.pending_redraw.clone(); - thread::spawn(move || { - thread::sleep(REDRAW_DEBOUNCE); - tx.send(AppEvent::Redraw); - pending_redraw.store(false, Ordering::Release); - }); + fn schedule_frame_in(&self, dur: Duration) { + let _ = self.frame_schedule_tx.send(Instant::now() + dur); } pub(crate) fn run(&mut self, terminal: &mut tui::Tui) -> Result<()> { - // Insert an event to trigger the first render. - let app_event_tx = self.app_event_tx.clone(); - app_event_tx.send(AppEvent::RequestRedraw); + // Schedule the first render immediately. + let _ = self.frame_schedule_tx.send(Instant::now()); while let Ok(event) = self.app_event_rx.recv() { match event { @@ -217,7 +240,10 @@ impl App<'_> { self.app_event_tx.send(AppEvent::RequestRedraw); } AppEvent::RequestRedraw => { - self.schedule_redraw(); + self.schedule_frame_in(REDRAW_DEBOUNCE); + } + AppEvent::ScheduleFrameIn(dur) => { + self.schedule_frame_in(dur); } AppEvent::Redraw => { std::io::stdout().sync_update(|_| self.draw_next_frame(terminal))??; @@ -447,7 +473,7 @@ impl App<'_> { widget: Box::new(ChatWidget::new( config, self.server.clone(), - app_event_tx.clone(), + self.app_event_tx.clone(), initial_prompt, initial_images, enhanced_keys_supported, diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index 9fe952ea..6b999984 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -2,6 +2,7 @@ use codex_core::protocol::Event; use codex_file_search::FileMatch; use crossterm::event::KeyEvent; use ratatui::text::Line; +use std::time::Duration; use crate::app::ChatWidgetArgs; use crate::slash_command::SlashCommand; @@ -17,6 +18,10 @@ pub(crate) enum AppEvent { /// Actually draw the next frame. Redraw, + /// Schedule a one-shot animation frame roughly after the given duration. + /// Multiple requests are coalesced by the central frame scheduler. + ScheduleFrameIn(Duration), + KeyEvent(KeyEvent), /// Text pasted from the terminal clipboard. diff --git a/codex-rs/tui/src/onboarding/auth.rs b/codex-rs/tui/src/onboarding/auth.rs index 62765528..ed573a41 100644 --- a/codex-rs/tui/src/onboarding/auth.rs +++ b/codex-rs/tui/src/onboarding/auth.rs @@ -20,7 +20,6 @@ use crate::colors::LIGHT_BLUE; use crate::colors::SUCCESS_GREEN; use crate::onboarding::onboarding_screen::KeyboardHandler; use crate::onboarding::onboarding_screen::StepStateProvider; -use crate::shimmer::FrameTicker; use crate::shimmer::shimmer_spans; use std::path::PathBuf; @@ -38,10 +37,9 @@ pub(crate) enum SignInState { } #[derive(Debug)] -/// Used to manage the lifecycle of SpawnedLogin and FrameTicker and ensure they get cleaned up. +/// Used to manage the lifecycle of SpawnedLogin and ensure it gets cleaned up. pub(crate) struct ContinueInBrowserState { login_child: Option, - _frame_ticker: Option, } impl Drop for ContinueInBrowserState { fn drop(&mut self) { @@ -180,9 +178,13 @@ impl AuthModeWidget { } fn render_continue_in_browser(&self, area: Rect, buf: &mut Buffer) { - let idx = self.current_frame(); let mut spans = vec![Span::from("> ")]; - spans.extend(shimmer_spans("Finish signing in via your browser", idx)); + // Schedule a follow-up frame to keep the shimmer animation going. + self.event_tx + .send(AppEvent::ScheduleFrameIn(std::time::Duration::from_millis( + 100, + ))); + spans.extend(shimmer_spans("Finish signing in via your browser")); let mut lines = vec![Line::from(spans), Line::from("")]; if let SignInState::ChatGptContinueInBrowser(state) = &self.sign_in_state { @@ -297,7 +299,6 @@ impl AuthModeWidget { self.sign_in_state = SignInState::ChatGptContinueInBrowser(ContinueInBrowserState { login_child: Some(child), - _frame_ticker: Some(FrameTicker::new(self.event_tx.clone())), }); self.event_tx.send(AppEvent::RequestRedraw); } @@ -353,16 +354,6 @@ impl AuthModeWidget { } }); } - - fn current_frame(&self) -> usize { - // Derive frame index from wall-clock time to avoid storing animation state. - // 100ms per frame to match the previous ticker cadence. - let now_ms = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .map(|d| d.as_millis()) - .unwrap_or(0); - (now_ms / 100) as usize - } } impl StepStateProvider for AuthModeWidget { diff --git a/codex-rs/tui/src/shimmer.rs b/codex-rs/tui/src/shimmer.rs index 7e0d9f57..0d0e5d0b 100644 --- a/codex-rs/tui/src/shimmer.rs +++ b/codex-rs/tui/src/shimmer.rs @@ -1,51 +1,35 @@ -use std::sync::Arc; -use std::sync::atomic::AtomicBool; -use std::sync::atomic::Ordering; +use std::sync::OnceLock; use std::time::Duration; +use std::time::Instant; use ratatui::style::Color; use ratatui::style::Modifier; use ratatui::style::Style; use ratatui::text::Span; -use crate::app_event::AppEvent; -use crate::app_event_sender::AppEventSender; +static PROCESS_START: OnceLock = OnceLock::new(); -#[derive(Debug)] -pub(crate) struct FrameTicker { - running: Arc, +fn elapsed_since_start() -> Duration { + let start = PROCESS_START.get_or_init(Instant::now); + start.elapsed() } -impl FrameTicker { - pub(crate) fn new(app_event_tx: AppEventSender) -> Self { - let running = Arc::new(AtomicBool::new(true)); - let running_clone = running.clone(); - let app_event_tx_clone = app_event_tx.clone(); - std::thread::spawn(move || { - while running_clone.load(Ordering::Relaxed) { - std::thread::sleep(Duration::from_millis(100)); - app_event_tx_clone.send(AppEvent::RequestRedraw); - } - }); - Self { running } - } -} - -impl Drop for FrameTicker { - fn drop(&mut self) { - self.running.store(false, Ordering::Relaxed); - } -} - -pub(crate) fn shimmer_spans(text: &str, frame_idx: usize) -> Vec> { +pub(crate) fn shimmer_spans(text: &str) -> Vec> { let chars: Vec = text.chars().collect(); + if chars.is_empty() { + return Vec::new(); + } + // Use time-based sweep synchronized to process start. let padding = 10usize; let period = chars.len() + padding * 2; - let pos = frame_idx % period; + let sweep_seconds = 2.5f32; + let pos_f = + (elapsed_since_start().as_secs_f32() % sweep_seconds) / sweep_seconds * (period as f32); + let pos = pos_f as usize; let has_true_color = supports_color::on_cached(supports_color::Stream::Stdout) .map(|level| level.has_16m) .unwrap_or(false); - let band_half_width = 6.0; + let band_half_width = 3.0; let mut spans: Vec> = Vec::with_capacity(chars.len()); for (i, ch) in chars.iter().enumerate() { diff --git a/codex-rs/tui/src/status_indicator_widget.rs b/codex-rs/tui/src/status_indicator_widget.rs index 1e236831..f63fc836 100644 --- a/codex-rs/tui/src/status_indicator_widget.rs +++ b/codex-rs/tui/src/status_indicator_widget.rs @@ -1,11 +1,6 @@ //! A live status indicator that shows the *latest* log line emitted by the //! application while the agent is processing a long‑running task. -use std::sync::Arc; -use std::sync::atomic::AtomicBool; -use std::sync::atomic::AtomicUsize; -use std::sync::atomic::Ordering; -use std::thread; use std::time::Duration; use std::time::Instant; @@ -23,6 +18,7 @@ use unicode_width::UnicodeWidthStr; use crate::app_event::AppEvent; use crate::app_event_sender::AppEventSender; +use crate::shimmer::shimmer_spans; // We render the live text using markdown so it visually matches the history // cells. Before rendering we strip any ANSI escape sequences to avoid writing @@ -41,42 +37,17 @@ pub(crate) struct StatusIndicatorWidget { last_target_len: usize, base_frame: usize, reveal_len_at_base: usize, - - frame_idx: Arc, - running: Arc, start_time: Instant, app_event_tx: AppEventSender, } impl StatusIndicatorWidget { - /// Create a new status indicator and start the animation timer. pub(crate) fn new(app_event_tx: AppEventSender) -> Self { - let frame_idx = Arc::new(AtomicUsize::new(0)); - let running = Arc::new(AtomicBool::new(true)); - - // Animation thread. - { - let frame_idx_clone = Arc::clone(&frame_idx); - let running_clone = Arc::clone(&running); - let app_event_tx_clone = app_event_tx.clone(); - thread::spawn(move || { - let mut counter = 0usize; - while running_clone.load(Ordering::Relaxed) { - std::thread::sleep(Duration::from_millis(100)); - counter = counter.wrapping_add(1); - frame_idx_clone.store(counter, Ordering::Relaxed); - app_event_tx_clone.send(AppEvent::RequestRedraw); - } - }); - } - Self { text: String::from("waiting for model"), last_target_len: 0, base_frame: 0, reveal_len_at_base: 0, - frame_idx, - running, start_time: Instant::now(), app_event_tx, @@ -108,7 +79,7 @@ impl StatusIndicatorWidget { // Compute how many characters are currently revealed so we can carry // this forward as the new baseline when target text changes. - let current_frame = self.frame_idx.load(std::sync::atomic::Ordering::Relaxed); + let current_frame = self.current_frame(); let shown_now = self.current_shown_len(current_frame); self.text = text; @@ -135,7 +106,7 @@ impl StatusIndicatorWidget { }; let new_len = stripped.chars().count(); - let current_frame = self.frame_idx.load(std::sync::atomic::Ordering::Relaxed); + let current_frame = self.current_frame(); self.text = sanitized; self.last_target_len = new_len; @@ -155,12 +126,12 @@ impl StatusIndicatorWidget { .saturating_add(frames.saturating_mul(TYPING_CHARS_PER_FRAME)); advanced.min(self.last_target_len) } -} -impl Drop for StatusIndicatorWidget { - fn drop(&mut self) { - use std::sync::atomic::Ordering; - self.running.store(false, Ordering::Relaxed); + fn current_frame(&self) -> usize { + // Derive frame index from wall-clock time. 100ms per frame to match + // the previous ticker cadence. + let since_start = self.start_time.elapsed(); + (since_start.as_millis() / 100) as usize } } @@ -171,45 +142,14 @@ impl WidgetRef for StatusIndicatorWidget { return; } - let idx = self.frame_idx.load(std::sync::atomic::Ordering::Relaxed); + // Schedule next animation frame. + self.app_event_tx + .send(AppEvent::ScheduleFrameIn(Duration::from_millis(100))); + let idx = self.current_frame(); let elapsed = self.start_time.elapsed().as_secs(); let shown_now = self.current_shown_len(idx); let status_prefix: String = self.text.chars().take(shown_now).collect(); - let animated_text = "Working"; - let header_chars: Vec = animated_text.chars().collect(); - let padding = 4usize; // virtual padding around the animated segment for smoother loop - let period = header_chars.len() + padding * 2; - let pos = idx % period; - let has_true_color = supports_color::on_cached(supports_color::Stream::Stdout) - .map(|level| level.has_16m) - .unwrap_or(false); - let band_half_width = 2.0; // width of the bright band in characters - - let mut animated_spans: Vec> = Vec::new(); - for (i, ch) in header_chars.iter().enumerate() { - let i_pos = i as isize + padding as isize; - let pos = pos as isize; - let dist = (i_pos - pos).abs() as f32; - - let t = if dist <= band_half_width { - let x = std::f32::consts::PI * (dist / band_half_width); - 0.5 * (1.0 + x.cos()) - } else { - 0.0 - }; - - let brightness = 0.4 + 0.6 * t; - let level = (brightness * 255.0).clamp(0.0, 255.0) as u8; - let style = if has_true_color { - Style::default() - .fg(Color::Rgb(level, level, level)) - .add_modifier(Modifier::BOLD) - } else { - color_for_level(level) - }; - - animated_spans.push(Span::styled(ch.to_string(), style)); - } + let animated_spans = shimmer_spans("Working"); // Plain rendering: no borders or padding so the live cell is visually indistinguishable from terminal scrollback. let inner_width = area.width as usize; @@ -268,16 +208,6 @@ impl WidgetRef for StatusIndicatorWidget { } } -fn color_for_level(level: u8) -> Style { - if level < 144 { - Style::default().add_modifier(Modifier::DIM) - } else if level < 208 { - Style::default() - } else { - Style::default().add_modifier(Modifier::BOLD) - } -} - #[cfg(test)] mod tests { use super::*;