From 40907470a7fda77c6a268647d942c2305c579a85 Mon Sep 17 00:00:00 2001 From: nakulbh Date: Thu, 30 Jul 2026 00:45:12 +0530 Subject: [PATCH 1/2] fix(notifications): wire up OSC notifications and fix silent macOS delivery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Desktop notifications reached Notification Center on macOS but never popped up a banner, and OSC-triggered notifications (e.g. "Claude is waiting for your input") never fired at all, even though the scanner and app-side plumbing for them were designed and documented (docs/guide/16-notifications.md, docs/guide/07-osc-notifications.md) but never actually wired together — TerminalPane fed PTY bytes to the terminal grid only, dropping them on the floor as far as notifications were concerned. - rmux-terminal/osc.rs: OSC 9 is overloaded — iTerm2/ConEmu also use it for progress bars (`OSC 9;4;[;]`), which is why this was disabled previously (produced junk notifications like "4;0;"). Add a targeted filter that recognizes and drops only that exact shape, so real notification text is unaffected. - rmux-app: give TerminalPane its own OscScanner over the same PTY bytes it feeds to TermState (scanning never mutates the stream), and thread completed notifications up through PaneNode -> Workspace -> WorkspaceManager -> RmuxApp, tagged with (workspace_id, pane_id), to the same NotificationManager::add() path the CLI's `notification.create` already used. - notifications/mod.rs: on macOS, mac-notification-sys silently posts every notification as com.apple.Finder when the running binary isn't inside a registered bundle (i.e. any dev/debug build) — delivery "succeeds" so it lands in Notification Center's history, but no banner ever animates, since most users have long muted Finder's routine trash/eject alerts. Explicitly claim rmux's own identity (matching the bundle id scripts/install.sh registers), falling back to Terminal's when rmux.app was never installed. Also set appname/icon on Linux so notifications group under "rmux" instead of a blank/PID-derived name. Co-Authored-By: Claude Sonnet 5 --- crates/rmux-app/src/app.rs | 40 ++++++++++++++++++++++-- crates/rmux-app/src/notifications/mod.rs | 40 ++++++++++++++++++++++++ crates/rmux-app/src/ui/terminal_pane.rs | 31 +++++++++++------- crates/rmux-app/src/workspace/mod.rs | 16 +++++++--- crates/rmux-app/src/workspace/model.rs | 4 +-- crates/rmux-app/src/workspace/splits.rs | 33 ++++++++++++------- crates/rmux-terminal/src/osc.rs | 33 +++++++++++++++++++ docs/API_REFERENCE.md | 2 +- docs/guide/16-notifications.md | 21 ++++++++++++- 9 files changed, 186 insertions(+), 34 deletions(-) diff --git a/crates/rmux-app/src/app.rs b/crates/rmux-app/src/app.rs index 9972ca0..aa2ec94 100644 --- a/crates/rmux-app/src/app.rs +++ b/crates/rmux-app/src/app.rs @@ -460,13 +460,17 @@ impl eframe::App for RmuxApp { ctx.send_viewport_cmd(egui::ViewportCommand::InnerSize(egui::vec2(size[0], size[1]))); } - // Process PTY output for all terminal panes (exit detection, grid). - // OSC → notification generation is disabled for now. + // Process PTY output for all terminal panes (exit detection, grid, + // OSC 9/99/777 notifications like "Claude is waiting for your input"). // Wake immediately when bytes arrived so key→nvim paint is not gated // solely on the 16 ms cursor-blink timer. - if self.workspace_manager.process_all_panes() { + let (any_output, osc_notifications) = self.workspace_manager.process_all_panes(); + if any_output { ctx.request_repaint(); } + for (workspace_id, pane_id, notification) in osc_notifications { + self.add_pane_notification(workspace_id, pane_id, notification); + } // cmux-style dynamic sidebar titles. The underlying `ps` / `git` / `gh` // work happens on the probe thread; here we only adopt its results and @@ -701,6 +705,36 @@ impl RmuxApp { let _ = self.api_event_tx.send(ApiEvent::new(event, data)); } + /// Store an OSC-derived notification and publish it (agent "waiting for + /// input" style messages, build-finished banners, etc). + /// + /// Shares the exact same [`NotificationManager::add`] path as the CLI's + /// `notification.create` method, so it stores the row, fires the desktop + /// notification, and updates unread counts identically either way. + fn add_pane_notification( + &mut self, + workspace_id: u64, + pane_id: u64, + notification: rmux_terminal::OscNotification, + ) { + let id = self.notifications.add( + notification.title.clone(), + notification.body.clone(), + Some(pane_id), + Some(workspace_id), + ); + self.publish_event( + "notification", + json!({ + "id": id, + "title": notification.title, + "body": notification.body, + "pane_id": pane_id, + "workspace_id": workspace_id, + }), + ); + } + /// Create a workspace with a live terminal in its initial pane. /// /// Shared by the Cmd/Ctrl+N shortcut and the `workspace.create` API diff --git a/crates/rmux-app/src/notifications/mod.rs b/crates/rmux-app/src/notifications/mod.rs index 640348c..d1b6b44 100644 --- a/crates/rmux-app/src/notifications/mod.rs +++ b/crates/rmux-app/src/notifications/mod.rs @@ -63,7 +63,17 @@ impl DesktopNotifier for SystemNotifier { // Best-effort: a failed desktop notification must never // crash the app (e.g. no notification daemon on Linux). let outcome = std::panic::catch_unwind(|| { + #[cfg(target_os = "macos")] + ensure_macos_identity(); + let mut notification = notify_rust::Notification::new(); + // No-op on macOS (both notify-rust backends silently ignore + // `appname`) but required on Linux/XDG so the banner groups + // under "rmux" — matching the `rmux.desktop` entry / icon + // name `scripts/install.sh` installs — instead of a blank + // or PID-derived app name. + notification.appname("rmux"); + notification.icon("rmux"); notification.summary(&title); if let Some(body) = &body { notification.body(body); @@ -81,3 +91,33 @@ impl DesktopNotifier for SystemNotifier { } } } + +/// Give rmux its own macOS Notification Center identity instead of silently +/// posting as `com.apple.Finder`. +/// +/// `notify-rust`/`mac-notification-sys` require a registered bundle +/// identifier to post through `NSUserNotificationCenter`. When the running +/// binary isn't inside a proper `.app` bundle (any debug build, `cargo run`, +/// or a `cargo install`ed binary), the crate silently falls back to +/// `com.apple.Finder`'s identity on first send. Notifications then land in +/// Notification Center's history — proving delivery "worked" — but never +/// pop up as a banner, because Finder posts constant low-priority ejects/ +/// trash notifications and most users have long since muted its alert style. +/// +/// `com.nakulbh.rmux` matches the `CFBundleIdentifier` `scripts/install.sh` +/// registers for `~/Applications/rmux.app`. Launch Services keys identity by +/// bundle id, not by which binary is currently executing, so this succeeds +/// even from a raw dev binary as long as that `.app` has been installed once. +/// If it hasn't (fresh clone, never ran the installer), fall back to +/// `com.apple.Terminal` — always registered, and a more sensible identity for +/// a terminal multiplexer than Finder in the meantime. +/// +/// Idempotent and cheap to call repeatedly: `mac_notification_sys` guards the +/// underlying `setApplication` call with a process-wide `Once`, so only the +/// very first call (across every notification ever sent) does any work. +#[cfg(target_os = "macos")] +fn ensure_macos_identity() { + if notify_rust::set_application("com.nakulbh.rmux").is_err() { + let _ = notify_rust::set_application("com.apple.Terminal"); + } +} diff --git a/crates/rmux-app/src/ui/terminal_pane.rs b/crates/rmux-app/src/ui/terminal_pane.rs index bb407f5..c1ea5ef 100644 --- a/crates/rmux-app/src/ui/terminal_pane.rs +++ b/crates/rmux-app/src/ui/terminal_pane.rs @@ -7,7 +7,8 @@ use anyhow::Result; use image::ImageEncoder; use image::codecs::png::PngEncoder; use rmux_terminal::{ - CoalescedSize, InputMapper, PtyBackend, PtyError, TermState, TerminalRenderer, + CoalescedSize, InputMapper, OscNotification, OscScanner, PtyBackend, PtyError, TermState, + TerminalRenderer, }; use std::path::{Path, PathBuf}; use std::sync::mpsc; @@ -83,6 +84,8 @@ pub struct TerminalPane { input_mapper: InputMapper, /// Channel receiver for PTY output from background thread. pty_rx: mpsc::Receiver>, + /// Scans raw PTY bytes for OSC 9/99/777 notification sequences. + osc_scanner: OscScanner, /// Lets the PTY reader thread request an immediate UI frame. repaint: RepaintHandle, /// Whether this pane currently has keyboard focus. @@ -213,6 +216,7 @@ impl TerminalPane { snapshot: rmux_terminal::GridSnapshot::default(), input_mapper, pty_rx: rx, + osc_scanner: OscScanner::new(), repaint, has_focus: false, show_cursor: true, @@ -295,17 +299,18 @@ impl TerminalPane { /// Process any new PTY output from the background reader thread. /// - /// Drains the channel and feeds bytes into the terminal state. - /// Should be called once per frame before rendering. + /// Drains the channel, feeds bytes into the terminal state, and scans the + /// same bytes for OSC 9/99/777 notification sequences. Should be called + /// once per frame before rendering. /// - /// Returns `true` if any PTY bytes were applied (caller may request repaint). - pub fn process_pty_output(&mut self) -> bool { + /// Returns `(got_output, notifications)`: `got_output` is `true` if any + /// PTY bytes were applied (caller may request repaint); `notifications` + /// holds any OSC notifications completed in this call. + pub fn process_pty_output(&mut self) -> (bool, Vec) { let mut got_output = false; + let mut notifications = Vec::new(); while let Ok(data) = self.pty_rx.try_recv() { - // OSC notification scanning is intentionally disabled: OSC 9 is also - // used by iTerm2 progress bars (`OSC 9;4;…`), which produced junk - // entries like "4;0;" in the notification panel. Re-enable only with - // a tighter parser that rejects progress / non-notify sequences. + notifications.extend(self.osc_scanner.feed(&data)); self.state.feed_bytes(&data); got_output = true; } @@ -336,7 +341,7 @@ impl TerminalPane { // background probe worker and handed over via [`Self::apply_probe`] — // this function must never fork a process. - got_output + (got_output, notifications) } /// Send a queued startup command once the shell looks ready. @@ -444,8 +449,10 @@ impl TerminalPane { // Reader thread can wake us; keep Context current for that path. self.repaint.bind(ui.ctx()); - // Process any new PTY output (also done at app level for all panes). - if self.process_pty_output() { + // Process any new PTY output (also done at app level for all panes, + // which is the authoritative pass for OSC notifications — this call + // will almost always find the channel already drained). + if self.process_pty_output().0 { ui.ctx().request_repaint(); } diff --git a/crates/rmux-app/src/workspace/mod.rs b/crates/rmux-app/src/workspace/mod.rs index ac28e4c..ba4c30d 100644 --- a/crates/rmux-app/src/workspace/mod.rs +++ b/crates/rmux-app/src/workspace/mod.rs @@ -327,13 +327,21 @@ impl WorkspaceManager { /// Process PTY output for all panes across all workspaces. /// - /// Returns `true` if any pane applied new PTY bytes (request a repaint). - pub fn process_all_panes(&mut self) -> bool { + /// Returns `(any_output, notifications)`: `any_output` is `true` if any + /// pane applied new PTY bytes (request a repaint); `notifications` is + /// every OSC notification completed this call, tagged with the raw + /// `(workspace_id, pane_id)` of the pane that raised it. + pub fn process_all_panes(&mut self) -> (bool, Vec<(u64, u64, rmux_terminal::OscNotification)>) { let mut any = false; + let mut notifications = Vec::new(); for workspace in &mut self.workspaces { - any |= workspace.process_pty_outputs(); + let (got, ws_notes) = workspace.process_pty_outputs(); + any |= got; + let workspace_id = workspace.id.0; + notifications + .extend(ws_notes.into_iter().map(|(pane_id, note)| (workspace_id, pane_id, note))); } - any + (any, notifications) } /// Shell pids of every terminal in every workspace (probe batch input). diff --git a/crates/rmux-app/src/workspace/model.rs b/crates/rmux-app/src/workspace/model.rs index b6a4a9a..55a5ecf 100644 --- a/crates/rmux-app/src/workspace/model.rs +++ b/crates/rmux-app/src/workspace/model.rs @@ -211,8 +211,8 @@ impl Workspace { /// Process PTY output for all panes in this workspace. /// - /// Returns `true` if any pane applied new PTY bytes. - pub fn process_pty_outputs(&mut self) -> bool { + /// Returns `(any_output, notifications)` — see [`PaneNode::process_pty_outputs`]. + pub fn process_pty_outputs(&mut self) -> (bool, Vec<(u64, rmux_terminal::OscNotification)>) { self.root.process_pty_outputs() } diff --git a/crates/rmux-app/src/workspace/splits.rs b/crates/rmux-app/src/workspace/splits.rs index 20f5bb6..8802fc5 100644 --- a/crates/rmux-app/src/workspace/splits.rs +++ b/crates/rmux-app/src/workspace/splits.rs @@ -5,6 +5,7 @@ #![allow(dead_code)] +use rmux_terminal::OscNotification; use thiserror::Error; use super::surface::Surface; @@ -269,28 +270,38 @@ impl PaneNode { /// Walks both the legacy `terminal` slot and every multi-surface tab /// so exit detection works for Cmd+T terminals too. /// - /// OSC → notification generation is disabled (iTerm2 progress OSC 9;4 - /// was mis-parsed as junk notifications). - /// Returns `true` if any pane consumed PTY output this call. - pub fn process_pty_outputs(&mut self) -> bool { + /// Returns `(any_output, notifications)`: `any_output` is `true` if any + /// pane consumed PTY output this call; `notifications` tags each OSC + /// notification with the raw id of the leaf pane that raised it (a + /// multi-surface leaf tags every tab's notifications with the shared + /// leaf id — surfaces don't have their own notification routing yet). + pub fn process_pty_outputs(&mut self) -> (bool, Vec<(u64, OscNotification)>) { match self { - Self::Leaf { terminal, surfaces, .. } => { + Self::Leaf { id, terminal, surfaces, .. } => { let mut any = false; + let mut notes = Vec::new(); if let Some(t) = terminal.as_mut() { - any |= t.process_pty_output(); + let (got, found) = t.process_pty_output(); + any |= got; + notes.extend(found.into_iter().map(|n| (id.0, n))); } for surface in surfaces.iter_mut() { - any |= surface.terminal.process_pty_output(); + let (got, found) = surface.terminal.process_pty_output(); + any |= got; + notes.extend(found.into_iter().map(|n| (id.0, n))); } - any + (any, notes) } - Self::Browser { .. } => false, + Self::Browser { .. } => (false, Vec::new()), Self::Split { children, .. } => { let mut any = false; + let mut notes = Vec::new(); for child in children.iter_mut() { - any |= child.process_pty_outputs(); + let (got, found) = child.process_pty_outputs(); + any |= got; + notes.extend(found); } - any + (any, notes) } } } diff --git a/crates/rmux-terminal/src/osc.rs b/crates/rmux-terminal/src/osc.rs index 35d63ad..7459527 100644 --- a/crates/rmux-terminal/src/osc.rs +++ b/crates/rmux-terminal/src/osc.rs @@ -209,6 +209,7 @@ impl OscScanner { fn parse_notification(code: u32, payload: &[u8]) -> Option { let text = String::from_utf8_lossy(payload); match code { + 9 if is_iterm2_progress_payload(&text) => None, 9 => Some(OscNotification { title: text.into_owned(), body: None, kind: OscKind::Simple9 }), 99 => Some(parse_rich99(&text)), 777 => parse_legacy777(&text), @@ -216,6 +217,21 @@ fn parse_notification(code: u32, payload: &[u8]) -> Option { } } +/// Whether an OSC 9 payload is actually an iTerm2/ConEmu progress-bar update +/// (`OSC 9;4;st[;pr]`, e.g. `4;1;42`) rather than free-text notification body. +/// +/// OSC 9 is overloaded: most tools use it for plain notification text, but +/// iTerm2 and ConEmu also use it for build/task progress bars, where the +/// payload is `4;` or `4;;` (state 0-4, percent +/// 0-100). Without this filter those progress updates surfaced as junk +/// notifications like "4;0;" every time a percentage changed. +fn is_iterm2_progress_payload(payload: &str) -> bool { + let Some(rest) = payload.strip_prefix("4;") else { + return false; + }; + rest.split(';').all(|segment| segment.is_empty() || segment.bytes().all(|b| b.is_ascii_digit())) +} + /// Parse an OSC 99 payload: `;`-separated `k=v` segments where `p=` carries /// `title:body`. If no `p=` segment exists, the whole payload is the title. fn parse_rich99(payload: &str) -> OscNotification { @@ -415,6 +431,23 @@ mod tests { assert!(scanner.buf.len() <= MAX_PAYLOAD); } + #[test] + fn test_iterm2_progress_sequences_produce_no_notification() { + assert!(scan(b"\x1b]9;4;0\x07").is_empty()); + assert!(scan(b"\x1b]9;4;1;42\x07").is_empty()); + assert!(scan(b"\x1b]9;4;1;100\x1b\\").is_empty()); + assert!(scan(b"\x1b]9;4;\x07").is_empty()); + } + + #[test] + fn test_osc9_text_starting_with_four_is_not_mistaken_for_progress() { + // Only the exact `4;[;]` shape is progress; real + // notification text that happens to start with "4" must still fire. + let found = scan(b"\x1b]9;4 builds finished\x07"); + assert_eq!(found.len(), 1); + assert_eq!(found[0].title, "4 builds finished"); + } + #[test] fn test_invalid_utf8_payload_is_lossy_decoded() { let found = scan(b"\x1b]9;bad\xff\xfeutf8\x07"); diff --git a/docs/API_REFERENCE.md b/docs/API_REFERENCE.md index 8a11d7f..f714824 100644 --- a/docs/API_REFERENCE.md +++ b/docs/API_REFERENCE.md @@ -508,7 +508,7 @@ All return `anyhow::Result<()>`: | `WorkspaceManager::split_active_right()` | fn | `Result` — Horizontal split | | `WorkspaceManager::split_active_down()` | fn | `Result` — Vertical split | | `WorkspaceManager::close_active_pane()` | fn | `Result<()>` | -| `WorkspaceManager::process_all_panes()` | fn | Drain PTY output everywhere. Returns `Vec<(u64, u64, OscNotification)>` | +| `WorkspaceManager::process_all_panes()` | fn | Drain PTY output everywhere. Returns `(bool, Vec<(u64, u64, OscNotification)>)` — any output applied, and every completed OSC notification tagged `(workspace_id, pane_id)` | | `WorkspaceManager::close_exited_panes()` | fn | Auto-close panes with dead processes | | `WorkspaceManager::rename_workspace(id, name)` | fn | Rename workspace | | `WorkspaceManager::close_active_workspace()` | fn | `Result` — Error if last | diff --git a/docs/guide/16-notifications.md b/docs/guide/16-notifications.md index feca23c..33e70bd 100644 --- a/docs/guide/16-notifications.md +++ b/docs/guide/16-notifications.md @@ -42,6 +42,13 @@ pub fn with_system_notifier() -> Self { Boxed trait means tests can use fake notifier. +On macOS, `SystemNotifier` also claims a real Notification Center identity +(`com.nakulbh.rmux`, falling back to `com.apple.Terminal`) before the first +send. Without this, `mac-notification-sys` silently posts every notification +as `com.apple.Finder` — delivery "succeeds" (the notification lands in +Notification Center's history) but no banner ever pops up, since most users +have long since muted Finder's routine trash/eject alerts. + ## Add notification `add()` assigns id, emits desktop notification, stores row. @@ -120,12 +127,24 @@ pub fn mark_read(&mut self, id: u64) { TerminalPane scans shell output. App collects parsed notifications in `update()`. ```rust -let osc_notifications = self.workspace_manager.process_all_panes(); +let (any_output, osc_notifications) = self.workspace_manager.process_all_panes(); +if any_output { + ctx.request_repaint(); +} for (workspace_id, pane_id, notification) in osc_notifications { self.add_pane_notification(workspace_id, pane_id, notification); } ``` +`TerminalPane` runs its own [`OscScanner`](07-osc-notifications.md) over the +same bytes it feeds to `TermState` — scanning never mutates the stream, it +only watches. OSC 9 is overloaded: iTerm2/ConEmu also use it for progress +bars (`OSC 9;4;[;]`), so the scanner's `parse_notification` +recognizes and drops that exact shape before it ever reaches here — real +notification text starting with a literal "4" (rare, but possible) still +gets through, since only the strict `4;[;]` form is treated +as progress. + App stores and publishes event: ```rust From da2d965d9e3cedfaa5aa60271e23dad93ed78e5e Mon Sep 17 00:00:00 2001 From: nakulbh Date: Sat, 1 Aug 2026 04:07:48 +0530 Subject: [PATCH 2/2] fix(terminal): scrambled text after sleep/wake and TUIs not filling the pane MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two separate rendering bugs, both cross-platform (macOS + Linux) since neither depends on OS-specific code paths: - Scrambled/garbled text after resume from sleep: our own per-glyph Arc cache holds mesh vertices with UV coordinates baked in for egui's font texture atlas. egui rebuilds that atlas whole (DPI change, max_texture_side change, or simply filling past ~80% over a long session) — and in practice, the GPU context getting recreated after the OS suspends/resumes triggers the same rebuild. Our cached galleys kept pointing at the old atlas's packing, so painting them against the freshly repacked (differently laid out) atlas sampled whatever now happened to sit at those old coordinates: scrambled glyphs. Fix: track the atlas's Arc identity and drop the whole glyph cache whenever it changes, for any reason, on any platform — no sleep/wake-specific detection needed. - Full-screen TUIs (nvim, lazygit, btop, etc.) leaving a gap below and sometimes beside their content: row height was a flat `font_size * 1.15` guess, chosen to keep block-element TUIs (LazyVim's logo) looking tight. That guess ran ~13-15% taller than JetBrains Mono's real line height, so the cols/rows we told the PTY (and thus the TUI) was consistently undercounted versus the pane's actual pixel size. A full-screen TUI queries its size once at startup and only repaints on SIGWINCH, so it latched onto geometry smaller than the pane and never grew to fill it. Fix: measure the font's real row height instead of guessing. The original guess's actual job — making sure adjacent rows of block-drawing / box-drawing characters tile without seams — is now handled directly by rendering those characters as geometry sized to the cell (already true for block elements; extended here to the common single-line box-drawing set: ─│┌┐└┘├┤┬┴┼, used by nvim splits, nvim-tree, lazygit, btop borders), so it no longer depends on how a specific font's glyphs sit inside its own line metrics. Co-Authored-By: Claude Sonnet 5 --- crates/rmux-terminal/src/glyph_cache.rs | 85 ++++++++++++++- crates/rmux-terminal/src/renderer.rs | 131 +++++++++++++++++++++--- 2 files changed, 200 insertions(+), 16 deletions(-) diff --git a/crates/rmux-terminal/src/glyph_cache.rs b/crates/rmux-terminal/src/glyph_cache.rs index 425826d..66c3cb1 100644 --- a/crates/rmux-terminal/src/glyph_cache.rs +++ b/crates/rmux-terminal/src/glyph_cache.rs @@ -33,11 +33,18 @@ pub(crate) struct GlyphCache { map: HashMap>, /// Memoised `has_glyph` answers per `(char, bold)`. coverage: HashMap<(char, bool), bool>, + /// Identity of the font texture atlas the cached entries were produced + /// against (see [`Self::ensure_fresh`]). `None` before the first frame. + atlas_identity: Option, } impl GlyphCache { pub(crate) fn with_capacity(cap: usize) -> Self { - Self { map: HashMap::with_capacity(cap), coverage: HashMap::with_capacity(cap) } + Self { + map: HashMap::with_capacity(cap), + coverage: HashMap::with_capacity(cap), + atlas_identity: None, + } } pub(crate) fn clear(&mut self) { @@ -45,6 +52,36 @@ impl GlyphCache { self.coverage.clear(); } + /// Drop every cached entry if egui recreated its font texture atlas + /// since the last call. Call this once per `draw()`, before looking + /// anything up. + /// + /// egui rebuilds the whole atlas (and its own internal galley cache) + /// when `pixels_per_point` changes, `max_texture_side` changes, or the + /// atlas fills past ~80% (`epaint::text::Fonts::begin_pass`). Any of + /// those can happen after a display/DPI change, and in practice also + /// after the OS suspends and resumes the GPU context — eframe rebuilds + /// font textures on the way back up. Our own cached `Arc`s bake + /// in mesh UV coordinates for the *old* atlas packing; painting them + /// against a freshly repacked atlas samples whatever now happens to + /// live at those old coordinates, which is exactly what "all the text + /// scrambled up after sleep" looks like. Comparing the atlas's `Arc` + /// identity (not its contents) catches every recreation reason at once, + /// on any platform, without needing to special-case sleep/wake events. + pub(crate) fn ensure_fresh(&mut self, ui: &Ui) { + let identity = ui.fonts(|f| Arc::as_ptr(&f.texture_atlas()) as *const () as usize); + self.invalidate_if_changed(identity); + } + + /// Core of [`Self::ensure_fresh`], split out so the invalidation logic + /// is testable without a live `egui::Ui`. + fn invalidate_if_changed(&mut self, atlas_identity: usize) { + if self.atlas_identity.replace(atlas_identity) != Some(atlas_identity) { + self.map.clear(); + self.coverage.clear(); + } + } + /// Whether the font cascade can render `c`, cached across frames. /// /// `Fonts::has_glyph` takes the shared font lock and walks the family's @@ -113,6 +150,52 @@ mod tests { assert_eq!(a, d); } + #[test] + fn same_atlas_identity_keeps_cached_entries() { + let mut cache = GlyphCache::with_capacity(8); + // Establish the baseline identity first — the very first call always + // "changes" it (from `None`), which would otherwise wipe the entries + // inserted below before the real assertion even runs. + cache.invalidate_if_changed(0xDEAD_BEEF); + cache.map.insert(GlyphKey::new('a', false, 14.0, Color32::WHITE), fake_galley()); + cache.coverage.insert(('a', false), true); + + cache.invalidate_if_changed(0xDEAD_BEEF); + + assert_eq!(cache.map.len(), 1, "same atlas identity must not evict entries"); + assert_eq!(cache.coverage.len(), 1); + } + + #[test] + fn atlas_identity_change_clears_cache() { + // A resized/recreated font atlas (DPI change, or the GPU context + // rebuilt after sleep/wake) invalidates every cached galley's UVs — + // this is the guard against "scrambled text after resume". + let mut cache = GlyphCache::with_capacity(8); + cache.map.insert(GlyphKey::new('a', false, 14.0, Color32::WHITE), fake_galley()); + cache.coverage.insert(('a', false), true); + cache.invalidate_if_changed(0x1111_1111); + + cache.invalidate_if_changed(0x2222_2222); + + assert!(cache.map.is_empty(), "atlas swap must evict stale galleys"); + assert!(cache.coverage.is_empty()); + } + + #[test] + fn first_call_does_not_panic_on_empty_cache() { + let mut cache = GlyphCache::with_capacity(8); + cache.invalidate_if_changed(0x1234); + assert!(cache.map.is_empty()); + } + + fn fake_galley() -> Arc { + let ctx = egui::Context::default(); + // Fonts aren't initialized until the first frame runs. + let _ = ctx.run(Default::default(), |_| {}); + ctx.fonts(|f| f.layout_no_wrap("x".to_string(), FontId::monospace(14.0), Color32::WHITE)) + } + #[test] fn clear_is_idempotent() { let mut cache = GlyphCache::with_capacity(8); diff --git a/crates/rmux-terminal/src/renderer.rs b/crates/rmux-terminal/src/renderer.rs index f3fb838..2e9beab 100644 --- a/crates/rmux-terminal/src/renderer.rs +++ b/crates/rmux-terminal/src/renderer.rs @@ -9,12 +9,12 @@ use egui::{Color32, FontFamily, FontId, Pos2, Rect, Stroke, Ui, Vec2}; const CURSOR_BLOCK_ALPHA: u8 = 200; const CURSOR_LINE_ALPHA: u8 = 255; -/// Extra vertical padding factor applied on top of measured glyph height so -/// descenders ("gypq") and combining marks don't clip, while still keeping -/// cells tight enough that box-drawing / block-element TUIs (LazyVim logo, -/// borders) tile without visible gaps. Ghostty/cmux use a similar tight -/// line height around JetBrains Mono. -const LINE_HEIGHT_PAD: f32 = 1.15; +/// Rough row-height guess used only until the real font metric is available +/// (see [`TerminalRenderer::ensure_cell_size_measured`]) — the very first +/// frame a pane paints, and for one frame after a font-size change. `1.0` +/// matches JetBrains Mono's tight actual line height closely enough that the +/// one-frame gap between this guess and the real measurement is imperceptible. +const ESTIMATED_LINE_HEIGHT_FACTOR: f32 = 1.0; fn cursor_color(alpha: u8, theme_color: Color32) -> Color32 { Color32::from_rgba_unmultiplied(theme_color.r(), theme_color.g(), theme_color.b(), alpha) @@ -99,6 +99,12 @@ fn paint_missing_symbol_fallback(painter: &egui::Painter, cell: Rect, c: char, f /// so without this path those render as hollow □ tofu boxes. /// - Common geometric triangles/squares (U+25B2–U+25C5, etc.) for a solid /// cmux/Ghostty-like look independent of font coverage. +/// - Light box-drawing lines (U+2500 ─ family) used for TUI window borders +/// and tree connectors (nvim splits, nvim-tree, lazygit, btop). Drawing +/// these as geometry — instead of relying on the font's own glyph metrics +/// for where the stroke sits inside its em-box — keeps them exact +/// regardless of the row height derived from the loaded font (see +/// [`TerminalRenderer::ensure_cell_size_measured`]). fn is_special_shape(c: char) -> bool { matches!( c, @@ -119,9 +125,42 @@ fn is_special_shape(c: char) -> bool { | '\u{2713}' | '\u{2714}' | '\u{2717}' | '\u{2718}' // Powerline solid arrows (common in prompts) | '\u{E0B0}'..='\u{E0B3}' + // Light box-drawing: ─ │ ┌ ┐ └ ┘ ├ ┤ ┬ ┴ ┼ + | '\u{2500}' | '\u{2502}' | '\u{250C}' | '\u{2510}' | '\u{2514}' | '\u{2518}' + | '\u{251C}' | '\u{2524}' | '\u{252C}' | '\u{2534}' | '\u{253C}' ) } +/// Draw a light box-drawing joint: a stroke from the cell center out to each +/// edge whose direction flag is set. Adjacent cells each draw their own half, +/// so a run of `─` connects seamlessly regardless of the exact cell size. +#[allow(clippy::too_many_arguments)] +fn paint_box_line( + painter: &egui::Painter, + cell: Rect, + fg: Color32, + up: bool, + down: bool, + left: bool, + right: bool, +) { + let thickness = (cell.width().min(cell.height()) * 0.09).max(1.0); + let stroke = Stroke::new(thickness, fg); + let center = cell.center(); + if up { + painter.line_segment([center, Pos2::new(center.x, cell.top())], stroke); + } + if down { + painter.line_segment([center, Pos2::new(center.x, cell.bottom())], stroke); + } + if left { + painter.line_segment([center, Pos2::new(cell.left(), center.y)], stroke); + } + if right { + painter.line_segment([center, Pos2::new(cell.right(), center.y)], stroke); + } +} + /// Draw a filled right-pointing triangle inset in `cell`. fn fill_triangle_right(painter: &egui::Painter, cell: Rect, fg: Color32, pad: f32) { let x0 = cell.left() + cell.width() * pad; @@ -471,6 +510,19 @@ fn paint_special_shape(painter: &egui::Painter, cell: Rect, c: char, fg: Color32 } } + // ── Light box-drawing (window borders, tree connectors) ──────── + '\u{2500}' => paint_box_line(painter, cell, fg, false, false, true, true), // ─ + '\u{2502}' => paint_box_line(painter, cell, fg, true, true, false, false), // │ + '\u{250C}' => paint_box_line(painter, cell, fg, false, true, false, true), // ┌ + '\u{2510}' => paint_box_line(painter, cell, fg, false, true, true, false), // ┐ + '\u{2514}' => paint_box_line(painter, cell, fg, true, false, false, true), // └ + '\u{2518}' => paint_box_line(painter, cell, fg, true, false, true, false), // ┘ + '\u{251C}' => paint_box_line(painter, cell, fg, true, true, false, true), // ├ + '\u{2524}' => paint_box_line(painter, cell, fg, true, true, true, false), // ┤ + '\u{252C}' => paint_box_line(painter, cell, fg, false, true, true, true), // ┬ + '\u{2534}' => paint_box_line(painter, cell, fg, true, false, true, true), // ┴ + '\u{253C}' => paint_box_line(painter, cell, fg, true, true, true, true), // ┼ + _ => return false, } true @@ -546,18 +598,32 @@ impl TerminalRenderer { /// Measure cell size from the actual loaded font on the first call. /// Subsequent calls are a no-op. + /// + /// Row height used to be a flat `font_size * 1.15` guess. That guess ran + /// ~13-15% taller than JetBrains Mono's real line height, so + /// `cols_rows_for_rect` (driven by this same `cell_size`) always counted + /// fewer rows — and sometimes columns — than the pane actually had pixels + /// for. A full-screen TUI queries its size once at startup and only + /// redraws on `SIGWINCH`, so it latched onto the undercounted geometry + /// and left a visible strip of pane background (or wallpaper, if + /// transparency is on) below and sometimes beside its content. Using the + /// font's own measured row height fits the real number of rows/cols — + /// the original guess existed to avoid glyphs (esp. block elements) + /// visually overlapping between rows, which is now handled directly by + /// rendering block elements and light box-drawing lines as geometry + /// sized to the cell (see [`is_special_shape`]) rather than depending on + /// how a specific font's glyphs sit inside its own line metrics. fn ensure_cell_size_measured(&mut self, ui: &Ui) { if self.cell_size_measured { return; } let font_id = FontId::monospace(self.font_size); - let glyph_width = ui.fonts(|f| { - f.layout("M".to_string(), font_id.clone(), Color32::WHITE, f32::INFINITY).size().x + let (glyph_width, row_height) = ui.fonts(|f| { + let width = + f.layout("M".to_string(), font_id.clone(), Color32::WHITE, f32::INFINITY).size().x; + let height = f.row_height(&font_id); + (width, height) }); - // Prefer a tight height derived from the font size rather than - // egui's paragraph `row_height`, which includes extra leading that - // leaves visible gaps between block-element rows (LazyVim logo). - let row_height = self.font_size * LINE_HEIGHT_PAD; self.cell_size = Vec2::new(glyph_width.max(1.0), row_height.max(1.0)); self.cell_size_measured = true; @@ -569,6 +635,10 @@ impl TerminalRenderer { } self.ensure_cell_size_measured(ui); + // Guards against stale galleys after egui rebuilds its font texture + // atlas (DPI change, or GPU context recreated after sleep/wake) — + // otherwise cached glyphs paint garbage from the new atlas's layout. + self.glyph_cache.ensure_fresh(ui); // Clip all cell paint to the pane so multi-tab / split content never // bleeds into a neighbour (GitHub #31). @@ -698,9 +768,9 @@ impl TerminalRenderer { } fn estimate_cell_size(font_size: f32) -> Vec2 { - // JetBrains Mono advance ≈ 0.6 × em; height uses the same pad factor - // as the measured path so resize math stays stable before first paint. - Vec2::new(font_size * 0.6, font_size * LINE_HEIGHT_PAD) + // JetBrains Mono advance ≈ 0.6 × em; corrected to the real measured + // value on the first `draw()` call. + Vec2::new(font_size * 0.6, font_size * ESTIMATED_LINE_HEIGHT_FACTOR) } pub fn cell_size(&self) -> Vec2 { @@ -840,6 +910,37 @@ mod tests { assert!(!is_special_shape('A')); } + #[test] + fn test_light_box_drawing_recognized_as_geometry() { + for c in ['─', '│', '┌', '┐', '└', '┘', '├', '┤', '┬', '┴', '┼'] { + assert!(is_special_shape(c), "{c} should be geometry-drawn"); + } + // Heavy/double variants are intentionally out of scope for now and + // still fall through to font rendering. + assert!(!is_special_shape('━')); + assert!(!is_special_shape('║')); + } + + #[test] + fn test_paint_box_line_draws_expected_segment_count() { + // Smoke test: every direction combination must paint without + // panicking, on a degenerate (zero-size) cell too. + let painter = egui::Painter::new( + egui::Context::default(), + egui::LayerId::debug(), + Rect::from_min_size(Pos2::ZERO, Vec2::splat(20.0)), + ); + let cell = Rect::from_min_size(Pos2::ZERO, Vec2::splat(20.0)); + for (up, down, left, right) in [ + (false, false, true, true), + (true, true, false, false), + (false, true, false, true), + (true, true, true, true), + ] { + paint_box_line(&painter, cell, Color32::WHITE, up, down, left, right); + } + } + #[test] fn test_symbol_range_covers_common_tofu_sources() { // Geometric / dingbat / technical — general anti-tofu policy.