mirror of
https://github.com/openai/codex.git
synced 2026-11-13 14:46:14 +00:00
feat(tui2): tune scrolling inpu based on (#8357)
## TUI2: Normalize Mouse Scroll Input Across Terminals (Wheel + Trackpad) This changes TUI2 scrolling to a stream-based model that normalizes terminal scroll event density into consistent wheel behavior (default: ~3 transcript lines per physical wheel notch) while keeping trackpad input higher fidelity via fractional accumulation. Primary code: `codex-rs/tui2/src/tui/scrolling/mouse.rs` Doc of record (model + probe-derived data): `codex-rs/tui2/docs/scroll_input_model.md` ### Why Terminals encode both mouse wheels and trackpads as discrete scroll up/down events with direction but no magnitude, and they vary widely in how many raw events they emit per physical wheel notch (commonly 1, 3, or 9+). Timing alone doesn’t reliably distinguish wheel vs trackpad, so cadence-based heuristics are unstable across terminals/hardware. This PR treats scroll input as short *streams* separated by silence or direction flips, normalizes raw event density into tick-equivalents, coalesces redraws for dense streams, and exposes explicit config overrides. ### What Changed #### Scroll Model (TUI2) - Stream detection - Start a stream on the first scroll event. - End a stream on an idle gap (`STREAM_GAP_MS`) or a direction flip. - Normalization - Convert raw events into tick-equivalents using per-terminal `tui.scroll_events_per_tick`. - Wheel-like vs trackpad-like behavior - Wheel-like: fixed “classic” lines per wheel notch; flush immediately for responsiveness. - Trackpad-like: fractional accumulation + carry across stream boundaries; coalesce flushes to ~60Hz to avoid floods and reduce “stop lag / overshoot”. - Trackpad divisor is intentionally capped: `min(scroll_events_per_tick, 3)` so terminals with dense wheel ticks (e.g. 9 events per notch) don’t make trackpads feel artificially slow. - Auto mode (default) - Start conservatively as trackpad-like (avoid overshoot). - Promote to wheel-like if the first tick-worth of events arrives quickly. - Fallback for 1-event-per-tick terminals (no tick-completion timing signal). #### Trackpad Acceleration Some terminals produce relatively low vertical event density for trackpad gestures, which makes large/faster swipes feel sluggish even when small motions feel correct. To address that, trackpad-like streams apply a bounded multiplier based on event count: - `multiplier = clamp(1 + abs(events) / scroll_trackpad_accel_events, 1..scroll_trackpad_accel_max)` The multiplier is applied to the trackpad stream’s computed line delta (including carried fractional remainder). Defaults are conservative and bounded. #### Config Knobs (TUI2) All keys live under `[tui]`: - `scroll_wheel_lines`: lines per physical wheel notch (default: 3). - `scroll_events_per_tick`: raw vertical scroll events per physical wheel notch (terminal-specific default; fallback: 3). - Wheel-like per-event contribution: `scroll_wheel_lines / scroll_events_per_tick`. - `scroll_trackpad_lines`: baseline trackpad sensitivity (default: 1). - Trackpad-like per-event contribution: `scroll_trackpad_lines / min(scroll_events_per_tick, 3)`. - `scroll_trackpad_accel_events` / `scroll_trackpad_accel_max`: bounded trackpad acceleration (defaults: 30 / 3). - `scroll_mode = auto|wheel|trackpad`: force behavior or use the heuristic (default: `auto`). - `scroll_wheel_tick_detect_max_ms`: auto-mode promotion threshold (ms). - `scroll_wheel_like_max_duration_ms`: auto-mode fallback for 1-event-per-tick terminals (ms). - `scroll_invert`: invert scroll direction (applies to wheel + trackpad). Config docs: `docs/config.md` and field docs in `codex-rs/core/src/config/types.rs`. #### App Integration - The app schedules follow-up ticks to close idle streams (via `ScrollUpdate::next_tick_in` and `schedule_frame_in`) and finalizes streams on draw ticks. - `codex-rs/tui2/src/app.rs` #### Docs - Single doc of record describing the model + preserved probe findings/spec: - `codex-rs/tui2/docs/scroll_input_model.md` #### Other (jj-only friendliness) - `codex-rs/tui2/src/diff_render.rs`: prefer stable cwd-relative paths when the file is under the cwd even if there’s no `.git`. ### Terminal Defaults Per-terminal defaults are derived from scroll-probe logs (see doc). Notable: - Ghostty currently defaults to `scroll_events_per_tick = 3` even though logs measured ~9 in one setup. This is a deliberate stopgap; if your Ghostty build emits ~9 events per wheel notch, set: ```toml [tui] scroll_events_per_tick = 9 ``` ### Testing - `just fmt` - `just fix -p codex-core --allow-no-vcs` - `cargo test -p codex-core --lib` (pass) - `cargo test -p codex-tui2` (scroll tests pass; remaining failures are known flaky VT100 color tests in `insert_history`) ### Review Focus - Stream finalization + frame scheduling in `codex-rs/tui2/src/app.rs`. - Auto-mode promotion thresholds and the 1-event-per-tick fallback behavior. - Trackpad divisor cap (`min(events_per_tick, 3)`) and acceleration defaults. - Ghostty default tradeoff (3 vs ~9) and whether we should change it.
This commit is contained in:
1 parent
a6974087e5
commit
63942b883c
9 files changed
+2219
-13
No files matched your search
+154
-9
@@ -19,6 +19,11 @@ use crate::render::renderable::Renderable;
|
||||
use crate::resume_picker::ResumeSelection;
|
||||
use crate::tui;
|
||||
use crate::tui::TuiEvent;
|
||||
use crate::tui::scrolling::MouseScrollState;
|
||||
use crate::tui::scrolling::ScrollConfig;
|
||||
use crate::tui::scrolling::ScrollConfigOverrides;
|
||||
use crate::tui::scrolling::ScrollDirection;
|
||||
use crate::tui::scrolling::ScrollUpdate;
|
||||
use crate::tui::scrolling::TranscriptLineMeta;
|
||||
use crate::tui::scrolling::TranscriptScroll;
|
||||
use crate::update_action::UpdateAction;
|
||||
@@ -42,6 +47,7 @@ use codex_core::protocol::Op;
|
||||
use codex_core::protocol::SessionSource;
|
||||
use codex_core::protocol::SkillErrorInfo;
|
||||
use codex_core::protocol::TokenUsage;
|
||||
use codex_core::terminal::terminal_info;
|
||||
use codex_protocol::ConversationId;
|
||||
use codex_protocol::openai_models::ModelPreset;
|
||||
use codex_protocol::openai_models::ModelUpgrade;
|
||||
@@ -336,6 +342,9 @@ pub(crate) struct App {
|
||||
/// Controls the animation thread that sends CommitTick events.
|
||||
pub(crate) commit_anim_running: Arc<AtomicBool>,
|
||||
|
||||
scroll_config: ScrollConfig,
|
||||
scroll_state: MouseScrollState,
|
||||
|
||||
// Esc-backtracking state grouped
|
||||
pub(crate) backtrack: crate::app_backtrack::BacktrackState,
|
||||
pub(crate) feedback: codex_feedback::CodexFeedback,
|
||||
@@ -371,6 +380,7 @@ struct TranscriptSelectionPoint {
|
||||
line_index: usize,
|
||||
column: u16,
|
||||
}
|
||||
|
||||
impl App {
|
||||
async fn shutdown_current_conversation(&mut self) {
|
||||
if let Some(conversation_id) = self.chat_widget.conversation_id() {
|
||||
@@ -478,6 +488,20 @@ impl App {
|
||||
let file_search = FileSearchManager::new(config.cwd.clone(), app_event_tx.clone());
|
||||
#[cfg(not(debug_assertions))]
|
||||
let upgrade_version = crate::updates::get_upgrade_version(&config);
|
||||
let scroll_config = ScrollConfig::from_terminal(
|
||||
&terminal_info(),
|
||||
ScrollConfigOverrides {
|
||||
events_per_tick: config.tui_scroll_events_per_tick,
|
||||
wheel_lines_per_tick: config.tui_scroll_wheel_lines,
|
||||
trackpad_lines_per_tick: config.tui_scroll_trackpad_lines,
|
||||
trackpad_accel_events: config.tui_scroll_trackpad_accel_events,
|
||||
trackpad_accel_max: config.tui_scroll_trackpad_accel_max,
|
||||
mode: Some(config.tui_scroll_mode),
|
||||
wheel_tick_detect_max_ms: config.tui_scroll_wheel_tick_detect_max_ms,
|
||||
wheel_like_max_duration_ms: config.tui_scroll_wheel_like_max_duration_ms,
|
||||
invert_direction: config.tui_scroll_invert,
|
||||
},
|
||||
);
|
||||
|
||||
let mut app = Self {
|
||||
server: conversation_manager.clone(),
|
||||
@@ -498,6 +522,8 @@ impl App {
|
||||
deferred_history_lines: Vec::new(),
|
||||
has_emitted_history_lines: false,
|
||||
commit_anim_running: Arc::new(AtomicBool::new(false)),
|
||||
scroll_config,
|
||||
scroll_state: MouseScrollState::default(),
|
||||
backtrack: BacktrackState::default(),
|
||||
feedback: feedback.clone(),
|
||||
pending_update_action: None,
|
||||
@@ -581,6 +607,10 @@ impl App {
|
||||
tui: &mut tui::Tui,
|
||||
event: TuiEvent,
|
||||
) -> Result<bool> {
|
||||
if matches!(&event, TuiEvent::Draw) {
|
||||
self.handle_scroll_tick(tui);
|
||||
}
|
||||
|
||||
if self.overlay.is_some() {
|
||||
let _ = self.handle_backtrack_overlay_event(tui, event).await?;
|
||||
} else {
|
||||
@@ -810,7 +840,8 @@ impl App {
|
||||
|
||||
/// Handle mouse interaction in the main transcript view.
|
||||
///
|
||||
/// - Mouse wheel movement scrolls the conversation history by small, fixed increments,
|
||||
/// - Mouse wheel movement scrolls the conversation history using stream-based
|
||||
/// normalization (events-per-line factor, discrete vs. continuous streams),
|
||||
/// independent of the terminal's own scrollback.
|
||||
/// - Mouse clicks and drags adjust a text selection defined in terms of
|
||||
/// flattened transcript lines and columns, so the selection is anchored
|
||||
@@ -875,21 +906,26 @@ impl App {
|
||||
|
||||
match mouse_event.kind {
|
||||
MouseEventKind::ScrollUp => {
|
||||
self.scroll_transcript(
|
||||
let scroll_update = self.mouse_scroll_update(ScrollDirection::Up);
|
||||
self.apply_scroll_update(
|
||||
tui,
|
||||
-3,
|
||||
scroll_update,
|
||||
transcript_area.height as usize,
|
||||
transcript_area.width,
|
||||
true,
|
||||
);
|
||||
}
|
||||
MouseEventKind::ScrollDown => {
|
||||
self.scroll_transcript(
|
||||
let scroll_update = self.mouse_scroll_update(ScrollDirection::Down);
|
||||
self.apply_scroll_update(
|
||||
tui,
|
||||
3,
|
||||
scroll_update,
|
||||
transcript_area.height as usize,
|
||||
transcript_area.width,
|
||||
true,
|
||||
);
|
||||
}
|
||||
MouseEventKind::ScrollLeft | MouseEventKind::ScrollRight => {}
|
||||
MouseEventKind::Down(MouseButton::Left) => {
|
||||
if let Some(point) = self.transcript_point_from_coordinates(
|
||||
transcript_area,
|
||||
@@ -931,18 +967,119 @@ impl App {
|
||||
}
|
||||
}
|
||||
|
||||
/// Scroll the transcript by a fixed number of visual lines.
|
||||
/// Convert a single mouse scroll event (direction-only) into a normalized scroll update.
|
||||
///
|
||||
/// This delegates to [`MouseScrollState::on_scroll_event`] using the current [`ScrollConfig`].
|
||||
/// The returned [`ScrollUpdate`] is intentionally split into:
|
||||
///
|
||||
/// - `lines`: a *delta* in visual lines to apply immediately to the transcript viewport.
|
||||
/// - Sign convention matches [`ScrollDirection`] (`Up` is negative; `Down` is positive).
|
||||
/// - May be 0 in trackpad-like mode while sub-line fractions are still accumulating.
|
||||
/// - `next_tick_in`: an optional delay after which we should trigger a follow-up tick.
|
||||
/// This is required because stream closure is defined by a *time gap* rather than an
|
||||
/// explicit "gesture end" event. See [`App::apply_scroll_update`] and
|
||||
/// [`App::handle_scroll_tick`].
|
||||
///
|
||||
/// In TUI2, that follow-up tick is driven via `TuiEvent::Draw`: we schedule a frame, and on
|
||||
/// the next draw we call [`MouseScrollState::on_tick`] to close idle streams and flush any
|
||||
/// newly-reached whole lines. This prevents perceived "stop lag" where accumulated scroll only
|
||||
/// applies once the next user input arrives.
|
||||
fn mouse_scroll_update(&mut self, direction: ScrollDirection) -> ScrollUpdate {
|
||||
self.scroll_state
|
||||
.on_scroll_event(direction, self.scroll_config)
|
||||
}
|
||||
|
||||
/// Apply a [`ScrollUpdate`] to the transcript viewport and schedule any needed follow-up tick.
|
||||
///
|
||||
/// `update.lines` is applied immediately via [`App::scroll_transcript`].
|
||||
///
|
||||
/// If `update.next_tick_in` is `Some`, we schedule a future frame so `TuiEvent::Draw` can call
|
||||
/// [`App::handle_scroll_tick`] and close the stream after it goes idle and/or cadence-flush
|
||||
/// pending whole lines.
|
||||
///
|
||||
/// `schedule_frame` is forwarded to [`App::scroll_transcript`] and controls whether scrolling
|
||||
/// should request an additional draw. Pass `false` when applying scroll during a
|
||||
/// `TuiEvent::Draw` tick to avoid redundant frames.
|
||||
fn apply_scroll_update(
|
||||
&mut self,
|
||||
tui: &mut tui::Tui,
|
||||
update: ScrollUpdate,
|
||||
visible_lines: usize,
|
||||
width: u16,
|
||||
schedule_frame: bool,
|
||||
) {
|
||||
if update.lines != 0 {
|
||||
self.scroll_transcript(tui, update.lines, visible_lines, width, schedule_frame);
|
||||
}
|
||||
if let Some(delay) = update.next_tick_in {
|
||||
tui.frame_requester().schedule_frame_in(delay);
|
||||
}
|
||||
}
|
||||
|
||||
/// Drive stream closure and cadence-based flushing for mouse scrolling.
|
||||
///
|
||||
/// This is called on every `TuiEvent::Draw` before rendering. If a scroll stream is active, it
|
||||
/// may:
|
||||
///
|
||||
/// - Close the stream once it has been idle for longer than the stream-gap threshold.
|
||||
/// - Flush whole-line deltas on the redraw cadence for trackpad-like streams, even if no new
|
||||
/// events arrive.
|
||||
///
|
||||
/// The resulting update is applied with `schedule_frame = false` because we are already in a
|
||||
/// draw tick.
|
||||
fn handle_scroll_tick(&mut self, tui: &mut tui::Tui) {
|
||||
let Some((visible_lines, width)) = self.transcript_scroll_dimensions(tui) else {
|
||||
return;
|
||||
};
|
||||
let update = self.scroll_state.on_tick();
|
||||
self.apply_scroll_update(tui, update, visible_lines, width, false);
|
||||
}
|
||||
|
||||
/// Compute the transcript viewport dimensions used for scrolling.
|
||||
///
|
||||
/// Mouse scrolling is applied in terms of "visible transcript lines": the terminal height
|
||||
/// minus the chat composer height. We compute this from the last known terminal size to avoid
|
||||
/// querying the terminal during non-draw events.
|
||||
///
|
||||
/// Returns `(visible_lines, width)` or `None` when the terminal is not yet sized or the chat
|
||||
/// area consumes the full height.
|
||||
fn transcript_scroll_dimensions(&self, tui: &tui::Tui) -> Option<(usize, u16)> {
|
||||
let size = tui.terminal.last_known_screen_size;
|
||||
let width = size.width;
|
||||
let height = size.height;
|
||||
if width == 0 || height == 0 {
|
||||
return None;
|
||||
}
|
||||
|
||||
let chat_height = self.chat_widget.desired_height(width);
|
||||
if chat_height >= height {
|
||||
return None;
|
||||
}
|
||||
|
||||
let transcript_height = height.saturating_sub(chat_height);
|
||||
if transcript_height == 0 {
|
||||
return None;
|
||||
}
|
||||
|
||||
Some((transcript_height as usize, width))
|
||||
}
|
||||
|
||||
/// Scroll the transcript by a number of visual lines.
|
||||
///
|
||||
/// This is the shared implementation behind mouse wheel movement and PgUp/PgDn keys in
|
||||
/// the main view. Scroll state is expressed in terms of transcript cells and their
|
||||
/// internal line indices, so scrolling refers to logical conversation content and
|
||||
/// remains stable even as wrapping or streaming causes visual reflows.
|
||||
///
|
||||
/// `schedule_frame` controls whether to request an extra draw; pass `false` when applying
|
||||
/// scroll during a `TuiEvent::Draw` tick to avoid redundant frames.
|
||||
fn scroll_transcript(
|
||||
&mut self,
|
||||
tui: &mut tui::Tui,
|
||||
delta_lines: i32,
|
||||
visible_lines: usize,
|
||||
width: u16,
|
||||
schedule_frame: bool,
|
||||
) {
|
||||
if visible_lines == 0 {
|
||||
return;
|
||||
@@ -953,9 +1090,11 @@ impl App {
|
||||
self.transcript_scroll
|
||||
.scrolled_by(delta_lines, &line_meta, visible_lines);
|
||||
|
||||
// Delay redraws slightly so scroll bursts coalesce into a single frame.
|
||||
tui.frame_requester()
|
||||
.schedule_frame_in(Duration::from_millis(16));
|
||||
if schedule_frame {
|
||||
// Delay redraws slightly so scroll bursts coalesce into a single frame.
|
||||
tui.frame_requester()
|
||||
.schedule_frame_in(Duration::from_millis(16));
|
||||
}
|
||||
}
|
||||
|
||||
/// Convert a `ToBottom` (auto-follow) scroll state into a fixed anchor at the current view.
|
||||
@@ -2011,6 +2150,7 @@ impl App {
|
||||
delta,
|
||||
usize::from(transcript_height),
|
||||
width,
|
||||
true,
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -2035,6 +2175,7 @@ impl App {
|
||||
delta,
|
||||
usize::from(transcript_height),
|
||||
width,
|
||||
true,
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -2177,6 +2318,8 @@ mod tests {
|
||||
has_emitted_history_lines: false,
|
||||
enhanced_keys_supported: false,
|
||||
commit_anim_running: Arc::new(AtomicBool::new(false)),
|
||||
scroll_config: ScrollConfig::default(),
|
||||
scroll_state: MouseScrollState::default(),
|
||||
backtrack: BacktrackState::default(),
|
||||
feedback: codex_feedback::CodexFeedback::new(),
|
||||
pending_update_action: None,
|
||||
@@ -2221,6 +2364,8 @@ mod tests {
|
||||
has_emitted_history_lines: false,
|
||||
enhanced_keys_supported: false,
|
||||
commit_anim_running: Arc::new(AtomicBool::new(false)),
|
||||
scroll_config: ScrollConfig::default(),
|
||||
scroll_state: MouseScrollState::default(),
|
||||
backtrack: BacktrackState::default(),
|
||||
feedback: codex_feedback::CodexFeedback::new(),
|
||||
pending_update_action: None,
|
||||
|
||||
@@ -300,6 +300,17 @@ fn render_change(change: &FileChange, out: &mut Vec<RtLine<'static>>, width: usi
|
||||
}
|
||||
|
||||
pub(crate) fn display_path_for(path: &Path, cwd: &Path) -> String {
|
||||
// Prefer a stable, user-local relative path when the file is under the current working
|
||||
// directory. This keeps output deterministic in jj-only repos (no `.git`) and matches user
|
||||
// expectations for "files in this project".
|
||||
if let Some(rel) = pathdiff::diff_paths(path, cwd)
|
||||
&& !rel
|
||||
.components()
|
||||
.any(|c| matches!(c, std::path::Component::ParentDir))
|
||||
{
|
||||
return rel.display().to_string();
|
||||
}
|
||||
|
||||
let path_in_same_repo = match (get_git_repo_root(cwd), get_git_repo_root(path)) {
|
||||
(Some(cwd_repo), Some(path_repo)) => cwd_repo == path_repo,
|
||||
_ => false,
|
||||
|
||||
@@ -19,6 +19,13 @@
|
||||
//! Spacer rows between non-continuation cells are represented as `TranscriptLineMeta::Spacer`.
|
||||
//! They are not valid anchors; `anchor_for` will pick the nearest non-spacer line when needed.
|
||||
|
||||
pub(crate) mod mouse;
|
||||
pub(crate) use mouse::MouseScrollState;
|
||||
pub(crate) use mouse::ScrollConfig;
|
||||
pub(crate) use mouse::ScrollConfigOverrides;
|
||||
pub(crate) use mouse::ScrollDirection;
|
||||
pub(crate) use mouse::ScrollUpdate;
|
||||
|
||||
/// Per-flattened-line metadata for the transcript view.
|
||||
///
|
||||
/// Each rendered line in the flattened transcript has a corresponding `TranscriptLineMeta` entry
|
||||
|
||||
File diff suppressed because it is too large.
Load diff
Reference in new issue
Block a user