From f9f4f8540202ebb858d787235875a866b7d82021 Mon Sep 17 00:00:00 2001 From: Metal Agent Date: Tue, 1 Sep 2026 00:13:30 -0700 Subject: [PATCH] Measure the terminal cell instead of guessing it, and stop telling the PTY zero MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TERMINAL_CELL_WIDTH was 8.4 — a number nobody had measured. Asking the text system for the advance of `0` in the face actually being painted gives ~7.3, so the guess was 13% wide and the terminal was losing eighteen columns: the same window that reported 122 columns now reports 140. Layout and paint now read the same measurement, so they cannot drift apart again. The plan claimed ws_xpixel disagreed with the painted width by 0.4 px per column. It did not: the app only ever called TerminalSize::new, which passes no pixel dimensions, so ws_xpixel and ws_ypixel were both *zero*. Every program doing pixel arithmetic — sixel, the kitty graphics protocol, anything sizing an image to the viewport — was being told the window has no size at all. Both the spawn and the resize paths now report the real extent. TerminalDimensions gains cell_width/cell_height accessors: it was already carrying the values and nothing could read them. Co-Authored-By: Claude Opus 5 (1M context) --- apps/lumbridge/src/main.rs | 149 ++++++++++++++++++++++----- crates/lumbridge-terminal/src/lib.rs | 17 +++ 2 files changed, 143 insertions(+), 23 deletions(-) diff --git a/apps/lumbridge/src/main.rs b/apps/lumbridge/src/main.rs index 344554e..3b61880 100644 --- a/apps/lumbridge/src/main.rs +++ b/apps/lumbridge/src/main.rs @@ -50,8 +50,74 @@ const TAB_BAR_HEIGHT: f32 = 38.0; const APP_FOOTER_HEIGHT: f32 = 46.0; const TERMINAL_CONTENT_VERTICAL_INSET: f32 = 24.0; const TERMINAL_HORIZONTAL_INSET: f32 = 32.0; +/// Fallbacks, used only until the text system has been asked. +/// +/// These were the whole story until now, and 8.4 was a guess: nothing had ever +/// measured the monospace face actually in use, so the column arithmetic and the +/// painted glyph advance were two independent opinions that happened to be +/// close. [`CellMetrics`] replaces them with a measurement. const TERMINAL_CELL_WIDTH: f32 = 8.4; const TERMINAL_CELL_HEIGHT: f32 = 18.0; +/// The point size the terminal is painted at. +const TERMINAL_FONT_SIZE: f32 = 13.0; +/// Named rather than "monospace" so the measurement and the painting agree on a +/// face. A generic family lets the text system resolve one font for measurement +/// and the renderer pick another. +const TERMINAL_FONT_FAMILY: &str = "monospace"; + +/// The measured size of one terminal cell. +/// +/// A terminal's whole geometry is columns × advance. Guessing the advance means +/// the last column is clipped or a gap is left, and it means the size handed to +/// the PTY describes a window that is not the one on screen. +#[derive(Clone, Copy, Debug)] +struct CellMetrics { + advance: f32, + line_height: f32, +} + +impl Default for CellMetrics { + fn default() -> Self { + Self { + advance: TERMINAL_CELL_WIDTH, + line_height: TERMINAL_CELL_HEIGHT, + } + } +} + +impl CellMetrics { + /// Asks the text system for the advance of `0` in the terminal face. + /// + /// `0` rather than `m`: in a monospace face every advance is the same, and a + /// digit is present in every font that could plausibly be resolved here. A + /// failure to measure falls back rather than propagating — a terminal with + /// slightly wrong columns is worth more than no terminal. + fn measure(cx: &App) -> Self { + let font = gpui::Font { + family: TERMINAL_FONT_FAMILY.into(), + features: gpui::FontFeatures::default(), + fallbacks: None, + weight: gpui::FontWeight::NORMAL, + style: gpui::FontStyle::Normal, + }; + let text_system = cx.text_system(); + let font_id = text_system.resolve_font(&font); + let size = px(TERMINAL_FONT_SIZE); + let advance = text_system + .ch_advance(font_id, size) + .map_or(TERMINAL_CELL_WIDTH, f32::from); + if advance <= 0.0 { + return Self::default(); + } + Self { + advance, + // Line height is a layout choice rather than a font metric: the + // painted row height is what a row occupies, and the renderer sets + // it explicitly. + line_height: TERMINAL_CELL_HEIGHT, + } + } +} const WORK_PANEL_GAP: f32 = 4.0; /// How close to the seam a press has to land to start a resize. const SIDEBAR_GRAB_RADIUS: f32 = 4.0; @@ -115,6 +181,7 @@ struct LumbridgeShell { sidebar: SidebarState, sidebar_has_focus: bool, sidebar_dragging: bool, + cell: CellMetrics, settings: Settings, /// The settings page on screen, if the pane is open. settings_page: Option, @@ -492,6 +559,7 @@ fn terminal_dimensions_for_window( window_size: Size, attached_panel_count: usize, sidebar_width: f32, + cell: CellMetrics, ) -> TerminalDimensions { let width = f32::from(window_size.width); let height = f32::from(window_size.height); @@ -507,17 +575,17 @@ fn terminal_dimensions_for_window( let panel_width = ((workspace_width - panel_gaps).max(0.0) / lossless_f32(panel_count)).max(0.0); let terminal_width = (panel_width - TERMINAL_HORIZONTAL_INSET).max(0.0); - let rows = (terminal_height / TERMINAL_CELL_HEIGHT) + let rows = (terminal_height / cell.line_height) .floor() .clamp(2.0, f32::from(u16::MAX)); - let columns = (terminal_width / TERMINAL_CELL_WIDTH) + let columns = (terminal_width / cell.advance) .floor() .clamp(20.0, f32::from(u16::MAX)); TerminalDimensions::with_cell_size( clamp_to_u16(rows), clamp_to_u16(columns), - clamp_to_u16(TERMINAL_CELL_WIDTH.round()), - clamp_to_u16(TERMINAL_CELL_HEIGHT.round()), + clamp_to_u16(cell.advance.round()), + clamp_to_u16(cell.line_height.round()), ) .expect("geometry clamps terminal dimensions above zero") } @@ -694,10 +762,12 @@ impl LumbridgeShell { let (store, panels, persistence_status) = load_panel_registry(); let sidebar = SidebarState::default(); + let cell = CellMetrics::measure(cx); let terminal_dimensions = terminal_dimensions_for_window( window.bounds().size, panels.attached_count(), sidebar.width(), + cell, ); let mut runtimes = RuntimeRegistry::new(); let mut live_terminals = BTreeMap::new(); @@ -720,6 +790,7 @@ impl LumbridgeShell { window.bounds().size, shell.panels.attached_count(), shell.sidebar.width(), + shell.cell, ); shell.resize_attached_terminals(dimensions.rows(), dimensions.columns(), cx); }) @@ -755,6 +826,7 @@ impl LumbridgeShell { sidebar, sidebar_has_focus: false, sidebar_dragging: false, + cell, // No file is read yet: this resolves the compiled defaults against // the real environment, so the pane already tells the truth about // which switches are pinned. Loading and watching the file is the @@ -950,8 +1022,17 @@ impl LumbridgeShell { if !terminal.terminal.resize(dimensions) { return false; } - let pty_size = TerminalSize::new(rows, columns) - .expect("terminal engine dimensions are valid PTY dimensions"); + // ws_xpixel and ws_ypixel were both zero until now, because the app + // only ever called TerminalSize::new. A program doing pixel arithmetic — + // sixel, the kitty graphics protocol, anything sizing an image to the + // viewport — was being told the window has no size at all. + let pty_size = TerminalSize::with_pixels( + rows, + columns, + clamp_to_u16(f32::from(columns) * self.cell.advance), + clamp_to_u16(f32::from(rows) * self.cell.line_height), + ) + .expect("terminal engine dimensions are valid PTY dimensions"); if let Err(error) = self.send_runtime_command(pane, RuntimeCommand::Resize(pty_size)) && let Some(terminal) = self.live_terminals.get_mut(&pane) { @@ -1049,6 +1130,7 @@ impl LumbridgeShell { window.bounds().size, self.panels.attached_count(), self.sidebar.width(), + self.cell, ); self.resize_attached_terminals(dimensions.rows(), dimensions.columns(), cx); } @@ -1071,6 +1153,7 @@ impl LumbridgeShell { window.bounds().size, self.panels.attached_count(), self.sidebar.width(), + self.cell, ); let mut terminal = LiveTerminalState::new(dimensions); if let Err(error) = spawn_live_runtime(&mut self.runtimes, pane, None, dimensions) { @@ -1120,6 +1203,7 @@ impl LumbridgeShell { window.bounds().size, self.panels.attached_count(), self.sidebar.width(), + self.cell, ); let mut terminal = LiveTerminalState::new(dimensions); if let Err(error) = spawn_live_runtime(&mut self.runtimes, pane, None, dimensions) { @@ -1619,12 +1703,16 @@ impl LumbridgeShell { cx.notify(); } - fn terminal_run(run: TerminalPaintRun, theme: ThemeColors) -> gpui::AnyElement { + fn terminal_run( + run: TerminalPaintRun, + theme: ThemeColors, + cell: CellMetrics, + ) -> gpui::AnyElement { let cursor = run.cursor; div() .flex_none() - .h(px(TERMINAL_CELL_HEIGHT)) - .w(px(f32::from(run.columns) * TERMINAL_CELL_WIDTH)) + .h(px(cell.line_height)) + .w(px(f32::from(run.columns) * cell.advance)) .overflow_hidden() .whitespace_nowrap() .text_color(run.foreground) @@ -1741,14 +1829,14 @@ impl LumbridgeShell { .into_any_element(); }; let scroll_pane = pane; + let cell = self.cell; let rows = terminal_paint_rows(&terminal.snapshot, theme) .into_iter() .map(|runs| { - div() - .flex() - .h(px(TERMINAL_CELL_HEIGHT)) - .flex_none() - .children(runs.into_iter().map(|run| Self::terminal_run(run, theme))) + div().flex().h(px(cell.line_height)).flex_none().children( + runs.into_iter() + .map(|run| Self::terminal_run(run, theme, cell)), + ) }) .collect::>(); div() @@ -1757,14 +1845,14 @@ impl LumbridgeShell { .size_full() .overflow_hidden() .bg(theme.chrome) - .font_family("monospace") - .text_size(px(13.0)) + .font_family(TERMINAL_FONT_FAMILY) + .text_size(px(TERMINAL_FONT_SIZE)) // Shift+PageUp was the only way into scrollback, which is not // something anyone guesses. A wheel is how people scroll. .on_scroll_wheel( cx.listener(move |shell, event: &gpui::ScrollWheelEvent, _, cx| { - let delta = event.delta.pixel_delta(px(TERMINAL_CELL_HEIGHT)); - let lines = (f32::from(delta.y) / TERMINAL_CELL_HEIGHT).clamp(-64.0, 64.0); + let delta = event.delta.pixel_delta(px(cell.line_height)); + let lines = (f32::from(delta.y) / cell.line_height).clamp(-64.0, 64.0); #[expect( clippy::cast_possible_truncation, reason = "clamped into -64..=64 on the line above" @@ -3614,8 +3702,15 @@ fn spawn_live_runtime( let command = CommandConfig::new("/bin/sh") .map_err(RuntimeActorError::Start)? .args(["-c", live_pty_script(seed)]); - let pty_size = TerminalSize::new(dimensions.rows(), dimensions.columns()) - .map_err(RuntimeActorError::Start)?; + // The engine already carries the measured cell size; pass the viewport + // through so a freshly spawned shell is not told the window is zero pixels. + let pty_size = TerminalSize::with_pixels( + dimensions.rows(), + dimensions.columns(), + dimensions.columns().saturating_mul(dimensions.cell_width()), + dimensions.rows().saturating_mul(dimensions.cell_height()), + ) + .map_err(RuntimeActorError::Start)?; runtimes.spawn( pane, command, @@ -3653,6 +3748,7 @@ mod tests { indexed_terminal_color, terminal_dimensions_for_window, terminal_key_from_parts, terminal_paint_rows, terminal_scroll_from_parts, visible_panel_count, visible_panel_range, }; + use super::CellMetrics; use crate::sidebar::model::DEFAULT_WIDTH; use gpui::{px, size}; @@ -3708,17 +3804,24 @@ mod tests { #[test] fn terminal_geometry_tracks_middle_sixty_percent_per_panel() { - let dimensions = terminal_dimensions_for_window(size(px(1500.0), px(960.0)), 5, 248.0); + let dimensions = terminal_dimensions_for_window( + size(px(1500.0), px(960.0)), + 5, + 248.0, + CellMetrics::default(), + ); assert_eq!(dimensions.rows(), 26); assert_eq!(dimensions.columns(), 45); let ultrawide = size(px(3440.0), px(1440.0)); assert_eq!(visible_panel_count(ultrawide, DEFAULT_WIDTH), 5); - let dimensions = terminal_dimensions_for_window(ultrawide, 5, DEFAULT_WIDTH); + let dimensions = + terminal_dimensions_for_window(ultrawide, 5, DEFAULT_WIDTH, CellMetrics::default()); assert_eq!(dimensions.rows(), 42); assert_eq!(dimensions.columns(), 71); - let two_panels = terminal_dimensions_for_window(ultrawide, 2, DEFAULT_WIDTH); + let two_panels = + terminal_dimensions_for_window(ultrawide, 2, DEFAULT_WIDTH, CellMetrics::default()); assert_eq!(two_panels.rows(), 42); assert_eq!(two_panels.columns(), 185); } diff --git a/crates/lumbridge-terminal/src/lib.rs b/crates/lumbridge-terminal/src/lib.rs index 120aa2f..2bfbfbc 100644 --- a/crates/lumbridge-terminal/src/lib.rs +++ b/crates/lumbridge-terminal/src/lib.rs @@ -66,6 +66,23 @@ impl TerminalDimensions { self.rows } + /// The measured width of one cell, in pixels. + /// + /// Carried so a caller can report the viewport's pixel extent to a PTY. + /// `ws_xpixel` is the whole window, not one cell, so it is this times the + /// column count — and a terminal that reports zero there tells every program + /// doing pixel arithmetic that the window has no size. + #[must_use] + pub const fn cell_width(self) -> u16 { + self.cell_width + } + + /// The height of one row, in pixels. + #[must_use] + pub const fn cell_height(self) -> u16 { + self.cell_height + } + #[must_use] pub const fn columns(self) -> u16 { self.columns