diff --git a/frontends/rioterm/src/application.rs b/frontends/rioterm/src/application.rs index 1b3c9066..f63fc525 100644 --- a/frontends/rioterm/src/application.rs +++ b/frontends/rioterm/src/application.rs @@ -1194,8 +1194,11 @@ impl ApplicationHandler for Application<'_> { let layout = route.window.screen.sugarloaf.window_size(); - let x = x.clamp(0.0, (layout.width as i32 - 1).into()) as usize; - let y = y.clamp(0.0, (layout.height as i32 - 1).into()) as usize; + // Keep f64 precision all the way to the cell-grid + // divide. The old `as usize` cast here dropped + // subpixel info from HiDPI events. + let x = x.clamp(0.0, (layout.width as i32 - 1) as f64); + let y = y.clamp(0.0, (layout.height as i32 - 1) as f64); // Snapshot the old mouse position before updating coordinates // so we can detect whether the cursor moved to a new cell. @@ -1291,7 +1294,7 @@ impl ApplicationHandler for Application<'_> { // Check if mouse is over island and set cursor to default use crate::renderer::island::ISLAND_HEIGHT; let scale_factor = route.window.screen.sugarloaf.scale_factor(); - let island_height_px = (ISLAND_HEIGHT * scale_factor) as usize; + let island_height_px = (ISLAND_HEIGHT * scale_factor) as f64; if route.window.screen.renderer.navigation.is_enabled() && y <= island_height_px { diff --git a/frontends/rioterm/src/context/title.rs b/frontends/rioterm/src/context/title.rs index 030fe2f6..3c72e0a7 100644 --- a/frontends/rioterm/src/context/title.rs +++ b/frontends/rioterm/src/context/title.rs @@ -287,6 +287,13 @@ pub mod test { width: 18., height: 9., }, + rio_backend::sugarloaf::layout::CellMetrics { + cell_width: 18, + cell_height: 9, + cell_baseline: 0, + face_width: 18.0, + face_height: 9.0, + }, 1.0, Margin::default(), ); @@ -338,6 +345,13 @@ pub mod test { width: 18., height: 9., }, + rio_backend::sugarloaf::layout::CellMetrics { + cell_width: 18, + cell_height: 9, + cell_baseline: 0, + face_width: 18.0, + face_height: 9.0, + }, 1.0, Margin::default(), ); diff --git a/frontends/rioterm/src/layout/compute_tests.rs b/frontends/rioterm/src/layout/compute_tests.rs index 2c02b413..a36779d2 100644 --- a/frontends/rioterm/src/layout/compute_tests.rs +++ b/frontends/rioterm/src/layout/compute_tests.rs @@ -3,6 +3,19 @@ use super::*; // This file tests compute function on different layouts. // I've added some real scenarios so I can make sure it doesn't go off again. +/// Build a `CellMetrics` whose integer cell stride matches the given +/// `TextDimensions`. Used by tests that construct dimensions +/// directly without going through sugarloaf's font path. +fn cell_for(dims: TextDimensions) -> rio_backend::sugarloaf::layout::CellMetrics { + rio_backend::sugarloaf::layout::CellMetrics { + cell_width: dims.width.round().max(1.0) as u32, + cell_height: dims.height.round().max(1.0) as u32, + cell_baseline: 0, + face_width: dims.width as f64, + face_height: dims.height as f64, + } +} + /// note: Computes the renderer's actual per-line height in physical pixels. /// /// The renderer gets metrics from Metrics::for_rich_text() which packs @@ -54,10 +67,11 @@ fn assert_rows_fit( let (cols, rows) = compute( panel_width, panel_height, - dimensions, - line_height_mod, + cell_for(dimensions), Margin::all(0.0), + scale, ); + let _ = line_height_mod; // line_height already baked into `dimensions.height`. let actual_line_height = renderer_line_height(ascent, descent, leading, line_height_mod, scale); @@ -167,7 +181,7 @@ fn test_compute_returns_min_for_zero_dimensions() { height: 32.0, scale: 2.0, }; - let (cols, rows) = compute(0.0, 0.0, dims, 1.0, Margin::all(0.0)); + let (cols, rows) = compute(0.0, 0.0, cell_for(dims), Margin::all(0.0), 2.0); assert_eq!(cols, MIN_COLS); assert_eq!(rows, MIN_LINES); } @@ -179,7 +193,7 @@ fn test_compute_returns_min_for_negative_dimensions() { height: 32.0, scale: 2.0, }; - let (cols, rows) = compute(-100.0, -100.0, dims, 1.0, Margin::all(0.0)); + let (cols, rows) = compute(-100.0, -100.0, cell_for(dims), Margin::all(0.0), 2.0); assert_eq!(cols, MIN_COLS); assert_eq!(rows, MIN_LINES); } @@ -191,7 +205,7 @@ fn test_compute_returns_min_for_zero_scale() { height: 32.0, scale: 0.0, }; - let (cols, rows) = compute(1600.0, 900.0, dims, 1.0, Margin::all(0.0)); + let (cols, rows) = compute(1600.0, 900.0, cell_for(dims), Margin::all(0.0), 0.0); assert_eq!(cols, MIN_COLS); assert_eq!(rows, MIN_LINES); } @@ -203,7 +217,7 @@ fn test_compute_basic_grid() { height: 33.0, scale: 2.0, }; - let (cols, rows) = compute(1600.0, 825.0, dims, 1.0, Margin::all(0.0)); + let (cols, rows) = compute(1600.0, 825.0, cell_for(dims), Margin::all(0.0), 2.0); assert_eq!(cols, 100); assert_eq!(rows, 25); } @@ -216,7 +230,7 @@ fn test_compute_floors_fractional_rows() { height: 33.0, scale: 1.0, }; - let (_, rows) = compute(1600.0, 840.0, dims, 1.0, Margin::all(0.0)); + let (_, rows) = compute(1600.0, 840.0, cell_for(dims), Margin::all(0.0), 1.0); assert_eq!(rows, 25); } @@ -228,7 +242,7 @@ fn test_compute_respects_margins() { scale: 2.0, }; let margin = Margin::new(0.0, 10.0, 0.0, 10.0); - let (cols, _) = compute(1600.0, 800.0, dims, 1.0, margin); + let (cols, _) = compute(1600.0, 800.0, cell_for(dims), margin, 2.0); // available = 1600 - 10*2 - 10*2 = 1560, cols = 1560/16 = 97 assert_eq!(cols, 97); } @@ -241,7 +255,7 @@ fn test_compute_margin_exceeds_size() { scale: 2.0, }; let margin = Margin::new(0.0, 0.0, 0.0, 1000.0); - let (cols, rows) = compute(100.0, 800.0, dims, 1.0, margin); + let (cols, rows) = compute(100.0, 800.0, cell_for(dims), margin, 2.0); assert_eq!(cols, MIN_COLS); assert_eq!(rows, MIN_LINES); } @@ -253,7 +267,7 @@ fn test_context_dimension_build() { height: 33.0, scale: 2.0, }; - let cd = ContextDimension::build(1650.0, 825.0, dims, 1.0, Margin::all(0.0)); + let cd = ContextDimension::build(1650.0, 825.0, dims, cell_for(dims), 1.0, Margin::all(0.0)); assert_eq!(cd.columns, 103); assert_eq!(cd.lines, 25); } @@ -265,7 +279,7 @@ fn test_context_dimension_update_width() { height: 33.0, scale: 2.0, }; - let mut cd = ContextDimension::build(1600.0, 825.0, dims, 1.0, Margin::all(0.0)); + let mut cd = ContextDimension::build(1600.0, 825.0, dims, cell_for(dims), 1.0, Margin::all(0.0)); assert_eq!(cd.columns, 100); cd.update_width(800.0); @@ -280,7 +294,7 @@ fn test_context_dimension_update_height() { height: 33.0, scale: 2.0, }; - let mut cd = ContextDimension::build(1600.0, 825.0, dims, 1.0, Margin::all(0.0)); + let mut cd = ContextDimension::build(1600.0, 825.0, dims, cell_for(dims), 1.0, Margin::all(0.0)); assert_eq!(cd.lines, 25); cd.update_height(660.0); @@ -295,7 +309,7 @@ fn test_context_dimension_update_dimensions() { height: 33.0, scale: 1.0, }; - let mut cd = ContextDimension::build(1600.0, 825.0, dims, 1.0, Margin::all(0.0)); + let mut cd = ContextDimension::build(1600.0, 825.0, dims, cell_for(dims), 1.0, Margin::all(0.0)); assert_eq!(cd.lines, 25); let new_dims = TextDimensions { @@ -303,7 +317,7 @@ fn test_context_dimension_update_dimensions() { height: 66.0, scale: 1.0, }; - cd.update_dimensions(new_dims); + cd.update_dimensions(new_dims, cell_for(new_dims)); assert_eq!(cd.lines, 12); // 825/66 = 12.5 → 12 } diff --git a/frontends/rioterm/src/layout/mod.rs b/frontends/rioterm/src/layout/mod.rs index 66ab0bbf..6b637ce2 100644 --- a/frontends/rioterm/src/layout/mod.rs +++ b/frontends/rioterm/src/layout/mod.rs @@ -47,17 +47,21 @@ pub struct ResizeState { fn compute( width: f32, height: f32, - dimensions: TextDimensions, - line_height: f32, + cell: rio_backend::sugarloaf::layout::CellMetrics, margin: Margin, + scale: f32, ) -> (usize, usize) { // Ensure we have positive dimensions - if width <= 0.0 || height <= 0.0 || dimensions.scale <= 0.0 || line_height <= 0.0 { + if width <= 0.0 + || height <= 0.0 + || scale <= 0.0 + || cell.cell_width == 0 + || cell.cell_height == 0 + { return (MIN_COLS, MIN_LINES); } // Calculate available space accounting for margins (scale margins to physical pixels) - let scale = dimensions.scale; let available_width = width - (margin.left * scale) - (margin.right * scale); let available_height = height - (margin.top * scale) - (margin.bottom * scale); @@ -66,24 +70,18 @@ fn compute( return (MIN_COLS, MIN_LINES); } - // Calculate columns - divide by the ROUNDED cell width. - // rounds `face_width` once in `font/Metrics.zig:265` (`cell_width = - // @round(face_width)`) and uses that integer everywhere — cols, - // grid shader, cursor hit-testing. Rio's grid renderer already - // does `.round()` on `cell_w` when building `GridUniforms`, so the - // column count has to use the same integer or the right edge of - // the grid floats `cols * (face_width - cell_width)` pixels short - // of the panel. Matches ; sacrifices at most 1 col vs - // fractional divide but keeps the render perfectly aligned. - let cell_width = dimensions.width.round().max(1.0); - let visible_columns = - std::cmp::max((available_width / cell_width) as usize, MIN_COLS); - - // Same treatment for rows: grid renders at `.round()`ed cell - // height, so cols-and-rows share the same integer snap. - let cell_height = dimensions.height.round().max(1.0); - let lines = (available_height / cell_height).floor(); - let visible_lines = std::cmp::max(lines as usize, MIN_LINES); + // Cols/rows divide by the canonical integer cell stride + // (`Metrics.cell_width / cell_height`). Same value the grid + // shader uses for `cell_size` and the mouse-hit-test divides by; + // single source of truth so the right/bottom edges of the grid + // stay aligned with the painted cells. + let cell_w = cell.cell_width as f32; + let cell_h = cell.cell_height as f32; + let visible_columns = std::cmp::max((available_width / cell_w) as usize, MIN_COLS); + let visible_lines = std::cmp::max( + (available_height / cell_h).floor() as usize, + MIN_LINES, + ); (visible_columns, visible_lines) } @@ -1169,6 +1167,7 @@ impl ContextGrid { self.width, self.height, current_context_dimension.dimension, + current_context_dimension.cell, current_context_dimension.line_height, unscaled_margin, ) @@ -1191,7 +1190,10 @@ impl ContextGrid { pub fn update_dimensions(&mut self, sugarloaf: &mut Sugarloaf) { for context in self.inner.values_mut() { if let Some(layout) = sugarloaf.get_text_layout(&context.val.rich_text_id) { - context.val.dimension.update_dimensions(layout.dimensions); + context + .val + .dimension + .update_dimensions(layout.dimensions, layout.cell); } } @@ -1568,6 +1570,12 @@ pub struct ContextDimension { pub dimension: TextDimensions, pub margin: Margin, pub line_height: f32, + /// Canonical cell metrics — single source of truth shared by the + /// GPU grid uniform, col/row count math, and mouse hit testing. + /// Rounded `u32` cell width / height / baseline plus unrounded + /// `f64` face dimensions for downstream subpixel math. Avoids + /// drift between painted cell stride and click→cell mapping. + pub cell: rio_backend::sugarloaf::layout::CellMetrics, } impl Default for ContextDimension { @@ -1580,6 +1588,7 @@ impl Default for ContextDimension { line_height: 1., dimension: TextDimensions::default(), margin: Margin::default(), + cell: rio_backend::sugarloaf::layout::CellMetrics::default(), } } } @@ -1589,10 +1598,11 @@ impl ContextDimension { width: f32, height: f32, dimension: TextDimensions, + cell: rio_backend::sugarloaf::layout::CellMetrics, line_height: f32, margin: Margin, ) -> Self { - let (columns, lines) = compute(width, height, dimension, line_height, margin); + let (columns, lines) = compute(width, height, cell, margin, dimension.scale); Self { width, height, @@ -1601,6 +1611,7 @@ impl ContextDimension { dimension, margin, line_height, + cell, } } @@ -1623,8 +1634,13 @@ impl ContextDimension { } #[inline] - pub fn update_dimensions(&mut self, dimensions: TextDimensions) { + pub fn update_dimensions( + &mut self, + dimensions: TextDimensions, + cell: rio_backend::sugarloaf::layout::CellMetrics, + ) { self.dimension = dimensions; + self.cell = cell; self.update(); } @@ -1633,9 +1649,9 @@ impl ContextDimension { let (columns, lines) = compute( self.width, self.height, - self.dimension, - self.line_height, + self.cell, self.margin, + self.dimension.scale, ); self.columns = columns; diff --git a/frontends/rioterm/src/mouse/mod.rs b/frontends/rioterm/src/mouse/mod.rs index efc55eaa..6459c55e 100644 --- a/frontends/rioterm/src/mouse/mod.rs +++ b/frontends/rioterm/src/mouse/mod.rs @@ -29,8 +29,11 @@ pub struct Mouse { pub accumulated_scroll: AccumulatedScroll, pub square_side: Side, pub inside_text_area: bool, - pub x: usize, - pub y: usize, + /// Cursor X in physical pixels. `f64` so subpixel precision from + /// the OS event survives all the way to the cell-grid divide. + pub x: f64, + /// Cursor Y in physical pixels. + pub y: f64, pub on_border: bool, /// Raw (unclamped) cursor Y in physical pixels, for selection scroll. pub raw_y: f64, @@ -51,8 +54,8 @@ impl Default for Mouse { inside_text_area: Default::default(), on_border: false, accumulated_scroll: AccumulatedScroll::default(), - x: Default::default(), - y: Default::default(), + x: 0.0, + y: 0.0, raw_y: 0.0, } } @@ -74,6 +77,15 @@ impl Mouse { } } +/// Map a physical-pixel cursor position to a terminal grid `Pos`. +/// +/// Pixel coords stay `f64` until the final integer truncation, and +/// the divide uses the canonical `u32` `cell_width / cell_height` +/// (the same integers the GPU shader paints with — no drift between +/// painted cell stride and click→cell mapping). +/// +/// `margin_x_left / margin_y_top` are already pre-scaled (physical +/// pixels), do not multiply by `scale_factor` here. #[inline] pub fn calculate_mouse_position( mouse: &Mouse, @@ -81,59 +93,63 @@ pub fn calculate_mouse_position( columns_rows: (usize, usize), margin_x_left: f32, margin_y_top: f32, - cell_dimension: (f32, f32), + cell: (u32, u32), ) -> Pos { - // In case sugarloaf hasn't obtained the dimensions - if cell_dimension.0 == 0.0 || cell_dimension.1 == 0.0 { + let (cell_w, cell_h) = (cell.0 as f64, cell.1 as f64); + if cell_w == 0.0 || cell_h == 0.0 { return Pos::default(); } - let cell_width = cell_dimension.0; - let cell_height = cell_dimension.1; - // Margins are already pre-scaled (multiplied by scale_factor in - // update_scaled_margin), so use them directly — do not scale again. - let margin_x = margin_x_left as usize; - let margin_y = margin_y_top as usize; + let margin_x = margin_x_left as f64; + let margin_y = margin_y_top as f64; - let col = if (margin_x + cell_width as usize) > mouse.x { - Column(0) - } else { - let col = ((mouse.x - margin_x) as f32 / cell_width) as usize; - std::cmp::min(Column(col), Column(columns_rows.0 - 1)) - }; + // f64 throughout. Negative-clamp via `.max(0.0)` so clicks in + // the margin map to col/row 0 rather than wrapping or + // overflowing on the cast. + let x_in_grid = (mouse.x - margin_x).max(0.0); + let y_in_grid = (mouse.y - margin_y).max(0.0); + let col_idx = (x_in_grid / cell_w) as usize; + let row_idx = (y_in_grid / cell_h) as usize; - let row = mouse.y.saturating_sub(margin_y) as f32 / cell_height; - let calc_row = std::cmp::min(row as usize, columns_rows.1 - 1); - let row = Line(calc_row as i32) - (display_offset); + let col = std::cmp::min(Column(col_idx), Column(columns_rows.0 - 1)); + let row = std::cmp::min(row_idx, columns_rows.1 - 1); + let row = Line(row as i32) - display_offset; Pos::new(row, col) } /// Determine which side of a cell the mouse x-position falls on. /// -/// `margin_x` is already pre-scaled (physical pixels). -/// `cell_width` is the float cell width. -/// `grid_width` is the total width of the grid area (physical pixels). +/// `margin_x` is pre-scaled (physical pixels). `cell_width` is the +/// canonical `u32` cell width (same value the renderer paints with). +/// `grid_width` is the total width of the grid area in physical +/// pixels. /// -/// Uses a 60% threshold (matching ghostty) rather than the 50% midpoint rio -/// inherited from alacritty. Clicks land on a cell until the cursor is past -/// 60% across it, and a drag must cross 60% of the next cell before it's -/// included — this reduces accidental half-cell snapping at the midpoint. +/// 60% threshold rather than the 50% midpoint inherited from +/// alacritty: clicks land on a cell until the cursor is past 60% +/// across it, and a drag must cross 60% of the next cell before +/// it's included — reduces accidental half-cell snapping at the +/// midpoint. #[inline] pub fn calculate_side_by_pos( - x: usize, + x: f64, margin_x: f32, - cell_width: f32, + cell_width: u32, grid_width: f32, ) -> Side { - let x_in_grid = (x as f32 - margin_x).max(0.0); - let cell_x = x_in_grid % cell_width; - let threshold = cell_width * 0.6; + let cell_w = cell_width as f64; + let margin = margin_x as f64; + let grid_w = grid_width as f64; - let additional_padding = (grid_width - margin_x) % cell_width; - let end_of_grid = grid_width - margin_x - additional_padding; + let x_in_grid = (x - margin).max(0.0); + let cell_x = x_in_grid % cell_w; + let threshold = cell_w * 0.6; - if cell_x >= threshold || x as f32 >= end_of_grid { + let usable = (grid_w - margin).max(0.0); + let additional_padding = usable % cell_w; + let end_of_grid = margin + usable - additional_padding; + + if cell_x >= threshold || x >= end_of_grid { Side::Right } else { Side::Left @@ -144,633 +160,278 @@ pub fn calculate_side_by_pos( pub mod test { use super::*; - /// Cell boundaries with width=9.4 and no margin: 0, 9.4, 18.8, 28.2, ... - #[test] - fn test_pos_calc_moving_mouse_x_with_scale_1() { - let display_offset = 0; - - let columns = 10; - let lines = 5; - let margin_x_left = 0.0; - let margin_y_top = 0.0; - let cell_dimension_width = 9.4; - let cell_dimension_height = 18.0; - - // x=8 → 8/9.4 = 0.85 → col 0 - let mouse = Mouse { - x: 8, - ..Default::default() - }; - let pos = calculate_mouse_position( - &mouse, - display_offset, - (columns, lines), - margin_x_left, - margin_y_top, - (cell_dimension_width, cell_dimension_height), - ); - assert_eq!(pos, Pos::new(Line(0), Column(0))); - - // x=9 → 9/9.4 = 0.96 → col 0 (still within first cell) - let mouse = Mouse { - x: 9, - ..Default::default() - }; - let pos = calculate_mouse_position( - &mouse, - display_offset, - (columns, lines), - margin_x_left, - margin_y_top, - (cell_dimension_width, cell_dimension_height), - ); - assert_eq!(pos, Pos::new(Line(0), Column(0))); - - // x=10 → 10/9.4 = 1.06 → col 1 - let mouse = Mouse { - x: 10, - ..Default::default() - }; - let pos = calculate_mouse_position( - &mouse, - display_offset, - (columns, lines), - margin_x_left, - margin_y_top, - (cell_dimension_width, cell_dimension_height), - ); - assert_eq!(pos, Pos::new(Line(0), Column(1))); - - // x=17 → 17/9.4 = 1.81 → col 1 - let mouse = Mouse { - x: 17, - ..Default::default() - }; - let pos = calculate_mouse_position( - &mouse, - display_offset, - (columns, lines), - margin_x_left, - margin_y_top, - (cell_dimension_width, cell_dimension_height), - ); - assert_eq!(pos, Pos::new(Line(0), Column(1))); - - // x=19 → 19/9.4 = 2.02 → col 2 - let mouse = Mouse { - x: 19, + fn mk_mouse(x: f64, y: f64) -> Mouse { + Mouse { + x, + y, ..Default::default() - }; - let pos = calculate_mouse_position( - &mouse, - display_offset, - (columns, lines), - margin_x_left, - margin_y_top, - (cell_dimension_width, cell_dimension_height), - ); - assert_eq!(pos, Pos::new(Line(0), Column(2))); + } } - /// Same as scale_1 — scale_factor doesn't affect column math when margins - /// are already pre-scaled (and here margin is 0). + /// Canonical stride: cell width = 9 (u32). Boundaries at 0, 9, 18, 27. #[test] - fn test_pos_calc_moving_mouse_x_with_scale_2() { - let display_offset = 0; - - let columns = 10; + fn pos_calc_basic_no_margin() { + let cols = 10; let lines = 5; - let margin_x_left = 0.0; // already scaled - let margin_y_top = 0.0; - let cell_dimension_width = 9.4; - let cell_dimension_height = 18.0; + let cell = (9u32, 18u32); - let mouse = Mouse { - x: 8, - ..Default::default() - }; - let pos = calculate_mouse_position( - &mouse, - display_offset, - (columns, lines), - margin_x_left, - margin_y_top, - (cell_dimension_width, cell_dimension_height), + // x=8 → 8/9 = 0 → col 0 + assert_eq!( + calculate_mouse_position(&mk_mouse(8.0, 0.0), 0, (cols, lines), 0.0, 0.0, cell) + .col, + Column(0), ); - assert_eq!(pos, Pos::new(Line(0), Column(0))); - - // x=9 → 9/9.4 = 0.96 → col 0 - let mouse = Mouse { - x: 9, - ..Default::default() - }; - let pos = calculate_mouse_position( - &mouse, - display_offset, - (columns, lines), - margin_x_left, - margin_y_top, - (cell_dimension_width, cell_dimension_height), + // x=8.99 → still col 0 (subpixel precision preserved) + assert_eq!( + calculate_mouse_position(&mk_mouse(8.99, 0.0), 0, (cols, lines), 0.0, 0.0, cell) + .col, + Column(0), ); - assert_eq!(pos, Pos::new(Line(0), Column(0))); - - // x=10 → 10/9.4 = 1.06 → col 1 - let mouse = Mouse { - x: 10, - ..Default::default() - }; - let pos = calculate_mouse_position( - &mouse, - display_offset, - (columns, lines), - margin_x_left, - margin_y_top, - (cell_dimension_width, cell_dimension_height), + // x=9 → boundary → col 1 + assert_eq!( + calculate_mouse_position(&mk_mouse(9.0, 0.0), 0, (cols, lines), 0.0, 0.0, cell) + .col, + Column(1), ); - assert_eq!(pos, Pos::new(Line(0), Column(1))); - - let mouse = Mouse { - x: 17, - ..Default::default() - }; - let pos = calculate_mouse_position( - &mouse, - display_offset, - (columns, lines), - margin_x_left, - margin_y_top, - (cell_dimension_width, cell_dimension_height), + // x=17.5 → 17.5/9 = 1.94 → col 1 + assert_eq!( + calculate_mouse_position(&mk_mouse(17.5, 0.0), 0, (cols, lines), 0.0, 0.0, cell) + .col, + Column(1), ); - assert_eq!(pos, Pos::new(Line(0), Column(1))); - - let mouse = Mouse { - x: 19, - ..Default::default() - }; - let pos = calculate_mouse_position( - &mouse, - display_offset, - (columns, lines), - margin_x_left, - margin_y_top, - (cell_dimension_width, cell_dimension_height), + // x=18 → col 2 + assert_eq!( + calculate_mouse_position(&mk_mouse(18.0, 0.0), 0, (cols, lines), 0.0, 0.0, cell) + .col, + Column(2), ); - assert_eq!(pos, Pos::new(Line(0), Column(2))); } - /// With pre-scaled margin=10, cell boundaries at: 10, 19.4, 28.8, 38.2, ... + /// Pre-scaled margin: clicks before the margin clamp to col 0. + /// Boundaries at 10, 19, 28, 37, ... (cell width 9 with margin 10). #[test] - fn test_pos_calc_moving_mouse_x_with_scale_1_with_margin_10() { - let display_offset = 0; - - let columns = 10; + fn pos_calc_with_prescaled_margin() { + let cols = 10; let lines = 5; - let margin_x_left = 10.0; // already scaled (10 * 1.0) - let margin_y_top = 0.0; - let cell_dimension_width = 9.4; - let cell_dimension_height = 18.0; - - // x=8 → before margin → col 0 - let mouse = Mouse { - x: 8, - ..Default::default() - }; - let pos = calculate_mouse_position( - &mouse, - display_offset, - (columns, lines), - margin_x_left, - margin_y_top, - (cell_dimension_width, cell_dimension_height), - ); - assert_eq!(pos, Pos::new(Line(0), Column(0))); + let cell = (9u32, 18u32); + let margin_x = 10.0_f32; - // x=9 → before margin → col 0 - let mouse = Mouse { - x: 9, - ..Default::default() - }; - let pos = calculate_mouse_position( - &mouse, - display_offset, - (columns, lines), - margin_x_left, - margin_y_top, - (cell_dimension_width, cell_dimension_height), + // Click before margin → col 0. + assert_eq!( + calculate_mouse_position(&mk_mouse(5.0, 0.0), 0, (cols, lines), margin_x, 0.0, cell) + .col, + Column(0), ); - assert_eq!(pos, Pos::new(Line(0), Column(0))); - - // x=18 → (18-10)/9.4 = 0.85 → col 0 - let mouse = Mouse { - x: 18, - ..Default::default() - }; - let pos = calculate_mouse_position( - &mouse, - display_offset, - (columns, lines), - margin_x_left, - margin_y_top, - (cell_dimension_width, cell_dimension_height), + // x=10 → col 0 (start of cell 0). + assert_eq!( + calculate_mouse_position(&mk_mouse(10.0, 0.0), 0, (cols, lines), margin_x, 0.0, cell) + .col, + Column(0), ); - assert_eq!(pos, Pos::new(Line(0), Column(0))); - - // x=20 → (20-10)/9.4 = 1.06 → col 1 - let mouse = Mouse { - x: 20, - ..Default::default() - }; - let pos = calculate_mouse_position( - &mouse, - display_offset, - (columns, lines), - margin_x_left, - margin_y_top, - (cell_dimension_width, cell_dimension_height), + // x=18.99 → still col 0. + assert_eq!( + calculate_mouse_position(&mk_mouse(18.99, 0.0), 0, (cols, lines), margin_x, 0.0, cell) + .col, + Column(0), ); - assert_eq!(pos, Pos::new(Line(0), Column(1))); - - // x=28 → (28-10)/9.4 = 1.91 → col 1 - let mouse = Mouse { - x: 28, - ..Default::default() - }; - let pos = calculate_mouse_position( - &mouse, - display_offset, - (columns, lines), - margin_x_left, - margin_y_top, - (cell_dimension_width, cell_dimension_height), + // x=19 → col 1. + assert_eq!( + calculate_mouse_position(&mk_mouse(19.0, 0.0), 0, (cols, lines), margin_x, 0.0, cell) + .col, + Column(1), ); - assert_eq!(pos, Pos::new(Line(0), Column(1))); + } - // x=29 → (29-10)/9.4 = 2.02 → col 2 - let mouse = Mouse { - x: 29, - ..Default::default() - }; - let pos = calculate_mouse_position( - &mouse, - display_offset, - (columns, lines), - margin_x_left, - margin_y_top, - (cell_dimension_width, cell_dimension_height), - ); - assert_eq!(pos, Pos::new(Line(0), Column(2))); + /// Regression for the actual mouse-positioning bug: with the + /// renderer painting at canonical `u32` stride `cell.cell_width`, + /// a click on the visual middle of a high-index column must map + /// to that same column index. The old code passed unrounded + /// `f32` cell_width to the divide, causing accumulating drift + /// (≈0.41px per col → wrong by 3 columns at col 100 with width + /// 16.41). + #[test] + fn pos_calc_no_drift_at_high_column_index() { + let cols = 200; + let lines = 50; + // u32 stride — the same value the GPU shader paints with. + let cell = (16u32, 33u32); - // x=38 → (38-10)/9.4 = 2.98 → col 2 - let mouse = Mouse { - x: 38, - ..Default::default() - }; + // Painted col 100 occupies pixels [1600, 1616). Click in the middle. let pos = calculate_mouse_position( - &mouse, - display_offset, - (columns, lines), - margin_x_left, - margin_y_top, - (cell_dimension_width, cell_dimension_height), + &mk_mouse(1608.0, 100.0), + 0, + (cols, lines), + 0.0, + 0.0, + cell, ); - assert_eq!(pos, Pos::new(Line(0), Column(2))); + assert_eq!(pos.col, Column(100)); - // x=39 → (39-10)/9.4 = 3.08 → col 3 - let mouse = Mouse { - x: 39, - ..Default::default() - }; + // Painted col 150 at pixels [2400, 2416). Click left edge. let pos = calculate_mouse_position( - &mouse, - display_offset, - (columns, lines), - margin_x_left, - margin_y_top, - (cell_dimension_width, cell_dimension_height), - ); - assert_eq!(pos, Pos::new(Line(0), Column(3))); + &mk_mouse(2400.0, 100.0), + 0, + (cols, lines), + 0.0, + 0.0, + cell, + ); + assert_eq!(pos.col, Column(150)); } - /// Margin=20 is already pre-scaled (e.g. config 10.0 * scale 2.0). - /// Cell boundaries at: 20, 29.4, 38.8, ... + /// Subpixel mouse precision survives the divide. With cell=16 + /// and HiDPI delivering `1608.5` from the OS, the f64 path gives + /// col 100; if we'd cast `mouse.x` to `usize` early (the old + /// behavior) we'd see `1608 / 16 = 100` too, but at the boundary + /// (e.g. `1599.9 → col 99` not `col 100`) precision matters. #[test] - fn test_pos_calc_moving_mouse_x_with_scale_2_with_margin_10() { - let display_offset = 0; - - let columns = 10; - let lines = 5; - let margin_x_left = 20.0; // already scaled (10 * 2.0) - let margin_y_top = 0.0; - let cell_dimension_width = 9.4; - let cell_dimension_height = 18.0; - - // x=9 → before margin → col 0 - let mouse = Mouse { - x: 9, - ..Default::default() - }; - let pos = calculate_mouse_position( - &mouse, - display_offset, - (columns, lines), - margin_x_left, - margin_y_top, - (cell_dimension_width, cell_dimension_height), - ); - assert_eq!(pos, Pos::new(Line(0), Column(0))); + fn pos_calc_subpixel_precision_preserved() { + let cell = (16u32, 33u32); + let cols = 200; + let lines = 50; - // x=28 → (28-20)/9.4 = 0.85 → col 0 - let mouse = Mouse { - x: 28, - ..Default::default() - }; + // 1599.9 should map to col 99 (cell 99 at [1584,1600)). let pos = calculate_mouse_position( - &mouse, - display_offset, - (columns, lines), - margin_x_left, - margin_y_top, - (cell_dimension_width, cell_dimension_height), + &mk_mouse(1599.9, 100.0), + 0, + (cols, lines), + 0.0, + 0.0, + cell, ); - assert_eq!(pos, Pos::new(Line(0), Column(0))); + assert_eq!(pos.col, Column(99)); - // x=30 → (30-20)/9.4 = 1.06 → col 1 - let mouse = Mouse { - x: 30, - ..Default::default() - }; + // 1600.1 should map to col 100. let pos = calculate_mouse_position( - &mouse, - display_offset, - (columns, lines), - margin_x_left, - margin_y_top, - (cell_dimension_width, cell_dimension_height), - ); - assert_eq!(pos, Pos::new(Line(0), Column(1))); + &mk_mouse(1600.1, 100.0), + 0, + (cols, lines), + 0.0, + 0.0, + cell, + ); + assert_eq!(pos.col, Column(100)); } - /// Regression: margins passed to calculate_mouse_position are already - /// pre-scaled (multiplied by scale_factor in update_scaled_margin), but the - /// function multiplied them by scale_factor again. With a 2× display and - /// margin_y_top=72 (already 36*2), the double-scaling produces 144 instead - /// of 72, shifting the row calculation by ~2 rows. - /// - /// Uses exact values observed on a Retina display: - /// cell 16.41×33, margin (4, 72) pre-scaled, scale 2.0, 96×27 grid. + /// Y axis: pre-scaled margin must not be re-scaled. Row stride + /// is the canonical `cell.1` (already includes line_height). #[test] - fn test_row_not_double_scaled() { - let display_offset = 0; - - let columns = 96; + fn pos_calc_row_with_prescaled_margin() { + let cols = 96; let lines = 27; - // These margins are ALREADY scaled (config margin * scale_factor). - let margin_x_left = 4.0; // e.g. config 2.0 * scale 2.0 - let margin_y_top = 72.0; // e.g. config 36.0 * scale 2.0 - let cell_w = 16.41; - let cell_h = 33.0; - - // Row 0 starts at y = margin_y_top = 72. - // Row 6 spans y = [72 + 6*33, 72 + 7*33) = [270, 303). - let mouse = Mouse { - x: 100, - y: 280, - ..Default::default() - }; + let margin_y = 72.0_f32; + let cell = (16u32, 33u32); + + // Row 0 begins at y = 72. Row 6 spans [72+198, 72+231) = [270, 303). let pos = calculate_mouse_position( - &mouse, - display_offset, - (columns, lines), - margin_x_left, - margin_y_top, - (cell_w, cell_h), + &mk_mouse(100.0, 280.0), + 0, + (cols, lines), + 0.0, + margin_y, + cell, ); - // (280 - 72) / 33 = 6.3 → row 6 - // Bug: (280 - 144) / 33 = 4.1 → row 4 (margin double-scaled) assert_eq!(pos.row, Line(6)); - // Row 22 spans y = [72 + 22*33, 72 + 23*33) = [798, 831). - let mouse = Mouse { - x: 100, - y: 820, - ..Default::default() - }; + // Row 22 spans [72 + 22*33, 72 + 23*33) = [798, 831). let pos = calculate_mouse_position( - &mouse, - display_offset, - (columns, lines), - margin_x_left, - margin_y_top, - (cell_w, cell_h), + &mk_mouse(100.0, 820.0), + 0, + (cols, lines), + 0.0, + margin_y, + cell, ); - // (820 - 72) / 33 = 22.7 → row 22 - // Bug: (820 - 144) / 33 = 20.5 → row 20 assert_eq!(pos.row, Line(22)); } - /// Same double-scaling issue on the X axis, but less visible with small - /// margins. With margin_x_left=20 (pre-scaled) and scale=2.0 the error - /// is 20 extra pixels — enough to shift a column. + /// Display offset shifts the reported row by the scrollback + /// position so callers get a viewport-relative `Line` index. #[test] - fn test_col_not_double_scaled() { - let display_offset = 0; - - let columns = 96; + fn pos_calc_display_offset_shifts_row() { + let cell = (16u32, 33u32); + let cols = 96; let lines = 27; - let margin_x_left = 20.0; // already scaled - let margin_y_top = 72.0; - let cell_w = 16.41; - let cell_h = 33.0; - - // Col 3 starts at x = 20 + 3*16.41 = 69.23. - let mouse = Mouse { - x: 70, - y: 200, - ..Default::default() - }; - let pos = calculate_mouse_position( - &mouse, - display_offset, - (columns, lines), - margin_x_left, - margin_y_top, - (cell_w, cell_h), - ); - // (70 - 20) / 16.41 = 3.05 → col 3 - // Bug: (70 - 40) / 16 = 1.87 → col 1 (margin double-scaled + int truncation) - assert_eq!(pos.col, Column(3)); - } - - /// Cell width=12.6, margin=10 (pre-scaled). Boundaries: 10, 22.6, 35.2, 47.8, ... - #[test] - fn test_pos_calc_font_size_12_6_moving_mouse_x_with_scale_1_with_margin_10() { - let display_offset = 0; - let columns = 10; - let lines = 5; - let margin_x_left = 10.0; - let margin_y_top = 0.0; - let cell_dimension_width = 12.6; - let cell_dimension_height = 18.0; - - // x=10 → (10-10)/12.6 = 0 → col 0 - let mouse = Mouse { - x: 10, - ..Default::default() - }; + // Row index 5 from top, display_offset 10 → Line(5) - 10 = Line(-5). let pos = calculate_mouse_position( - &mouse, - display_offset, - (columns, lines), - margin_x_left, - margin_y_top, - (cell_dimension_width, cell_dimension_height), - ); - assert_eq!(pos, Pos::new(Line(0), Column(0))); - - // x=22 → (22-10)/12.6 = 0.95 → col 0 - let mouse = Mouse { - x: 22, - ..Default::default() - }; - let pos = calculate_mouse_position( - &mouse, - display_offset, - (columns, lines), - margin_x_left, - margin_y_top, - (cell_dimension_width, cell_dimension_height), - ); - assert_eq!(pos, Pos::new(Line(0), Column(0))); - - // x=23 → (23-10)/12.6 = 1.03 → col 1 - let mouse = Mouse { - x: 23, - ..Default::default() - }; - let pos = calculate_mouse_position( - &mouse, - display_offset, - (columns, lines), - margin_x_left, - margin_y_top, - (cell_dimension_width, cell_dimension_height), - ); - assert_eq!(pos, Pos::new(Line(0), Column(1))); - - // x=35 → (35-10)/12.6 = 1.98 → col 1 - let mouse = Mouse { - x: 35, - ..Default::default() - }; - let pos = calculate_mouse_position( - &mouse, - display_offset, - (columns, lines), - margin_x_left, - margin_y_top, - (cell_dimension_width, cell_dimension_height), - ); - assert_eq!(pos, Pos::new(Line(0), Column(1))); - - // x=36 → (36-10)/12.6 = 2.06 → col 2 - let mouse = Mouse { - x: 36, - ..Default::default() - }; - let pos = calculate_mouse_position( - &mouse, - display_offset, - (columns, lines), - margin_x_left, - margin_y_top, - (cell_dimension_width, cell_dimension_height), - ); - assert_eq!(pos, Pos::new(Line(0), Column(2))); + &mk_mouse(100.0, 5.0 * 33.0 + 1.0), + 10, + (cols, lines), + 0.0, + 0.0, + cell, + ); + assert_eq!(pos.row, Line(-5)); } - /// With cell_width=16.41 the 60% threshold sits at 9.846, so Left - /// spans [0, 9.846) and Right spans [9.846, 16.41). Using float math - /// avoids truncation that would shift the boundary. + /// 60% threshold: cell_x < 0.6*cell_w → Left, otherwise Right. + /// Boundary lands on subpixel position. #[test] - fn test_side_by_pos_float_precision() { - let cell_width = 16.41_f32; - let margin_x = 8.0; // pre-scaled - let grid_width = 8.0 + 96.0 * cell_width; - - // pixel 16 → cell_x = 8.0 < threshold 9.846 → Left - assert_eq!( - calculate_side_by_pos(16, margin_x, cell_width, grid_width), - Side::Left, - ); + fn side_by_pos_60_percent_threshold() { + let cell = 16u32; + let margin_x = 0.0_f32; + let grid_w = 16.0 * 200.0; - // pixel 17 → cell_x = 9.0 < threshold 9.846 → still Left (was Right at 50%) + // cell_x = 9.59 < 9.6 → Left. assert_eq!( - calculate_side_by_pos(17, margin_x, cell_width, grid_width), + calculate_side_by_pos(9.59, margin_x, cell, grid_w), Side::Left, ); - - // pixel 18 → cell_x = 10.0 >= threshold 9.846 → Right + // cell_x ≥ 9.6 → Right. Use 9.61 to dodge f64 rep of 9.6. assert_eq!( - calculate_side_by_pos(18, margin_x, cell_width, grid_width), + calculate_side_by_pos(9.61, margin_x, cell, grid_w), Side::Right, ); } - /// Regression: integer truncation of cell_width (16.41 → 16) caused - /// the modulo to drift at higher pixel positions, making the side - /// detection wrong for cells far from the left edge. + /// Regression: side detection had drift at high columns when the + /// f32 divide was off by half a pixel per cell. With u32 stride + /// the modulo stays exact at any column. #[test] - fn test_side_by_pos_no_drift_at_high_columns() { - let cell_width = 16.41_f32; - let margin_x = 8.0; - let grid_width = 8.0 + 96.0 * cell_width; + fn side_by_pos_no_drift_at_high_columns() { + let cell = 16u32; + let margin_x = 0.0_f32; + let grid_w = 16.0 * 200.0; - // Column 76 starts at margin + 76 * 16.41 = 8 + 1247.16 = 1255.16 - // Left side: pixel 1256 → cell_x = 1248 % 16.41 ≈ 0.66 → Left + // Col 100 = pixel [1600, 1616). Left side: cell_x = 0..9.6. assert_eq!( - calculate_side_by_pos(1256, margin_x, cell_width, grid_width), + calculate_side_by_pos(1600.0, margin_x, cell, grid_w), Side::Left, ); - - // Pixel 1266 → cell_x = 1258 % 16.41 ≈ 10.48 >= 9.846 → Right assert_eq!( - calculate_side_by_pos(1266, margin_x, cell_width, grid_width), + calculate_side_by_pos(1609.5, margin_x, cell, grid_w), + Side::Left, + ); + // ≥ 9.6 → Right. Use 9.61 to dodge f64 representation of 9.6. + assert_eq!( + calculate_side_by_pos(1609.61, margin_x, cell, grid_w), Side::Right, ); } - /// Margin must not be double-scaled in side calculation. + /// Margin pre-scaled, must not be re-scaled in the side + /// calculation. Pixel before margin clamps to Left. #[test] - fn test_side_by_pos_prescaled_margin() { - let cell_width = 16.0; - let margin_x = 40.0; // already scaled (e.g. 20 * 2.0) - let grid_width = 40.0 + 80.0 * 16.0; + fn side_by_pos_prescaled_margin() { + let cell = 16u32; + let margin_x = 40.0_f32; + let grid_w = 40.0 + 80.0 * 16.0; - // Pixel 41: just past margin, left side of cell 0 - // cell_x = (41 - 40) % 16 = 1.0, threshold = 9.6 → Left + // Before margin → Left. assert_eq!( - calculate_side_by_pos(41, margin_x, cell_width, grid_width), + calculate_side_by_pos(30.0, margin_x, cell, grid_w), Side::Left, ); - - // Pixel 50: past the 60% threshold of cell 0 - // cell_x = (50 - 40) % 16 = 10.0, threshold = 9.6 → Right - assert_eq!( - calculate_side_by_pos(50, margin_x, cell_width, grid_width), - Side::Right, - ); - - // Pixel 49: cell_x = 9.0 < threshold 9.6 → Left (was Right at 50%) + // x=49: cell_x = 9 < 9.6 → Left. assert_eq!( - calculate_side_by_pos(49, margin_x, cell_width, grid_width), + calculate_side_by_pos(49.0, margin_x, cell, grid_w), Side::Left, ); - - // Pixel 30: before margin → clamped to 0, Left + // x=49.61 → cell_x ≥ 9.6 → Right (49.6 hits f64 rep edge). assert_eq!( - calculate_side_by_pos(30, margin_x, cell_width, grid_width), - Side::Left, + calculate_side_by_pos(49.61, margin_x, cell, grid_w), + Side::Right, ); } } diff --git a/frontends/rioterm/src/renderer/mod.rs b/frontends/rioterm/src/renderer/mod.rs index 9d1a15cd..9f577a88 100644 --- a/frontends/rioterm/src/renderer/mod.rs +++ b/frontends/rioterm/src/renderer/mod.rs @@ -380,10 +380,12 @@ impl Renderer { let has_overlays = !terminal_snapshot.kitty_placements.is_empty(); let has_virtual = !terminal_snapshot.kitty_virtual_placements.is_empty(); if has_overlays || has_virtual { - let line_height = sugarloaf.style().line_height; let layout = context.dimension; - let cell_width = layout.dimension.width; - let cell_height = layout.dimension.height * line_height; + // Canonical integer cell stride — line_height already + // baked into `cell.cell_height`. Same value the GPU + // grid uniform paints with. + let cell_width = layout.cell.cell_width as f32; + let cell_height = layout.cell.cell_height as f32; let origin_x = panel_rect[0] + grid_scaled_margin.left; let origin_y = panel_rect[1] + grid_scaled_margin.top; @@ -571,8 +573,8 @@ impl Renderer { // taffy allocates fractional sizes while the grid // snaps to whole cells. let dim = grid_context.val.dimension; - let cell_w = dim.dimension.width.round().max(1.0); - let cell_h = dim.dimension.height.round().max(1.0); + let cell_w = dim.cell.cell_width as f32; + let cell_h = dim.cell.cell_height as f32; let cols = dim.columns.max(1) as f32; let rows = dim.lines.max(1) as f32; let panel_left = diff --git a/frontends/rioterm/src/screen/mod.rs b/frontends/rioterm/src/screen/mod.rs index a6d86221..49a626ff 100644 --- a/frontends/rioterm/src/screen/mod.rs +++ b/frontends/rioterm/src/screen/mod.rs @@ -243,6 +243,9 @@ impl Screen<'_> { sugarloaf .get_text_dimensions(&rich_text_id) .unwrap_or_default(), + sugarloaf + .get_text_cell_metrics(&rich_text_id) + .unwrap_or_default(), config.line_height, margin, ); @@ -399,7 +402,9 @@ impl Screen<'_> { let current_grid = self.context_manager.current_grid(); let (context, margin) = current_grid.current_context_with_computed_dimension(); let context_dimension = context.dimension; - let style = self.sugarloaf.style(); + // Canonical integer cell stride — `cell.cell_height` already + // has line_height baked in by sugarloaf, do NOT re-multiply. + // Single source of truth shared with the GPU grid uniform. calculate_mouse_position( &self.mouse, display_offset, @@ -407,8 +412,8 @@ impl Screen<'_> { margin.left, margin.top, ( - context_dimension.dimension.width, - context_dimension.dimension.height * style.line_height, + context_dimension.cell.cell_width, + context_dimension.cell.cell_height, ), ) } @@ -2214,10 +2219,11 @@ impl Screen<'_> { let current_grid = self.context_manager.current_grid(); let (context, margin) = current_grid.current_context_with_computed_dimension(); let layout = context.dimension; - // All values in physical pixels — margin is pre-scaled, cell - // dimensions are in physical pixels, position.y is physical. - let cell_height = - (layout.dimension.height * self.sugarloaf.style().line_height) as f64; + // Canonical integer cell stride. line_height is already + // baked into `cell.cell_height`; the previous code + // multiplied by line_height again, breaking the + // edge-of-viewport detection at line_height ≠ 1.0. + let cell_height = layout.cell.cell_height as f64; let text_area_top = margin.top as f64; let text_area_bottom = text_area_top + layout.lines as f64 * cell_height; let window_height = self.sugarloaf.window_size().height as f64; @@ -2256,21 +2262,25 @@ impl Screen<'_> { } #[inline] - pub fn contains_point(&self, x: usize, y: usize) -> bool { + pub fn contains_point(&self, x: f64, y: f64) -> bool { let current_grid = self.context_manager.current_grid(); let (context, margin) = current_grid.current_context_with_computed_dimension(); let layout = context.dimension; - // Margin is already pre-scaled (physical pixels), same as x/y. - let cell_width = layout.dimension.width; - let cell_height = layout.dimension.height * self.sugarloaf.style().line_height; - x > margin.left as usize - && x <= (margin.left + layout.columns as f32 * cell_width) as usize - && y > margin.top as usize - && y <= (margin.top + layout.lines as f32 * cell_height) as usize + // Canonical integer stride — same as the GPU paints with. + // line_height is already baked into `cell.cell_height`; do + // NOT multiply again here. + let cell_w = layout.cell.cell_width as f64; + let cell_h = layout.cell.cell_height as f64; + let left = margin.left as f64; + let top = margin.top as f64; + x > left + && x <= left + layout.columns as f64 * cell_w + && y > top + && y <= top + layout.lines as f64 * cell_h } #[inline] - pub fn side_by_pos(&self, x: usize) -> Side { + pub fn side_by_pos(&self, x: f64) -> Side { let current_grid = self.context_manager.current_grid(); let (_, margin) = current_grid.current_context_with_computed_dimension(); let current_context = self.context_manager.current(); @@ -2279,7 +2289,7 @@ impl Screen<'_> { crate::mouse::calculate_side_by_pos( x, margin.left, - layout.dimension.width, + layout.cell.cell_width, layout.width, ) } @@ -2575,7 +2585,7 @@ impl Screen<'_> { use crate::renderer::island::ISLAND_HEIGHT; let scale_factor = self.sugarloaf.scale_factor(); - let island_height_px = (ISLAND_HEIGHT * scale_factor) as usize; + let island_height_px = (ISLAND_HEIGHT * scale_factor) as f64; let window_width = self.sugarloaf.window_size().width; let num_tabs = self.context_manager.len(); @@ -3446,9 +3456,10 @@ impl Screen<'_> { if let Some(current_item) = current_grid.current_item() { let layout = current_item.val.dimension; - let cell_width = layout.dimension.width; - let line_height = self.sugarloaf.style().line_height; - let cell_height = layout.dimension.height * line_height; + // Canonical integer stride — same value the GPU + // shader uses; line_height is already baked in. + let cell_width = layout.cell.cell_width as f32; + let cell_height = layout.cell.cell_height as f32; let scale_factor = self.sugarloaf.scale_factor(); let panel_rect = current_item.layout_rect; @@ -3596,8 +3607,10 @@ impl Screen<'_> { // columns end up 7 vs 8 px wide → visible seams. // Rounding both to the same integer stride the cell // grid is actually drawn on removes the drift. - let cell_w = dim.dimension.width.round().max(1.0); - let cell_h = dim.dimension.height.round().max(1.0); + // Canonical integer cell stride — single source of + // truth for paint, layout and mouse hit-test. + let cell_w = dim.cell.cell_width as f32; + let cell_h = dim.cell.cell_height as f32; // Per-panel font size (zoom is per-rich-text, not root). // Falls back to root × scale if the text id can't be // found — shouldn't happen post-init but keeps the emit @@ -4028,10 +4041,10 @@ impl Screen<'_> { let cursor_pos = terminal.grid.cursor.pos; drop(terminal); - // Calculate pixel position of cursor - let cell_width = layout.dimension.width; - let line_height = self.sugarloaf.style().line_height; - let cell_height = layout.dimension.height * line_height; + // Calculate pixel position of cursor — canonical integer + // stride (line_height already baked into cell_height). + let cell_width = layout.cell.cell_width as f32; + let cell_height = layout.cell.cell_height as f32; // Validate dimensions before calculation if cell_width <= 0.0 || cell_height <= 0.0 { diff --git a/frontends/rioterm/src/screen/touch.rs b/frontends/rioterm/src/screen/touch.rs index bc2d8abe..96f529bb 100644 --- a/frontends/rioterm/src/screen/touch.rs +++ b/frontends/rioterm/src/screen/touch.rs @@ -144,8 +144,8 @@ fn on_touch_motion( let layout = route.window.screen.sugarloaf.window_size(); // Start simulated mouse input. - let x = start_location.x.clamp(0.0, layout.width.into()) as usize; - let y = start_location.y.clamp(0.0, layout.height.into()) as usize; + let x = start_location.x.clamp(0.0, layout.width.into()); + let y = start_location.y.clamp(0.0, layout.height.into()); route.window.screen.mouse.x = x; route.window.screen.mouse.y = y; @@ -197,8 +197,8 @@ fn on_touch_motion( } TouchPurpose::Select(_) => { let layout = route.window.screen.sugarloaf.window_size(); - let x = touch.location.x.clamp(0.0, layout.width.into()) as usize; - let y = touch.location.y.clamp(0.0, layout.height.into()) as usize; + let x = touch.location.x.clamp(0.0, layout.width.into()); + let y = touch.location.y.clamp(0.0, layout.height.into()); route.window.screen.mouse.x = x; route.window.screen.mouse.y = y; tracing::info!("select motion"); @@ -224,8 +224,8 @@ fn on_touch_end( let layout = route.window.screen.sugarloaf.window_size(); - let x = start_location.x.clamp(0.0, layout.width.into()) as usize; - let y = start_location.y.clamp(0.0, layout.height.into()) as usize; + let x = start_location.x.clamp(0.0, layout.width.into()); + let y = start_location.y.clamp(0.0, layout.height.into()); route.window.screen.mouse.x = x; route.window.screen.mouse.y = y; diff --git a/sugarloaf/src/layout/content.rs b/sugarloaf/src/layout/content.rs index 1afcb4c9..9e19eb0b 100644 --- a/sugarloaf/src/layout/content.rs +++ b/sugarloaf/src/layout/content.rs @@ -519,8 +519,10 @@ impl Content { let mut builder_state = BuilderState::from_layout(rich_text_layout); // Immediately calculate dimensions for a representative character - builder_state.layout.dimensions = + let (dims, cell) = self.calculate_character_cell_dimensions(rich_text_layout); + builder_state.layout.dimensions = dims; + builder_state.layout.cell = cell; if let Some(content_state) = self.states.get_mut(&id) { content_state.data = ContentData::Text(builder_state); @@ -532,119 +534,126 @@ impl Content { } } - /// Calculate character cell dimensions + /// Calculate character cell dimensions and the canonical + /// [`CellMetrics`] used by the renderer + mouse + layout. + /// + /// Integer cell width / height / baseline are produced by + /// `.round()`-ing the unrounded `face_*` values once at this + /// layer. All downstream consumers read those integers — + /// there's no second rounding stage anywhere in the pipeline, + /// so the renderer's painted stride and the mouse hit-test + /// divide by the same value. + /// + /// `line_height` (the user's config multiplier) is applied to + /// `face_height` here, so callers must NOT re-apply it. fn calculate_character_cell_dimensions( &self, layout: &TextLayout, - ) -> crate::layout::TextDimensions { + ) -> (crate::layout::TextDimensions, crate::layout::CellMetrics) { let font_size = layout.font_size; + let scale_f64 = layout.dimensions.scale as f64; + let line_height_mod = layout.line_height as f64; - // macOS: read metrics + space-glyph advance straight from the - // primary CTFont. This is the last byte-dependent path that - // mattered on mac — using CTFont here means `FONT_DATA_CACHE` - // never holds the primary font either. - #[cfg(target_os = "macos")] - if let Some(handle) = self.fonts.ct_font(0) { - let metrics = crate::font::macos::font_metrics(&handle, font_size); - // Cell width = max advance across all printable ASCII, - // queried on a CTFont clone at the real render size (not - // the 1pt base — that returns bogus 1.0-per-glyph - // advances on some fonts). Mirrors - //. Progressive fallbacks: - // 1. max-ASCII at this size (right answer on every - // real font we've seen) - // 2. advance of space (pre-existing behaviour; may - // return None) - // 3. `font_size` itself (the em — last-resort, wider - // than any real monospace advance) - let char_width = crate::font::macos::max_ascii_advance_px(&handle, font_size) - .or_else(|| { - crate::font::macos::advance_units_for_char(&handle, ' ') - .map(|(units, upem)| units * font_size / upem as f32) + // (char_width, ascent, descent, leading) in pixels at the + // *logical* font size (i.e. before `scale` is applied). + let raw: Option<(f64, f64, f64, f64)> = { + #[cfg(target_os = "macos")] + { + self.fonts.ct_font(0).map(|handle| { + let m = crate::font::macos::font_metrics(&handle, font_size); + // Cell width = max advance across printable ASCII at + // the real render size. Same progressive fallback as + // before — final fallback is `font_size` (the em). + let cw = + crate::font::macos::max_ascii_advance_px(&handle, font_size) + .or_else(|| { + crate::font::macos::advance_units_for_char( + &handle, ' ', + ) + .map(|(units, upem)| { + units * font_size / upem as f32 + }) + }) + .unwrap_or(font_size); + ( + cw as f64, + m.ascent as f64, + m.descent as f64, + m.leading as f64, + ) }) - .unwrap_or(font_size); - let line_height = - (metrics.ascent + metrics.descent + metrics.leading) * layout.line_height; - let scale = layout.dimensions.scale; - return crate::layout::TextDimensions { - width: char_width * scale, - height: (line_height * scale).ceil(), - scale, - }; - } - - #[cfg(not(target_os = "macos"))] - if let Some(font_library_data) = self.fonts.inner.try_read() { - let font_id = 0; // FONT_ID_REGULAR - - // Get font data to create swash FontRef - if let Some((font_data, offset, _key)) = font_library_data.get_data(&font_id) + } + #[cfg(not(target_os = "macos"))] { - // Create swash FontRef directly from font data - if let Some(font_ref) = - swash::FontRef::from_index(&font_data, offset as usize) - { - // Get metrics using swash - let font_metrics = font_ref.metrics(&[]); - - // Calculate character cell width using space character - let glyph_id = font_ref.charmap().map(' ' as u32); - let char_width = { - // Get advance width for space character using GlyphMetrics - let glyph_metrics = font_ref.glyph_metrics(&[]); - let advance = glyph_metrics.advance_width(glyph_id); - - // Scale to font size - let units_per_em = font_metrics.units_per_em as f32; - let scale_factor = font_size / units_per_em; - - if advance > 0.0 { - advance * scale_factor - } else { - // Fallback: approximate monospace character width - font_size - } - }; - - // Calculate line height using scaled metrics - let units_per_em = font_metrics.units_per_em as f32; - let scale_factor = font_size / units_per_em; - let ascent = font_metrics.ascent * scale_factor; - let descent = font_metrics.descent.abs() * scale_factor; - let leading = font_metrics.leading * scale_factor; - let line_height = (ascent + descent + leading) * layout.line_height; - - // Scale to physical pixels to match what the brush returns. - // physical scale — the renderer uses that ceiled value. - let char_width_physical = char_width * layout.dimensions.scale; - let line_height_physical = - (line_height * layout.dimensions.scale).ceil(); - - // Return dimensions in physical pixels (matching brush behavior) - let result = crate::layout::TextDimensions { - width: char_width_physical, - height: line_height_physical, - scale: layout.dimensions.scale, - }; - - // println!(" -> Returning dimensions (physical): width={}, height={}, scale={}", - // result.width, result.height, result.scale); - - return result; - } + self.fonts.inner.try_read().and_then(|lib| { + let id = 0; + let (data, offset, _key) = lib.get_data(&id)?; + let font_ref = + swash::FontRef::from_index(&data, offset as usize)?; + let m = font_ref.metrics(&[]); + let upem = m.units_per_em as f32; + let s = font_size / upem; + let glyph = font_ref.charmap().map(' ' as u32); + let advance = + font_ref.glyph_metrics(&[]).advance_width(glyph); + let cw = if advance > 0.0 { advance * s } else { font_size }; + Some(( + cw as f64, + (m.ascent * s) as f64, + (m.descent.abs() * s) as f64, + (m.leading * s) as f64, + )) + }) } - } + }; - // Fallback to reasonable defaults if font metrics unavailable - // Return in physical pixels to match brush behavior - let fallback_width = layout.font_size; - let fallback_height = layout.font_size * layout.line_height; + let (face_width, face_height, descent_phys, leading_phys) = + if let Some((cw, ascent, descent, leading)) = raw { + let face_width = cw * scale_f64; + let face_height = + (ascent + descent + leading) * line_height_mod * scale_f64; + ( + face_width, + face_height, + descent * line_height_mod * scale_f64, + leading * line_height_mod * scale_f64, + ) + } else { + // Last-resort fallback: square em cell at the configured + // line_height. No baseline information, so cell_baseline + // ends up 0. + let fw = font_size as f64 * scale_f64; + let fh = font_size as f64 * line_height_mod * scale_f64; + (fw, fh, 0.0, 0.0) + }; - crate::layout::TextDimensions { - width: fallback_width * layout.dimensions.scale, - height: fallback_height * layout.dimensions.scale, + let cell_width = face_width.round().max(1.0) as u32; + let cell_height = face_height.round().max(1.0) as u32; + // Pixels from the bottom of the cell to the text baseline. + // Same formula as `font::metrics::Metrics::calc`: descent + + // half the line_gap (so the line gap is split evenly above + // and below the glyph). All in physical pixels. + let cell_baseline = (descent_phys + leading_phys * 0.5).round().max(0.0) as u32; + + let dims = crate::layout::TextDimensions { + // Legacy fields kept for back-compat with sugarloaf-side + // consumers that still read TextDimensions. Now snapped + // to the same integer cell stride (was: `.ceil()` on + // height, raw on width — produced drift at high column + // indexes when paired with the mouse path's unrounded + // divide). + width: cell_width as f32, + height: cell_height as f32, scale: layout.dimensions.scale, - } + }; + let cell = crate::layout::CellMetrics { + cell_width, + cell_height, + cell_baseline, + face_width, + face_height, + }; + (dims, cell) } #[inline] @@ -666,8 +675,9 @@ impl Content { #[inline] pub fn add_transient_text(&mut self, layout: &TextLayout) -> usize { let mut builder_state = BuilderState::from_layout(layout); - builder_state.layout.dimensions = - self.calculate_character_cell_dimensions(layout); + let (dims, cell) = self.calculate_character_cell_dimensions(layout); + builder_state.layout.dimensions = dims; + builder_state.layout.cell = cell; let mut content_state = ContentState::new(ContentData::Text(builder_state)); content_state.render_data.transient = true; @@ -740,10 +750,12 @@ impl Content { return; }; - let new_dimension = self.calculate_character_cell_dimensions(&layout); + let (new_dimension, new_cell) = + self.calculate_character_cell_dimensions(&layout); if let Some(text_state) = self.get_state_mut(state_id) { text_state.layout.dimensions = new_dimension; + text_state.layout.cell = new_cell; } } diff --git a/sugarloaf/src/layout/mod.rs b/sugarloaf/src/layout/mod.rs index a9af5e70..580554d8 100644 --- a/sugarloaf/src/layout/mod.rs +++ b/sugarloaf/src/layout/mod.rs @@ -59,12 +59,54 @@ impl Default for TextDimensions { } } +/// Canonical cell metrics in physical pixels. Rounded `u32` cell +/// width / height / baseline are the single source of truth for the +/// GPU grid uniform, the col/row count math, and mouse hit testing. +/// The unrounded `f64` `face_width / face_height` are retained for +/// downstream subpixel math (image positioning, baseline-relative +/// offsets). +/// +/// Important invariants: +/// - `cell_width = round(face_width)` (half-away-from-zero) +/// - `cell_height = round(face_height)` (half-away-from-zero) — +/// `face_height` already has the user's `line_height` multiplier +/// baked in; consumers MUST NOT re-apply it. +/// - `cell_baseline` is pixels from the **bottom** of the cell to +/// the text baseline. +#[derive(Copy, Clone, Debug, PartialEq)] +pub struct CellMetrics { + pub cell_width: u32, + pub cell_height: u32, + pub cell_baseline: u32, + pub face_width: f64, + pub face_height: f64, +} + +impl Default for CellMetrics { + fn default() -> Self { + Self { + cell_width: 8, + cell_height: 16, + cell_baseline: 4, + face_width: 8.0, + face_height: 16.0, + } + } +} + #[derive(Debug, PartialEq, Copy, Clone)] pub struct TextLayout { pub line_height: f32, pub font_size: f32, pub original_font_size: f32, pub dimensions: TextDimensions, + /// Canonical cell metrics. Single source of truth for the + /// integer cell stride and baseline. Consumers should prefer + /// this over `dimensions` for any cell-coordinate math (renderer + /// grid uniform, layout col/row count, mouse hit testing) — + /// `dimensions` is kept for legacy callers that read the raw f32 + /// width/height. + pub cell: CellMetrics, } impl TextLayout { @@ -85,6 +127,7 @@ impl TextLayout { scale: default_layout.scale_factor, ..TextDimensions::default() }, + cell: CellMetrics::default(), } } } @@ -96,6 +139,7 @@ impl Default for TextLayout { font_size: 0.0, original_font_size: 0.0, dimensions: TextDimensions::default(), + cell: CellMetrics::default(), } } } diff --git a/sugarloaf/src/sugarloaf.rs b/sugarloaf/src/sugarloaf.rs index 0f6f6b09..9b3895b8 100644 --- a/sugarloaf/src/sugarloaf.rs +++ b/sugarloaf/src/sugarloaf.rs @@ -1060,6 +1060,26 @@ impl Sugarloaf<'_> { } } + /// Canonical [`CellMetrics`] for `id`, mirroring + /// [`get_text_dimensions`]'s mark-for-repaint side effect. + /// Use alongside `get_text_dimensions` when constructing a + /// `ContextDimension` so the layout / GPU / mouse pipeline all + /// see the same `u32` cell stride. + #[inline] + pub fn get_text_cell_metrics( + &mut self, + id: &usize, + ) -> Option { + if self.state.content.get_text_by_id(*id).is_some() { + if let Some(content_state) = self.state.content.states.get_mut(id) { + content_state.render_data.needs_repaint = true; + } + Some(self.state.get_state_layout(id).cell) + } else { + None + } + } + #[inline] pub fn clear(&mut self) { self.state.clean_screen();