diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 83df038fe..bacb553b0 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -1988,8 +1988,12 @@ fn destroyPageExt( } /// Fast-path function to erase exactly 1 row. Erasing means that the row -/// is completely removed, not just cleared. All rows below the removed row -/// will be moved up by 1 to account for this. +/// is completely REMOVED, not just cleared. All rows following the removed +/// row will be shifted up by 1 to fill the empty space. +/// +/// Unlike eraseRows, eraseRow does not change the size of any pages. The +/// caller is responsible for adjusting the row count of the final page if +/// that behavior is required. pub fn eraseRow( self: *PageList, pt: point.Point, @@ -2029,7 +2033,7 @@ pub fn eraseRow( // 1 | 2 2 2 // 2 | 3 3 3 // 3 <' 0 <. 4 4 - // --- --- | --- --- + // --- --- | --- --- <- page boundary // 4 4 -' 4 -. 5 // 5 5 5 | 6 // 6 6 6 | 7 @@ -2057,35 +2061,34 @@ pub fn eraseRow( } } - // The final row needs to be cleared in case we re-use it. + // Clear the final row which was rotated from the top of the page. page.data.clearCells(&rows[page.data.size.rows - 1], 0, page.data.size.cols); - - // We don't trim off the final row if we erased active, since one of - // our invariants is that we always have full active space. - if (pt != .active) { - page.data.size.rows -= 1; - } } -/// A special-case of eraseRow that shifts only a bounded number of following +/// A variant of eraseRow that shifts only a bounded number of following /// rows up, filling the space they leave behind with blank rows. /// /// `limit` is exclusive of the erased row. A limit of 1 will erase the target /// row and shift the row below in to its position, leaving a blank row below. -/// -/// This function has a lot of repeated code in it because it is a hot path. pub fn eraseRowBounded( self: *PageList, pt: point.Point, limit: usize, ) !void { + // This function has a lot of repeated code in it because it is a hot path. + // + // To get a better idea of what's happening, read eraseRow first for more + // in-depth explanatory comments. To avoid repetition, the only comments for + // this function are for where it differs from eraseRow. + const pn = self.pin(pt).?; var page = pn.page; var rows = page.data.rows.ptr(page.data.memory.ptr); - // Special case where we'll reach the limit in the same page as the erased - // row, so we don't have to handle cloning rows between pages. + // If the row limit is less than the remaining rows before the end of the + // page, then we clear the row, rotate it to the end of the boundary limit + // and update our pins. if (page.data.size.rows - pn.y > limit) { page.data.clearCells(&rows[pn.y], 0, page.data.size.cols); fastmem.rotateOnce(Row, rows[pn.y..][0..limit + 1]); @@ -2100,14 +2103,14 @@ pub fn eraseRowBounded( return; } - var shifted: usize = 0; - fastmem.rotateOnce(Row, rows[pn.y..page.data.size.rows]); - shifted += page.data.size.rows - pn.y; + // We need to keep track of how many rows we've shifted so that we can + // determine at what point we need to do a partial shift on subsequent + // pages. + var shifted: usize = page.data.size.rows - pn.y; - // We adjust the tracked pins in this page, moving up any that were below - // the removed row. + // Update tracked pins. { var pin_it = self.tracked_pins.keyIterator(); while (pin_it.next()) |p_ptr| { @@ -2124,6 +2127,11 @@ pub fn eraseRowBounded( page = next; rows = next_rows; + // We check to see if this page contains enough rows to satisfy the + // specified limit, accounting for rows we've already shifted in prior + // pages. + // + // The logic here is very similar to the one before the loop. const shifted_limit = limit - shifted; if (page.data.size.rows > shifted_limit) { page.data.clearCells(&rows[0], 0, page.data.size.cols); @@ -2147,11 +2155,10 @@ pub fn eraseRowBounded( fastmem.rotateOnce(Row, rows[0..page.data.size.rows]); + // Account for the rows shifted in this page. shifted += page.data.size.rows; - // 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. + // Update tracked pins. var pin_it = self.tracked_pins.keyIterator(); while (pin_it.next()) |p_ptr| { const p = p_ptr.*; diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index 43299df45..52267fa13 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -1103,19 +1103,10 @@ pub fn index(self: *Terminal) !void { self.screen.cursorAbsolute(cursor_x, cursor_y); } - if (self.scrolling_region.bottom < self.rows) { - try self.screen.pages.eraseRowBounded( - .{ .active = .{ .y = self.scrolling_region.top } }, - self.scrolling_region.bottom - self.scrolling_region.top - ); - } else { - // If we have no bottom margin we don't need to worry about - // potentially damaging rows below the scrolling region, - // and eraseRow is cheaper than eraseRowBounded. - try self.screen.pages.eraseRow( - .{ .active = .{ .y = self.scrolling_region.top } }, - ); - } + try self.screen.pages.eraseRowBounded( + .{ .active = .{ .y = self.scrolling_region.top } }, + self.scrolling_region.bottom - self.scrolling_region.top + ); // The operations above can prune our cursor style so we need to // update. This should never fail because the above can only FREE