diff --git a/src/renderer/Metal.zig b/src/renderer/Metal.zig index 4b5863e3d..5395554df 100644 --- a/src/renderer/Metal.zig +++ b/src/renderer/Metal.zig @@ -98,10 +98,11 @@ current_background_color: terminal.color.RGB, /// cells goes into a separate shader. cells: mtl_cell.Contents, -/// If this is true, it forces a full cell contents rebuild on the next frame. -/// This can be used to force a rebuild whenever internal state changes for -/// our cells structures (i.e. a resize). -cells_rebuild: bool = true, +/// The last viewport that we based our rebuild off of. If this changes, +/// then we do a full rebuild of the cells. The pointer values in this pin +/// are NOT SAFE to read because they may be modified, freed, etc from the +/// termio thread. We treat the pointers as integers for comparison only. +cells_viewport: ?terminal.Pin = null, /// Set to true after rebuildCells is called. This can be used /// to determine if any possible changes have been made to the @@ -844,6 +845,7 @@ pub fn updateFrame( preedit: ?renderer.State.Preedit, cursor_style: ?renderer.CursorStyle, color_palette: terminal.color.Palette, + viewport_pin: terminal.Pin, /// If true, rebuild the full screen. full_rebuild: bool, @@ -883,6 +885,9 @@ pub fn updateFrame( return; } + // Get the viewport pin so that we can compare it to the current. + const viewport_pin = state.terminal.screen.pages.pin(.{ .viewport = .{} }).?; + // We used to share terminal state, but we've since learned through // analysis that it is faster to copy the terminal state than to // hold the lock while rebuilding GPU cells. @@ -929,8 +934,11 @@ pub fn updateFrame( if (v > 0) break :rebuild true; } - // Temporary while: https://github.com/mitchellh/ghostty/issues/1731 - if (true) break :rebuild true; + // If our viewport changed then we need to rebuild the entire + // screen because it means we scrolled. If we have no previous + // viewport then we must rebuild. + const prev_viewport = self.cells_viewport orelse break :rebuild true; + if (!prev_viewport.eql(viewport_pin)) break :rebuild true; break :rebuild false; }; @@ -960,6 +968,7 @@ pub fn updateFrame( .preedit = preedit, .cursor_style = cursor_style, .color_palette = state.terminal.color_palette.colors, + .viewport_pin = viewport_pin, .full_rebuild = full_rebuild, }; }; @@ -970,7 +979,7 @@ pub fn updateFrame( // Build our GPU cells try self.rebuildCells( - critical.full_rebuild or self.cells_rebuild, + critical.full_rebuild, &critical.screen, critical.mouse, critical.preedit, @@ -978,6 +987,9 @@ pub fn updateFrame( &critical.color_palette, ); + // Update our viewport pin + self.cells_viewport = critical.viewport_pin; + // Update our background color self.current_background_color = critical.bg; @@ -1784,7 +1796,9 @@ pub fn setScreenSize( // Reset our cell contents. try self.cells.resize(self.alloc, grid_size); - self.cells_rebuild = true; + + // Reset our viewport to force a rebuild + self.cells_viewport = null; // If we have custom shaders then we update the state if (self.custom_shader_state) |*state| { @@ -2043,7 +2057,6 @@ fn rebuildCells( // Update that our cells rebuilt self.cells_rebuilt = true; - self.cells_rebuild = false; // Log some things // log.debug("rebuildCells complete cached_runs={}", .{ diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index e1da55ad4..6681b733e 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -473,6 +473,11 @@ pub fn clone( continue; } + // We want to maintain the dirty bits from the original page so + // instead of setting a range we grab the dirty bit and then + // set it on the new page in the new location. + var dirty = page.data.dirtyBitSet(); + // Kind of slow, we want to shift the rows up in the page up to // end and then resize down. const rows = page.data.rows.ptr(page.data.memory); @@ -483,6 +488,7 @@ pub fn clone( const old_dst = dst.*; dst.* = src.*; src.* = old_dst; + dirty.setValue(i, dirty.isSet(i + chunk.start)); } page.data.size.rows = @intCast(len); total_rows += len; @@ -2023,6 +2029,12 @@ pub fn eraseRow( } } + { + // Set all the rows as dirty in this page + var dirty = page.data.dirtyBitSet(); + dirty.setRangeValue(.{ .start = pn.y, .end = page.data.size.rows }, true); + } + // We iterate through all of the following pages in order to move their // rows up by 1 as well. while (page.next) |next| { @@ -2054,6 +2066,10 @@ pub fn eraseRow( fastmem.rotateOnce(Row, rows[0..page.data.size.rows]); + // Set all the rows as dirty + var dirty = page.data.dirtyBitSet(); + dirty.setRangeValue(.{ .start = 0, .end = page.data.size.rows }, true); + // Our tracked pins for this page need to be updated. // If the pin is in row 0 that means the corresponding row has // been moved to the previous page. Otherwise, move it up by 1. @@ -2306,6 +2322,10 @@ pub fn eraseRows( // Our new size is the amount we scrolled chunk.page.data.size.rows = @intCast(scroll_amount); erased += chunk.end; + + // Set all the rows as dirty + var dirty = chunk.page.data.dirtyBitSet(); + dirty.setRangeValue(.{ .start = 0, .end = chunk.page.data.size.rows }, true); } // If we deleted active, we need to regrow because one of our invariants diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 3ab434048..6490376b4 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -565,10 +565,17 @@ pub fn cursorDownScroll(self: *Screen) !void { self.cursor.page_row, self.cursor.page_pin.page.data.getCells(self.cursor.page_row), ); + + var dirty = self.cursor.page_pin.page.data.dirtyBitSet(); + dirty.set(0); } else { // eraseRow will shift everything below it up. try self.pages.eraseRow(.{ .active = .{} }); + // Note we don't need to mark anything dirty in this branch + // because eraseRow will mark all the rotated rows as dirty + // in the entire page. + // We need to move our cursor down one because eraseRows will // preserve our pin directly and we're erasing one row. const page_pin = self.cursor.page_pin.down(1).?; @@ -626,6 +633,9 @@ pub fn cursorDownScroll(self: *Screen) !void { self.cursor.page_row = page_rac.row; self.cursor.page_cell = page_rac.cell; + // Our new row is always dirty + self.cursorMarkDirty(); + // Clear the new row so it gets our bg color. We only do this // if we have a bg color at all. if (self.cursor.style.bg_color != .none) { @@ -2882,6 +2892,7 @@ test "Screen: cursorAbsolute across pages preserves style" { try testing.expect(styleval.flags.bold); } } + test "Screen: scrolling" { const testing = std.testing; const alloc = testing.allocator; @@ -2909,6 +2920,11 @@ test "Screen: scrolling" { }, cell.content.color_rgb); } + // Everything is dirty because we have no scrollback + try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 0 } })); + try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 1 } })); + try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 2 } })); + // Scrolling to the bottom does nothing s.scroll(.{ .active = {} }); @@ -2934,6 +2950,9 @@ test "Screen: scrolling with a single-row screen no scrollback" { defer alloc.free(contents); try testing.expectEqualStrings("", contents); } + + // Screen should be dirty + try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 0 } })); } test "Screen: scrolling with a single-row screen with scrollback" { @@ -2952,6 +2971,12 @@ test "Screen: scrolling with a single-row screen with scrollback" { try testing.expectEqualStrings("", contents); } + // Active should be dirty + try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 0 } })); + + // Our scrollback should not be dirty + try testing.expect(!s.pages.isDirty(.{ .screen = .{ .x = 0, .y = 0 } })); + s.scroll(.{ .delta_row = -1 }); { const contents = try s.dumpStringAlloc(alloc, .{ .viewport = .{} });