From 7cfe77f3f0349dde739bb1af1c4fcb2467b3102f Mon Sep 17 00:00:00 2001 From: KOGA Mitsuhiro Date: Fri, 1 May 2026 20:07:13 +0900 Subject: [PATCH] fix emoji glyph placement on the terminal grid MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Emoji rendered slightly upper-right of its 2-cell slot because `grid_emit::ensure_glyph_by_id` rasterized at the text point size and placed the bitmap on the primary font's baseline (`cell_h - ascent + top`), using bearings from the emoji font's own coordinate system. The two metric systems don't compose, so the offset was unavoidable for any non-trivial emoji font. Also fixes a related bug in `find_best_font_match{,_strict}`: the symbol-map fast path hardcoded `is_emoji = false`, so config-pinned emoji families (e.g. `Segoe UI Emoji` via `[fonts.symbol-map]`) bypassed the emoji rasterization path entirely. Now inherits `is_emoji` from the mapped font's color-table detection. For emoji `ensure_glyph_by_id` now: - rasterizes at `min(2*cell_w, cell_h)` so the bitmap fills its 2-cell slot regardless of the text font size, - buckets the atlas key by that cell-derived size so it doesn't alias text rasterizations during resize, - centers the bitmap horizontally in `2*cell_w` and vertically in `cell_h` (matches the symbol-centering rationale in changelog 42 for PUA constraint glyphs). Hot path is unchanged — atlas lookups still hit on the first call per emoji glyph and stay cached. --- rio-grid/src/lib.rs | 57 +++++++++++++++++++++++++++++++++------ sugarloaf/src/font/mod.rs | 26 ++++++++++++++++-- 2 files changed, 73 insertions(+), 10 deletions(-) diff --git a/rio-grid/src/lib.rs b/rio-grid/src/lib.rs index 713b7f0326..126bd946c3 100644 --- a/rio-grid/src/lib.rs +++ b/rio-grid/src/lib.rs @@ -2372,6 +2372,7 @@ pub fn build_row_fg( glyph_id, size_bucket, size_u16, + cell_w, cell_h, ascent_px, is_emoji, @@ -2688,6 +2689,7 @@ fn emit_preedit_cluster( glyph_id, size_bucket, size_u16, + cell_w, cell_h, ascent_px, is_emoji, @@ -2930,6 +2932,18 @@ fn emit_strikethroughs( /// Look up or rasterize-and-insert a glyph into the grid atlas by /// `glyph_id`. Platform-agnostic entry point; cfg branches inside to /// call the CT or swash rasterizer. +/// +/// Two placement modes: +/// - **Text** (non-emoji): glyph rasterized at the text point size, +/// placed on the primary font's baseline via `cell_h - ascent + top`. +/// - **Emoji**: glyph rasterized at a size chosen from cell metrics +/// (`min(2*cell_w, cell_h)`) so the bitmap fills its 2-cell slot, +/// then centered horizontally in `2*cell_w` and vertically in +/// `cell_h`. The emoji font's own bbox/ascent is irrelevant — its +/// metrics don't compose with the primary font's baseline, so cell- +/// centered placement (matches the symbol-centering rationale in +/// `compositor.rs` for PUA constraint glyphs) is the only metric- +/// independent answer. #[allow(clippy::too_many_arguments)] fn ensure_glyph_by_id( rasterizer: &mut GridGlyphRasterizer, @@ -2938,16 +2952,30 @@ fn ensure_glyph_by_id( glyph_id: u16, size_bucket: u16, size_u16: u16, + cell_w: f32, cell_h: f32, ascent_px: i16, is_emoji: bool, synthetic_italic: bool, synthetic_bold: bool, ) -> Option<(GlyphKey, AtlasSlot, bool)> { + // Emoji rasterizes at a cell-derived size, not the text size, so + // its atlas bucket has to differ — otherwise resizing-induced text- + // size changes that fall in the same `size_bucket` would alias the + // cached emoji bitmap. + let (raster_size_u16, raster_size_bucket) = if is_emoji { + let target = (2.0 * cell_w).min(cell_h).max(1.0); + let s = target.round().clamp(1.0, u16::MAX as f32) as u16; + let b = (target * 4.0).round().clamp(0.0, u16::MAX as f32) as u16; + (s, b) + } else { + (size_u16, size_bucket) + }; + let key = GlyphKey { font_id, glyph_id: glyph_id as u32, - size_bucket, + size_bucket: raster_size_bucket, }; if let Some(slot) = grid.lookup_glyph(key) { return Some((key, slot, false)); @@ -2961,25 +2989,38 @@ fn ensure_glyph_by_id( rasterizer, font_id, glyph_id, - size_u16, + raster_size_u16, is_emoji, synthetic_bold, synthetic_italic, )?; let is_color = raw.is_color; - // Convert CG-convention `left`/`top` into grid-convention - // `bearing_y` = `cell_h - ascent + top`. See the long comment in - // the original macOS rasterizer for the geometry. - let bearing_y = { + let (bearing_x, bearing_y) = if is_emoji { + // Center the rasterized bitmap within the 2-cell slot. + // `bearing_y` is "bitmap top measured from cell bottom" — see + // `grid.metal:265-269` (`offset.y = cell_size.y - bearing_y`). + let slot_w = (2.0 * cell_w).round() as i32; + let slot_h = cell_h.round() as i32; + let bx = ((slot_w - raw.width as i32) / 2).clamp(i16::MIN as i32, i16::MAX as i32) + as i16; + let by = ((slot_h + raw.height as i32) / 2) + .clamp(i16::MIN as i32, i16::MAX as i32) as i16; + (bx, by) + } else { + // Convert CG-convention `left`/`top` into grid-convention + // `bearing_y` = `cell_h - ascent + top`. let top_i16 = raw.top.clamp(i16::MIN as i32, i16::MAX as i32) as i16; let cell_h_i16 = cell_h.round().clamp(0.0, i16::MAX as f32) as i16; - cell_h_i16.saturating_sub(ascent_px).saturating_add(top_i16) + let by = cell_h_i16.saturating_sub(ascent_px).saturating_add(top_i16); + let bx = raw.left.clamp(i16::MIN as i32, i16::MAX as i32) as i16; + (bx, by) }; + let raster = RasterizedGlyph { width: raw.width.min(u16::MAX as u32) as u16, height: raw.height.min(u16::MAX as u32) as u16, - bearing_x: raw.left.clamp(i16::MIN as i32, i16::MAX as i32) as i16, + bearing_x, bearing_y, bytes: &raw.bytes, }; diff --git a/sugarloaf/src/font/mod.rs b/sugarloaf/src/font/mod.rs index 5c469f9ebf..3bb217778f 100644 --- a/sugarloaf/src/font/mod.rs +++ b/sugarloaf/src/font/mod.rs @@ -653,7 +653,18 @@ impl FontLibraryData { if let Some(symbol_maps) = &self.symbol_maps { for symbol_map in symbol_maps { if symbol_map.range.contains(&ch) { - return Some((symbol_map.font_index, false)); + // Inherit `is_emoji` from the mapped font's color-table + // detection so config-pinned emoji families (e.g. + // `Segoe UI Emoji` via `[fonts.symbol-map]`) flow into + // the wide-cell rasterization / centering path in + // `grid_emit::ensure_glyph_by_id`. Hardcoding `false` + // here was a bug — it bypassed all the emoji-specific + // placement. + let is_emoji = self + .try_get(&symbol_map.font_index) + .map(|fd| fd.is_emoji) + .unwrap_or(false); + return Some((symbol_map.font_index, is_emoji)); } } } @@ -716,7 +727,18 @@ impl FontLibraryData { if let Some(symbol_maps) = &self.symbol_maps { for symbol_map in symbol_maps { if symbol_map.range.contains(&ch) { - return Some((symbol_map.font_index, false)); + // Inherit `is_emoji` from the mapped font's color-table + // detection so config-pinned emoji families (e.g. + // `Segoe UI Emoji` via `[fonts.symbol-map]`) flow into + // the wide-cell rasterization / centering path in + // `grid_emit::ensure_glyph_by_id`. Hardcoding `false` + // here was a bug — it bypassed all the emoji-specific + // placement. + let is_emoji = self + .try_get(&symbol_map.font_index) + .map(|fd| fd.is_emoji) + .unwrap_or(false); + return Some((symbol_map.font_index, is_emoji)); } } }