From 0048e2dbbc7d74664b8a650da2b0cc1a9dde8a2a Mon Sep 17 00:00:00 2001 From: ZN-Ice Date: Sun, 19 Jul 2026 21:15:30 +0800 Subject: [PATCH 1/2] fix(capture): correct headless glyph baseline offset (letter clipping) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The headless renderer derived each glyph's baseline offset from `ascent_unscaled / units_per_em`, but ab_glyph's `PxScale.y` maps the font's full `height_unscaled` (ascent − descent) — not units_per_em — to pixels. For CJK-capable fonts (Noto CJK on Linux, Arial Unicode) whose ascent−descent exceeds 1em, the offset was too large and pushed the baseline below the cell. Because rows are painted top-to-bottom, the next row's background fill then erased the lower half of every tall glyph — on Linux screenshots every ASCII letter showed only its top half. Use `as_scaled(scale).ascent()`, which divides by height_unscaled and so matches the outline rasterization. Add a regression test asserting a tall glyph reaches into the top of its cell (fails at y=9 on the old code, passes after the fix). Root-caused via systematic-debugging: per-row ink in the Linux screenshot only occupied rel=[10,17] of each 18px cell; a metrics probe confirmed the ab_glyph scale semantics (Arial Unicode: 19.24px offset > 18px cell with the bug, 14.36px correct). Co-Authored-By: Claude --- crates/cli-box-core/src/capture/headless.rs | 68 ++++++++++++++++++++- 1 file changed, 65 insertions(+), 3 deletions(-) diff --git a/crates/cli-box-core/src/capture/headless.rs b/crates/cli-box-core/src/capture/headless.rs index c0456be..3365964 100644 --- a/crates/cli-box-core/src/capture/headless.rs +++ b/crates/cli-box-core/src/capture/headless.rs @@ -138,7 +138,7 @@ impl HeadlessTerminal { /// `scroll_offset` is a line offset into the scrollback (large values clamp /// to the top). Returns an error if no font is available for rendering. pub fn render_png(&self, scroll_offset: usize) -> Result> { - use ab_glyph::{point, Font, PxScale}; + use ab_glyph::{point, Font, PxScale, ScaleFont}; use image::{ImageBuffer, Rgba, RgbaImage}; use std::io::Cursor; @@ -157,8 +157,16 @@ impl HeadlessTerminal { let cell_h = line_h.round() as u32; // Monospace cell width ≈ 0.6 * height (avoids glyph_advance unit ambiguity). let cell_w = (line_h * 0.6).round() as u32; - let upem = font.units_per_em().unwrap_or(1.0); - let ascent_px = font.ascent_unscaled() / upem * line_h; + // Baseline offset MUST match how the rasterizer interprets `scale`: + // ab_glyph's `PxScale.y` maps the font's full `height_unscaled` + // (ascent - descent) to pixels — NOT `units_per_em`. Dividing the + // ascent by upem (the old code) overestimates the offset for any font + // whose ascent-descent exceeds 1em (all CJK faces: Noto CJK, Arial + // Unicode, ...), pushing the baseline below the cell so the next row's + // background fill clips the lower half of every tall glyph. + // `as_scaled(scale).ascent()` divides by height_unscaled and so stays + // consistent with the outline rasterization above. + let ascent_px = font.as_scaled(scale).ascent(); let mut parser = self .parser @@ -316,6 +324,60 @@ mod tests { assert!(has_ink, "rendered PNG should contain non-background pixels"); } + #[test] + fn render_png_tall_glyphs_span_cell_height() { + // Regression: render_png used `ascent_unscaled / units_per_em` as the + // baseline offset, but ab_glyph's `PxScale.y` maps the font's full + // `height_unscaled` (ascent - descent) — not units_per_em — to pixels. + // For CJK-capable fonts (Arial Unicode, Noto CJK) the height exceeds + // upem, so the offset was too large and pushed the baseline *below* the + // cell. Because rows are painted top-to-bottom, the next row's + // background fill then erased the lower half of every tall glyph — + // ASCII letters showed only their top, sitting at the bottom of the row. + // Fix: derive the baseline offset from the rasterizer-consistent + // `as_scaled(scale).ascent()`. + let term = HeadlessTerminal::new(8, 2); + term.feed(b"M"); + let png = match term.render_png(0) { + Ok(p) => p, + Err(e) => { + eprintln!("skipped (no font): {e}"); + return; + } + }; + let img = image::load_from_memory(&png).expect("decode").to_rgba8(); + let (img_w, img_h) = (img.width(), img.height()); + let cell_h = img_h / 2; + + // Scan the first text row for foreground ink and record its vertical span. + let is_fg = |p: &image::Rgba| p[0] > 50 || p[1] > 50 || p[2] > 50; + let mut top: Option = None; + let mut bot: Option = None; + for y in 0..cell_h { + for x in 0..img_w { + if is_fg(img.get_pixel(x, y)) { + top.get_or_insert(y); + bot = Some(y); + } + } + } + let top = top.expect("no foreground ink in row 0"); + let bot = bot.unwrap(); + + // With the bug the glyph's top landed past the cell midline (ink only in + // the bottom band). After the fix it must reach into the top of the cell. + assert!( + top < cell_h * 2 / 5, + "tall glyph top ink at y={top}, cell_h={cell_h}; glyph is shoved to the \ + bottom of its cell (baseline-offset / clipping bug)", + ); + // Sanity: the glyph still reaches the lower portion of the cell. + assert!( + bot >= cell_h / 2, + "tall glyph bottom ink at y={bot}, cell_h={cell_h}; glyph too short", + ); + } + #[test] fn render_png_resets_scrollback_offset() { // Regression: render_png(scroll) used to mutate the parser's persistent From 79ec5ec37c8342b13139a7293fd9b62eaaa9e740 Mon Sep 17 00:00:00 2001 From: ZN-Ice Date: Sun, 19 Jul 2026 22:30:54 +0800 Subject: [PATCH 2/2] test(capture): guard headless regression test against false-pass on Latin fonts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code review (#53) noted the regression test would silently pass for the wrong reason on a host whose `load_font()` resolves a Latin-only font (height≈upem), where the clipping bug never manifests — so it couldn't actually guard the regression there. Resolve by computing the *buggy* baseline offset (ascent_unscaled/upem*18) up front and skipping loudly (with a clear reason) when it does not exceed the cell height, rather than false-passing. On Arial Unicode / Noto CJK the offset is 19.24 > 18, so the test still runs and discriminates bug vs fix. Co-Authored-By: Claude --- crates/cli-box-core/src/capture/headless.rs | 34 +++++++++++++++++---- 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/crates/cli-box-core/src/capture/headless.rs b/crates/cli-box-core/src/capture/headless.rs index 3365964..66cc118 100644 --- a/crates/cli-box-core/src/capture/headless.rs +++ b/crates/cli-box-core/src/capture/headless.rs @@ -336,15 +336,37 @@ mod tests { // ASCII letters showed only their top, sitting at the bottom of the row. // Fix: derive the baseline offset from the rasterizer-consistent // `as_scaled(scale).ascent()`. - let term = HeadlessTerminal::new(8, 2); - term.feed(b"M"); - let png = match term.render_png(0) { - Ok(p) => p, - Err(e) => { - eprintln!("skipped (no font): {e}"); + // The bug only manifests on fonts whose ascender exceeds the em square + // (ascent_unscaled > units_per_em) — CJK-capable faces (Arial Unicode, + // Noto CJK). On a Latin-only font the old buggy offset stays inside the + // cell and this test would pass for the wrong reason, so compute the + // *buggy* offset and skip loudly when it would not overflow the cell, + // instead of false-passing on a host without a CJK-scale font. + use ab_glyph::Font; + let font = match load_font() { + Some(f) => f, + None => { + eprintln!("skipped (no font available)"); return; } }; + let upem = font.units_per_em().unwrap_or(1.0); + let buggy_offset = font.ascent_unscaled() / upem * 18.0; + if buggy_offset <= 18.0 { + eprintln!( + "skipped: loaded font's buggy baseline offset is {buggy_offset:.2} px, \ + which does not exceed the cell height (18 px); the clipping bug does \ + not manifest on this font, so the test would pass for the wrong reason" + ); + return; + } + drop(font); + + let term = HeadlessTerminal::new(8, 2); + term.feed(b"M"); + let png = term + .render_png(0) + .expect("font was confirmed loadable above"); let img = image::load_from_memory(&png).expect("decode").to_rgba8(); let (img_w, img_h) = (img.width(), img.height()); let cell_h = img_h / 2;