From f5b4a65ec4855072890db6a6776cdd7a16313bec Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Mon, 27 Jul 2026 10:35:58 +0800 Subject: [PATCH] fix(terminal): render emoji presentation sequences at their real width MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An emoji written as base + U+FE0F rendered wrong twice over. Both halves came from the variation selector arriving as a zero-width combining mark after the column budget was already spent. Width, in the `alacritty_terminal` pin (bumped to the fork's b79e704): `input` reserves columns one `char` at a time, so a base whose East Asian Width is Neutral -- U+2764 in `❀️`, U+1F5C2 in `πŸ—‚οΈ`, U+26A0 in `⚠️` -- kept the single column it was given, its glyph bled over the next cell, and every column after it on the line shifted left by one. The fork re-scores the sequence with `UnicodeWidthStr`, which is where UTS #51's width-2 rule lives, and widens the cell to match. Presentation, here: `snapshot_cell` copied only `cell.c` into `RenderCell`, so `cell.zerowidth()` was dropped before the shaper ever saw it. `❀` and `❀\u{FE0F}` reached gpui as the same string and picked the same text-presentation face -- a black heart where every other terminal shows a red one. `RenderCell` now carries the marks and a new `RowSeg::Cluster` shapes them with their base. That restores every combining mark, not just the selectors: `e` + U+0301 was being dropped the same way. A marked cell never joins a batched run. Marks add characters without adding columns, which is exactly the correspondence `force_width` uses to pin one glyph per column in a `Run` or `Wide` segment. Fixes #203. --- Cargo.lock | 2 +- Cargo.toml | 28 ++++++---- src/terminal/element.rs | 115 ++++++++++++++++++++++++++++++++++++++-- src/terminal/remote.rs | 44 +++++++++++++++ 4 files changed, 176 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 78da59d6..e0521fd2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -196,7 +196,7 @@ dependencies = [ [[package]] name = "alacritty_terminal" version = "0.26.1-dev" -source = "git+https://github.com/l0ng-ai/alacritty?rev=22b1d74842732f6ea30d8f4eec768b3ac659d0b6#22b1d74842732f6ea30d8f4eec768b3ac659d0b6" +source = "git+https://github.com/l0ng-ai/alacritty?rev=b79e70484b13308ce766a763531ac25a8302c012#b79e70484b13308ce766a763531ac25a8302c012" dependencies = [ "base64", "bitflags 2.13.0", diff --git a/Cargo.toml b/Cargo.toml index f0590e1d..75633a1f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -69,15 +69,25 @@ reqwest_client = { git = "https://github.com/zed-industries/zed", rev = "1d217ee # (`terminal::remote`) for the VT parser + grid (`Term`/`ansi::Processor`) that # renders the mirror. The daemon's PTY itself is driven by `portable-pty` below. # -# The `tty7` branch is Zed's `fcf32fe` (the rev Zed pins) plus one commit: -# `push_keyboard_mode` capped its stack by removing from `title_stack` instead of -# `keyboard_mode_stack` β€” a copy-paste slip from `push_title` that compiles because -# both are `Vec`s. With `kitty_keyboard` on (see `terminal_config_from_user`) every -# overflowing push silently drops a saved title, and once the title stack is empty -# `Vec::remove(0)` panics β€” 4097 unpopped `CSI > 1 u` pushes, about 20KB of output, -# kill the reader thread and freeze the pane. Still present on alacritty master as -# of 852e971; drop this fork once it lands upstream. -alacritty_terminal = { git = "https://github.com/l0ng-ai/alacritty", rev = "22b1d74842732f6ea30d8f4eec768b3ac659d0b6" } +# The `tty7` branch is Zed's `fcf32fe` (the rev Zed pins) plus two commits, both +# still missing from alacritty master as of 852e971: +# +# 1. `push_keyboard_mode` capped its stack by removing from `title_stack` instead +# of `keyboard_mode_stack` β€” a copy-paste slip from `push_title` that compiles +# because both are `Vec`s. With `kitty_keyboard` on (see +# `terminal_config_from_user`) every overflowing push silently drops a saved +# title, and once the title stack is empty `Vec::remove(0)` panics β€” 4097 +# unpopped `CSI > 1 u` pushes, about 20KB of output, kill the reader thread and +# freeze the pane. +# 2. `input` reserves columns per `char`, so an emoji written as base + U+FE0F +# (`❀️`, `πŸ—‚οΈ`, `⚠️` β€” any base whose East Asian Width is Neutral) gets one +# column instead of two and shifts the rest of the line left by one. The fork +# re-scores the sequence with `UnicodeWidthStr` and widens the cell (issue +# #203). +# +# Each patch has a guard test in `src/terminal/remote.rs`; drop this fork once +# both land upstream. +alacritty_terminal = { git = "https://github.com/l0ng-ai/alacritty", rev = "b79e70484b13308ce766a763531ac25a8302c012" } # Desktop notifications driven by OSC 9 / OSC 777 escape sequences. Cross-platform; # the macOS backend uses the deprecated NSUserNotification (weak β€” a completion diff --git a/src/terminal/element.rs b/src/terminal/element.rs index 6d8e173d..2a6135f0 100644 --- a/src/terminal/element.rs +++ b/src/terminal/element.rs @@ -42,6 +42,11 @@ enum UnderlineKind { #[derive(Clone)] struct RenderCell { c: char, + /// Combining marks the emulator stacked on this cell: accents, variation + /// selectors, ZWJ-sequence tails. They carry no column of their own, but + /// dropping them changes what the shaper sees β€” `❀` and `❀\u{FE0F}` pick + /// different faces β€” so they ride along and get shaped with their base. + marks: Option>, fg: Hsla, bg: Hsla, draw_bg: bool, @@ -66,6 +71,7 @@ impl Default for RenderCell { fn default() -> Self { Self { c: ' ', + marks: None, fg: Hsla::default(), bg: Hsla::default(), draw_bg: false, @@ -195,6 +201,7 @@ fn snapshot_cell( let mut rc = RenderCell { c: cell.c, + marks: cell.zerowidth().map(Box::from), fg: to_hsla(fgc), bg: to_hsla(bgc), draw_bg, @@ -442,9 +449,11 @@ impl GlyphStyle { } /// A blank cell paints no glyph (and today, no underline either β€” see -/// `GlyphStyle::draws_on_blanks`). +/// `GlyphStyle::draws_on_blanks`). A blank carrying combining marks is not +/// blank: a mark that opens a line lands on the space the grid starts with, and +/// it still has to be drawn. fn is_blank(cell: &RenderCell) -> bool { - cell.c == '\0' || cell.c == ' ' + (cell.c == '\0' || cell.c == ' ') && cell.marks.is_none() } /// One paintable piece of a row produced by [`segment_row`]. @@ -472,6 +481,16 @@ enum RowSeg { /// drawing, accented Latin, …) that may route to a fallback face whose /// advance isn't the cell width. Solo { col: usize }, + /// A base plus the combining marks stacked on it, shaped as one string so + /// the marks reach the shaper. Never batched with neighbours: the marks add + /// characters without adding columns, which is exactly the correspondence + /// `force_width` relies on in a [`RowSeg::Run`] or [`RowSeg::Wide`]. + Cluster { + col: usize, + /// Columns the base occupies β€” 2 once the grid marked it wide. + cells: usize, + text: String, + }, } /// Split one grid row into paintable segments. @@ -499,6 +518,21 @@ fn segment_row(row: &[RenderCell]) -> Vec { col += 1; continue; } + // Combining marks come first: they can sit on an ASCII base too, and + // either way the whole cluster has to reach the shaper in one string. + if let Some(marks) = &cell.marks { + let cells = if col + 1 < row.len() && row[col + 1].spacer { + 2 + } else { + 1 + }; + let mut text = String::with_capacity(1 + marks.len()); + text.push(cell.c); + text.extend(marks.iter()); + segs.push(RowSeg::Cluster { col, cells, text }); + col += cells; + continue; + } if !cell.c.is_ascii_graphic() { // Wide (two-column) glyph? The trailing spacer is the grid's own // width marker, so no Unicode width guessing is needed. @@ -513,6 +547,7 @@ fn segment_row(row: &[RenderCell]) -> Vec { while col + 1 < row.len() && !row[col].spacer && !is_blank(&row[col]) + && row[col].marks.is_none() && !row[col].c.is_ascii_graphic() && row[col + 1].spacer && GlyphStyle::of(&row[col]) == style @@ -552,7 +587,11 @@ fn segment_row(row: &[RenderCell]) -> Vec { col += 1; continue; } - if c.spacer || !c.c.is_ascii_graphic() || GlyphStyle::of(c) != style { + if c.spacer + || c.marks.is_some() + || !c.c.is_ascii_graphic() + || GlyphStyle::of(c) != style + { break; } for _ in 0..gap { @@ -797,6 +836,16 @@ fn paint_glyphs( } (col, 1, char_string(cell.c), None, true) } + // Same pinning as the batched runs, just for one base: two + // columns get `force_width` so a fallback emoji face can't + // drift, one column paints at the origin like `Solo`. + RowSeg::Cluster { col, cells, text } => ( + col, + cells, + SharedString::from(text), + (cells == 2).then(|| geom.cell_width * 2.), + cells == 1, + ), }; let style = GlyphStyle::of(&buf[row_base + start]); @@ -2064,6 +2113,66 @@ mod tests { assert!(segment_row(&row).is_empty()); } + fn cluster(col: usize, cells: usize, text: &str) -> RowSeg { + RowSeg::Cluster { + col, + cells, + text: text.to_string(), + } + } + + /// Combining marks reach the shaper attached to their base, in one string. + #[test] + fn segment_row_shapes_combining_marks_with_their_base() { + // Single column: `e` + U+0301 β†’ Γ©. + let mut row = vec![cell('a'), cell('e'), cell('b')]; + row[1].marks = Some(Box::from(['\u{0301}'])); + assert_eq!( + segment_row(&row), + [run(0, 1, "a"), cluster(1, 1, "e\u{0301}"), run(2, 1, "b"),] + ); + + // Two columns: the emulator widened the base, so the cluster owns the + // spacer too (❀ + U+FE0F). + let mut row = wide_cells("\u{2764}"); + row[0].marks = Some(Box::from(['\u{FE0F}'])); + assert_eq!(segment_row(&row), [cluster(0, 2, "\u{2764}\u{FE0F}")]); + } + + /// A marked cell never joins a batch: marks add characters without adding + /// columns, which would desync `force_width`'s glyph-per-column pinning. + #[test] + fn segment_row_never_batches_a_marked_cell() { + // ASCII run splits around it. + let mut row: Vec<_> = "abc".chars().map(cell).collect(); + row[1].marks = Some(Box::from(['\u{0301}'])); + assert_eq!( + segment_row(&row), + [run(0, 1, "a"), cluster(1, 1, "b\u{0301}"), run(2, 1, "c"),] + ); + + // Wide run splits around it. + let mut row = wide_cells("δ½ ε₯½δΈ–"); + row[2].marks = Some(Box::from(['\u{FE0F}'])); + assert_eq!( + segment_row(&row), + [ + wide(0, 2, "δ½ "), + cluster(2, 2, "ε₯½\u{FE0F}"), + wide(4, 2, "δΈ–"), + ] + ); + } + + /// A mark opening a line lands on the space the grid starts with β€” that + /// cell still has ink, so it must not be skipped as blank. + #[test] + fn segment_row_keeps_a_blank_that_carries_marks() { + let mut row = vec![cell(' '), cell(' ')]; + row[0].marks = Some(Box::from(['\u{0301}'])); + assert_eq!(segment_row(&row), [cluster(0, 1, " \u{0301}")]); + } + #[test] fn char_string_memoizes_per_char() { let a = char_string('η•Œ'); diff --git a/src/terminal/remote.rs b/src/terminal/remote.rs index 5971763a..b281b8c3 100644 --- a/src/terminal/remote.rs +++ b/src/terminal/remote.rs @@ -1732,6 +1732,50 @@ mod tests { ); } + /// Guards the `alacritty_terminal` pin, not our own code. Upstream reserves + /// columns one `char` at a time, so an emoji written as base + `U+FE0F` + /// (`❀️`, `πŸ—‚οΈ`, `⚠️` β€” anything whose base is East Asian Width Neutral) gets + /// one column instead of two and shoves the rest of the line left by one. + /// Our fork re-scores the sequence and widens the cell; a bump back to an + /// unpatched rev must fail here rather than in the field (issue #203). + #[test] + fn emoji_presentation_sequences_reserve_two_columns() { + let (client_side, mut daemon_side) = UnixStream::pair().unwrap(); + let term = RemoteTerminal::from_stream(client_side, TermSize::new(80, 24)).unwrap(); + + // `x` marks where the emoji ended: column 2 if ❀️ got its two columns, + // column 1 if the selector was counted as free. + DaemonMsg::Output("\u{2764}\u{FE0F}x".as_bytes().to_vec()) + .encode(&mut daemon_side) + .unwrap(); + daemon_side.flush().unwrap(); + + let mut row = String::new(); + for _ in 0..200 { + { + let t = term.term.lock(); + let grid = t.grid(); + row.clear(); + for col in 0..3usize { + row.push( + grid[alacritty_terminal::index::Line(0)] + [alacritty_terminal::index::Column(col)] + .c, + ); + } + } + if row.contains('x') { + break; + } + std::thread::sleep(std::time::Duration::from_millis(5)); + } + + assert_eq!( + row, "\u{2764} x", + "❀️ must hold two columns (glyph + spacer) before the next glyph" + ); + } + #[test] fn spawn_retry_only_for_daemon_disconnects() { let eof: anyhow::Error =