fix(terminal): render emoji presentation sequences at their real width

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.
2026-09-22 00:02:23 +00:00 · 2026-07-27 10:35:58 +08:00
parent 313c1c3c8b
commit f5b4a65ec4
4 changed files with 176 additions and 13 deletions
@@ -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",
@@ -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
@@ -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<Box<[char]>>,
    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<RowSeg> {
            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<RowSeg> {
                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<RowSeg> {
                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('界');
@@ -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 =