From b81d4fddae48a2332453dc8a1525cbc56bcf4c1f Mon Sep 17 00:00:00 2001 From: "shingo.imota" Date: Fri, 17 Jul 2026 18:16:59 +0900 Subject: [PATCH 1/3] perf(render): add pane cache guards, invalidation, and full-rebuild predictor Groundwork for reusing cached pane instances across occlusion cycles: - cached_frame_matches_viewport: a cached frame is reusable only when the live viewport, the viewport the layout was built against, the current pane set (PaneId + rect), and the shared glyph atlas identity/eviction generation all still match, and the frame is not atlas-unstable. - invalidate_pane: clears a pane's row-cache key so its next rebuild is unconditionally full (reads every row from the live snapshot, independent of consumed row-dirty bits). - pane_rebuild_would_be_full: read-only predictor replicating rebuild_pane_cached's exact full-rebuild decision; the shared frame_invalidation_key_matches extraction keeps the two from drifting. --- crates/noa-render/src/renderer/cell.rs | 15 +- crates/noa-render/src/renderer/mod.rs | 204 ++++++ crates/noa-render/src/renderer/tests/cache.rs | 596 ++++++++++++++++++ 3 files changed, 804 insertions(+), 11 deletions(-) diff --git a/crates/noa-render/src/renderer/cell.rs b/crates/noa-render/src/renderer/cell.rs index 7c13bcaa..19fcb7fc 100644 --- a/crates/noa-render/src/renderer/cell.rs +++ b/crates/noa-render/src/renderer/cell.rs @@ -484,18 +484,11 @@ pub(super) fn rebuild_pane_cached( // something actually changed. On the steady-state frame nothing is // cloned at all. let atlas_identity = font.atlas_identity(); + // Shared with `Renderer::pane_rebuild_would_be_full`'s read-only + // prediction of this same decision (see `super::frame_invalidation_key_matches`'s + // doc comment) — kept as one function so the two can never drift apart. let key_fields_match = |k: &FrameInvalidationKey, atlas_gen: u64| { - k.active_is_alt == snap.active_is_alt - && k.cols == snap.cols - && k.rows == snap.rows_n - && k.cell_size == cell_size - && k.atlas_identity == atlas_identity - && k.atlas_eviction_generation == atlas_gen - && k.selection == snap.selection - && k.hover_link == snap.hover_link - && k.colors == snap.colors - && k.theme == *theme - && k.search == snap.search + super::frame_invalidation_key_matches(k, snap, theme, cell_size, atlas_identity, atlas_gen) }; let rows = snap.rows.len(); diff --git a/crates/noa-render/src/renderer/mod.rs b/crates/noa-render/src/renderer/mod.rs index 2e9af4df..0247dbee 100644 --- a/crates/noa-render/src/renderer/mod.rs +++ b/crates/noa-render/src/renderer/mod.rs @@ -192,6 +192,33 @@ struct FrameInvalidationKey { atlas_eviction_generation: u64, } +/// Whether `key` (a pane's previously-cached [`FrameInvalidationKey`]) still +/// matches `snap`'s pane-wide invalidation triggers — i.e. none of the +/// bundled fields in the doc comment above changed since `key` was recorded. +/// Shared between [`cell::rebuild_pane_cached`]'s actual rebuild decision and +/// [`Renderer::pane_rebuild_would_be_full`]'s read-only prediction of it, so +/// the two can never drift apart. +fn frame_invalidation_key_matches( + key: &FrameInvalidationKey, + snap: &FrameSnapshot, + theme: &Theme, + cell_size: (f32, f32), + atlas_identity: u64, + atlas_eviction_generation: u64, +) -> bool { + key.active_is_alt == snap.active_is_alt + && key.cols == snap.cols + && key.rows == snap.rows_n + && key.cell_size == cell_size + && key.atlas_identity == atlas_identity + && key.atlas_eviction_generation == atlas_eviction_generation + && key.selection == snap.selection + && key.hover_link == snap.hover_link + && key.colors == snap.colors + && key.theme == *theme + && key.search == snap.search +} + /// Shell-cursor render state plus copy-mode presence and its projected cursor. /// Presence is kept separately so an offscreen copy cursor still suppresses /// and invalidates the shell cursor. @@ -269,6 +296,30 @@ pub struct Renderer { /// resolved into image quads at draw time (parallel to `pane_layout`). pane_images: Vec, pane_layout: Vec<(PaneId, PaneRect)>, + /// `self.viewport` as of the `rebuild_panes` call that last populated + /// `pane_layout`/`instances` — deliberately NOT the same as the live + /// `viewport` field below, which `resize()` mutates unconditionally + /// (including while occluded, before the cache catches up). See + /// `cached_frame_matches_viewport`. + pane_layout_viewport: PixelSize, + /// The shared glyph atlas's identity + eviction generation as of the + /// `rebuild_panes` call that last populated `pane_layout`/`instances` + /// (the same pair `FrameInvalidationKey` uses). The atlas is shared + /// across every window's renderer, so another window's background + /// refresh can evict/reallocate it between this renderer's last rebuild + /// and now; if it has, this renderer's cached instances may reference + /// reclaimed atlas rectangles. See `cached_frame_matches_viewport`. + pane_layout_atlas_identity: u64, + pane_layout_atlas_generation: u64, + /// `self.frame_unstable` as of the `rebuild_panes` call that last + /// populated `pane_layout`/`instances`: that call could not fully + /// stabilize against glyph-atlas evictions, so its instances may + /// reference reallocated atlas rectangles. Normal rendering converges by + /// requesting one more frame (`needs_follow_up_frame`); a background + /// refresh while occluded has no such follow-up loop, so a cache built + /// unstable must never be presented as-is by the reveal fast path. See + /// `cached_frame_matches_viewport`. + pane_layout_unstable: bool, divider_range: Range, focus_indicator_range: Range, viewport: PixelSize, @@ -377,6 +428,10 @@ impl Renderer { pane_instances: Vec::new(), pane_images: Vec::new(), pane_layout: Vec::new(), + pane_layout_viewport: PixelSize { w: 0, h: 0 }, + pane_layout_atlas_identity: 0, + pane_layout_atlas_generation: 0, + pane_layout_unstable: false, divider_range: 0..0, focus_indicator_range: 0..0, viewport: PixelSize { w: 0, h: 0 }, @@ -527,6 +582,142 @@ impl Renderer { self.rows_rebuilt_last_frame } + /// Whether this renderer already holds a built pane layout + instance + /// list matching `viewport` and `expected_panes` — i.e. a previous + /// `rebuild_panes` (possibly during a background cache refresh while + /// occluded) produced something safe to present as-is. Used by the + /// tab-switch-stall reveal fast path: `false` means that frame must fall + /// back to a normal full rebuild instead of drawing a stale, + /// wrongly-sized, wrong-glyph, or stale-split layout. Independent ways + /// this can be `false`: + /// - never rendered (`pane_layout` empty); + /// - resized since the cache was last built — checked against BOTH the + /// live `viewport` (in case the caller's `viewport` argument is itself + /// stale) and `pane_layout_viewport` (what the cache was actually + /// built against; `resize()` mutates the live field unconditionally, + /// including while occluded, well before any rebuild follows — e.g. a + /// macOS tab group resizing every member window together); + /// - the shared glyph atlas moved since the cache was last built — it is + /// shared across every window's renderer, so another window's + /// background refresh can evict/reallocate it in between, which would + /// leave this renderer's cached instances referencing reclaimed atlas + /// rectangles; + /// - `expected_panes` (the CURRENT split layout's `(PaneId, PaneRect)` + /// set, in the exact form the caller would pass to `rebuild_panes`) + /// no longer matches `pane_layout` — a split added/closed or a pane + /// closed (e.g. via IPC or a pty exit) while occluded, which changes + /// nothing about viewport or atlas but must still fall back, or the + /// reveal frame would present closed/stale panes; + /// - the cached frame never stabilized against glyph-atlas evictions + /// (`pane_layout_unstable`) — normal rendering converges via + /// `needs_follow_up_frame`, but a background refresh has no such + /// follow-up loop, so an unstable cache must never be presented as-is. + pub fn cached_frame_matches_viewport( + &self, + viewport: PixelSize, + font: &FontGrid, + expected_panes: &[(PaneId, PaneRect)], + ) -> bool { + !self.pane_layout.is_empty() + && self.viewport == viewport + && self.pane_layout_viewport == viewport + && self.pane_layout_atlas_identity == font.atlas_identity() + && self.pane_layout_atlas_generation == font.atlas_eviction_generation() + && !self.pane_layout_unstable + && self.pane_layout == expected_panes + } + + /// Force `pane`'s NEXT `rebuild_panes` call to fully rebuild every + /// visible row, regardless of `FrameSnapshot::row_dirty` — bypassing the + /// per-row cache-key reuse entirely, not just the reveal fast-path guard + /// above. A full rebuild reads every row straight from that call's fresh + /// `FrameSnapshot` (which always reflects the terminal's actual current + /// content; only `row_dirty` — which per-row reuse trusts — can be + /// stale), so this restores correctness regardless of how the terminal's + /// dirty bits got out of sync with what this cache has already applied. + /// + /// Used when a pane's captured-but-never-applied snapshot (`noa-app`'s + /// `Surface::pending_reveal_snapshot`) is about to be discarded — e.g. a + /// window re-occludes before its guaranteed catch-up redraw ran (kaizen + /// cycle 5): that snapshot's read already consumed the terminal's row + /// damage, so a later rebuild trusting `row_dirty` would see those rows + /// as clean and never re-derive them, even though this cache never + /// actually applied that content. A no-op if `pane` has no cache entry + /// yet (its first rebuild is already full). + pub fn invalidate_pane(&mut self, pane: PaneId) { + if let Some(cache) = self.pane_render_cache.get_mut(&pane) { + cache.key = None; + } + } + + /// Read-only prediction of whether rebuilding `pane` against `snap` right + /// now would hit `cell::rebuild_pane_cached`'s FULL-rebuild path (every + /// visible row) rather than its incremental one (only dirty rows, or the + /// scroll-shift translation) — without doing any of that work or + /// mutating any cache state. Mirrors that function's `full` decision + /// exactly (same shared `frame_invalidation_key_matches`), so the two can + /// never disagree. + /// + /// Used to bound the tab-switch-stall background refresh's per-invocation + /// cost (kaizen cycle 6, finding P1): a hidden tab that scrolled more than + /// one viewport since its cache was last built can never hit the + /// scroll-shift fast path, so refreshing it now would always be a full, + /// synchronous, main-thread rebuild (measured 38–93ms) — and that work is + /// wasted anyway, since the eventual reveal's catch-up frame will be a + /// full rebuild regardless of whether this background refresh ran. The + /// caller skips refreshing (and, per its own dirty-set policy, does not + /// keep retrying) any pane this returns `true` for. + pub fn pane_rebuild_would_be_full( + &self, + pane: PaneId, + snap: &FrameSnapshot, + font: &FontGrid, + theme: &Theme, + ) -> bool { + let Some(cache) = self.pane_render_cache.get(&pane) else { + // No cache entry yet: this pane's first-ever rebuild is always + // full (mirrors `cell::rebuild_pane_cached`'s `cache.bg.len() != + // rows` check against a fresh, empty cache). + return true; + }; + let rows = snap.rows.len(); + if cache.bg.len() != rows { + return true; + } + let cell_size = (font.metrics().cell_w, font.metrics().cell_h); + let atlas_identity = font.atlas_identity(); + let atlas_gen = font.atlas_eviction_generation(); + if snap.scroll_shift > 0 { + let scroll_path_applies = snap.scroll_shift < rows + && cache + .prev_row_base + .is_some_and(|base| base + snap.scroll_shift == snap.row_base) + && cache.key.as_ref().is_some_and(|key| { + key.abs_row_base + snap.scroll_shift == snap.abs_row_base + && frame_invalidation_key_matches( + key, + snap, + theme, + cell_size, + atlas_identity, + atlas_gen, + ) + }); + return !scroll_path_applies; + } + !cache.key.as_ref().is_some_and(|key| { + key.abs_row_base == snap.abs_row_base + && frame_invalidation_key_matches( + key, + snap, + theme, + cell_size, + atlas_identity, + atlas_gen, + ) + }) + } + /// `true` when the most recent `rebuild_panes` could not stabilize /// against glyph-atlas evictions, so the frame just built may draw some /// glyphs with another glyph's pixels. The caller should request one @@ -674,6 +865,19 @@ impl Renderer { self.focus_indicator_color = [accent[0], accent[1], accent[2], FOCUS_INDICATOR_ALPHA]; self.cell_instance_len = self.instances.len(); self.rows_rebuilt_last_frame = rows_rebuilt_total; + // The viewport + shared-atlas state `pane_layout` was actually built + // against, for `cached_frame_matches_viewport`'s guard: `self.viewport` + // is mutated unconditionally by `resize()` (including for windows + // resized while occluded, before their pane cache catches up), and + // the glyph atlas is shared across every window's renderer, so both + // must be pinned here rather than re-read live at guard-check time. + self.pane_layout_viewport = self.viewport; + self.pane_layout_atlas_identity = font.atlas_identity(); + self.pane_layout_atlas_generation = font.atlas_eviction_generation(); + // `self.frame_unstable` reflects the FINAL cross-pane pass's + // stability (see the loop above), so this is exactly this call's + // settled outcome, not a stale value from an earlier pass. + self.pane_layout_unstable = self.frame_unstable; } /// Draw the current instance list into `view`, uploading updated GPU diff --git a/crates/noa-render/src/renderer/tests/cache.rs b/crates/noa-render/src/renderer/tests/cache.rs index e425d8a9..cda7a627 100644 --- a/crates/noa-render/src/renderer/tests/cache.rs +++ b/crates/noa-render/src/renderer/tests/cache.rs @@ -691,3 +691,599 @@ fn active_screen_switch_forces_rebuild_even_when_rows_are_clean() { "alt -> primary switch must rebuild every row even when row_dirty is clean" ); } + +#[test] +fn cached_frame_matches_viewport_tracks_the_viewport_the_layout_was_built_against() { + // Regression test (kaizen cycle 3, bug A1): `resize()` mutates the live + // `viewport` field unconditionally — including for a window resized + // while occluded, well before its pane cache next rebuilds (a macOS tab + // group resizes every member window together, occluded or not). The + // tab-switch-stall reveal fast path must not compare its guard against + // that live field, or a resize-while-occluded would make the guard + // wrongly report the (still A-sized) cached layout as usable at the new + // size B. + let Some((device, queue)) = device_queue() else { + eprintln!("no wgpu adapter available — skipping cached_frame_matches_viewport test"); + return; + }; + let Some(mut font) = skip_font() else { return }; + let mut renderer = Renderer::new( + &device, + &queue, + wgpu::TextureFormat::Bgra8Unorm, + &mut font, + GridPadding::ZERO, + ) + .expect("build renderer"); + + let size_a = PixelSize { w: 64, h: 64 }; + let size_b = PixelSize { w: 96, h: 96 }; + renderer.resize(size_a); + + let mut terminal = Terminal::new(GridSize::new(4, 2)); + let theme = Theme::new(); + let pane = PaneId::new(1); + let rect = PaneRect::new(0, 0, 64, 64); + let expected = [(pane, rect)]; + let snap = FrameSnapshot::from_terminal(&mut terminal); + renderer.rebuild_panes( + &[PaneFrame { + pane, + rect, + snapshot: &snap, + }], + &mut font, + &theme, + ); + assert!( + renderer.cached_frame_matches_viewport(size_a, &font, &expected), + "a freshly rebuilt layout must match the viewport it was built against" + ); + + // Simulate a resize landing while this window is occluded: the live + // viewport moves, but nothing has rebuilt the cache against it yet. + renderer.resize(size_b); + assert!( + !renderer.cached_frame_matches_viewport(size_b, &font, &expected), + "a resize with no rebuild since must not report the stale cache as usable at the new size" + ); + assert!( + !renderer.cached_frame_matches_viewport(size_a, &font, &expected), + "the live viewport moved past size_a too, so presenting at the old size is equally wrong" + ); + + // Once the cache actually rebuilds against the new size, the guard + // must report it usable again. + let snap2 = FrameSnapshot::from_terminal(&mut terminal); + renderer.rebuild_panes( + &[PaneFrame { + pane, + rect, + snapshot: &snap2, + }], + &mut font, + &theme, + ); + assert!( + renderer.cached_frame_matches_viewport(size_b, &font, &expected), + "a rebuild after the resize must make the guard match the new viewport again" + ); +} + +#[test] +fn cached_frame_matches_viewport_is_invalidated_by_a_shared_atlas_eviction() { + // Regression test (kaizen cycle 3, finding P2-1): the glyph atlas is + // shared across every window's renderer (`crate::SharedGlyphAtlases`), + // so another window's background refresh rasterizing new glyphs can + // evict/reallocate atlas rectangles this renderer's cached instances + // already reference — even though this renderer's own viewport never + // changed. The reveal fast-path guard must catch that too. + let Some((device, queue)) = device_queue() else { + eprintln!("no wgpu adapter available — skipping cached_frame_matches_viewport atlas test"); + return; + }; + // A capped atlas (mirrors `atlas_eviction_epoch_forces_full_row_cache_rebuild` + // in `atlas.rs`) so flooding distinct glyphs deterministically forces an + // eviction instead of just growing the atlas. + let mut font = match FontGrid::new_with_capped_atlas_for_tests(14.0, FontConfig::default(), 48) + { + Ok(font) => font, + Err(err) => { + eprintln!("skipping: no system monospace font available: {err}"); + return; + } + }; + let mut renderer = Renderer::new( + &device, + &queue, + wgpu::TextureFormat::Bgra8Unorm, + &mut font, + GridPadding::ZERO, + ) + .expect("build renderer"); + + let size = PixelSize { w: 64, h: 64 }; + renderer.resize(size); + + let mut terminal = Terminal::new(GridSize::new(4, 2)); + let theme = Theme::new(); + let pane = PaneId::new(1); + let rect = PaneRect::new(0, 0, 64, 64); + let expected = [(pane, rect)]; + let snap = FrameSnapshot::from_terminal(&mut terminal); + renderer.rebuild_panes( + &[PaneFrame { + pane, + rect, + snapshot: &snap, + }], + &mut font, + &theme, + ); + assert!( + renderer.cached_frame_matches_viewport(size, &font, &expected), + "a freshly rebuilt layout must be usable" + ); + + // Simulate another window's background refresh evicting the shared + // atlas: flood distinct glyphs through the SAME `FontGrid` this renderer + // was built with, without calling `rebuild_panes` on this renderer again. + let before_eviction = font.atlas_eviction_generation(); + for ch in ('!'..='~').chain('\u{3041}'..='\u{3096}') { + font.get_or_raster(ch); + if font.atlas_eviction_generation() > before_eviction { + break; + } + } + assert!( + font.atlas_eviction_generation() > before_eviction, + "capped atlas must evict after flooding distinct glyphs" + ); + + assert!( + !renderer.cached_frame_matches_viewport(size, &font, &expected), + "an atlas eviction after this renderer's last rebuild must invalidate \ + its cached instances even though the viewport never changed" + ); +} + +#[test] +fn cached_frame_matches_viewport_is_invalidated_by_a_pane_layout_change() { + // Regression test (kaizen cycle 4, finding P2-C): a split added/closed + // or a pane closed while occluded (IPC, or a pty exit) changes the + // split tree's pane id + rect set without touching viewport or atlas at + // all, so those two guards alone would let a stale layout — including + // panes that no longer exist — through the reveal fast path. + let Some((device, queue)) = device_queue() else { + eprintln!( + "no wgpu adapter available — skipping cached_frame_matches_viewport pane-layout test" + ); + return; + }; + let Some(mut font) = skip_font() else { return }; + let mut renderer = Renderer::new( + &device, + &queue, + wgpu::TextureFormat::Bgra8Unorm, + &mut font, + GridPadding::ZERO, + ) + .expect("build renderer"); + + let size = PixelSize { w: 64, h: 64 }; + renderer.resize(size); + + let mut terminal = Terminal::new(GridSize::new(4, 2)); + let theme = Theme::new(); + let pane_a = PaneId::new(1); + let rect_a = PaneRect::new(0, 0, 64, 64); + let snap = FrameSnapshot::from_terminal(&mut terminal); + renderer.rebuild_panes( + &[PaneFrame { + pane: pane_a, + rect: rect_a, + snapshot: &snap, + }], + &mut font, + &theme, + ); + let expected_unchanged = [(pane_a, rect_a)]; + assert!( + renderer.cached_frame_matches_viewport(size, &font, &expected_unchanged), + "a freshly rebuilt layout must match the panes it was built from" + ); + + // A second pane appeared (split while occluded) — same viewport, same + // atlas, but the expected pane set now differs. + let pane_b = PaneId::new(2); + let rect_b = PaneRect::new(0, 32, 64, 32); + let expected_after_split = [(pane_a, PaneRect::new(0, 0, 64, 32)), (pane_b, rect_b)]; + assert!( + !renderer.cached_frame_matches_viewport(size, &font, &expected_after_split), + "a split appearing while occluded must invalidate the cached layout \ + even though viewport and atlas never changed" + ); + + // A pane closed while occluded — the cache still has it, but it is no + // longer part of the expected set. + let expected_empty: [(PaneId, PaneRect); 0] = []; + assert!( + !renderer.cached_frame_matches_viewport(size, &font, &expected_empty), + "a closed pane must invalidate the cached layout that still includes it" + ); +} + +#[test] +fn cached_frame_matches_viewport_refuses_an_unstable_cache() { + // Regression test (kaizen cycle 4, finding P2-D): normal visible-window + // rendering converges an atlas-eviction-unstable frame via + // `needs_follow_up_frame` (the app schedules one more redraw). A + // background refresh while occluded has no such follow-up loop, so if + // its `rebuild_panes` gives up unstable, the reveal fast path must + // refuse to present that cache as-is — even though viewport, atlas + // identity+generation (both are read fresh, not evicted further), and + // pane layout may all still match. + let Some((device, queue)) = device_queue() else { + eprintln!( + "no wgpu adapter available — skipping cached_frame_matches_viewport instability test" + ); + return; + }; + // An atlas too small to hold even one row's distinct glyphs at once + // forces continuous eviction within a single `rebuild_panes` call, + // across every retry pass — the same failure mode + // `MAX_ATLAS_EVICTION_REBUILD_PASSES` guards against. + let mut font = match FontGrid::new_with_capped_atlas_for_tests(14.0, FontConfig::default(), 24) + { + Ok(font) => font, + Err(err) => { + eprintln!("skipping: no system monospace font available: {err}"); + return; + } + }; + let mut renderer = Renderer::new( + &device, + &queue, + wgpu::TextureFormat::Bgra8Unorm, + &mut font, + GridPadding::ZERO, + ) + .expect("build renderer"); + + let size = PixelSize { w: 64, h: 64 }; + renderer.resize(size); + + // One row packed with many distinct glyphs (letters, digits, symbols, + // and CJK) so a 24x24-px atlas cannot hold this row's working set at + // once, regardless of system font metrics. + let chars: Vec = ('!'..='~').chain('\u{3041}'..='\u{3096}').collect(); + let mut terminal = Terminal::new(GridSize::new(chars.len() as u16, 1)); + for (col, ch) in chars.iter().enumerate() { + terminal.primary.grid[0].cells[col].ch = *ch; + } + let theme = Theme::new(); + let pane = PaneId::new(1); + let rect = PaneRect::new(0, 0, 64, 64); + let expected = [(pane, rect)]; + let snap = FrameSnapshot::from_terminal(&mut terminal); + renderer.rebuild_panes( + &[PaneFrame { + pane, + rect, + snapshot: &snap, + }], + &mut font, + &theme, + ); + + if !renderer.needs_follow_up_frame() { + eprintln!( + "could not force a genuinely atlas-eviction-unstable rebuild on this \ + system's font metrics — skipping the instability-refusal assertion" + ); + return; + } + assert!( + !renderer.cached_frame_matches_viewport(size, &font, &expected), + "an unstable cache must never be presented by the reveal fast path, \ + even with matching viewport, atlas identity+generation, and pane layout" + ); +} + +#[test] +fn invalidate_pane_forces_a_full_rebuild_even_with_unchanged_row_dirty_bits() { + // Regression test (kaizen cycle 5, P1 continued): `noa-app` calls + // `invalidate_pane` when a pane's captured-but-never-applied + // `pending_reveal_snapshot` is discarded (re-occluded before its + // guaranteed catch-up redraw ran) — the terminal's row-dirty bits were + // already consumed by that capture, so a normal per-row-cache rebuild + // trusting them would see every row clean and rebuild nothing, even + // though this cache never actually applied that content. This pins that + // `invalidate_pane` defeats per-row cache-key reuse directly — not just + // the (separate) reveal fast-path guard — by forcing a full rebuild on + // the very next call despite `row_dirty` being all-false throughout. + let Some((device, queue)) = device_queue() else { + eprintln!("no wgpu adapter available — skipping invalidate_pane test"); + return; + }; + let Some(mut font) = skip_font() else { return }; + let mut renderer = Renderer::new( + &device, + &queue, + wgpu::TextureFormat::Bgra8Unorm, + &mut font, + GridPadding::ZERO, + ) + .expect("build renderer"); + renderer.resize(PixelSize { w: 64, h: 64 }); + + let mut terminal = Terminal::new(GridSize::new(4, 2)); + terminal.primary.grid[0].cells[0].ch = 'A'; + let theme = Theme::new(); + let pane = PaneId::new(1); + let rect = PaneRect::new(0, 0, 64, 64); + + let snap1 = FrameSnapshot::from_terminal(&mut terminal); + renderer.rebuild_panes( + &[PaneFrame { + pane, + rect, + snapshot: &snap1, + }], + &mut font, + &theme, + ); + assert!(renderer.rows_rebuilt_last_frame() > 0); + + // Baseline (mirrors `rebuild_panes_reports_zero_rows_rebuilt_when_nothing_changed`): + // an unmutated second read reports every row clean, so a normal rebuild + // does zero work — this is the exact state a discarded + // `pending_reveal_snapshot` would otherwise leave behind. + let snap2 = FrameSnapshot::from_terminal(&mut terminal); + renderer.rebuild_panes( + &[PaneFrame { + pane, + rect, + snapshot: &snap2, + }], + &mut font, + &theme, + ); + assert_eq!( + renderer.rows_rebuilt_last_frame(), + 0, + "sanity check: an unchanged snapshot must rebuild zero rows without invalidation" + ); + + renderer.invalidate_pane(pane); + + // Same unchanged content, same all-clean `row_dirty` — but this rebuild + // must now touch every row anyway. + let snap3 = FrameSnapshot::from_terminal(&mut terminal); + assert!( + snap3.row_dirty.iter().all(|&dirty| !dirty), + "the snapshot itself still reports no damage — the rebuild below must \ + ignore that and rebuild fully purely because of invalidate_pane" + ); + renderer.rebuild_panes( + &[PaneFrame { + pane, + rect, + snapshot: &snap3, + }], + &mut font, + &theme, + ); + assert_eq!( + renderer.rows_rebuilt_last_frame(), + 2, + "invalidate_pane must force every row to rebuild on the next call, \ + even though row_dirty reported nothing changed" + ); +} + +#[test] +fn pane_rebuild_would_be_full_predicts_full_when_scroll_exceeds_a_viewport() { + // Regression test (kaizen cycle 6, finding P1): the tab-switch-stall + // background refresh bounds its own main-thread cost by skipping the + // (expensive) rebuild entirely for any pane where a rebuild right now + // would be full — this pins the read-only predictor it relies on for + // that decision, covering the three cases `noa-app`'s guard needs: + // never-built, unchanged (incremental), and scrolled-past-the-viewport + // (full, because the scroll-shift translation can't apply beyond one + // viewport of movement). + let Some((device, queue)) = device_queue() else { + eprintln!("no wgpu adapter available — skipping pane_rebuild_would_be_full test"); + return; + }; + let Some(mut font) = skip_font() else { return }; + let mut renderer = Renderer::new( + &device, + &queue, + wgpu::TextureFormat::Bgra8Unorm, + &mut font, + GridPadding::ZERO, + ) + .expect("build renderer"); + renderer.resize(PixelSize { w: 64, h: 64 }); + + let theme = Theme::new(); + let pane = PaneId::new(1); + let never_built_pane = PaneId::new(2); + let rect = PaneRect::new(0, 0, 64, 64); + + let mut terminal = Terminal::new(GridSize::new(4, 4)); + let snap1 = FrameSnapshot::from_terminal(&mut terminal); + renderer.rebuild_panes( + &[PaneFrame { + pane, + rect, + snapshot: &snap1, + }], + &mut font, + &theme, + ); + + assert!( + renderer.pane_rebuild_would_be_full(never_built_pane, &snap1, &font, &theme), + "a pane with no cache entry yet must always predict full" + ); + + // Unchanged content since the last rebuild: the incremental path + // (zero dirty rows) applies, so this must predict NOT full. + let snap2 = FrameSnapshot::from_terminal(&mut terminal); + assert!( + !renderer.pane_rebuild_would_be_full(pane, &snap2, &font, &theme), + "an unchanged snapshot must predict incremental, not full" + ); + + // Scroll well past this 4-row viewport in one go — beyond what the + // scroll-shift fast path can translate (it requires `scroll_shift < rows`). + let mut stream = Stream::new(); + let mut burst = Vec::new(); + for line in 1..=20 { + burst.extend_from_slice(format!("{line}\r\n").as_bytes()); + } + stream.feed(&burst, &mut terminal); + let snap3 = FrameSnapshot::from_terminal(&mut terminal); + assert!( + snap3.scroll_shift >= 4, + "test setup must actually exceed the viewport: scroll_shift={}", + snap3.scroll_shift + ); + assert!( + renderer.pane_rebuild_would_be_full(pane, &snap3, &font, &theme), + "a scroll exceeding the viewport must predict full — this is exactly \ + the scenario the background-refresh skip guard exists to catch" + ); +} + +#[test] +fn invalidate_pane_after_a_skipped_mixed_window_still_surfaces_the_in_place_edit() { + // Regression test (kaizen cycle 7, CRITICAL): a mixed occluded window + // where one pane (A) is scrolling past its viewport (predicts full) and + // another (B) got a plain in-place edit with its cache key otherwise + // unchanged (would have been incremental on its own) must not lose B's + // edit when `noa-app`'s background refresh skips the WHOLE window + // because of A. The fix invalidates every visible pane it captured a + // (damage-consuming) snapshot for that round — not just the one that + // forced the skip — so this pins that B's edit still surfaces fully on + // the next rebuild after exactly that sequence: capture (consumes B's + // damage) -> discard both -> invalidate both -> an unrelated later read + // -> rebuild. + let Some((device, queue)) = device_queue() else { + eprintln!("no wgpu adapter available — skipping mixed-window invalidate test"); + return; + }; + let Some(mut font) = skip_font() else { return }; + let mut renderer = Renderer::new( + &device, + &queue, + wgpu::TextureFormat::Bgra8Unorm, + &mut font, + GridPadding::ZERO, + ) + .expect("build renderer"); + renderer.resize(PixelSize { w: 128, h: 64 }); + + let theme = Theme::new(); + let pane_a = PaneId::new(1); + let pane_b = PaneId::new(2); + let rect_a = PaneRect::new(0, 0, 64, 64); + let rect_b = PaneRect::new(64, 0, 64, 64); + + let mut terminal_a = Terminal::new(GridSize::new(4, 4)); + let mut terminal_b = Terminal::new(GridSize::new(4, 4)); + + // Establish both panes' caches. + let snap_a0 = FrameSnapshot::from_terminal(&mut terminal_a); + let snap_b0 = FrameSnapshot::from_terminal(&mut terminal_b); + renderer.rebuild_panes( + &[ + PaneFrame { + pane: pane_a, + rect: rect_a, + snapshot: &snap_a0, + }, + PaneFrame { + pane: pane_b, + rect: rect_b, + snapshot: &snap_b0, + }, + ], + &mut font, + &theme, + ); + + // Round N: A scrolls well past its 4-row viewport; B gets a plain + // in-place edit (no scroll, invalidation-key fields unchanged). + let mut stream_a = Stream::new(); + let mut burst = Vec::new(); + for line in 1..=20 { + burst.extend_from_slice(format!("{line}\r\n").as_bytes()); + } + stream_a.feed(&burst, &mut terminal_a); + terminal_b.primary.grid[0].cells[0].ch = 'X'; + terminal_b.primary.grid[0].dirty = true; + + // This is exactly `background_refresh_pane_cache`'s capture loop: every + // visible pane's snapshot is taken (and its damage consumed) regardless + // of what happens next. + let snap_a1 = FrameSnapshot::from_terminal(&mut terminal_a); + let snap_b1 = FrameSnapshot::from_terminal(&mut terminal_b); + + assert!( + snap_a1.scroll_shift >= 4, + "test setup must actually exceed the viewport" + ); + assert!( + renderer.pane_rebuild_would_be_full(pane_a, &snap_a1, &font, &theme), + "pane A must predict full — this is what forces the whole-window skip" + ); + assert!( + !renderer.pane_rebuild_would_be_full(pane_b, &snap_b1, &font, &theme), + "pane B alone would have been incremental — its edit must not be \ + lost just because A forces the whole-window skip" + ); + + // The app's skip path: discard both captured snapshots (never fed to + // `rebuild_panes`), but invalidate BOTH panes' caches — not just A's. + renderer.invalidate_pane(pane_a); + renderer.invalidate_pane(pane_b); + drop(snap_a1); + drop(snap_b1); + + // A later rebuild, with no further mutation, must still show B's edit + // in full — even though B's own dirty bit from the skipped round is + // gone (consumed and discarded, never applied). + let snap_a2 = FrameSnapshot::from_terminal(&mut terminal_a); + let snap_b2 = FrameSnapshot::from_terminal(&mut terminal_b); + assert!( + snap_b2.row_dirty.iter().all(|&dirty| !dirty), + "sanity: B's edit damage was already consumed by the skipped round's capture" + ); + renderer.rebuild_panes( + &[ + PaneFrame { + pane: pane_a, + rect: rect_a, + snapshot: &snap_a2, + }, + PaneFrame { + pane: pane_b, + rect: rect_b, + snapshot: &snap_b2, + }, + ], + &mut font, + &theme, + ); + assert_eq!( + renderer.rows_rebuilt_last_frame(), + 8, + "invalidate_pane on BOTH panes must force a full rebuild of both \ + (4 rows each = 8 total), so B's in-place edit is not lost even \ + though its own dirty bit was consumed and discarded by the \ + skipped round" + ); +} From 39f5136680ec1778d05ba9898211fe48c5da348e Mon Sep 17 00:00:00 2001 From: "shingo.imota" Date: Fri, 17 Jul 2026 18:17:17 +0900 Subject: [PATCH 2/3] test(render): add headless tab-switch full-rebuild baseline bench MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Measures the cost a busy tab pays on its first frame after a long occlusion (full viewport rebuild, cold vs warm glyph atlas) on a real GPU adapter; skips when none is available. Baseline on Apple Silicon at 200x60: ~93ms cold / ~38ms warm vs a ~16ms frame budget — the numbers motivating the background-refresh + instant-reveal design. --- crates/noa-render/tests/tab_switch_bench.rs | 157 ++++++++++++++++++++ 1 file changed, 157 insertions(+) create mode 100644 crates/noa-render/tests/tab_switch_bench.rs diff --git a/crates/noa-render/tests/tab_switch_bench.rs b/crates/noa-render/tests/tab_switch_bench.rs new file mode 100644 index 00000000..14288b82 --- /dev/null +++ b/crates/noa-render/tests/tab_switch_bench.rs @@ -0,0 +1,157 @@ +//! Headless baseline benchmark for the "tab switch to a busy tab momentarily +//! freezes" problem (Kaizen cycle 1, measurement only — no behavior change). +//! +//! Diagnosis: while a macOS-tab window is occluded, `redraw()` early-returns +//! so `PaneRenderCache` goes stale; on reveal, the first frame's cache lookup +//! misses (the pane's absolute row base advanced while backgrounded, or the +//! shared glyph atlas evicted entries other tabs needed), forcing +//! `rebuild_pane_cached`'s `full=true` path — every visible row is +//! re-shaped and, for glyphs no longer resident, re-rasterized. +//! +//! This benchmark isolates that cost on a real (not synthetic) VT-parsed +//! terminal: a 200×60 grid of varied colored ASCII + CJK + emoji text, fed +//! through an actual `noa_vt::Stream` into a real `noa_grid::Terminal`, then +//! rebuilt with `Renderer::rebuild_cells`. It measures: +//! +//! - a cold-atlas full rebuild (fresh `FontGrid`, nothing rasterized yet — +//! the worst case: every glyph shapes AND rasterizes), and +//! - a warm-atlas full rebuild (same glyphs, but the atlas already holds +//! them from a prior rebuild — isolates row re-shaping/layout cost from +//! rasterization). +//! +//! A full rebuild is forced for both by advancing `abs_row_base` between +//! builds (mirrors a backgrounded tab's shell producing output while +//! occluded), the same cache-miss trigger `rebuild_pane_cached` uses in +//! production — no internal (`pub(super)`) API is touched. +//! +//! Skips gracefully where no GPU adapter is available (headless CI without a +//! Metal/Vulkan device), matching `pipeline.rs`'s convention. + +// Shared with `pipeline.rs`'s test binary; this binary only needs +// `device_queue`, so the rest of the module's helpers go unused here. +#[path = "pipeline/shared.rs"] +#[allow(dead_code)] +mod shared; + +use noa_core::{DEFAULT_GRID_PADDING, GridSize, PixelSize}; +use noa_font::FontGrid; +use noa_grid::Terminal; +use noa_render::{FrameSnapshot, Renderer, Theme}; +use noa_vt::Stream; +use std::time::Instant; + +const COLS: u16 = 200; +const ROWS: u16 = 60; + +/// Feeds a real VT byte stream into a fresh `Terminal`, producing a busy +/// full-viewport frame: every row cycles through 8 SGR foreground colors and +/// a mix of ASCII, CJK, and emoji text (so glyph shaping/rasterization sees +/// realistic width and script variety, not a single repeated glyph). +fn busy_terminal() -> Terminal { + let mut terminal = Terminal::new(GridSize::new(COLS, ROWS)); + let mut stream = Stream::new(); + let words = [ + "fn", + "ホスト", + "端末", + "🦀", + "match", + "パイプ", + "Ok(0)", + "😀", + ]; + let mut bytes = Vec::new(); + for row in 0..ROWS { + let color = 31 + (row as usize % 8); // SGR 31..=38 + bytes.extend_from_slice(format!("\x1b[{color}m").as_bytes()); + let mut written = 0usize; + let mut i = 0; + while written < COLS as usize { + let word = words[(row as usize + i) % words.len()]; + bytes.extend_from_slice(word.as_bytes()); + written += word.chars().count(); + i += 1; + } + bytes.extend_from_slice(b"\x1b[0m\r\n"); + } + stream.feed(&bytes, &mut terminal); + terminal +} + +/// A snapshot of `terminal` with `abs_row_base` bumped by `bump` — mirrors +/// what a backgrounded tab's advancing scrollback does across an occluded +/// period, which is what forces `rebuild_pane_cached`'s cache-miss `full` +/// path on reveal (a real cache, not touched here, keys off exactly this +/// field — see `FrameInvalidationKey` in `noa-render/src/renderer/cell.rs`). +fn snapshot_with_row_base(terminal: &mut Terminal, bump: usize) -> FrameSnapshot { + let mut snap = FrameSnapshot::from_terminal(terminal); + snap.abs_row_base += bump; + snap.row_dirty = vec![true; snap.rows.len()]; + snap +} + +#[test] +fn tab_switch_full_rebuild_baseline() { + let Some((device, queue)) = shared::device_queue() else { + eprintln!("no wgpu adapter available — skipping tab-switch bench"); + return; + }; + + let mut terminal = busy_terminal(); + let theme = Theme::new(); + let viewport = PixelSize { + w: COLS as u32 * 10, + h: ROWS as u32 * 20, + }; + + // Cold atlas: nothing rasterized yet, so the first rebuild both shapes + // every row AND rasterizes every distinct glyph into the atlas. + let mut cold_font = + FontGrid::new(14.0, noa_font::FontConfig::default()).expect("load a system monospace font"); + let mut cold_renderer = Renderer::new( + &device, + &queue, + wgpu::TextureFormat::Bgra8UnormSrgb, + &mut cold_font, + DEFAULT_GRID_PADDING, + ) + .expect("build cold-atlas renderer"); + cold_renderer.resize(viewport); + + let cold_snap = snapshot_with_row_base(&mut terminal, 0); + let cold_start = Instant::now(); + cold_renderer.rebuild_cells(&cold_snap, &mut cold_font, &theme); + cold_renderer.sync_atlas(&device, &queue, &mut cold_font); + let cold_elapsed = cold_start.elapsed(); + let cold_rows_rebuilt = cold_renderer.rows_rebuilt_last_frame(); + + // Warm atlas: same font/atlas already holding every glyph from the + // build above, but a *fresh* cache miss (bumped `abs_row_base`, as a + // backgrounded tab's scrollback would) forces a second full rebuild — + // isolating row-shaping/layout cost with rasterization taken out. + let warm_snap = snapshot_with_row_base(&mut terminal, 5_000); + let warm_start = Instant::now(); + cold_renderer.rebuild_cells(&warm_snap, &mut cold_font, &theme); + cold_renderer.sync_atlas(&device, &queue, &mut cold_font); + let warm_elapsed = warm_start.elapsed(); + let warm_rows_rebuilt = cold_renderer.rows_rebuilt_last_frame(); + + eprintln!( + "[tab-switch-bench] {}x{} full rebuild: cold-atlas {:.3}ms (rows_rebuilt={}), warm-atlas {:.3}ms (rows_rebuilt={})", + COLS, + ROWS, + cold_elapsed.as_secs_f64() * 1e3, + cold_rows_rebuilt, + warm_elapsed.as_secs_f64() * 1e3, + warm_rows_rebuilt, + ); + + assert_eq!( + cold_rows_rebuilt, ROWS as u64, + "first-ever rebuild must be a full rebuild of every row" + ); + assert_eq!( + warm_rows_rebuilt, ROWS as u64, + "bumped abs_row_base must force a second full rebuild (cache-miss path)" + ); +} From 70b78c28f3e6c126f229d39b6d48450b13ad4c3d Mon Sep 17 00:00:00 2001 From: "shingo.imota" Date: Fri, 17 Jul 2026 18:17:38 +0900 Subject: [PATCH 3/3] perf(app): eliminate tab-switch stall with background refresh and instant reveal Switching to a tab with heavy output froze for a frame: while occluded, redraw() early-returns, so the pane render cache goes fully stale and the first frame after reveal rebuilds the whole viewport synchronously (93ms cold-atlas / 38ms warm-atlas at 200x60, per the baseline bench). - Throttled background cache refresh: dirty occluded windows refresh the pane cache and glyph atlas at most once per 250ms globally (one window per interval, least-recently-refreshed first), driven by pty output events plus a one-shot trailing wake-up per throttle window (fully idle again once the dirty backlog drains), without touching the 1x1 occluded swapchain. A refresh that could not reuse the row cache incrementally (e.g. the tab scrolled past a whole viewport) is skipped to keep the event-loop cost bounded; its panes are invalidated so the consumed row damage is deferred to the next full rebuild, never lost. - Instant reveal: the first redraw after Occluded(false) presents the cached instances (when the renderer-side guards allow reuse) and defers the now mostly-incremental rebuild to an immediately scheduled follow-up frame. The fast-path frame stashes its captured snapshots so the follow-up rebuilds against the exact damage it consumed, then schedules one more redraw to pick up output that landed in between. Re-occluding before the follow-up frame invalidates the stashed panes' row caches so the consumed damage is never lost. - NOA_TAB_SWITCH_TRACE=1 env-gated tracer logging the reveal-to-present breakdown and background refreshes. --- crates/noa-app/src/app.rs | 27 ++ crates/noa-app/src/app/event_loop.rs | 58 ++- crates/noa-app/src/app/helpers/dispatch.rs | 85 ++++ crates/noa-app/src/app/helpers/tests.rs | 102 +++++ crates/noa-app/src/app/lifecycle.rs | 2 + crates/noa-app/src/app/quick_terminal.rs | 2 + crates/noa-app/src/app/render.rs | 499 ++++++++++++++++++--- crates/noa-app/src/app/state.rs | 22 + crates/noa-app/src/app/timers.rs | 20 + crates/noa-app/src/lib.rs | 1 + crates/noa-app/src/tab_switch_trace.rs | 197 ++++++++ 11 files changed, 959 insertions(+), 56 deletions(-) create mode 100644 crates/noa-app/src/tab_switch_trace.rs diff --git a/crates/noa-app/src/app.rs b/crates/noa-app/src/app.rs index dcfffa59..f16ca238 100644 --- a/crates/noa-app/src/app.rs +++ b/crates/noa-app/src/app.rs @@ -429,6 +429,30 @@ pub struct App { /// spawn finds the slot empty and gets the normal shell. `Mutex` only /// because `spawn_pane_surface` takes `&self`. initial_command: Mutex>>, + /// Global throttle gate for the tab-switch-stall background pane-cache + /// refresh (see `helpers::dispatch::background_refresh_selection`): + /// when any occluded window last had its cache opportunistically + /// refreshed, across ALL windows — not per-window. A per-window gate + /// would admit up to one full-viewport rebuild per window per interval, + /// which with N busy occluded tabs stalls the event loop (and so the + /// foreground window) for up to `N` rebuilds every interval. + last_bg_refresh: Option, + /// Occluded windows with pty output observed since their last background + /// refresh (or since becoming occluded, if never refreshed) — candidates + /// for the next globally-throttled refresh. Populated only by + /// `UserEvent::Redraw` (pty-driven), never by a self-armed wake-up, so an + /// app with no occluded pty activity never spends a cycle on this. + dirty_occluded_windows: HashSet, + /// One-shot trailing wake-up for the background-refresh backlog (kaizen + /// cycle 6, finding P2): armed whenever `dirty_occluded_windows` is + /// non-empty, at the earliest instant the global throttle reopens — + /// so a candidate blocked purely by timing (its output landed inside the + /// throttle window, and no further pty output ever arrives to re-trigger + /// a check) still gets exactly one retry, rather than sitting stale + /// until an unrelated event. `None` whenever the backlog is empty: the + /// `about_to_wait` + `WaitUntil` mechanism then arms no timer for this at + /// all, same as every other idle-power-sensitive tick here. + bg_refresh_wake_deadline: Option, } /// The wgpu foundation prewarmed on a worker: adapter/device are requested @@ -718,6 +742,9 @@ impl App { prespawned_pty: Mutex::new(prespawned_pty), startup_tasks: Some(startup_tasks), initial_command: Mutex::new(initial_command), + last_bg_refresh: None, + dirty_occluded_windows: HashSet::new(), + bg_refresh_wake_deadline: None, } } diff --git a/crates/noa-app/src/app/event_loop.rs b/crates/noa-app/src/app/event_loop.rs index 24d87107..3ac4fc93 100644 --- a/crates/noa-app/src/app/event_loop.rs +++ b/crates/noa-app/src/app/event_loop.rs @@ -189,6 +189,17 @@ impl ApplicationHandler for App { && let Some(state) = self.windows.get(&window_id) { state.window.request_redraw(); + } else if pane_decision == TargetedRedrawDecision::Suppress { + // The window is occluded, so the request above is dropped — + // this pty output would otherwise sit invisible until the + // window reveals, at which point a long-hidden busy pane + // forces a full pane-cache rebuild synchronously (the + // tab-switch stall). Mark it dirty and let the GLOBAL + // (not per-window — see `background_refresh_selection`) + // throttle decide whether — and which occluded window's — + // cache gets opportunistically refreshed right now. + self.dirty_occluded_windows.insert(window_id); + self.maybe_background_refresh_pane_cache(); } if overview_decision == TargetedRedrawDecision::Request { self.request_overview_redraw(); @@ -433,22 +444,65 @@ impl ApplicationHandler for App { } if !occluded { self.reset_cursor_blink_phase(); + // NOA_TAB_SWITCH_TRACE t0: begin the occlusion-reveal + // sample (see `tab_switch_trace` module docs). + crate::tab_switch_trace::on_reveal_start(); } let gpu = self.gpu.as_ref(); if let Some(state) = self.windows.get_mut(&window_id) { state.occluded = occluded; + if occluded { + // A stashed reveal snapshot from a previous fast-path + // frame that never got its guaranteed follow-up + // redraw (e.g. re-occluded before winit delivered it) + // must not simply be dropped (kaizen cycle 5): its + // capturing read already consumed the terminal's row + // damage, so a later rebuild trusting `row_dirty` + // would see those rows as clean and never re-derive + // them — even though this pane's render cache never + // actually applied that content, leaving it stuck + // showing pre-capture output indefinitely if nothing + // else ever re-dirties those exact rows (e.g. a + // one-shot in-place TUI redraw with no further + // output). `invalidate_pane` sidesteps needing the + // stash's content at all: it forces this pane's next + // rebuild (whichever comes first — a real redraw's + // fresh terminal read, or a background refresh) to + // rebuild every row unconditionally, straight from + // the terminal's actual (still-correct) live content + // at that time, bypassing `row_dirty` entirely. Only + // needed for panes that actually had a stash — a + // pane with none is already consistent. + for (&pane_id, surface) in state.surfaces.iter_mut() { + if surface.pending_reveal_snapshot.take().is_some() { + state.renderer.invalidate_pane(render_pane_id(pane_id)); + } + } + } if let Some(gpu) = gpu { + let trace_configure_start = crate::tab_switch_trace::configure_start(); configure_wgpu_surface( &state.surface, &gpu.device, &state.surface_config, state.occluded, ); + crate::tab_switch_trace::on_surface_configured(trace_configure_start); } } if !occluded { + // A reveal redraws with fresh terminal content, so any + // pending background-refresh candidacy is obsolete; keep + // dirty state scoped to a single occlusion cycle so a + // stale candidate can't consume a refresh slot later. + self.dirty_occluded_windows.remove(&window_id); self.sync_current_background_image_to_window(window_id); - if let Some(state) = self.windows.get(&window_id) { + if let Some(state) = self.windows.get_mut(&window_id) { + // Tab-switch stall fix: let the next `redraw()` present + // whatever the renderer already has cached (kept warm + // by background refreshes while occluded) instead of + // forcing a full rebuild synchronously on this frame. + state.reveal_fast_path_pending = true; state.window.request_redraw(); } } @@ -803,6 +857,7 @@ impl ApplicationHandler for App { let live_wallpaper_deadline = self.tick_live_wallpaper(); let kitty_anim_deadline = self.tick_kitty_animations(); let memory_trim_deadline = self.tick_memory_trim(); + let bg_refresh_wake_deadline = self.tick_bg_refresh_wake(); let deadline = [ blink_deadline, resize_throttle_deadline, @@ -817,6 +872,7 @@ impl ApplicationHandler for App { live_wallpaper_deadline, kitty_anim_deadline, memory_trim_deadline, + bg_refresh_wake_deadline, ] .into_iter() .flatten() diff --git a/crates/noa-app/src/app/helpers/dispatch.rs b/crates/noa-app/src/app/helpers/dispatch.rs index d71ef455..acc47e50 100644 --- a/crates/noa-app/src/app/helpers/dispatch.rs +++ b/crates/noa-app/src/app/helpers/dispatch.rs @@ -237,6 +237,91 @@ pub(crate) fn keyboard_preedit_should_swallow_key( modal_preedit_owner.is_some_and(|owner| owner == window_id) || pane_preedit_active } +/// Throttle interval for an occluded window's background pane-cache refresh +/// (tab-switch stall fix). Chosen so a long-hidden busy tab's row cache and +/// glyph atlas stay warm enough that the reveal frame's catch-up rebuild is +/// small, without running a rebuild on every single pty-output redraw a busy +/// occluded pane requests. +pub(crate) const BG_REFRESH_INTERVAL: Duration = Duration::from_millis(250); + +/// Which (if any) occluded window's pane cache should be opportunistically +/// rebuilt right now (present-free, no swapchain touch), given every +/// currently-dirty occluded window and its own last-refresh time. +/// +/// Throttle is GLOBAL, not per-window (kaizen cycle 4, finding P1-B): a +/// per-window `BG_REFRESH_INTERVAL` gate admits up to one full-viewport +/// rebuild PER WINDOW per interval, so N busy occluded tabs stall the event +/// loop — and so the foreground window's own rendering and input handling — +/// for up to N rebuilds every interval. At most one candidate is returned per +/// `interval`, app-wide. +/// +/// Fairness among ready candidates: the one refreshed longest ago (or never, +/// which sorts as "most overdue") wins, so no single busy tab can starve the +/// others by continuously re-dirtying itself — every dirty occluded window +/// eventually gets its turn, oldest-refresh-first. +/// +/// Called only from the `UserEvent::Redraw` path, which already fires +/// exclusively when the io thread observed new pty output — so this is never +/// invoked on a self-armed wake-up; an app with no occluded pty activity +/// never spends a cycle here. +pub(crate) fn background_refresh_selection( + dirty_windows: &[(Id, Option)], + global_last_refresh: Option, + now: Instant, + interval: Duration, +) -> Option { + if let Some(last) = global_last_refresh + && now.saturating_duration_since(last) < interval + { + return None; + } + dirty_windows + .iter() + .max_by_key(|(_, last_refresh)| { + last_refresh.map_or(Duration::MAX, |t| now.saturating_duration_since(t)) + }) + .map(|(id, _)| *id) +} + +/// Next trailing wake-up deadline for the background-refresh backlog +/// (kaizen cycle 6, finding P2): `None` while the backlog is empty (fully +/// idle — the caller arms no timer at all), otherwise the earliest instant +/// the global throttle reopens. Closes the gap where a dirty candidate is +/// blocked purely by timing (its output landed inside the throttle window, +/// and nothing else ever triggers another check) — without this, it would +/// sit stale until an unrelated event happened to redraw again. +/// +/// `dirty_remaining` is `!dirty_occluded_windows.is_empty()` evaluated AFTER +/// whatever this invocation of `maybe_background_refresh_pane_cache` did (or +/// didn't do): the same rule applies whether this call just serviced one +/// candidate and others remain (chain to drain the existing backlog one +/// throttle interval at a time) or was itself blocked by the throttle with +/// candidates still waiting (arm the one retry). Either way, `last_refresh` +/// (the GLOBAL last-refresh instant) is the anchor: the throttle can't +/// reopen before `last_refresh + interval`, regardless of which of those two +/// cases produced this call. +pub(crate) fn bg_refresh_wake_deadline( + dirty_remaining: bool, + last_refresh: Option, + interval: Duration, +) -> Option { + if !dirty_remaining { + return None; + } + last_refresh.map(|last| last + interval) +} + +/// Whether `redraw()`'s reveal frame may skip the pane-cache rebuild this +/// frame and present the renderer's already-built instances as-is. +/// `pending` is the window's one-shot post-`Occluded(false)` flag; +/// `has_renderable_frame` guards the case where the renderer has nothing +/// usable yet (never rendered, or the viewport changed since its last build) +/// — that case must fall back to the normal full rebuild instead of +/// presenting garbage or a wrongly-sized layout. +pub(crate) fn reveal_fast_path_decision(pending: bool, has_renderable_frame: bool) -> bool { + pending && has_renderable_frame +} + pub(crate) fn pane_user_event_redraw_decision( pane_state: Option<(bool, bool)>, ) -> TargetedRedrawDecision { diff --git a/crates/noa-app/src/app/helpers/tests.rs b/crates/noa-app/src/app/helpers/tests.rs index bc6e8dc3..05367e8a 100644 --- a/crates/noa-app/src/app/helpers/tests.rs +++ b/crates/noa-app/src/app/helpers/tests.rs @@ -1294,6 +1294,108 @@ fn targeted_redraw_decision_drops_stale_and_suppresses_occluded_tabs() { ); } +#[test] +fn background_refresh_selection_returns_none_with_no_dirty_windows() { + let now = Instant::now(); + assert_eq!( + background_refresh_selection::(&[], None, now, BG_REFRESH_INTERVAL), + None + ); +} + +#[test] +fn background_refresh_selection_is_due_immediately_with_no_prior_global_refresh() { + let now = Instant::now(); + assert_eq!( + background_refresh_selection(&[(1u32, None)], None, now, BG_REFRESH_INTERVAL), + Some(1) + ); +} + +/// The global gate, not a per-window one (kaizen cycle 4, finding P1-B): even +/// though window 2 has never been refreshed (individually "due"), a global +/// refresh within the interval blocks EVERY window's turn, not just window 1's. +#[test] +fn background_refresh_selection_throttles_globally_across_every_window() { + let start = Instant::now(); + let dirty = [(1u32, Some(start)), (2u32, None)]; + assert_eq!( + background_refresh_selection( + &dirty, + Some(start), + start + BG_REFRESH_INTERVAL - Duration::from_millis(1), + BG_REFRESH_INTERVAL + ), + None + ); + assert_eq!( + background_refresh_selection( + &dirty, + Some(start), + start + BG_REFRESH_INTERVAL, + BG_REFRESH_INTERVAL + ), + Some(2) + ); +} + +/// Fairness: among ready candidates, the one refreshed longest ago (or never) +/// wins — a continuously-busy window can't starve a quieter one. +#[test] +fn background_refresh_selection_prefers_the_least_recently_refreshed_window() { + let now = Instant::now(); + let long_ago = now - Duration::from_secs(10); + let recently = now - Duration::from_millis(1); + let dirty = [(1u32, Some(recently)), (2u32, Some(long_ago)), (3u32, None)]; + // Window 3 has never been refreshed at all — more overdue than any + // timestamped window, however old. + assert_eq!( + background_refresh_selection(&dirty, None, now, BG_REFRESH_INTERVAL), + Some(3) + ); + let dirty_without_never_refreshed = [(1u32, Some(recently)), (2u32, Some(long_ago))]; + assert_eq!( + background_refresh_selection( + &dirty_without_never_refreshed, + None, + now, + BG_REFRESH_INTERVAL + ), + Some(2) + ); +} + +#[test] +fn bg_refresh_wake_deadline_is_none_when_no_dirty_backlog_remains() { + // Fully idle: no dirty candidates left, so no timer is armed at all — + // regardless of when the last refresh happened. + let now = Instant::now(); + assert_eq!( + bg_refresh_wake_deadline(false, Some(now), BG_REFRESH_INTERVAL), + None + ); + assert_eq!( + bg_refresh_wake_deadline(false, None, BG_REFRESH_INTERVAL), + None + ); +} + +#[test] +fn bg_refresh_wake_deadline_is_the_next_throttle_reopening_when_backlog_remains() { + let last_refresh = Instant::now(); + assert_eq!( + bg_refresh_wake_deadline(true, Some(last_refresh), BG_REFRESH_INTERVAL), + Some(last_refresh + BG_REFRESH_INTERVAL) + ); +} + +#[test] +fn reveal_fast_path_requires_both_pending_and_a_renderable_cached_frame() { + assert!(!reveal_fast_path_decision(false, true)); + assert!(!reveal_fast_path_decision(true, false)); + assert!(reveal_fast_path_decision(true, true)); +} + #[test] fn stale_pane_user_event_redraw_decision_noops_without_panicking() { assert_eq!( diff --git a/crates/noa-app/src/app/lifecycle.rs b/crates/noa-app/src/app/lifecycle.rs index b145a651..e06a7f4a 100644 --- a/crates/noa-app/src/app/lifecycle.rs +++ b/crates/noa-app/src/app/lifecycle.rs @@ -587,6 +587,8 @@ impl App { title_override: None, native_overlays: Default::default(), applied_window_bg: None, + bg_refresh_last: None, + reveal_fast_path_pending: false, }, ); if let Some(identity) = remote_card_identity.as_ref() { diff --git a/crates/noa-app/src/app/quick_terminal.rs b/crates/noa-app/src/app/quick_terminal.rs index 839dcf71..36061c82 100644 --- a/crates/noa-app/src/app/quick_terminal.rs +++ b/crates/noa-app/src/app/quick_terminal.rs @@ -849,6 +849,8 @@ impl App { bell_flash_until: None, native_overlays: Default::default(), applied_window_bg: None, + bg_refresh_last: None, + reveal_fast_path_pending: false, }, ); self.relayout_and_resize_window(window_id); diff --git a/crates/noa-app/src/app/render.rs b/crates/noa-app/src/app/render.rs index a68fca0c..28e1d5b6 100644 --- a/crates/noa-app/src/app/render.rs +++ b/crates/noa-app/src/app/render.rs @@ -3,6 +3,247 @@ use super::*; impl App { + /// Globally-throttled entry point for occluded windows' background + /// pane-cache refresh (tab-switch stall fix). At most one window's cache + /// is refreshed per `BG_REFRESH_INTERVAL` ACROSS THE WHOLE APP (kaizen + /// cycle 4, finding P1-B) — a per-window gate would let N busy occluded + /// tabs each admit their own rebuild every interval, stalling the event + /// loop (and so the foreground window) for up to N rebuilds per + /// interval. Only called from the `UserEvent::Redraw` path, which fires + /// exclusively on real pty output, so an idle app never wakes up on its + /// own. `dirty_occluded_windows` may retain ids for windows that have + /// since un-occluded or closed; those are filtered out before selection + /// and dropped from the set here so it can't grow unbounded. + /// + /// Kaizen cycle 6, finding P2: whether or not this call actually + /// refreshes a window, it always re-derives `bg_refresh_wake_deadline` + /// (via the pure `bg_refresh_wake_deadline`) from whatever backlog + /// remains afterward — so a candidate that's dirty but blocked purely by + /// the throttle (no further pty output ever arrives to re-trigger this + /// function) still gets exactly one trailing retry at the moment the + /// throttle reopens, instead of sitting stale until an unrelated event. + /// `timers::tick_bg_refresh_wake` is what actually fires that retry, via + /// the same `about_to_wait` + `WaitUntil` mechanism every other timer + /// uses. Once the backlog drains, this reports `None` and the app goes + /// fully idle again — no periodic self-wakeup while there is truly + /// nothing left to do. + pub(super) fn maybe_background_refresh_pane_cache(&mut self) { + self.dirty_occluded_windows + .retain(|id| self.windows.get(id).is_some_and(|state| state.occluded)); + let now = Instant::now(); + let candidates: Vec<(WindowId, Option)> = self + .dirty_occluded_windows + .iter() + .filter_map(|&id| { + self.windows + .get(&id) + .map(|state| (id, state.bg_refresh_last)) + }) + .collect(); + if let Some(target) = background_refresh_selection( + &candidates, + self.last_bg_refresh, + now, + BG_REFRESH_INTERVAL, + ) { + self.last_bg_refresh = Some(now); + self.dirty_occluded_windows.remove(&target); + if let Some(state) = self.windows.get_mut(&target) { + state.bg_refresh_last = Some(now); + } + self.background_refresh_pane_cache(target); + } + self.bg_refresh_wake_deadline = bg_refresh_wake_deadline( + !self.dirty_occluded_windows.is_empty(), + self.last_bg_refresh, + BG_REFRESH_INTERVAL, + ); + } + + /// Rebuild an occluded window's `PaneRenderCache` against its terminals' + /// current content, WITHOUT touching the (shrunk 1×1) swapchain or ever + /// presenting — so it keeps the row cache overlap and glyph atlas warm + /// for whenever this window reveals, instead of a long occlusion forcing + /// a full rebuild synchronously on the reveal frame. Deliberately a + /// stripped-down version of `redraw()`'s snapshot loop: title, sidebar, + /// scrollbar-thumb, and modal-overlay state are all display-only and + /// need no upkeep while nothing is on screen. + fn background_refresh_pane_cache(&mut self, window_id: WindowId) { + let (Some(gpu), Some(state)) = (self.gpu.as_mut(), self.windows.get_mut(&window_id)) else { + return; + }; + if !state.occluded { + return; + } + let mut snapshots = Vec::new(); + let visible_panes = visible_pane_ids(&state.split_tree, state.zoomed); + let now = Instant::now(); + for pane_id in visible_panes { + let Some(surface) = state.surfaces.get_mut(&pane_id) else { + continue; + }; + let mut term = surface.terminal.lock(); + let active = term.active(); + let cursor = active.cursor; + let (active_cols, active_rows) = (active.cols, active.rows); + surface.cursor_blink_state = CursorBlinkState { + visible: cursor.visible, + style: cursor.style, + at_live_viewport: term.viewport_offset() == 0, + }; + // Kaizen cycle 5: structurally, this pane's window is occluded + // (checked above), and `redraw()` never runs — so never stashes + // a NEW `pending_reveal_snapshot` — while occluded; any stash + // from just before occlusion is drained (and its pane cache + // invalidated) by the `Occluded(true)` handler before the window + // is considered occluded at all. So this should always be `None` + // here. Still checked, not assumed: consuming it exactly like + // `redraw()`'s per-pane loop does closes the same lost/backward- + // damage class this function would otherwise reopen if that + // invariant were ever violated by a future change to either side. + let mut snapshot = if let Some(pending) = surface.pending_reveal_snapshot.take() { + pending + } else { + // Same synchronized-output hold bookkeeping `redraw()` uses + // (see `sync_output_snapshot_decision`), so a pane using + // DECSET 2026 isn't left with mismatched hold state once its + // window reveals. + let synchronized = term.modes.synchronized_output(); + let dimensions_match = surface.held_snapshot.as_ref().is_some_and(|held| { + held.snapshot.cols == active_cols && held.snapshot.rows_n == active_rows + }); + let decision = sync_output_snapshot_decision( + synchronized, + surface.held_snapshot.as_ref().map(|held| held.captured_at), + now, + dimensions_match, + false, + ); + match decision { + SyncSnapshotDecision::Reuse => surface + .held_snapshot + .as_ref() + .expect("Reuse is only decided when held_snapshot is Some") + .snapshot + .clone(), + SyncSnapshotDecision::Fresh => { + let fresh = FrameSnapshot::from_terminal_recycle( + &mut term, + std::mem::take(&mut surface.snapshot_recycle), + ); + if sync_output_snapshot_release_decision(synchronized) { + if surface.held_snapshot.is_some() { + surface.held_snapshot = None; + } + } else { + surface.held_snapshot = Some(HeldSnapshot { + snapshot: fresh.clone(), + captured_at: now, + }); + } + fresh + } + } + }; + snapshot.focused = + pane_owns_keyboard_focus(window_id, pane_id, self.os_focused, state.focused_pane); + snapshot.cursor_blink_visible = self.cursor_blink_visible; + snapshot.hover_link = surface.hover_link; + snapshots.push((pane_id, surface.rect, snapshot)); + } + let panes = snapshots + .iter() + .map(|(pane_id, rect, snapshot)| PaneFrame { + pane: render_pane_id(*pane_id), + rect: render_pane_rect(*rect), + snapshot, + }) + .collect::>(); + let theme = active_theme(&gpu.theme, &gpu.preview_theme); + // Kaizen cycle 6, finding P1: bound this call's main-thread cost. A + // hidden pane that scrolled more than one viewport (or has no cache + // yet, or its invalidation key otherwise diverged) since its cache + // was last built can never hit `cell::rebuild_pane_cached`'s + // incremental/scroll-shift path — rebuilding it now would always be + // a full, synchronous rebuild (measured 38-93ms for a 200x60 pane), + // and that cost is wasted anyway: the eventual reveal's catch-up + // frame rebuilds fully regardless of what this background refresh + // did or didn't do (see `Renderer::pane_rebuild_would_be_full`'s doc + // comment). `rebuild_panes` always rebuilds every pane in `panes` + // together (there is no way to apply only the cheap panes without + // dropping the expensive one out of `pane_layout` entirely), so the + // check and the skip are both whole-window, not per-pane. + // + // Worst-case bounded cost this call can still incur: this window's + // visible-pane count worth of snapshot captures (terminal lock + + // row-damage take, no font rasterization — cheap), plus, only when + // EVERY pane predicts incremental, one incremental `rebuild_panes` + // bounded by that snapshot's actual dirty-row count (a pane that + // rewrites every row in place with no scrolling is not caught by + // this guard and can still cost close to a full rebuild — an + // accepted residual per the reviewed design, distinct from the + // scroll-volume case this guard specifically targets). + let any_pane_would_be_full = panes.iter().any(|pane| { + state + .renderer + .pane_rebuild_would_be_full(pane.pane, pane.snapshot, &gpu.font, theme) + }); + if any_pane_would_be_full { + // CRITICAL fix (kaizen cycle 7): every visible pane's snapshot + // above already had its terminal row damage consumed — the + // capture loop runs unconditionally regardless of what happens + // next. Simply discarding them here (as an earlier version of + // this skip did) loses whichever OTHER pane's in-place edits got + // captured alongside the one that actually forced this skip: a + // mixed window where pane A is scrolling (predicts full) and + // pane B just got a same-key in-place edit (would have been + // incremental) would show B's pre-edit content indefinitely, + // since B's dirty bits are gone and its cache key still matches. + // + // Fix: force EVERY visible pane onto a full rebuild next time + // (`invalidate_pane`), so whichever rebuild happens next — + // another bg-refresh attempt, or the reveal catch-up — reads + // every row straight from the terminal's actual live content at + // that time, independent of any dirty bit this round already + // consumed. This forfeits this round's incremental opportunity + // for every pane in the window (not just the one that scrolled), + // accepted per the reviewed design. + // + // Rejected alternative: stash-on-skip (reusing + // `Surface::pending_reveal_snapshot`, the P1-A mechanism). + // Rejected because that field is consumed unconditionally by + // EVERY `redraw()` call for that pane (not gated to its + // fast-path frame) — a stash planted here could sit across + // arbitrarily many bg-refresh throttle intervals and then get + // applied to the REAL presented reveal frame in place of a + // fresh terminal read, showing stale (potentially very old) + // content on an actual visible frame — a worse bug than the one + // being fixed. `invalidate_pane` has no such hazard: it never + // substitutes stale content for a fresh read: it only ever + // forces a rebuild to look at MORE rows than the dirty bits + // alone would ask for, straight from whatever's actually there + // at rebuild time. + for (pane_id, _, _) in &snapshots { + state.renderer.invalidate_pane(render_pane_id(*pane_id)); + } + } else { + let trace_start = crate::tab_switch_trace::bg_refresh_start(); + state.renderer.rebuild_panes(&panes, &mut gpu.font, theme); + crate::tab_switch_trace::on_bg_refresh( + trace_start, + state.renderer.rows_rebuilt_last_frame(), + ); + state + .renderer + .sync_atlas(&gpu.device, &gpu.queue, &mut gpu.font); + } + for (pane_id, _, snapshot) in snapshots { + if let Some(surface) = state.surfaces.get_mut(&pane_id) { + surface.snapshot_recycle = snapshot.into_recycle(); + } + } + } + pub(super) fn redraw(&mut self, window_id: WindowId) { // NOA_LATENCY_TRACE: stamp before the FrameSnapshot is built so // `on_present` can tell whether this frame could contain a pending @@ -141,8 +382,52 @@ impl App { if state.occluded { return; } + // The CURRENT split layout's `(PaneId, PaneRect)` set, in the exact + // form `rebuild_panes` would receive this frame — cheap (no terminal + // lock), and computed up front so the fast-path guard below can + // catch a split added/closed or a pane closed while occluded (P2-C): + // that changes nothing about viewport or atlas, so without this + // check the guard would otherwise present a stale, closed-pane + // layout. + let expected_panes: Vec<(RenderPaneId, PaneRect)> = + visible_pane_ids(&state.split_tree, state.zoomed) + .into_iter() + .filter_map(|pane_id| { + state + .surfaces + .get(&pane_id) + .map(|surface| (render_pane_id(pane_id), render_pane_rect(surface.rect))) + }) + .collect(); + // Tab-switch stall fix: consume the one-shot post-reveal flag + // regardless of outcome — it only ever applies to the very next + // redraw after `Occluded(false)`, win or lose. `has_renderable_frame` + // is the fallback guard (never rendered, the viewport or pane layout + // changed while occluded, the shared glyph atlas moved since, or the + // cache never stabilized — see `Renderer::cached_frame_matches_viewport`), + // which forces the normal full rebuild instead. + let reveal_fast_path = reveal_fast_path_decision( + std::mem::take(&mut state.reveal_fast_path_pending), + state.renderer.cached_frame_matches_viewport( + PixelSize { + w: state.surface_config.width, + h: state.surface_config.height, + }, + &gpu.font, + &expected_panes, + ), + ); let mut snapshots = Vec::new(); + // Set when any pane this frame consumed a `pending_reveal_snapshot` + // (kaizen cycle 4, finding P1-A): that stash reflects the terminal + // as of the FAST-PATH frame, not now — any pty output arriving in + // between coalesces into this same redraw (winit merges repeated + // `request_redraw` calls) and would otherwise sit unpresented until + // an unrelated event (cursor blink, input) happens to redraw again. + // Closing this deterministically costs one extra (normal, + // incremental) redraw rather than an unbounded wait. + let mut consumed_pending_reveal_snapshot = false; // A user-set override wins over the focused pane's shell title // (tab-title REQ-TTL-2/5); the shell path below only applies while // there is no override. @@ -234,58 +519,72 @@ impl App { viewport_rows: term.active().rows, }); } - // Synchronized output (DECSET 2026, read under the lock already - // held above — no second lock, see R3's cursor-blink cache): a - // redraw triggered from outside the io thread's own pacing (focus - // change, cursor blink, an unrelated pane's redraw in the same - // window) can otherwise land mid-update and capture a torn frame. - // `sync_output_snapshot_decision` picks between reading the - // terminal fresh and reusing this pane's last held snapshot. - let synchronized = term.modes.synchronized_output(); - let dimensions_match = surface.held_snapshot.as_ref().is_some_and(|held| { - held.snapshot.cols == active_cols && held.snapshot.rows_n == active_rows - }); - let decision = sync_output_snapshot_decision( - synchronized, - surface.held_snapshot.as_ref().map(|held| held.captured_at), - now, - dimensions_match, - copy_mode_active, - ); - let mut snapshot = match decision { - SyncSnapshotDecision::Reuse => surface - .held_snapshot - .as_ref() - .expect("Reuse is only decided when held_snapshot is Some") - .snapshot - .clone(), - SyncSnapshotDecision::Fresh => { - let fresh = FrameSnapshot::from_terminal_recycle( - &mut term, - std::mem::take(&mut surface.snapshot_recycle), - ); - // Only retained while synchronized output is actually - // active: an app that never uses mode 2026 never pays for - // this clone (see `sync_output_snapshot_decision`'s doc - // comment on the performance trade-off). - if sync_output_snapshot_release_decision(synchronized) { - // Sync just ended (or was never on): a held snapshot - // no longer serves any purpose, and keeping it around - // would retain a stale full-grid `FrameSnapshot` for - // the rest of this pane's lifetime. The `is_some()` - // guard keeps the common case — a pane that has never - // used mode 2026 — a single no-op check, not a write - // every frame. - if surface.held_snapshot.is_some() { - surface.held_snapshot = None; + // Tab-switch stall fix (kaizen cycle 3, finding P1-1): a pane + // this window presented via the reveal fast path last frame + // already had its terminal damage consumed into this exact + // snapshot (`FrameSnapshot::from_terminal_recycle` clears the + // grid's dirty bits on read) — that frame deliberately did not + // feed it into `rebuild_panes`. Reuse it here unconditionally + // instead of reading the terminal fresh (which would now see + // zero row damage and rebuild nothing), so this catch-up frame + // rebuilds against the content the fast path actually presented. + let mut snapshot = if let Some(pending) = surface.pending_reveal_snapshot.take() { + consumed_pending_reveal_snapshot = true; + pending + } else { + // Synchronized output (DECSET 2026, read under the lock already + // held above — no second lock, see R3's cursor-blink cache): a + // redraw triggered from outside the io thread's own pacing (focus + // change, cursor blink, an unrelated pane's redraw in the same + // window) can otherwise land mid-update and capture a torn frame. + // `sync_output_snapshot_decision` picks between reading the + // terminal fresh and reusing this pane's last held snapshot. + let synchronized = term.modes.synchronized_output(); + let dimensions_match = surface.held_snapshot.as_ref().is_some_and(|held| { + held.snapshot.cols == active_cols && held.snapshot.rows_n == active_rows + }); + let decision = sync_output_snapshot_decision( + synchronized, + surface.held_snapshot.as_ref().map(|held| held.captured_at), + now, + dimensions_match, + copy_mode_active, + ); + match decision { + SyncSnapshotDecision::Reuse => surface + .held_snapshot + .as_ref() + .expect("Reuse is only decided when held_snapshot is Some") + .snapshot + .clone(), + SyncSnapshotDecision::Fresh => { + let fresh = FrameSnapshot::from_terminal_recycle( + &mut term, + std::mem::take(&mut surface.snapshot_recycle), + ); + // Only retained while synchronized output is actually + // active: an app that never uses mode 2026 never pays for + // this clone (see `sync_output_snapshot_decision`'s doc + // comment on the performance trade-off). + if sync_output_snapshot_release_decision(synchronized) { + // Sync just ended (or was never on): a held snapshot + // no longer serves any purpose, and keeping it around + // would retain a stale full-grid `FrameSnapshot` for + // the rest of this pane's lifetime. The `is_some()` + // guard keeps the common case — a pane that has never + // used mode 2026 — a single no-op check, not a write + // every frame. + if surface.held_snapshot.is_some() { + surface.held_snapshot = None; + } + } else { + surface.held_snapshot = Some(HeldSnapshot { + snapshot: fresh.clone(), + captured_at: now, + }); } - } else { - surface.held_snapshot = Some(HeldSnapshot { - snapshot: fresh.clone(), - captured_at: now, - }); + fresh } - fresh } }; snapshot.search_prompt = self @@ -363,11 +662,47 @@ impl App { snapshot, }) .collect::>(); - state.renderer.rebuild_panes( - &panes, - &mut gpu.font, - active_theme(&gpu.theme, &gpu.preview_theme), - ); + if reveal_fast_path { + // Instant reveal frame: present the renderer's already-cached + // instances (kept warm by background refreshes while occluded, + // or simply the last frame drawn before occlusion) as-is, and + // request an immediate follow-up redraw to do the real + // (now mostly-incremental, thanks to the warm cache) rebuild. + // `panes` above is unused this frame — its only job was feeding + // `rebuild_panes`, which this branch deliberately skips. + // + // P1-1: the snapshot loop above already read each pane's + // terminal fresh (as it does every frame) and so already + // consumed its row damage — that read is simply not fed into a + // rebuild this frame. Stash it per pane so the follow-up redraw + // requested below rebuilds against exactly this content instead + // of re-reading the terminal and finding no damage left. + for (pane_id, _, snapshot) in &snapshots { + if let Some(surface) = state.surfaces.get_mut(pane_id) { + surface.pending_reveal_snapshot = Some(snapshot.clone()); + } + } + crate::tab_switch_trace::on_fast_path_reveal(); + state.window.request_redraw(); + } else { + // NOA_TAB_SWITCH_TRACE t2: the pane cache rebuild — a full rebuild + // of every visible row is the dominant suspected cost on the + // occlusion-reveal path (see `tab_switch_trace` module docs). + let trace_rebuild_start = crate::tab_switch_trace::rebuild_start(); + state.renderer.rebuild_panes( + &panes, + &mut gpu.font, + active_theme(&gpu.theme, &gpu.preview_theme), + ); + crate::tab_switch_trace::on_pane_rebuild( + trace_rebuild_start, + state.renderer.rows_rebuilt_last_frame(), + ); + } + // Atlas sync still runs on the fast path: any glyphs rasterized by a + // background refresh while occluded need their texture uploaded + // before this frame's (reused) instances reference them; cheap no-op + // when nothing changed since the last sync. state .renderer .sync_atlas(&gpu.device, &gpu.queue, &mut gpu.font); @@ -749,6 +1084,9 @@ impl App { // NOA_LATENCY_TRACE t2: the echo frame has been handed to the // compositor (present-call proxy; see `latency_trace` module docs). crate::latency_trace::on_present(trace_frame_start); + // NOA_TAB_SWITCH_TRACE t3: closes the occlusion-reveal sample (if + // one is pending) and logs the full breakdown. + crate::tab_switch_trace::on_present(); { static FIRST_FRAME: std::sync::atomic::AtomicBool = std::sync::atomic::AtomicBool::new(false); @@ -761,6 +1099,14 @@ impl App { if state.renderer.needs_follow_up_frame() { state.window.request_redraw(); } + // P1-A: this frame presented at least one pane's stashed reveal + // snapshot instead of reading the terminal, so any pty output that + // arrived between the fast-path frame and this one was never read — + // request one more (now normal, no stash left, so a real terminal + // read) redraw to pick it up deterministically. + if consumed_pending_reveal_snapshot { + state.window.request_redraw(); + } // Hand each snapshot's row buffer back to its pane so the next // frame's `from_terminal_recycle` reuses allocations and clean rows. @@ -1102,4 +1448,47 @@ mod tests { assert_eq!(snapshot.selection, terminal.active().selection); assert_eq!(snapshot.row_base, terminal.active().visible_row_base()); } + + /// Regression test (kaizen cycle 3, finding P1-1), pinned at the pure + /// `FrameSnapshot` mechanism `redraw`'s fast path depends on: reading a + /// terminal's snapshot consumes its row damage, so a caller that reads + /// one snapshot and discards it (as the reveal fast path does — it + /// deliberately does not feed that read into `rebuild_panes`) MUST carry + /// that exact snapshot forward for the next rebuild, because a second, + /// independent read finds the damage already gone. This is the bug the + /// fast path had before `Surface::pending_reveal_snapshot` was added: + /// dirty rows captured on the fast-path frame would otherwise vanish + /// until unrelated pty output re-dirtied them. + #[test] + fn a_second_snapshot_read_sees_no_damage_the_first_read_already_consumed() { + let mut terminal = Terminal::new(GridSize::new(4, 2)); + Stream::new().feed(b"before", &mut terminal); + // Establish a clean baseline (mirrors production: `redraw` always + // takes a snapshot every frame, so any earlier frame's damage is + // already consumed by the time this scenario starts). + let _ = FrameSnapshot::from_terminal(&mut terminal); + + Stream::new().feed(b"\x1b[Hafter", &mut terminal); + let fast_path_read = FrameSnapshot::from_terminal(&mut terminal); + assert!( + fast_path_read.row_dirty.iter().any(|&dirty| dirty), + "the mutated row must show up as damaged on the read that observes it" + ); + + // Simulate the bug: a naive follow-up frame re-reads the terminal + // instead of reusing `fast_path_read`. + let naive_second_read = FrameSnapshot::from_terminal(&mut terminal); + assert!( + naive_second_read.row_dirty.iter().all(|&dirty| !dirty), + "a second independent read must see zero damage — proving the \ + fast-path frame's read has to be carried forward, not discarded, \ + or this content silently never reaches a rebuild" + ); + + // The fix: `fast_path_read` itself (stashed in + // `Surface::pending_reveal_snapshot` and consumed by the very next + // redraw) still carries the correct damage, independent of whatever + // a second terminal read would have seen. + assert!(fast_path_read.row_dirty.iter().any(|&dirty| dirty)); + } } diff --git a/crates/noa-app/src/app/state.rs b/crates/noa-app/src/app/state.rs index 5035d275..23d893c9 100644 --- a/crates/noa-app/src/app/state.rs +++ b/crates/noa-app/src/app/state.rs @@ -332,6 +332,18 @@ pub(super) struct WindowState { /// dragging a full AppKit layout + CA commit into every cursor-blink /// frame (measured as the largest single main-thread idle cost). pub(super) applied_window_bg: Option<(noa_core::Rgb, u32)>, + /// Last time this window's pane cache was refreshed while occluded (tab + /// switch stall fix). `None` means no background refresh has run yet + /// since this window last became occluded — the throttle in + /// `background_refresh_decision` treats that as immediately due. + pub(super) bg_refresh_last: Option, + /// Set on `WindowEvent::Occluded(false)`; consumed by the very next + /// `redraw()` call, which presents whatever the renderer already has + /// cached (possibly refreshed in the background while occluded, up to + /// `BG_REFRESH_INTERVAL` stale) instead of forcing a full pane-cache + /// rebuild synchronously on the reveal frame. That redraw always requests + /// a follow-up frame to catch the cache up. + pub(super) reveal_fast_path_pending: bool, } /// How long the `cols × rows` resize toast stays up after the last grid @@ -963,6 +975,15 @@ pub(super) struct Surface { /// captured while synchronized output was active — see /// `sync_output_snapshot_decision` in `render.rs`. pub(super) held_snapshot: Option, + /// Set for one frame when the tab-switch-stall reveal fast path presents + /// this pane without rebuilding (`render.rs`'s `reveal_fast_path` + /// branch): the fresh snapshot that frame captured (and so already + /// consumed the terminal's row damage for), carried forward so the + /// guaranteed follow-up redraw rebuilds against it directly instead of + /// re-reading the terminal and finding the damage already cleared + /// (kaizen cycle 3, finding P1-1). Consumed (`take`n) by the very next + /// redraw of this pane, fast path or not. + pub(super) pending_reveal_snapshot: Option, } /// Transport-specific ownership for a pane. Keeping this explicit prevents a @@ -1069,6 +1090,7 @@ impl Surface { kitty_animation_flag, cursor_blink_state: CursorBlinkState::default(), held_snapshot: None, + pending_reveal_snapshot: None, } } diff --git a/crates/noa-app/src/app/timers.rs b/crates/noa-app/src/app/timers.rs index a43562fe..9fa77d20 100644 --- a/crates/noa-app/src/app/timers.rs +++ b/crates/noa-app/src/app/timers.rs @@ -676,6 +676,26 @@ impl App { self.request_overview_redraw(); None } + + /// One-shot trailing retry for the tab-switch-stall background pane-cache + /// refresh's backlog (kaizen cycle 6, finding P2): fires exactly one + /// `maybe_background_refresh_pane_cache` attempt once the global throttle + /// reopens, closing the gap where a dirty occluded window is blocked + /// purely by timing and no further pty output ever arrives to re-trigger + /// a check. That call itself re-derives the next deadline (see + /// `bg_refresh_wake_deadline`) from whatever backlog remains afterward — + /// draining `dirty_occluded_windows` one throttle interval at a time + /// until it empties, at which point this returns `None` and the app goes + /// fully idle again (no periodic self-wakeup once there is truly nothing + /// left to do). + pub(super) fn tick_bg_refresh_wake(&mut self) -> Option { + let deadline = self.bg_refresh_wake_deadline?; + if Instant::now() < deadline { + return Some(deadline); + } + self.maybe_background_refresh_pane_cache(); + self.bg_refresh_wake_deadline + } } #[cfg(test)] diff --git a/crates/noa-app/src/lib.rs b/crates/noa-app/src/lib.rs index d80f68d2..00bb34ed 100644 --- a/crates/noa-app/src/lib.rs +++ b/crates/noa-app/src/lib.rs @@ -43,6 +43,7 @@ mod session_store; mod sidebar; pub mod split_tree; pub mod startup_trace; +mod tab_switch_trace; mod theme; mod theme_favorites; mod theme_settings; diff --git a/crates/noa-app/src/tab_switch_trace.rs b/crates/noa-app/src/tab_switch_trace.rs new file mode 100644 index 00000000..5c61ef6d --- /dev/null +++ b/crates/noa-app/src/tab_switch_trace.rs @@ -0,0 +1,197 @@ +//! Env-gated tab-switch (occlusion-reveal) latency instrumentation +//! (`NOA_TAB_SWITCH_TRACE=1`). +//! +//! macOS tabs are separate windows: switching tabs occludes the outgoing +//! window and un-occludes the incoming one. This traces the incoming +//! window's reveal path: +//! +//! ```text +//! WindowEvent::Occluded(false) [t0] +//! → configure_wgpu_surface (swapchain 1×1 → full size) [t1] +//! → request_redraw → redraw() +//! → rebuild_panes (PaneRenderCache rebuild, possibly +//! a full rebuild of every visible row) [t2] +//! → surface present() [t3] +//! ``` +//! +//! and logs `t1 − t0` (surface configure), `t2 − t1` (pane cache rebuild, +//! plus rows rebuilt), and `t3 − t0` (total reveal→present) to stderr. +//! +//! Zero cost when disabled: every public hook first checks one cached bool +//! (a `OnceLock` env read) and returns. +//! +//! Attribution model: the tracer is process-global and keeps a single +//! pending reveal (like `latency_trace`'s single pending keypress) — it is +//! meant for a one-window-at-a-time tab-switch benchmark, not for +//! attribution across concurrently occluding/revealing windows. A reveal +//! whose window never presents again (e.g. immediately re-occluded) never +//! logs; the next reveal simply replaces it. + +use std::sync::OnceLock; +use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; +use std::time::Instant; + +/// `0` = no pending sample; real timestamps are nudged to ≥ 1. +static REVEAL_AT: AtomicU64 = AtomicU64::new(0); +static CONFIGURE_NS: AtomicU64 = AtomicU64::new(0); +static REBUILD_NS: AtomicU64 = AtomicU64::new(0); +static ROWS_REBUILT: AtomicU64 = AtomicU64::new(0); +/// Set when the pending reveal's frame took the fast path (presented the +/// renderer's already-cached instances instead of forcing a synchronous +/// rebuild). Read and cleared by `on_present`. +static FAST_PATH: AtomicBool = AtomicBool::new(false); + +fn enabled() -> bool { + static ENABLED: OnceLock = OnceLock::new(); + *ENABLED.get_or_init(|| { + std::env::var("NOA_TAB_SWITCH_TRACE").is_ok_and(|value| !value.is_empty() && value != "0") + }) +} + +fn epoch() -> Instant { + static EPOCH: OnceLock = OnceLock::new(); + *EPOCH.get_or_init(Instant::now) +} + +fn now_ns() -> u64 { + (Instant::now().saturating_duration_since(epoch()).as_nanos() as u64).max(1) +} + +/// Main thread, on `WindowEvent::Occluded(false)`, before +/// `configure_wgpu_surface` runs. Starts a new sample (replacing any +/// unfinished one — see the attribution model above). +pub(crate) fn on_reveal_start() { + if !enabled() { + return; + } + CONFIGURE_NS.store(0, Ordering::Relaxed); + REBUILD_NS.store(0, Ordering::Relaxed); + ROWS_REBUILT.store(0, Ordering::Relaxed); + FAST_PATH.store(false, Ordering::Relaxed); + REVEAL_AT.store(now_ns(), Ordering::Relaxed); +} + +/// Main thread, immediately before `configure_wgpu_surface` runs on the +/// reveal path. Returns `0` when disabled. +pub(crate) fn configure_start() -> u64 { + if !enabled() { 0 } else { now_ns() } +} + +/// Main thread, immediately after `configure_wgpu_surface` returns. +pub(crate) fn on_surface_configured(start: u64) { + if start == 0 || !enabled() || REVEAL_AT.load(Ordering::Relaxed) == 0 { + return; + } + CONFIGURE_NS.store(now_ns().saturating_sub(start), Ordering::Relaxed); +} + +/// Main thread, at the top of `redraw()`'s `rebuild_panes` call. Returns `0` +/// when disabled. +pub(crate) fn rebuild_start() -> u64 { + if !enabled() { 0 } else { now_ns() } +} + +/// Main thread, immediately after `rebuild_panes` returns, with the row +/// count it reports via `Renderer::rows_rebuilt_last_frame()`. +pub(crate) fn on_pane_rebuild(start: u64, rows_rebuilt: u64) { + if start == 0 || !enabled() || REVEAL_AT.load(Ordering::Relaxed) == 0 { + return; + } + REBUILD_NS.store(now_ns().saturating_sub(start), Ordering::Relaxed); + ROWS_REBUILT.store(rows_rebuilt, Ordering::Relaxed); +} + +/// Main thread, when `redraw()` takes the reveal fast path (presents the +/// renderer's already-cached instances instead of rebuilding this frame). +pub(crate) fn on_fast_path_reveal() { + if !enabled() || REVEAL_AT.load(Ordering::Relaxed) == 0 { + return; + } + FAST_PATH.store(true, Ordering::Relaxed); +} + +/// Main thread, at the top of a background pane-cache refresh for an occluded +/// window (see `App::background_refresh_pane_cache`). Returns `0` when +/// disabled. +pub(crate) fn bg_refresh_start() -> u64 { + if !enabled() { 0 } else { now_ns() } +} + +/// Main thread, immediately after a background refresh's `rebuild_panes` +/// returns. Logs its own line — independent of the reveal-sample state above, +/// since a background refresh has no `present()` to pair with. +pub(crate) fn on_bg_refresh(start: u64, rows_rebuilt: u64) { + if start == 0 || !enabled() { + return; + } + let us = now_ns().saturating_sub(start) / 1_000; + eprintln!("[tab-switch-trace] bg-refresh {us}us rows={rows_rebuilt}"); +} + +/// Main thread, immediately after `SurfaceTexture::present()`. Closes the +/// pending reveal sample (if any) and logs the breakdown. +pub(crate) fn on_present() { + if !enabled() { + return; + } + let reveal_at = REVEAL_AT.swap(0, Ordering::Relaxed); + if reveal_at == 0 { + return; + } + let total_ns = now_ns().saturating_sub(reveal_at); + let configure_ns = CONFIGURE_NS.load(Ordering::Relaxed); + let rebuild_ns = REBUILD_NS.load(Ordering::Relaxed); + let rows = ROWS_REBUILT.load(Ordering::Relaxed); + let fast_path = FAST_PATH.swap(false, Ordering::Relaxed); + eprintln!( + "[tab-switch-trace] reveal→present {}us (surface-configure {}us, pane-rebuild {}us, rows_rebuilt={}){}", + total_ns / 1_000, + configure_ns / 1_000, + rebuild_ns / 1_000, + rows, + if fast_path { + " (fast-path, rows_rebuilt=0)" + } else { + "" + }, + ); +} + +#[cfg(test)] +mod tests { + use super::*; + + // The hooks must be inert when the env var is unset (the default in the + // test runner): no state is written, no output produced. This pins the + // zero-cost-when-disabled contract at its observable surface. Running + // `cargo test` with `NOA_TAB_SWITCH_TRACE=1` set is legitimate (e.g. to + // eyeball the trace output alongside the test run), so this test's + // contract simply does not apply then — skip rather than assert-fail. + #[test] + fn disabled_hooks_leave_state_untouched() { + if std::env::var("NOA_TAB_SWITCH_TRACE") + .is_ok_and(|value| !value.is_empty() && value != "0") + { + eprintln!( + "NOA_TAB_SWITCH_TRACE is set in the environment — skipping the \ + disabled-hooks contract test" + ); + return; + } + assert!(!enabled(), "test runner must not set NOA_TAB_SWITCH_TRACE"); + on_reveal_start(); + assert_eq!(configure_start(), 0); + on_surface_configured(0); + assert_eq!(rebuild_start(), 0); + on_pane_rebuild(0, 42); + on_fast_path_reveal(); + assert_eq!(bg_refresh_start(), 0); + on_bg_refresh(0, 7); + on_present(); + assert_eq!(REVEAL_AT.load(Ordering::Relaxed), 0); + assert!(!FAST_PATH.load(Ordering::Relaxed)); + assert_eq!(CONFIGURE_NS.load(Ordering::Relaxed), 0); + assert_eq!(REBUILD_NS.load(Ordering::Relaxed), 0); + assert_eq!(ROWS_REBUILT.load(Ordering::Relaxed), 0); + } +}