From 3815878cde9625b1ecabdbbdafd19a974cbd9c7a Mon Sep 17 00:00:00 2001 From: Raphael Amorim Date: Wed, 29 Apr 2026 20:41:49 +0200 Subject: [PATCH] change baseline formula --- frontends/rioterm/src/application.rs | 21 +-- frontends/rioterm/src/context/title.rs | 2 + frontends/rioterm/src/layout/compute_tests.rs | 37 +++- frontends/rioterm/src/layout/mod.rs | 6 +- frontends/rioterm/src/mouse/mod.rs | 106 +++++++++-- frontends/rioterm/src/renderer/mod.rs | 19 +- frontends/rioterm/src/router/mod.rs | 28 ++- frontends/rioterm/src/screen/mod.rs | 35 ++-- sugarloaf/src/layout/content.rs | 172 ++++++++++++++---- sugarloaf/src/layout/mod.rs | 9 +- sugarloaf/src/sugarloaf.rs | 13 +- 11 files changed, 321 insertions(+), 127 deletions(-) diff --git a/frontends/rioterm/src/application.rs b/frontends/rioterm/src/application.rs index f63fc525..51b420d6 100644 --- a/frontends/rioterm/src/application.rs +++ b/frontends/rioterm/src/application.rs @@ -1200,11 +1200,6 @@ impl ApplicationHandler for Application<'_> { 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. - let old_x = route.window.screen.mouse.x; - let old_y = route.window.screen.mouse.y; - route.window.screen.mouse.x = x; route.window.screen.mouse.y = y; route.window.screen.mouse.raw_y = position.y; @@ -1408,16 +1403,19 @@ impl ApplicationHandler for Application<'_> { let display_offset = route.window.screen.display_offset(); let point = route.window.screen.mouse_position(display_offset); - // Detect cell change by comparing pixel positions against cell - // dimensions, avoiding a second mouse_position() call. - let square_changed = x != old_x || y != old_y; + // Compare *cell* coordinates, not pixel coordinates, so + // subpixel HiDPI jitter inside the same cell doesn't + // re-fire hint / OSC-8 / hyperlink work every event. + let prev_cell = route.window.screen.mouse.last_cell; + let cell_changed = prev_cell != Some(point); + route.window.screen.mouse.last_cell = Some(point); let inside_text_area = route.window.screen.contains_point(x, y); let square_side = route.window.screen.side_by_pos(x); - // If the mouse hasn't changed cells, do nothing. + // If the cursor hasn't changed cells, do nothing. // Force update when transitioning off a border so the cursor resets. - if !square_changed + if !cell_changed && !was_on_border && route.window.screen.mouse.square_side == square_side && route.window.screen.mouse.inside_text_area == inside_text_area @@ -1470,8 +1468,7 @@ impl ApplicationHandler for Application<'_> { if is_selecting { route.window.screen.update_selection(point, square_side); route.window.screen.context_manager.request_render(); - } else if square_changed - && route.window.screen.has_mouse_motion_and_drag() + } else if cell_changed && route.window.screen.has_mouse_motion_and_drag() { if lmb_pressed { route.window.screen.mouse_report(32, ElementState::Pressed); diff --git a/frontends/rioterm/src/context/title.rs b/frontends/rioterm/src/context/title.rs index 3c72e0a7..8d62d143 100644 --- a/frontends/rioterm/src/context/title.rs +++ b/frontends/rioterm/src/context/title.rs @@ -293,6 +293,7 @@ pub mod test { cell_baseline: 0, face_width: 18.0, face_height: 9.0, + face_y: 0.0, }, 1.0, Margin::default(), @@ -351,6 +352,7 @@ pub mod test { cell_baseline: 0, face_width: 18.0, face_height: 9.0, + face_y: 0.0, }, 1.0, Margin::default(), diff --git a/frontends/rioterm/src/layout/compute_tests.rs b/frontends/rioterm/src/layout/compute_tests.rs index a36779d2..6ceaf272 100644 --- a/frontends/rioterm/src/layout/compute_tests.rs +++ b/frontends/rioterm/src/layout/compute_tests.rs @@ -13,6 +13,7 @@ fn cell_for(dims: TextDimensions) -> rio_backend::sugarloaf::layout::CellMetrics cell_baseline: 0, face_width: dims.width as f64, face_height: dims.height as f64, + face_y: 0.0, } } @@ -267,7 +268,14 @@ fn test_context_dimension_build() { height: 33.0, scale: 2.0, }; - let cd = ContextDimension::build(1650.0, 825.0, dims, cell_for(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); } @@ -279,7 +287,14 @@ fn test_context_dimension_update_width() { height: 33.0, scale: 2.0, }; - let mut cd = ContextDimension::build(1600.0, 825.0, dims, cell_for(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); @@ -294,7 +309,14 @@ fn test_context_dimension_update_height() { height: 33.0, scale: 2.0, }; - let mut cd = ContextDimension::build(1600.0, 825.0, dims, cell_for(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); @@ -309,7 +331,14 @@ fn test_context_dimension_update_dimensions() { height: 33.0, scale: 1.0, }; - let mut cd = ContextDimension::build(1600.0, 825.0, dims, cell_for(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 { diff --git a/frontends/rioterm/src/layout/mod.rs b/frontends/rioterm/src/layout/mod.rs index 6b637ce2..c50e91e0 100644 --- a/frontends/rioterm/src/layout/mod.rs +++ b/frontends/rioterm/src/layout/mod.rs @@ -78,10 +78,8 @@ fn compute( 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, - ); + let visible_lines = + std::cmp::max((available_height / cell_h).floor() as usize, MIN_LINES); (visible_columns, visible_lines) } diff --git a/frontends/rioterm/src/mouse/mod.rs b/frontends/rioterm/src/mouse/mod.rs index 6459c55e..629d4651 100644 --- a/frontends/rioterm/src/mouse/mod.rs +++ b/frontends/rioterm/src/mouse/mod.rs @@ -37,6 +37,12 @@ pub struct Mouse { pub on_border: bool, /// Raw (unclamped) cursor Y in physical pixels, for selection scroll. pub raw_y: f64, + /// Last cell (line, column) the cursor was over. `None` until the + /// first `CursorMoved` event arrives. Used by the input dispatcher + /// to skip hint / OSC-8 / hyperlink work when the cursor moves + /// within the same cell — replaces the old pixel-equality check + /// that fired on every subpixel HiDPI jitter. + pub last_cell: Option, } impl Default for Mouse { @@ -57,6 +63,7 @@ impl Default for Mouse { x: 0.0, y: 0.0, raw_y: 0.0, + last_cell: None, } } } @@ -177,32 +184,67 @@ pub mod test { // 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, + calculate_mouse_position( + &mk_mouse(8.0, 0.0), + 0, + (cols, lines), + 0.0, + 0.0, + cell + ) + .col, Column(0), ); // 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, + calculate_mouse_position( + &mk_mouse(8.99, 0.0), + 0, + (cols, lines), + 0.0, + 0.0, + cell + ) + .col, Column(0), ); // 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, + calculate_mouse_position( + &mk_mouse(9.0, 0.0), + 0, + (cols, lines), + 0.0, + 0.0, + cell + ) + .col, Column(1), ); // 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, + calculate_mouse_position( + &mk_mouse(17.5, 0.0), + 0, + (cols, lines), + 0.0, + 0.0, + cell + ) + .col, Column(1), ); // x=18 → col 2 assert_eq!( - calculate_mouse_position(&mk_mouse(18.0, 0.0), 0, (cols, lines), 0.0, 0.0, cell) - .col, + calculate_mouse_position( + &mk_mouse(18.0, 0.0), + 0, + (cols, lines), + 0.0, + 0.0, + cell + ) + .col, Column(2), ); } @@ -218,26 +260,54 @@ pub mod test { // 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, + calculate_mouse_position( + &mk_mouse(5.0, 0.0), + 0, + (cols, lines), + margin_x, + 0.0, + cell + ) + .col, Column(0), ); // 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, + calculate_mouse_position( + &mk_mouse(10.0, 0.0), + 0, + (cols, lines), + margin_x, + 0.0, + cell + ) + .col, Column(0), ); // 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, + calculate_mouse_position( + &mk_mouse(18.99, 0.0), + 0, + (cols, lines), + margin_x, + 0.0, + cell + ) + .col, Column(0), ); // x=19 → col 1. assert_eq!( - calculate_mouse_position(&mk_mouse(19.0, 0.0), 0, (cols, lines), margin_x, 0.0, cell) - .col, + calculate_mouse_position( + &mk_mouse(19.0, 0.0), + 0, + (cols, lines), + margin_x, + 0.0, + cell + ) + .col, Column(1), ); } diff --git a/frontends/rioterm/src/renderer/mod.rs b/frontends/rioterm/src/renderer/mod.rs index 9f577a88..891b7c87 100644 --- a/frontends/rioterm/src/renderer/mod.rs +++ b/frontends/rioterm/src/renderer/mod.rs @@ -737,14 +737,13 @@ impl Renderer { } /// Scan visible rows for kitty Unicode-placeholder cells (U+10EEEE) and - /// push one `GraphicOverlay` per row-run. Ports the four key behaviors - /// from ghostty's `graphics_unicode.zig`: + /// push one `GraphicOverlay` per row-run. Implements four key behaviors + /// of the Kitty graphics Unicode-placeholder protocol: /// /// 1. Per-row `kitty_virtual_placeholder` flag check skips rows /// with no placeholders. /// 2. Continuation rules — a cell with missing diacritics inherits - /// from the previous cell on the row (`canAppend`, - /// `graphics_unicode.zig:506-513`). + /// from the previous cell on the row (`canAppend`). /// 3. Run aggregation — consecutive cells with same image / row / /// sequential column collapse into one Placement /// (`PlacementIterator.next`, `graphics_unicode.zig:36-99`). @@ -764,7 +763,9 @@ impl Renderer { IncompletePlacement, PlaceholderRun, PLACEHOLDER, }; - // Below text — matches ghostty's default for virtual placements. + // Below text by default for virtual placements — apps that + // want them above the glyphs set z-index explicitly via the + // graphics protocol. const VIRTUAL_Z_INDEX: i32 = -1; for (line_idx, row) in snapshot.visible_rows.iter().enumerate() { @@ -832,11 +833,9 @@ impl Renderer { ); } // Default missing row/col on the FIRST cell of a - // run — matches ghostty's - // `graphics_unicode.zig:84-86`. Without this, - // a subsequent cell with `Some(col)` couldn't - // sequentially extend a run started by a cell - // with `None`. + // run. Without this, a subsequent cell with + // `Some(col)` couldn't sequentially extend a + // run started by a cell with `None`. if cell.row.is_none() { cell.row = Some(0); } diff --git a/frontends/rioterm/src/router/mod.rs b/frontends/rioterm/src/router/mod.rs index ad3adcde..f1afc260 100644 --- a/frontends/rioterm/src/router/mod.rs +++ b/frontends/rioterm/src/router/mod.rs @@ -789,9 +789,8 @@ fn compute_window_size_from_grid( + panel.margin.left + panel.margin.right) * scale; - let raw = (columns as f32 * dim.dimension.width).ceil() as u32 - + margin as u32 - + panel_edge as u32; + let raw = + columns as u32 * dim.cell.cell_width + margin as u32 + panel_edge as u32; raw.next_multiple_of(scale_u32) } _ => window_size.width, @@ -805,9 +804,8 @@ fn compute_window_size_from_grid( + panel.margin.top + panel.margin.bottom) * scale; - let raw = (rows as f32 * dim.dimension.height).ceil() as u32 - + margin as u32 - + panel_edge as u32; + let raw = + rows as u32 * dim.cell.cell_height + margin as u32 + panel_edge as u32; raw.next_multiple_of(scale_u32) } _ => window_size.height, @@ -837,6 +835,17 @@ mod grid_size_tests { height, scale, }, + // Canonical cell stride matches the dims so router math + // (`cols * cell_width`) lines up with what the test + // labels imply. + cell: rio_backend::sugarloaf::layout::CellMetrics { + cell_width: width.round().max(1.0) as u32, + cell_height: height.round().max(1.0) as u32, + cell_baseline: 0, + face_width: width as f64, + face_height: height as f64, + face_y: 0.0, + }, margin, ..Default::default() } @@ -922,8 +931,9 @@ mod grid_size_tests { #[test] fn rounds_up_on_hidpi() { let dim = make_dim(16.41, 33.0, 2.0, Margin::all(0.0)); - // 80 * 16.41 = 1312.8 → ceil = 1313, next_multiple_of(2) = 1314 - // 24 * 33.0 = 792, next_multiple_of(2) = 792 + // Canonical cell stride: round(16.41) = 16, round(33.0) = 33. + // 80 * 16 = 1280, next_multiple_of(2) = 1280 + // 24 * 33 = 792, next_multiple_of(2) = 792 assert_eq!( compute_window_size_from_grid( Some(80), @@ -932,7 +942,7 @@ mod grid_size_tests { &dim, win(1000, 600) ), - (1314, 792) + (1280, 792) ); } diff --git a/frontends/rioterm/src/screen/mod.rs b/frontends/rioterm/src/screen/mod.rs index 49a626ff..ac65a073 100644 --- a/frontends/rioterm/src/screen/mod.rs +++ b/frontends/rioterm/src/screen/mod.rs @@ -3503,9 +3503,7 @@ impl Screen<'_> { // - `damage == Partial(lines)`: // rebuild only those rows. // Unchanged rows keep their CellBg + CellText resident in - // the grid's CPU state, which is re-uploaded verbatim. Same - // pattern as `.partial` path at - // `ghostty/src/renderer/generic.zig:2431-2440`. + // the grid's CPU state, which is re-uploaded verbatim. { struct PanelFrame { route_id: usize, @@ -3561,9 +3559,7 @@ impl Screen<'_> { /// Search-hint matches for this panel. `None` when /// search is inactive. Consumed alongside `selection` /// inside `build_row_bg` / `build_row_fg` to apply - /// `search_match_background` / `_foreground`. Mirrors - /// `row_data.highlights` at - /// `ghostty/src/renderer/generic.zig:1317`. + /// `search_match_background` / `_foreground`. hint_matches: Option>, /// Currently-focused search match (↑/↓ navigation). /// Rendered with `search_focused_match_background` / @@ -3598,17 +3594,14 @@ impl Screen<'_> { { let ctx = &mut item.val; let dim = ctx.dimension; - // Snap to integer pixel cells. `dim.dimension.width` - // comes from `char_width * scale` (fractional); - // `dim.dimension.height` is already `.ceil()`'d in - // sugarloaf's layout. Mixed fractional widths drift - // the bg fragment's `floor((pixel - padding) / - // cell_size)` across cell boundaries — adjacent - // 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. // Canonical integer cell stride — single source of - // truth for paint, layout and mouse hit-test. + // truth for paint, layout, and mouse hit-test. The + // bg fragment shader does + // `floor((pixel - padding) / cell_size)` and the text + // vertex multiplies `grid_pos * cell_size`, so both + // sides must agree on the same integer stride or + // adjacent columns drift to 7 vs 8 px wide and seams + // show up. 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). @@ -3908,12 +3901,10 @@ impl Screen<'_> { // window scaled_margin + the panel's layout rect // offset inside the root container. Snap to integer // pixels so `cell_size * grid_pos + grid_padding` - // always lands on pixel boundaries — same approach - // as `@floatFromInt(blank.top)` at - // `ghostty/src/renderer/generic.zig:1976-1981`. - // Without this, a fractional margin (e.g. Taffy - // layout computing 10.5px offsets) shifts the whole - // grid half a pixel and the bg fragment's + // always lands on pixel boundaries. Without this, a + // fractional margin (e.g. Taffy layout computing + // 10.5px offsets) shifts the whole grid half a pixel + // and the bg fragment's // `floor((pixel - padding) / cell_size)` disagrees // with the text vertex's `cell_size * grid_pos` // about where cell boundaries are → visible seams. diff --git a/sugarloaf/src/layout/content.rs b/sugarloaf/src/layout/content.rs index 9e19eb0b..fa93eeb5 100644 --- a/sugarloaf/src/layout/content.rs +++ b/sugarloaf/src/layout/content.rs @@ -388,10 +388,11 @@ pub struct SpanStyle { /// Does NOT affect positioning/advance — only compositor scaling. pub pua_constraint: Option, /// Optional per-glyph Nerd Font constraint (size / alignment / - /// padding) ported from ghostty's patcher table. When set, the - /// compositor uses ghostty's constrain() math to lay the glyph out - /// instead of the generic cell-centered fit. Only populated by the - /// renderer for codepoints with a table entry (`get_constraint`). + /// padding) sourced from the Nerd Fonts patcher table. When set, + /// the compositor lays the glyph out using the constraint math + /// in `nerd_font_attributes` instead of the generic cell-centered + /// fit. Only populated by the renderer for codepoints with a + /// table entry (`get_constraint`). pub nerd_font_constraint: Option, } @@ -430,6 +431,48 @@ pub struct Content { selector: Option, } +/// Compute the canonical [`CellMetrics`] from face metrics already +/// scaled to physical pixels. Pure function so the formula is +/// testable without standing up a `Content`. +/// +/// Inputs: +/// - `face_width / face_height`: unrounded cell dims in physical px +/// (`face_height` already has the user's `line_height` multiplier +/// applied). +/// - `descent_phys / leading_phys`: descent and line gap in physical +/// px, also multiplied by `line_height`. Descent is a *positive* +/// magnitude (swash convention). +/// +/// Centering invariant: if `face_height` is `33.4` and rounds to +/// `33`, the baseline shifts up by `0.2` so the glyph stays +/// centered in the rounded cell — matches the half-rounding-delta +/// adjustment used elsewhere for vertical pixel-snapping. +#[inline] +pub(crate) fn canonical_cell_metrics( + face_width: f64, + face_height: f64, + descent_phys: f64, + leading_phys: f64, +) -> crate::layout::CellMetrics { + let cell_width = face_width.round().max(1.0) as u32; + let cell_height = face_height.round().max(1.0) as u32; + // Unrounded baseline: line_gap split evenly above and below the + // glyph, descent below. Sign-flipped from Zig's negative-descent + // convention since swash reports descent as positive. + let face_baseline = leading_phys * 0.5 + descent_phys; + let baseline_centered = face_baseline - (cell_height as f64 - face_height) * 0.5; + let cell_baseline = baseline_centered.round().max(0.0) as u32; + let face_y = cell_baseline as f64 - face_baseline; + crate::layout::CellMetrics { + cell_width, + cell_height, + cell_baseline, + face_width, + face_height, + face_y, + } +} + impl Content { /// Creates a new layout context with the specified font library. pub fn new(font_library: &FontLibrary) -> Self { @@ -519,8 +562,7 @@ impl Content { let mut builder_state = BuilderState::from_layout(rich_text_layout); // Immediately calculate dimensions for a representative character - let (dims, cell) = - self.calculate_character_cell_dimensions(rich_text_layout); + let (dims, cell) = self.calculate_character_cell_dimensions(rich_text_layout); builder_state.layout.dimensions = dims; builder_state.layout.cell = cell; @@ -564,17 +606,12 @@ impl Content { // 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); + 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, @@ -588,15 +625,17 @@ impl Content { 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 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 }; + 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, @@ -627,13 +666,8 @@ impl Content { (fw, fh, 0.0, 0.0) }; - 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 cell = + canonical_cell_metrics(face_width, face_height, descent_phys, leading_phys); let dims = crate::layout::TextDimensions { // Legacy fields kept for back-compat with sugarloaf-side @@ -642,17 +676,10 @@ impl Content { // 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, + width: cell.cell_width as f32, + height: cell.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) } @@ -750,8 +777,7 @@ impl Content { return; }; - let (new_dimension, new_cell) = - 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; @@ -1513,6 +1539,72 @@ mod tests { use super::*; use swash::shape::cluster::Glyph; + /// Pixel-perfect when face dimensions are already integer: + /// rounding is a no-op so cell == face and the centering + /// adjustment vanishes. + #[test] + fn canonical_cell_metrics_no_rounding_delta() { + let cell = canonical_cell_metrics(16.0, 33.0, 4.0, 2.0); + assert_eq!(cell.cell_width, 16); + assert_eq!(cell.cell_height, 33); + // baseline = leading*0.5 + descent = 1 + 4 = 5 + assert_eq!(cell.cell_baseline, 5); + assert_eq!(cell.face_y, 0.0); + } + + /// Centering: face_height = 32.4 rounds DOWN to 32, so the + /// baseline must shift by half the rounding delta (≈ +0.2) + /// so the glyph stays vertically centered in the rounded cell. + #[test] + fn canonical_cell_metrics_centers_after_round_down() { + let cell = canonical_cell_metrics(16.0, 32.4, 4.0, 2.0); + assert_eq!(cell.cell_height, 32); + // face_baseline = 1 + 4 = 5 + // baseline_centered = 5 - (32 - 32.4) / 2 = 5 + 0.2 = 5.2 → round 5 + assert_eq!(cell.cell_baseline, 5); + // face_y = 5 - 5 = 0 (cell_baseline rounds back to face_baseline here) + assert!((cell.face_y - 0.0).abs() < 1e-9); + } + + /// Centering: face_height = 32.6 rounds UP to 33, baseline + /// shifts by ~ -0.3 (so the glyph drops slightly to stay + /// centered in the now-taller rounded cell). + #[test] + fn canonical_cell_metrics_centers_after_round_up() { + let cell = canonical_cell_metrics(16.0, 32.6, 4.0, 2.0); + assert_eq!(cell.cell_height, 33); + // face_baseline = 5 + // baseline_centered = 5 - (33 - 32.6) / 2 = 5 - 0.2 = 4.8 → round 5 + assert_eq!(cell.cell_baseline, 5); + } + + /// Invariants: 0 ≤ cell_baseline ≤ cell_height. + /// Holds across a sweep of face dimensions, including small + /// fonts where descent + half_gap might be larger than the + /// rounding delta. + #[test] + fn canonical_cell_metrics_baseline_within_cell() { + for face_h in [12.0, 13.4, 18.7, 24.0, 33.0, 49.5, 66.0] { + let cell = canonical_cell_metrics(8.0, face_h, 3.0, 1.0); + assert!( + cell.cell_baseline <= cell.cell_height, + "baseline {} > cell_height {} at face_h={}", + cell.cell_baseline, + cell.cell_height, + face_h + ); + } + } + + /// `face_width.round().max(1.0)` clamps degenerate sub-pixel + /// widths so the divide path in `compute()` doesn't get a + /// zero stride. + #[test] + fn canonical_cell_metrics_clamps_width_floor() { + let cell = canonical_cell_metrics(0.3, 16.0, 4.0, 2.0); + assert_eq!(cell.cell_width, 1); + } + fn create_test_glyph(id: u16, x: f32, y: f32, advance: f32) -> Glyph { Glyph { id, diff --git a/sugarloaf/src/layout/mod.rs b/sugarloaf/src/layout/mod.rs index 580554d8..2288427c 100644 --- a/sugarloaf/src/layout/mod.rs +++ b/sugarloaf/src/layout/mod.rs @@ -72,7 +72,12 @@ impl Default for TextDimensions { /// `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. +/// the text baseline. Centered in the rounded cell so the glyph +/// doesn't drift up/down by half a pixel after rounding. +/// - `face_y = cell_baseline - face_baseline` — offset between the +/// actual (rounded) baseline and the unrounded face baseline. +/// Used by underline / strikethrough position conversion when +/// moving from baseline-relative to cell-top-relative coords. #[derive(Copy, Clone, Debug, PartialEq)] pub struct CellMetrics { pub cell_width: u32, @@ -80,6 +85,7 @@ pub struct CellMetrics { pub cell_baseline: u32, pub face_width: f64, pub face_height: f64, + pub face_y: f64, } impl Default for CellMetrics { @@ -90,6 +96,7 @@ impl Default for CellMetrics { cell_baseline: 4, face_width: 8.0, face_height: 16.0, + face_y: 0.0, } } } diff --git a/sugarloaf/src/sugarloaf.rs b/sugarloaf/src/sugarloaf.rs index 9b3895b8..1b6886da 100644 --- a/sugarloaf/src/sugarloaf.rs +++ b/sugarloaf/src/sugarloaf.rs @@ -1408,13 +1408,12 @@ impl Sugarloaf<'_> { // doesn't yet interleave kitty image layers with // the grid bg/text split (BrushRenderer::render // owns kitty image draws inline), so for now the - // bg+text passes run back-to-back per panel — same - // visual result as the prior single render call - // and unchanged from ghostty's wgpu builds. - // Re-ordering kitty layers around the bg/text - // split would require pulling image draws out of - // BrushRenderer::render — Metal already does that; - // wgpu follow-up. + // bg+text passes run back-to-back per panel — + // same visual result as the prior single render + // call. Re-ordering kitty layers around the + // bg/text split would require pulling image + // draws out of BrushRenderer::render — Metal + // already does that; wgpu follow-up. for (grid, uniforms) in grids.iter_mut() { grid.render_bg_wgpu(&mut rpass, uniforms); grid.render_text_wgpu(&mut rpass, uniforms); -- 2.51.2