rio-vt: intern extras by content, copy-on-write attach

frontends/rioterm/src/hints.rs

@@ -254,11 +254,12 @@ impl HintState {
     ) {
         // Walk the visible region looking for OSC 8 hyperlink spans.
         //
-        // After the cell repack, hyperlinks live in the per-grid
-        // `extras_table`. Each cell carries an `extras_id: u16`; cells
-        // in the same hyperlink span share that id. We compare ids
-        // (cheap u16 compare) to find the start and end of each span,
-        // then look up the URI once via `Crosswords::cell_hyperlink`.
+        // Spans are found by comparing the hyperlink itself, not the
+        // cell's `extras_id`: extras slots are interned by content, so
+        // a cell that also carries combining marks holds a different
+        // slot than its neighbors while belonging to the same link.
+        // `Hyperlink` is an `Arc` around (id, uri); the comparison is
+        // content equality, matching the OSC 8 `id=` semantics.
         let grid = &term.grid;
         let display_offset = grid.display_offset();
         let visible_lines = grid.screen_lines();
@@ -272,20 +273,20 @@ impl HintState {
             let mut col = 0usize;
             let cols = grid.columns();
             while col < cols {
-                let id = match term.cell_hyperlink_id(line, Column(col)) {
-                    Some(id) => id,
+                let link = match term.cell_hyperlink(line, Column(col)) {
+                    Some(link) => link,
                     None => {
                         col += 1;
                         continue;
                     }
                 };
 
                 // Found the start of a hyperlink span. Walk forward
-                // until the extras_id changes.
+                // while the cells carry the same hyperlink.
                 let start_col = col;
                 let mut end_col = col;
                 while end_col < cols
-                    && term.cell_hyperlink_id(line, Column(end_col)) == Some(id)
+                    && term.cell_hyperlink(line, Column(end_col)).as_ref() == Some(&link)
                 {
                     end_col += 1;
                 }

frontends/rioterm/src/screen/mod.rs

@@ -2105,32 +2105,32 @@ impl Screen<'_> {
 
         // Look up the cell's hyperlink via the per-grid extras table.
         // Cells in the same OSC 8 span share an `extras_id`, so we
-        // walk left/right comparing ids (cheap u16 compare) to find
-        // the span boundaries, then look up the URI once.
-        let id = terminal.cell_hyperlink_id(point.row, point.col)?;
+        // walk left/right comparing the hyperlink itself (extras slots
+        // are interned by content, so a cell with combining marks has
+        // a different id while belonging to the same link) to find the
+        // span boundaries.
+        let hyperlink = terminal.cell_hyperlink(point.row, point.col)?;
 
         let mut start_col = point.col;
         let mut end_col = point.col;
 
         while start_col > rio_backend::crosswords::pos::Column(0) {
             let prev_col = start_col - 1;
-            if terminal.cell_hyperlink_id(point.row, prev_col) == Some(id) {
+            if terminal.cell_hyperlink(point.row, prev_col).as_ref() == Some(&hyperlink) {
                 start_col = prev_col;
             } else {
                 break;
             }
         }
         while end_col < grid.columns() - 1 {
             let next_col = end_col + 1;
-            if terminal.cell_hyperlink_id(point.row, next_col) == Some(id) {
+            if terminal.cell_hyperlink(point.row, next_col).as_ref() == Some(&hyperlink) {
                 end_col = next_col;
             } else {
                 break;
             }
         }
 
-        let hyperlink = terminal.cell_hyperlink(point.row, point.col)?;
-
         // Build a synthetic hint config so the rest of the hint
         // pipeline (highlighting, click action) treats this just like
         // a regex/url match.

rio-vt/src/crosswords/grid/mod.rs

@@ -126,6 +126,16 @@ impl ReflowRemap {
 pub struct ExtrasTable {
     slots: Vec<Option<crate::crosswords::square::Extras>>,
     free: Vec<u16>,
+    /// Content-hash interning: identical extras share one slot.
+    /// Emoji and combining sequences repeat constantly, and every
+    /// entry here used to burn a fresh slot toward the `u16::MAX`
+    /// cap (whose overflow silently drops extras). Invariant: every
+    /// live slot's content maps back to its id, maintained by
+    /// `alloc` and `sweep_unmarked`/`free`/`clear`. Interning also
+    /// means slots are shared — mutating one in place would edit
+    /// every referencing cell, so writers must copy-on-write
+    /// (clone, modify, re-alloc).
+    lookup: rustc_hash::FxHashMap<crate::crosswords::square::Extras, u16>,
     /// Allocations since the last mark-and-sweep. The table holds
     /// hyperlink and zero-width data whose slots stay referenced by
     /// cells until their rows scroll off the ring; without a periodic
@@ -148,6 +158,7 @@ impl ExtrasTable {
         Self {
             slots: vec![None],
             free: Vec::new(),
+            lookup: rustc_hash::FxHashMap::default(),
             allocs_since_reclaim: 0,
         }
     }
@@ -166,13 +177,19 @@ impl ExtrasTable {
         self.slots.get_mut(id as usize)?.as_mut()
     }
 
-    /// Allocate a new extras slot, returning its id (always non-zero).
+    /// Return the slot holding `extras`, interning by content: repeat
+    /// content reuses its existing slot, new content allocates one.
+    /// The returned id is always non-zero (0 on slot exhaustion).
     pub fn alloc(
         &mut self,
         extras: crate::crosswords::square::Extras,
     ) -> crate::crosswords::square::ExtrasId {
+        if let Some(&id) = self.lookup.get(&extras) {
+            return id;
+        }
         self.allocs_since_reclaim += 1;
         if let Some(id) = self.free.pop() {
+            self.lookup.insert(extras.clone(), id);
             self.slots[id as usize] = Some(extras);
             return id;
         }
@@ -181,6 +198,7 @@ impl ExtrasTable {
             return 0;
         }
         let id = self.slots.len() as u16;
+        self.lookup.insert(extras.clone(), id);
         self.slots.push(Some(extras));
         id
     }
@@ -205,7 +223,9 @@ impl ExtrasTable {
         for id in 1..self.slots.len() {
             let marked = live[id / 64] & (1 << (id % 64)) != 0;
             if !marked && self.slots[id].is_some() {
-                self.slots[id] = None;
+                if let Some(extras) = self.slots[id].take() {
+                    self.lookup.remove(&extras);
+                }
                 self.free.push(id as u16);
             }
         }
@@ -217,7 +237,8 @@ impl ExtrasTable {
             return;
         }
         if let Some(slot) = self.slots.get_mut(id as usize) {
-            if slot.take().is_some() {
+            if let Some(extras) = slot.take() {
+                self.lookup.remove(&extras);
                 self.free.push(id);
             }
         }
@@ -227,6 +248,7 @@ impl ExtrasTable {
         self.slots.clear();
         self.slots.push(None);
         self.free.clear();
+        self.lookup.clear();
     }
 }
 

rio-vt/src/crosswords/mod.rs

@@ -3236,21 +3236,22 @@ impl<U: EventListener> Handler for Crosswords<U> {
                 column.0 = column.saturating_sub(1);
             }
 
+            // Copy-on-write: slots are interned and shared — every
+            // cell written under one OSC 8 template references the
+            // same slot, so pushing into it in place would attach the
+            // mark to the whole hyperlink span. Clone, extend, and
+            // re-intern instead; the marked cell gets its own
+            // (marks + hyperlink) slot and its neighbors keep theirs.
+            let existing_id = self.grid[row][column].extras_id();
+            let mut extras = existing_id
+                .and_then(|id| self.grid.extras_table.get(id).cloned())
+                .unwrap_or_default();
+            extras.zerowidth.push(c);
+            let id = self.grid.alloc_extras(extras);
             let cell = &mut self.grid[row][column];
-            let existing_id = cell.extras_id();
-            if let Some(id) = existing_id {
-                if let Some(extras) = self.grid.extras_table.get_mut(id) {
-                    extras.zerowidth.push(c);
-                }
-            } else {
-                let mut extras = crate::crosswords::square::Extras::default();
-                extras.zerowidth.push(c);
-                let id = self.grid.alloc_extras(extras);
-                let cell = &mut self.grid[row][column];
-                cell.set_extras_id(Some(id));
-                cell.insert_cell_flag(CellFlags::GRAPHEME);
-                self.grid[row].has_extras = true;
-            }
+            cell.set_extras_id(Some(id));
+            cell.insert_cell_flag(CellFlags::GRAPHEME);
+            self.grid[row].has_extras = true;
             return;
         }
 
@@ -7404,6 +7405,78 @@ mod tests {
         assert!(!cw.grid.cursor.should_wrap);
     }
 
+    /// Identical extras content interns into one slot: the u16 id
+    /// space stops being a per-occurrence budget.
+    #[test]
+    fn extras_intern_by_content() {
+        use crate::performer::handler::Handler;
+        let mut cw = new_term(10, 2);
+        cw.input('e');
+        cw.input('\u{301}');
+        cw.input('e');
+        cw.input('\u{301}');
+        let a = cw.grid[Line(0)][Column(0)].extras_id().unwrap();
+        let b = cw.grid[Line(0)][Column(1)].extras_id().unwrap();
+        assert_eq!(a, b);
+        // Different content gets its own slot.
+        cw.input('e');
+        cw.input('\u{302}');
+        let c = cw.grid[Line(0)][Column(2)].extras_id().unwrap();
+        assert_ne!(a, c);
+    }
+
+    /// A combining mark typed inside an OSC 8 hyperlink must attach to
+    /// its own cell only. The old in-place mutation edited the shared
+    /// template slot, leaking the mark into every cell of the span —
+    /// and the marked cell must keep the hyperlink.
+    #[test]
+    fn combining_mark_inside_hyperlink_stays_on_one_cell() {
+        use crate::performer::handler::Handler;
+        let mut cw = new_term(10, 2);
+        let link = Hyperlink::new(None::<&str>, "https://example.com");
+        cw.set_hyperlink(Some(link.clone()));
+        cw.input('a');
+        cw.input('\u{301}');
+        cw.input('b');
+        cw.set_hyperlink(None);
+
+        // The marked cell carries mark + hyperlink.
+        let a_id = cw.grid[Line(0)][Column(0)].extras_id().unwrap();
+        let a = cw.grid.extras_table.get(a_id).unwrap();
+        assert_eq!(a.zerowidth, vec!['\u{301}']);
+        assert_eq!(a.hyperlink.as_ref(), Some(&link));
+
+        // Its neighbor keeps the pristine hyperlink-only slot.
+        let b_id = cw.grid[Line(0)][Column(1)].extras_id().unwrap();
+        let b = cw.grid.extras_table.get(b_id).unwrap();
+        assert!(b.zerowidth.is_empty());
+        assert_eq!(b.hyperlink.as_ref(), Some(&link));
+        assert_ne!(a_id, b_id);
+    }
+
+    /// The interning lookup must be purged when slots are swept, or a
+    /// re-alloc of old content would return a dead slot id.
+    #[test]
+    fn reclaim_purges_interning_lookup() {
+        use crate::performer::handler::Handler;
+        let mut cw = new_term(10, 2);
+        cw.input('e');
+        cw.input('\u{301}');
+        let content = cw
+            .grid
+            .extras_table
+            .get(cw.grid[Line(0)][Column(0)].extras_id().unwrap())
+            .unwrap()
+            .clone();
+        // Drop the only reference and sweep.
+        cw.clear_screen(ClearMode::All);
+        cw.grid.reclaim_extras();
+        // Re-alloc of the same content must yield a live slot.
+        let id = cw.grid.alloc_extras(content.clone());
+        assert_ne!(id, 0);
+        assert_eq!(cw.grid.extras_table.get(id), Some(&content));
+    }
+
     #[test]
     fn vs16_at_last_column_preserves_base_extras() {
         use crate::performer::handler::Handler;
@@ -7421,10 +7494,15 @@ mod tests {
 
         cw.input('\u{FE0F}');
 
-        // The wide base on the new row should still carry the same extras
-        // entry (the ZWJ we attached earlier).
+        // The wide base on the new row still carries the earlier ZWJ
+        // plus the VS16 that widened it. Slots are interned with
+        // copy-on-write now, so the id may change; the content is the
+        // contract (comparing ids would pin the old in-place-mutation
+        // behavior, which leaked pushes into shared slots).
         let moved_extras = cw.grid[Line(1)][Column(0)].extras_id();
-        assert_eq!(moved_extras, original_extras);
+        assert!(moved_extras.is_some());
+        let extras = cw.grid.extras_table.get(moved_extras.unwrap()).unwrap();
+        assert_eq!(extras.zerowidth, vec!['\u{200D}', '\u{FE0F}']);
         assert_eq!(cw.grid[Line(1)][Column(0)].wide(), Wide::Wide);
         assert_eq!(cw.grid[Line(0)][Column(2)].wide(), Wide::LeadingSpacer);
     }

rio-vt/src/crosswords/square.rs

@@ -199,7 +199,7 @@ pub type ExtrasId = u16;
 
 /// Storage for the rare per-cell data that used to live inside `CellExtra`.
 /// Allocated only for cells that need it; pooled in a `Vec` on the grid.
-#[derive(Default, Debug, Clone, PartialEq, Eq)]
+#[derive(Default, Debug, Clone, PartialEq, Eq, Hash)]
 pub struct Extras {
     pub zerowidth: Vec<char>,
     pub hyperlink: Option<Hyperlink>,