From da9a63aa10e1f0b98f153baf8082339902ef7f5a Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Sat, 30 May 2026 10:06:59 -0400 Subject: [PATCH] =?UTF-8?q?B1=20polish=20=E2=80=94=20fix=20cursor-motion?= =?UTF-8?q?=20slowness=20+=20whole-line=20"selection"=20look?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two issues from visual validation now that arrow keys work: 1. Far too slow. The `Decorations` arm called `self.reshape()` (set_rich_text + shape_until_scroll — a full text re-shape) on *every* decoration change. B1's own-window `CurrentLine` decoration changes on every up/down move, so each vertical cursor step forced a full re-shape. But only diagnostic decorations affect the rich text (they override glyph fg in `projected_rich_chunks`); Selection / CurrentLine / search are background quads rebuilt cheaply in `render()`. Now reshape runs only when the fg-affecting set changed (`fg_decoration_fingerprint` compares before/after); a background-only change just requests a redraw. 2. The entire line looked selected. The own-window `CurrentLine` wash paints the whole cursor line, which reads as a persistent selection — unwanted as default. The caret already marks the own cursor, so `collect_own_decoration_rects` now skips `CurrentLine` (renders only own `Selection`). Revises Q#B4: the caret is the own-cursor indicator, not a line wash. Peer presence still shows other frontends' lines. Test `fg_fingerprint_ignores_background_decoration_changes`: a CurrentLine-only change leaves the fingerprint equal (no reshape); a diagnostic change alters it (reshape). Gates green: fmt; clippy --all-targets --workspace -D warnings; pmacs-gpu unit 19 (+1). pmacs lib / daemon untouched. Deferred (noted for follow-up sessions, not B1): - Mouse click → cursor: needs the Q#B5 wire decision (no FrontendEvent::SetCursor variant; semantic frontends can't use grid-cell Mouse coords). Its own session. - PageUp/PageDown: keys are forwarded and move the daemon cursor, but pmacs-gpu renders from the top with no scroll, so the caret would leave the viewport. Needs GPU scrolling first. Co-Authored-By: Claude Opus 4.8 (1M context) --- pmacs-gpu/src/main.rs | 79 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 74 insertions(+), 5 deletions(-) diff --git a/pmacs-gpu/src/main.rs b/pmacs-gpu/src/main.rs index a724eea..56e3d12 100644 --- a/pmacs-gpu/src/main.rs +++ b/pmacs-gpu/src/main.rs @@ -782,12 +782,26 @@ impl State { if self.current_buffer_id != Some(buffer_id) { return None; } + // Only diagnostic decorations affect the *rich text* + // (they override glyph fg in `projected_rich_chunks`); + // background kinds (Selection / CurrentLine / Search) + // are quads rebuilt cheaply in `render()`. A full + // `reshape()` (set_rich_text + shape_until_scroll) on + // every decoration change made cursor motion crawl — + // B1's own `CurrentLine` changes on every up/down move. + // Reshape only when the fg-affecting set changed; else + // just repaint the quads. + let fg_before = fg_decoration_fingerprint(&self.current_decorations); if full { self.replace_decorations(segments); } else { self.merge_decorations(segments); } - self.reshape(); + if fg_before == fg_decoration_fingerprint(&self.current_decorations) { + self.window.request_redraw(); + } else { + self.reshape(); + } None } InstanceMessage::InlineAdornments { buffer_id, items } => { @@ -1286,12 +1300,18 @@ impl State { rects_to_vertex_bytes(&rects, self.config.width, self.config.height) } - /// Own-window `Selection` / `CurrentLine` washes from - /// `current_decorations` (Q#B4). The producer emits these for this - /// frontend's window; they become non-trivial once B1's cursor - /// motion moves the window cursor off byte 0. + /// Own-window `Selection` washes from `current_decorations`. The + /// caret already marks the own cursor, so the own *`CurrentLine`* + /// wash is deliberately NOT rendered — a whole-line highlight on + /// every cursor line reads as a persistent selection, which is not + /// wanted as default editor behavior (revising Q#B4: the caret is + /// the own-cursor indicator; the line wash isn't). Peer presence + /// still shows other frontends' lines via `collect_peer_rects`. fn collect_own_decoration_rects(&self, rects: &mut Vec, line_offsets: &[u64]) { for d in &self.current_decorations { + if d.kind == DecorationKind::CurrentLine { + continue; + } if let Some(color) = decoration_kind_to_bg_color(d.kind) { self.push_glyph_extent_rects( rects, @@ -2041,6 +2061,21 @@ fn indexed_to_glyphon(idx: u8) -> glyphon::Color { glyphon::Color::rgb(level, level, level) } +/// The decorations that affect the *rich text* (a glyph fg override in +/// `projected_rich_chunks`), as an ordered `(range, kind)` set. Only +/// kinds with a foreground color qualify — i.e. the diagnostic +/// severities; background kinds (`Selection` / `CurrentLine` / search) +/// are quads. Equal fingerprints across a `Decorations` update mean the +/// shaped text is unaffected and a `reshape()` can be skipped (the perf +/// fix for cursor-motion-driven `CurrentLine` churn). +fn fg_decoration_fingerprint(decos: &[Decoration]) -> Vec<(ByteRange, DecorationKind)> { + decos + .iter() + .filter(|d| decoration_kind_to_color(d.kind).is_some()) + .map(|d| (d.range, d.kind)) + .collect() +} + /// Map a [`DecorationKind`] to a foreground color override, or `None` /// for kinds whose visual is a background and can't be expressed in /// the current `Attrs`-only rendering pipeline. @@ -2227,6 +2262,40 @@ mod tests { assert!(!m.contains(Modifiers::SHIFT)); } + #[test] + fn fg_fingerprint_ignores_background_decoration_changes() { + let deco = |start, end, kind| Decoration { + range: ByteRange { start, end }, + kind, + }; + // A diagnostic (fg) decoration + a CurrentLine (bg) decoration. + let before = vec![ + deco(10, 14, DecorationKind::DiagnosticError), + deco(0, 20, DecorationKind::CurrentLine), + ]; + // The cursor moved: CurrentLine now spans a different line, the + // diagnostic is unchanged. + let after = vec![ + deco(10, 14, DecorationKind::DiagnosticError), + deco(40, 60, DecorationKind::CurrentLine), + ]; + assert_eq!( + fg_decoration_fingerprint(&before), + fg_decoration_fingerprint(&after), + "a CurrentLine-only change must not change the fg fingerprint (no reshape)" + ); + + // A diagnostic change DOES alter the fingerprint (reshape needed). + let after_diag = vec![ + deco(10, 18, DecorationKind::DiagnosticError), + deco(0, 20, DecorationKind::CurrentLine), + ]; + assert_ne!( + fg_decoration_fingerprint(&before), + fg_decoration_fingerprint(&after_diag) + ); + } + #[test] fn line_byte_offsets_indexes_each_logical_line() { // "abc\nde\nfgh": lines start at bytes 0, 4, 7. Indexed by