Measure the terminal cell instead of guessing it, and stop telling the PTY zero
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) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
e5d7a3efd5
commit
f9f4f85402
+126
-23
@@ -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<Page>,
|
||||
@@ -492,6 +559,7 @@ fn terminal_dimensions_for_window(
|
||||
window_size: Size<Pixels>,
|
||||
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::<Vec<_>>();
|
||||
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);
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user