From 815ccb060b1f983272dab86af6bacb156dfcbfd9 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 15 Apr 2026 13:42:38 -0700 Subject: [PATCH] terminal: fix viewport pin during resize reflow Maybe related to #12298? When Screen resize forwards the active cursor into PageList reflow, a history-pinned viewport can be remapped into the active area before the preserved-cursor grow step finishes. The old code kept treating that viewport as a history pin during the intermediate grow calls, which left too few rows beneath the pin and tripped the viewport integrity checks. Fix this by normalizing the viewport back to active as soon as reflow moves the pinned row into the active area. Add a Screen-level regression test that exercises the full resize path with bounded scrollback and wrapped rows, and document the setup so the unwrap and viewport transition are clear. --- src/terminal/PageList.zig | 11 ++++++ src/terminal/Screen.zig | 80 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 398733745..17eea73d0 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -1137,6 +1137,17 @@ fn resizeCols( for (total..self.rows) |_| _ = try self.grow(); } + // Reflow can unwrap enough rows that a history viewport pin lands in the + // active area before we do any preserved-cursor growth below. Switch back + // to the active viewport now so intermediate grow() integrity checks stay + // valid. + switch (self.viewport) { + .active, .top => {}, + .pin => if (self.pinIsActive(self.viewport_pin.*)) { + self.viewport = .active; + }, + } + // See preserved_cursor setup for why. if (preserved_cursor) |c| cursor: { const active_pt = self.pointFromPin( diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index b56701838..39fdd6109 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -6497,6 +6497,86 @@ test "Screen: resize more cols with populated scrollback" { } } +test "Screen: resize more cols bounded scrollback keeps viewport valid" { + // Regression test for issue #12298. + // + // This needs to live at the Screen layer rather than PageList because the + // bad state only appears once Screen forwards the active cursor into the + // resize path. A direct PageList resize repro does not hit the same bug. + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, .{ + .cols = 2, + .rows = 10, + .max_scrollback = 10_000, + }); + defer s.deinit(); + + // Build 30 rows of scrollback on top of our 10-row viewport so we have a + // 40-row screen with history above the active area. + for (0..30) |_| _ = try s.pages.grow(); + s.cursorReload(); + try testing.expectEqual(@as(usize, 40), s.pages.scrollbar().total); + + // Fill the entire screen with two-row wrapped runs: + // - even rows mark the end of a wrapped line + // - odd rows mark the continuation + // + // With 2 columns, each logical line occupies two rows. When we grow to 4 + // columns with reflow enabled, those pairs unwrap back into single rows. + // That cuts the total row count down and is what stresses the viewport pin. + var it = s.pages.pageIterator(.right_down, .{ .screen = .{} }, null); + while (it.next()) |chunk| { + const page = &chunk.node.data; + for (chunk.start..chunk.end) |y| { + const rac = page.getRowAndCell(0, y); + if (y % 2 == 0) { + rac.row.wrap = true; + } else { + rac.row.wrap_continuation = true; + } + + for (0..s.pages.cols) |x| { + page.getRowAndCell(x, y).cell.* = .{ + .content_tag = .codepoint, + .content = .{ .codepoint = 'A' }, + }; + } + } + } + + // Pin the viewport to a history row just above the active area. + // + // Before resize: + // - total rows = 40 + // - active area starts at row 30 + // - viewport is pinned at row 28 + // + // After unwrap during resize: + // - total rows shrinks to 20 + // - the old row 28 remaps into what is now the active area + // + // The bug was that resize/grow would temporarily keep the viewport as a + // history pin even after reflow had moved it into the active area, leaving + // fewer than `rows` visible rows beneath the pin and tripping integrity + // checks. + s.pages.scroll(.{ .pin = s.pages.pin(.{ .screen = .{ .y = 28 } }).? }); + try testing.expect(s.pages.viewport == .pin); + try testing.expect(s.pages.getBottomRight(.viewport) != null); + + // Growing columns triggers reflow, which unwraps the synthetic wrapped + // rows above. This used to panic during the resize path. + try s.resize(.{ .cols = 4, .rows = s.pages.rows, .reflow = true }); + + // After the fix, the viewport is normalized back to the active area as + // soon as the pinned row lands there, so viewport queries remain valid. + try testing.expectEqual(@as(usize, 4), s.pages.cols); + try testing.expect(s.pages.scrollbar().total < 40); + try testing.expect(s.pages.viewport == .active); + try testing.expect(s.pages.getBottomRight(.viewport) != null); +} + test "Screen: resize more cols with reflow" { const testing = std.testing; const alloc = testing.allocator;