perf: separate clearing graphemes/hyperlinks from updating row flag

This improves the `clearCells` function since it only has to update once
after clearing all of the individual cells, or not at all if the whole
row was cleared since then it knows for sure that it cleared them all.

This also makes it so that the row style flag is properly tracked when
cells are cleared but not the whole row.
pull/9645/head
Qwerasd 2025-11-18 17:43:04 -07:00
parent 5ffa7f8f45
commit f9e245ab7f
3 changed files with 121 additions and 35 deletions

View File

@ -1322,8 +1322,9 @@ pub fn clearRows(
} }
} }
/// Clear the cells with the blank cell. This takes care to handle /// Clear the cells with the blank cell.
/// cleaning up graphemes and styles. ///
/// This takes care to handle cleaning up graphemes and styles.
pub fn clearCells( pub fn clearCells(
self: *Screen, self: *Screen,
page: *Page, page: *Page,
@ -1350,30 +1351,54 @@ pub fn clearCells(
assert(@intFromPtr(&cells[cells.len - 1]) <= @intFromPtr(&row_cells[row_cells.len - 1])); assert(@intFromPtr(&cells[cells.len - 1]) <= @intFromPtr(&row_cells[row_cells.len - 1]));
} }
// If this row has graphemes, then we need go through a slow path // If we have managed memory (styles, graphemes, or hyperlinks)
// and delete the cell graphemes. // in this row then we go cell by cell and clear them if present.
if (row.grapheme) { if (row.grapheme) {
for (cells) |*cell| { for (cells) |*cell| {
if (cell.hasGrapheme()) page.clearGrapheme(row, cell); if (cell.hasGrapheme())
page.clearGrapheme(cell);
}
// If we have no left/right scroll region we can be sure
// that we've cleared all the graphemes, so we clear the
// flag, otherwise we ask the page to update the flag.
if (cells.len == self.pages.cols) {
row.grapheme = false;
} else {
page.updateRowGraphemeFlag(row);
} }
} }
// If we have hyperlinks, we need to clear those.
if (row.hyperlink) { if (row.hyperlink) {
for (cells) |*cell| { for (cells) |*cell| {
if (cell.hyperlink) page.clearHyperlink(row, cell); if (cell.hyperlink)
page.clearHyperlink(cell);
}
// If we have no left/right scroll region we can be sure
// that we've cleared all the hyperlinks, so we clear the
// flag, otherwise we ask the page to update the flag.
if (cells.len == self.pages.cols) {
row.hyperlink = false;
} else {
page.updateRowHyperlinkFlag(row);
} }
} }
if (row.styled) { if (row.styled) {
for (cells) |*cell| { for (cells) |*cell| {
if (cell.style_id == style.default_id) continue; if (cell.hasStyling())
page.styles.release(page.memory, cell.style_id); page.styles.release(page.memory, cell.style_id);
} }
// If we have no left/right scroll region we can be sure that // If we have no left/right scroll region we can be sure
// the row is no longer styled. // that we've cleared all the styles, so we clear the
if (cells.len == self.pages.cols) row.styled = false; // flag, otherwise we ask the page to update the flag.
if (cells.len == self.pages.cols) {
row.styled = false;
} else {
page.updateRowStyledFlag(row);
}
} }
if (comptime build_options.kitty_graphics) { if (comptime build_options.kitty_graphics) {

View File

@ -683,10 +683,9 @@ fn printCell(
// If the prior value had graphemes, clear those // If the prior value had graphemes, clear those
if (cell.hasGrapheme()) { if (cell.hasGrapheme()) {
self.screens.active.cursor.page_pin.node.data.clearGrapheme( const page = &self.screens.active.cursor.page_pin.node.data;
self.screens.active.cursor.page_row, page.clearGrapheme(cell);
cell, page.updateRowGraphemeFlag(self.screens.active.cursor.page_row);
);
} }
// We don't need to update the style refs unless the // We don't need to update the style refs unless the
@ -745,7 +744,8 @@ fn printCell(
} else if (had_hyperlink) { } else if (had_hyperlink) {
// If the previous cell had a hyperlink then we need to clear it. // If the previous cell had a hyperlink then we need to clear it.
var page = &self.screens.active.cursor.page_pin.node.data; var page = &self.screens.active.cursor.page_pin.node.data;
page.clearHyperlink(self.screens.active.cursor.page_row, cell); page.clearHyperlink(cell);
page.updateRowHyperlinkFlag(self.screens.active.cursor.page_row);
} }
} }
@ -1474,7 +1474,8 @@ fn rowWillBeShifted(
if (left_cell.wide == .spacer_tail) { if (left_cell.wide == .spacer_tail) {
const wide_cell: *Cell = &cells[self.scrolling_region.left - 1]; const wide_cell: *Cell = &cells[self.scrolling_region.left - 1];
if (wide_cell.hasGrapheme()) { if (wide_cell.hasGrapheme()) {
page.clearGrapheme(row, wide_cell); page.clearGrapheme(wide_cell);
page.updateRowGraphemeFlag(row);
} }
wide_cell.content.codepoint = 0; wide_cell.content.codepoint = 0;
wide_cell.wide = .narrow; wide_cell.wide = .narrow;
@ -1484,7 +1485,8 @@ fn rowWillBeShifted(
if (right_cell.wide == .wide) { if (right_cell.wide == .wide) {
const tail_cell: *Cell = &cells[self.scrolling_region.right + 1]; const tail_cell: *Cell = &cells[self.scrolling_region.right + 1];
if (right_cell.hasGrapheme()) { if (right_cell.hasGrapheme()) {
page.clearGrapheme(row, right_cell); page.clearGrapheme(right_cell);
page.updateRowGraphemeFlag(row);
} }
right_cell.content.codepoint = 0; right_cell.content.codepoint = 0;
right_cell.wide = .narrow; right_cell.wide = .narrow;

View File

@ -1059,26 +1059,54 @@ pub const Page = struct {
const cells = row.cells.ptr(self.memory)[left..end]; const cells = row.cells.ptr(self.memory)[left..end];
// If we have managed memory (styles, graphemes, or hyperlinks)
// in this row then we go cell by cell and clear them if present.
if (row.grapheme) { if (row.grapheme) {
for (cells) |*cell| { for (cells) |*cell| {
if (cell.hasGrapheme()) self.clearGrapheme(row, cell); if (cell.hasGrapheme())
self.clearGrapheme(cell);
}
// If we have no left/right scroll region we can be sure
// that we've cleared all the graphemes, so we clear the
// flag, otherwise we use the update function to update.
if (cells.len == self.size.cols) {
row.grapheme = false;
} else {
self.updateRowGraphemeFlag(row);
} }
} }
if (row.hyperlink) { if (row.hyperlink) {
for (cells) |*cell| { for (cells) |*cell| {
if (cell.hyperlink) self.clearHyperlink(row, cell); if (cell.hyperlink)
self.clearHyperlink(cell);
}
// If we have no left/right scroll region we can be sure
// that we've cleared all the hyperlinks, so we clear the
// flag, otherwise we use the update function to update.
if (cells.len == self.size.cols) {
row.hyperlink = false;
} else {
self.updateRowHyperlinkFlag(row);
} }
} }
if (row.styled) { if (row.styled) {
for (cells) |*cell| { for (cells) |*cell| {
if (cell.style_id == stylepkg.default_id) continue; if (cell.hasStyling())
self.styles.release(self.memory, cell.style_id);
self.styles.release(self.memory, cell.style_id);
} }
if (cells.len == self.size.cols) row.styled = false; // If we have no left/right scroll region we can be sure
// that we've cleared all the styles, so we clear the
// flag, otherwise we use the update function to update.
if (cells.len == self.size.cols) {
row.styled = false;
} else {
self.updateRowStyledFlag(row);
}
} }
if (comptime build_options.kitty_graphics) { if (comptime build_options.kitty_graphics) {
@ -1106,7 +1134,11 @@ pub const Page = struct {
} }
/// Clear the hyperlink from the given cell. /// Clear the hyperlink from the given cell.
pub inline fn clearHyperlink(self: *Page, row: *Row, cell: *Cell) void { ///
/// In order to update the hyperlink flag on the row, call
/// `updateRowHyperlinkFlag` after you finish clearing any
/// hyperlinks in the row.
pub inline fn clearHyperlink(self: *Page, cell: *Cell) void {
defer self.assertIntegrity(); defer self.assertIntegrity();
// Get our ID // Get our ID
@ -1118,9 +1150,13 @@ pub const Page = struct {
self.hyperlink_set.release(self.memory, entry.value_ptr.*); self.hyperlink_set.release(self.memory, entry.value_ptr.*);
map.removeByPtr(entry.key_ptr); map.removeByPtr(entry.key_ptr);
cell.hyperlink = false; cell.hyperlink = false;
}
// Mark that we no longer have hyperlinks, also search the row /// Checks if the row contains any hyperlinks and sets
// to make sure its state is correct. /// the hyperlink flag to false if none are found.
///
/// Call after removing hyperlinks in a row.
pub inline fn updateRowHyperlinkFlag(self: *Page, row: *Row) void {
const cells = row.cells.ptr(self.memory)[0..self.size.cols]; const cells = row.cells.ptr(self.memory)[0..self.size.cols];
for (cells) |c| if (c.hyperlink) return; for (cells) |c| if (c.hyperlink) return;
row.hyperlink = false; row.hyperlink = false;
@ -1434,7 +1470,11 @@ pub const Page = struct {
} }
/// Clear the graphemes for a given cell. /// Clear the graphemes for a given cell.
pub inline fn clearGrapheme(self: *Page, row: *Row, cell: *Cell) void { ///
/// In order to update the grapheme flag on the row, call
/// `updateRowGraphemeFlag` after you finish clearing any
/// graphemes in the row.
pub inline fn clearGrapheme(self: *Page, cell: *Cell) void {
defer self.assertIntegrity(); defer self.assertIntegrity();
if (build_options.slow_runtime_safety) assert(cell.hasGrapheme()); if (build_options.slow_runtime_safety) assert(cell.hasGrapheme());
@ -1450,9 +1490,15 @@ pub const Page = struct {
// Remove the entry // Remove the entry
map.removeByPtr(entry.key_ptr); map.removeByPtr(entry.key_ptr);
// Mark that we no longer have graphemes, also search the row // Mark that we no longer have graphemes by changing the content tag.
// to make sure its state is correct.
cell.content_tag = .codepoint; cell.content_tag = .codepoint;
}
/// Checks if the row contains any graphemes and sets
/// the grapheme flag to false if none are found.
///
/// Call after removing graphemes in a row.
pub inline fn updateRowGraphemeFlag(self: *Page, row: *Row) void {
const cells = row.cells.ptr(self.memory)[0..self.size.cols]; const cells = row.cells.ptr(self.memory)[0..self.size.cols];
for (cells) |c| if (c.hasGrapheme()) return; for (cells) |c| if (c.hasGrapheme()) return;
row.grapheme = false; row.grapheme = false;
@ -1470,6 +1516,16 @@ pub const Page = struct {
return self.grapheme_map.map(self.memory).capacity(); return self.grapheme_map.map(self.memory).capacity();
} }
/// Checks if the row contains any styles and sets
/// the styled flag to false if none are found.
///
/// Call after removing styles in a row.
pub inline fn updateRowStyledFlag(self: *Page, row: *Row) void {
const cells = row.cells.ptr(self.memory)[0..self.size.cols];
for (cells) |c| if (c.hasStyling()) return;
row.styled = false;
}
/// Returns true if this page is dirty at all. /// Returns true if this page is dirty at all.
pub inline fn isDirty(self: *const Page) bool { pub inline fn isDirty(self: *const Page) bool {
if (self.dirty) return true; if (self.dirty) return true;
@ -1750,7 +1806,7 @@ pub const Row = packed struct(u64) {
/// Returns true if this row has any managed memory outside of the /// Returns true if this row has any managed memory outside of the
/// row structure (graphemes, styles, etc.) /// row structure (graphemes, styles, etc.)
fn managedMemory(self: Row) bool { inline fn managedMemory(self: Row) bool {
return self.grapheme or self.styled or self.hyperlink; return self.grapheme or self.styled or self.hyperlink;
} }
}; };
@ -2076,7 +2132,8 @@ test "Page appendGrapheme small" {
try testing.expectEqualSlices(u21, &.{ 0x0A, 0x0B }, page.lookupGrapheme(rac.cell).?); try testing.expectEqualSlices(u21, &.{ 0x0A, 0x0B }, page.lookupGrapheme(rac.cell).?);
// Clear it // Clear it
page.clearGrapheme(rac.row, rac.cell); page.clearGrapheme(rac.cell);
page.updateRowGraphemeFlag(rac.row);
try testing.expect(!rac.row.grapheme); try testing.expect(!rac.row.grapheme);
try testing.expect(!rac.cell.hasGrapheme()); try testing.expect(!rac.cell.hasGrapheme());
} }
@ -2121,7 +2178,8 @@ test "Page clearGrapheme not all cells" {
try page.appendGrapheme(rac2.row, rac2.cell, 0x0A); try page.appendGrapheme(rac2.row, rac2.cell, 0x0A);
// Clear it // Clear it
page.clearGrapheme(rac.row, rac.cell); page.clearGrapheme(rac.cell);
page.updateRowGraphemeFlag(rac.row);
try testing.expect(rac.row.grapheme); try testing.expect(rac.row.grapheme);
try testing.expect(!rac.cell.hasGrapheme()); try testing.expect(!rac.cell.hasGrapheme());
try testing.expect(rac2.cell.hasGrapheme()); try testing.expect(rac2.cell.hasGrapheme());
@ -2385,7 +2443,8 @@ test "Page cloneFrom graphemes" {
// Write again // Write again
for (0..page.capacity.rows) |y| { for (0..page.capacity.rows) |y| {
const rac = page.getRowAndCell(1, y); const rac = page.getRowAndCell(1, y);
page.clearGrapheme(rac.row, rac.cell); page.clearGrapheme(rac.cell);
page.updateRowGraphemeFlag(rac.row);
rac.cell.* = .{ rac.cell.* = .{
.content_tag = .codepoint, .content_tag = .codepoint,
.content = .{ .codepoint = 0 }, .content = .{ .codepoint = 0 },