diff --git a/codex-rs/tui/src/tui.rs b/codex-rs/tui/src/tui.rs index e35e192a5..ff9ce94d2 100644 --- a/codex-rs/tui/src/tui.rs +++ b/codex-rs/tui/src/tui.rs @@ -47,6 +47,7 @@ use crate::tui::event_stream::TuiEventStream; use crate::tui::job_control::SuspendContext; mod event_stream; +mod frame_rate_limiter; mod frame_requester; #[cfg(unix)] mod job_control; diff --git a/codex-rs/tui/src/tui/frame_rate_limiter.rs b/codex-rs/tui/src/tui/frame_rate_limiter.rs new file mode 100644 index 000000000..56dd752e6 --- /dev/null +++ b/codex-rs/tui/src/tui/frame_rate_limiter.rs @@ -0,0 +1,62 @@ +//! Limits how frequently frame draw notifications may be emitted. +//! +//! Widgets sometimes call `FrameRequester::schedule_frame()` more frequently than a user can +//! perceive. This limiter clamps draw notifications to a maximum of 60 FPS to avoid wasted work. +//! +//! This is intentionally a small, pure helper so it can be unit-tested in isolation and used by +//! the async frame scheduler without adding complexity to the app/event loop. + +use std::time::Duration; +use std::time::Instant; + +/// A 60 FPS minimum frame interval (≈16.67ms). +pub(super) const MIN_FRAME_INTERVAL: Duration = Duration::from_nanos(16_666_667); + +/// Remembers the most recent emitted draw, allowing deadlines to be clamped forward. +#[derive(Debug, Default)] +pub(super) struct FrameRateLimiter { + last_emitted_at: Option, +} + +impl FrameRateLimiter { + /// Returns `requested`, clamped forward if it would exceed the maximum frame rate. + pub(super) fn clamp_deadline(&self, requested: Instant) -> Instant { + let Some(last_emitted_at) = self.last_emitted_at else { + return requested; + }; + let min_allowed = last_emitted_at + .checked_add(MIN_FRAME_INTERVAL) + .unwrap_or(last_emitted_at); + requested.max(min_allowed) + } + + /// Records that a draw notification was emitted at `emitted_at`. + pub(super) fn mark_emitted(&mut self, emitted_at: Instant) { + self.last_emitted_at = Some(emitted_at); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + + #[test] + fn default_does_not_clamp() { + let t0 = Instant::now(); + let limiter = FrameRateLimiter::default(); + assert_eq!(limiter.clamp_deadline(t0), t0); + } + + #[test] + fn clamps_to_min_interval_since_last_emit() { + let t0 = Instant::now(); + let mut limiter = FrameRateLimiter::default(); + + assert_eq!(limiter.clamp_deadline(t0), t0); + limiter.mark_emitted(t0); + + let too_soon = t0 + Duration::from_millis(1); + assert_eq!(limiter.clamp_deadline(too_soon), t0 + MIN_FRAME_INTERVAL); + } +} diff --git a/codex-rs/tui/src/tui/frame_requester.rs b/codex-rs/tui/src/tui/frame_requester.rs index 4f7886aa2..88db9d71f 100644 --- a/codex-rs/tui/src/tui/frame_requester.rs +++ b/codex-rs/tui/src/tui/frame_requester.rs @@ -18,6 +18,8 @@ use std::time::Instant; use tokio::sync::broadcast; use tokio::sync::mpsc; +use super::frame_rate_limiter::FrameRateLimiter; + /// A requester for scheduling future frame draws on the TUI event loop. /// /// This is the handler side of an actor/handler pair with `FrameScheduler`, which coalesces @@ -68,15 +70,23 @@ impl FrameRequester { /// A scheduler for coalescing frame draw requests and notifying the TUI event loop. /// /// This type is internal to `FrameRequester` and is spawned as a task to handle scheduling logic. +/// +/// To avoid wasted redraw work, draw notifications are clamped to a maximum of 60 FPS (see +/// [`FrameRateLimiter`]). struct FrameScheduler { receiver: mpsc::UnboundedReceiver, draw_tx: broadcast::Sender<()>, + rate_limiter: FrameRateLimiter, } impl FrameScheduler { /// Create a new FrameScheduler with the provided receiver and draw notification sender. fn new(receiver: mpsc::UnboundedReceiver, draw_tx: broadcast::Sender<()>) -> Self { - Self { receiver, draw_tx } + Self { + receiver, + draw_tx, + rate_limiter: FrameRateLimiter::default(), + } } /// Run the scheduling loop, coalescing frame requests and notifying the TUI event loop. @@ -97,6 +107,7 @@ impl FrameScheduler { // All senders dropped; exit the scheduler. break }; + let draw_at = self.rate_limiter.clamp_deadline(draw_at); next_deadline = Some(next_deadline.map_or(draw_at, |cur| cur.min(draw_at))); // Do not send a draw immediately here. By continuing the loop, @@ -107,6 +118,7 @@ impl FrameScheduler { _ = &mut deadline => { if next_deadline.is_some() { next_deadline = None; + self.rate_limiter.mark_emitted(target); let _ = self.draw_tx.send(()); } } @@ -116,6 +128,7 @@ impl FrameScheduler { } #[cfg(test)] mod tests { + use super::super::frame_rate_limiter::MIN_FRAME_INTERVAL; use super::*; use tokio::time; use tokio_util::time::FutureExt; @@ -218,6 +231,98 @@ mod tests { assert!(second.is_err(), "unexpected extra draw received"); } + #[tokio::test(flavor = "current_thread", start_paused = true)] + async fn test_limits_draw_notifications_to_60fps() { + let (draw_tx, mut draw_rx) = broadcast::channel(16); + let requester = FrameRequester::new(draw_tx); + + requester.schedule_frame(); + time::advance(Duration::from_millis(1)).await; + let first = draw_rx + .recv() + .timeout(Duration::from_millis(50)) + .await + .expect("timed out waiting for first draw"); + assert!(first.is_ok(), "broadcast closed unexpectedly"); + + requester.schedule_frame(); + time::advance(Duration::from_millis(1)).await; + let early = draw_rx.recv().timeout(Duration::from_millis(1)).await; + assert!( + early.is_err(), + "draw fired too early; expected max 60fps (min interval {MIN_FRAME_INTERVAL:?})" + ); + + time::advance(MIN_FRAME_INTERVAL).await; + let second = draw_rx + .recv() + .timeout(Duration::from_millis(50)) + .await + .expect("timed out waiting for second draw"); + assert!(second.is_ok(), "broadcast closed unexpectedly"); + } + + #[tokio::test(flavor = "current_thread", start_paused = true)] + async fn test_rate_limit_clamps_early_delayed_requests() { + let (draw_tx, mut draw_rx) = broadcast::channel(16); + let requester = FrameRequester::new(draw_tx); + + requester.schedule_frame(); + time::advance(Duration::from_millis(1)).await; + let first = draw_rx + .recv() + .timeout(Duration::from_millis(50)) + .await + .expect("timed out waiting for first draw"); + assert!(first.is_ok(), "broadcast closed unexpectedly"); + + requester.schedule_frame_in(Duration::from_millis(1)); + + time::advance(Duration::from_millis(10)).await; + let too_early = draw_rx.recv().timeout(Duration::from_millis(1)).await; + assert!( + too_early.is_err(), + "draw fired too early; expected max 60fps (min interval {MIN_FRAME_INTERVAL:?})" + ); + + time::advance(MIN_FRAME_INTERVAL).await; + let second = draw_rx + .recv() + .timeout(Duration::from_millis(50)) + .await + .expect("timed out waiting for clamped draw"); + assert!(second.is_ok(), "broadcast closed unexpectedly"); + } + + #[tokio::test(flavor = "current_thread", start_paused = true)] + async fn test_rate_limit_does_not_delay_future_draws() { + let (draw_tx, mut draw_rx) = broadcast::channel(16); + let requester = FrameRequester::new(draw_tx); + + requester.schedule_frame(); + time::advance(Duration::from_millis(1)).await; + let first = draw_rx + .recv() + .timeout(Duration::from_millis(50)) + .await + .expect("timed out waiting for first draw"); + assert!(first.is_ok(), "broadcast closed unexpectedly"); + + requester.schedule_frame_in(Duration::from_millis(50)); + + time::advance(Duration::from_millis(49)).await; + let early = draw_rx.recv().timeout(Duration::from_millis(1)).await; + assert!(early.is_err(), "draw fired too early"); + + time::advance(Duration::from_millis(1)).await; + let second = draw_rx + .recv() + .timeout(Duration::from_millis(50)) + .await + .expect("timed out waiting for delayed draw"); + assert!(second.is_ok(), "broadcast closed unexpectedly"); + } + #[tokio::test(flavor = "current_thread", start_paused = true)] async fn test_multiple_delayed_requests_coalesce_to_earliest() { let (draw_tx, mut draw_rx) = broadcast::channel(16); diff --git a/codex-rs/tui2/src/app.rs b/codex-rs/tui2/src/app.rs index 78ccfb038..d567ce961 100644 --- a/codex-rs/tui2/src/app.rs +++ b/codex-rs/tui2/src/app.rs @@ -1114,9 +1114,8 @@ impl App { .scrolled_by(delta_lines, &line_meta, visible_lines); if schedule_frame { - // Delay redraws slightly so scroll bursts coalesce into a single frame. - tui.frame_requester() - .schedule_frame_in(Duration::from_millis(16)); + // Request a redraw; the frame scheduler coalesces bursts and clamps to 60fps. + tui.frame_requester().schedule_frame(); } } diff --git a/codex-rs/tui2/src/pager_overlay.rs b/codex-rs/tui2/src/pager_overlay.rs index 06a7cad46..3d24aef96 100644 --- a/codex-rs/tui2/src/pager_overlay.rs +++ b/codex-rs/tui2/src/pager_overlay.rs @@ -1,6 +1,5 @@ use std::io::Result; use std::sync::Arc; -use std::time::Duration; use crate::history_cell::HistoryCell; use crate::history_cell::UserHistoryCell; @@ -280,8 +279,8 @@ impl PagerView { return Ok(()); } } - tui.frame_requester() - .schedule_frame_in(Duration::from_millis(16)); + // Request a redraw; the frame scheduler coalesces bursts and clamps to 60fps. + tui.frame_requester().schedule_frame(); Ok(()) } @@ -298,8 +297,8 @@ impl PagerView { return Ok(()); } } - tui.frame_requester() - .schedule_frame_in(Duration::from_millis(16)); + // Request a redraw; the frame scheduler coalesces bursts and clamps to 60fps. + tui.frame_requester().schedule_frame(); Ok(()) } diff --git a/codex-rs/tui2/src/tui.rs b/codex-rs/tui2/src/tui.rs index 2b79b1231..3cb1fb01c 100644 --- a/codex-rs/tui2/src/tui.rs +++ b/codex-rs/tui2/src/tui.rs @@ -47,6 +47,7 @@ use crate::tui::job_control::SUSPEND_KEY; use crate::tui::job_control::SuspendContext; mod alt_screen_nesting; +mod frame_rate_limiter; mod frame_requester; #[cfg(unix)] mod job_control; diff --git a/codex-rs/tui2/src/tui/frame_rate_limiter.rs b/codex-rs/tui2/src/tui/frame_rate_limiter.rs new file mode 100644 index 000000000..56dd752e6 --- /dev/null +++ b/codex-rs/tui2/src/tui/frame_rate_limiter.rs @@ -0,0 +1,62 @@ +//! Limits how frequently frame draw notifications may be emitted. +//! +//! Widgets sometimes call `FrameRequester::schedule_frame()` more frequently than a user can +//! perceive. This limiter clamps draw notifications to a maximum of 60 FPS to avoid wasted work. +//! +//! This is intentionally a small, pure helper so it can be unit-tested in isolation and used by +//! the async frame scheduler without adding complexity to the app/event loop. + +use std::time::Duration; +use std::time::Instant; + +/// A 60 FPS minimum frame interval (≈16.67ms). +pub(super) const MIN_FRAME_INTERVAL: Duration = Duration::from_nanos(16_666_667); + +/// Remembers the most recent emitted draw, allowing deadlines to be clamped forward. +#[derive(Debug, Default)] +pub(super) struct FrameRateLimiter { + last_emitted_at: Option, +} + +impl FrameRateLimiter { + /// Returns `requested`, clamped forward if it would exceed the maximum frame rate. + pub(super) fn clamp_deadline(&self, requested: Instant) -> Instant { + let Some(last_emitted_at) = self.last_emitted_at else { + return requested; + }; + let min_allowed = last_emitted_at + .checked_add(MIN_FRAME_INTERVAL) + .unwrap_or(last_emitted_at); + requested.max(min_allowed) + } + + /// Records that a draw notification was emitted at `emitted_at`. + pub(super) fn mark_emitted(&mut self, emitted_at: Instant) { + self.last_emitted_at = Some(emitted_at); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + + #[test] + fn default_does_not_clamp() { + let t0 = Instant::now(); + let limiter = FrameRateLimiter::default(); + assert_eq!(limiter.clamp_deadline(t0), t0); + } + + #[test] + fn clamps_to_min_interval_since_last_emit() { + let t0 = Instant::now(); + let mut limiter = FrameRateLimiter::default(); + + assert_eq!(limiter.clamp_deadline(t0), t0); + limiter.mark_emitted(t0); + + let too_soon = t0 + Duration::from_millis(1); + assert_eq!(limiter.clamp_deadline(too_soon), t0 + MIN_FRAME_INTERVAL); + } +} diff --git a/codex-rs/tui2/src/tui/frame_requester.rs b/codex-rs/tui2/src/tui/frame_requester.rs index 4f7886aa2..88db9d71f 100644 --- a/codex-rs/tui2/src/tui/frame_requester.rs +++ b/codex-rs/tui2/src/tui/frame_requester.rs @@ -18,6 +18,8 @@ use std::time::Instant; use tokio::sync::broadcast; use tokio::sync::mpsc; +use super::frame_rate_limiter::FrameRateLimiter; + /// A requester for scheduling future frame draws on the TUI event loop. /// /// This is the handler side of an actor/handler pair with `FrameScheduler`, which coalesces @@ -68,15 +70,23 @@ impl FrameRequester { /// A scheduler for coalescing frame draw requests and notifying the TUI event loop. /// /// This type is internal to `FrameRequester` and is spawned as a task to handle scheduling logic. +/// +/// To avoid wasted redraw work, draw notifications are clamped to a maximum of 60 FPS (see +/// [`FrameRateLimiter`]). struct FrameScheduler { receiver: mpsc::UnboundedReceiver, draw_tx: broadcast::Sender<()>, + rate_limiter: FrameRateLimiter, } impl FrameScheduler { /// Create a new FrameScheduler with the provided receiver and draw notification sender. fn new(receiver: mpsc::UnboundedReceiver, draw_tx: broadcast::Sender<()>) -> Self { - Self { receiver, draw_tx } + Self { + receiver, + draw_tx, + rate_limiter: FrameRateLimiter::default(), + } } /// Run the scheduling loop, coalescing frame requests and notifying the TUI event loop. @@ -97,6 +107,7 @@ impl FrameScheduler { // All senders dropped; exit the scheduler. break }; + let draw_at = self.rate_limiter.clamp_deadline(draw_at); next_deadline = Some(next_deadline.map_or(draw_at, |cur| cur.min(draw_at))); // Do not send a draw immediately here. By continuing the loop, @@ -107,6 +118,7 @@ impl FrameScheduler { _ = &mut deadline => { if next_deadline.is_some() { next_deadline = None; + self.rate_limiter.mark_emitted(target); let _ = self.draw_tx.send(()); } } @@ -116,6 +128,7 @@ impl FrameScheduler { } #[cfg(test)] mod tests { + use super::super::frame_rate_limiter::MIN_FRAME_INTERVAL; use super::*; use tokio::time; use tokio_util::time::FutureExt; @@ -218,6 +231,98 @@ mod tests { assert!(second.is_err(), "unexpected extra draw received"); } + #[tokio::test(flavor = "current_thread", start_paused = true)] + async fn test_limits_draw_notifications_to_60fps() { + let (draw_tx, mut draw_rx) = broadcast::channel(16); + let requester = FrameRequester::new(draw_tx); + + requester.schedule_frame(); + time::advance(Duration::from_millis(1)).await; + let first = draw_rx + .recv() + .timeout(Duration::from_millis(50)) + .await + .expect("timed out waiting for first draw"); + assert!(first.is_ok(), "broadcast closed unexpectedly"); + + requester.schedule_frame(); + time::advance(Duration::from_millis(1)).await; + let early = draw_rx.recv().timeout(Duration::from_millis(1)).await; + assert!( + early.is_err(), + "draw fired too early; expected max 60fps (min interval {MIN_FRAME_INTERVAL:?})" + ); + + time::advance(MIN_FRAME_INTERVAL).await; + let second = draw_rx + .recv() + .timeout(Duration::from_millis(50)) + .await + .expect("timed out waiting for second draw"); + assert!(second.is_ok(), "broadcast closed unexpectedly"); + } + + #[tokio::test(flavor = "current_thread", start_paused = true)] + async fn test_rate_limit_clamps_early_delayed_requests() { + let (draw_tx, mut draw_rx) = broadcast::channel(16); + let requester = FrameRequester::new(draw_tx); + + requester.schedule_frame(); + time::advance(Duration::from_millis(1)).await; + let first = draw_rx + .recv() + .timeout(Duration::from_millis(50)) + .await + .expect("timed out waiting for first draw"); + assert!(first.is_ok(), "broadcast closed unexpectedly"); + + requester.schedule_frame_in(Duration::from_millis(1)); + + time::advance(Duration::from_millis(10)).await; + let too_early = draw_rx.recv().timeout(Duration::from_millis(1)).await; + assert!( + too_early.is_err(), + "draw fired too early; expected max 60fps (min interval {MIN_FRAME_INTERVAL:?})" + ); + + time::advance(MIN_FRAME_INTERVAL).await; + let second = draw_rx + .recv() + .timeout(Duration::from_millis(50)) + .await + .expect("timed out waiting for clamped draw"); + assert!(second.is_ok(), "broadcast closed unexpectedly"); + } + + #[tokio::test(flavor = "current_thread", start_paused = true)] + async fn test_rate_limit_does_not_delay_future_draws() { + let (draw_tx, mut draw_rx) = broadcast::channel(16); + let requester = FrameRequester::new(draw_tx); + + requester.schedule_frame(); + time::advance(Duration::from_millis(1)).await; + let first = draw_rx + .recv() + .timeout(Duration::from_millis(50)) + .await + .expect("timed out waiting for first draw"); + assert!(first.is_ok(), "broadcast closed unexpectedly"); + + requester.schedule_frame_in(Duration::from_millis(50)); + + time::advance(Duration::from_millis(49)).await; + let early = draw_rx.recv().timeout(Duration::from_millis(1)).await; + assert!(early.is_err(), "draw fired too early"); + + time::advance(Duration::from_millis(1)).await; + let second = draw_rx + .recv() + .timeout(Duration::from_millis(50)) + .await + .expect("timed out waiting for delayed draw"); + assert!(second.is_ok(), "broadcast closed unexpectedly"); + } + #[tokio::test(flavor = "current_thread", start_paused = true)] async fn test_multiple_delayed_requests_coalesce_to_earliest() { let (draw_tx, mut draw_rx) = broadcast::channel(16);