From 66875206bb3315c6d9315f5dff5df2817b4e81e5 Mon Sep 17 00:00:00 2001 From: Tim Culverhouse Date: Mon, 25 Sep 2023 10:43:39 -0500 Subject: [PATCH 1/3] terminal: move charset to screen Each screen (primary and alternate) retains the state of the current charset when DECSC (save cursor) is called. Move the CharsetState into the screen to enable saving the state with each screen. Add a test for charset state on screen change --- src/terminal/Screen.zig | 30 +++++++++++++++ src/terminal/Terminal.zig | 80 +++++++++++++++++++-------------------- 2 files changed, 70 insertions(+), 40 deletions(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index f289e096d..a949d858d 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -63,9 +63,27 @@ const point = @import("point.zig"); const CircBuf = @import("circ_buf.zig").CircBuf; const Selection = @import("Selection.zig"); const fastmem = @import("../fastmem.zig"); +const charsets = @import("charsets.zig"); const log = std.log.scoped(.screen); +/// State required for all charset operations. +const CharsetState = struct { + /// The list of graphical charsets by slot + charsets: CharsetArray = CharsetArray.initFill(charsets.Charset.utf8), + + /// GL is the slot to use when using a 7-bit printable char (up to 127) + /// GR used for 8-bit printable chars. + gl: charsets.Slots = .G0, + gr: charsets.Slots = .G2, + + /// Single shift where a slot is used for exactly one char. + single_shift: ?charsets.Slots = null, + + /// An array to map a charset slot to a lookup table. + const CharsetArray = std.EnumArray(charsets.Slots, charsets.Charset); +}; + /// Cursor represents the cursor state. pub const Cursor = struct { /// x, y where the cursor currently exists (0-indexed). This x/y is @@ -897,6 +915,18 @@ kitty_keyboard: kitty.KeyFlagStack = .{}, /// Kitty graphics protocol state. kitty_images: kitty.graphics.ImageStorage = .{}, +/// The charset state +charset: CharsetState = .{}, + +/// The saved charset state. This state is saved / restored along with the +/// cursor state +saved_charset: CharsetState = .{}, + +/// The saved state of origin mode. This mode gets special handling in Screen +/// because it's state is saved/restored with the cursor and must have a state +/// independent to each screen (primary and alternate) +saved_origin_mode: bool = false, + /// Initialize a new screen. pub fn init( alloc: Allocator, diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index 7b3a12aca..5187dc56a 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -74,13 +74,6 @@ scrolling_region: ScrollingRegion, /// The last reported pwd, if any. pwd: std.ArrayList(u8), -/// The charset state -charset: CharsetState = .{}, - -/// The saved charset state. This state is saved / restored along with the -/// cursor state -saved_charset: CharsetState = .{}, - /// The color palette to use color_palette: color.Palette = color.default, @@ -111,23 +104,6 @@ flags: packed struct { mouse_format: MouseFormat = .x10, } = .{}, -/// State required for all charset operations. -const CharsetState = struct { - /// The list of graphical charsets by slot - charsets: CharsetArray = CharsetArray.initFill(charsets.Charset.utf8), - - /// GL is the slot to use when using a 7-bit printable char (up to 127) - /// GR used for 8-bit printable chars. - gl: charsets.Slots = .G0, - gr: charsets.Slots = .G2, - - /// Single shift where a slot is used for exactly one char. - single_shift: ?charsets.Slots = null, - - /// An array to map a charset slot to a lookup table. - const CharsetArray = std.EnumArray(charsets.Slots, charsets.Charset); -}; - /// The event types that can be reported for mouse-related activities. /// These are all mutually exclusive (hence in a single enum). pub const MouseEvents = enum(u3) { @@ -423,8 +399,8 @@ fn plainString(self: *Terminal, alloc: Allocator) ![]const u8 { /// was already saved it is overwritten. pub fn saveCursor(self: *Terminal) void { self.screen.saved_cursor = self.screen.cursor; - self.saved_charset = self.charset; - self.modes.save(.origin); + self.screen.saved_charset = self.screen.charset; + self.screen.saved_origin_mode = self.modes.get(.origin); } /// Restore cursor position and other state. @@ -433,8 +409,8 @@ pub fn saveCursor(self: *Terminal) void { /// If no save was done before values are reset to their initial values. pub fn restoreCursor(self: *Terminal) void { self.screen.cursor = self.screen.saved_cursor; - self.charset = self.saved_charset; - _ = self.modes.restore(.origin); + self.screen.charset = self.screen.saved_charset; + self.modes.set(.origin, self.screen.saved_origin_mode); } /// TODO: test @@ -583,7 +559,7 @@ pub fn setAttribute(self: *Terminal, attr: sgr.Attribute) !void { /// Set the charset into the given slot. pub fn configureCharset(self: *Terminal, slot: charsets.Slots, set: charsets.Charset) void { - self.charset.charsets.set(slot, set); + self.screen.charset.charsets.set(slot, set); } /// Invoke the charset in slot into the active slot. If single is true, @@ -596,13 +572,13 @@ pub fn invokeCharset( ) void { if (single) { assert(active == .GL); - self.charset.single_shift = slot; + self.screen.charset.single_shift = slot; return; } switch (active) { - .GL => self.charset.gl = slot, - .GR => self.charset.gr = slot, + .GL => self.screen.charset.gl = slot, + .GR => self.screen.charset.gr = slot, } } @@ -769,11 +745,11 @@ fn printCell(self: *Terminal, unmapped_c: u21) *Screen.Cell { // TODO: non-utf8 handling, gr // If we're single shifting, then we use the key exactly once. - const key = if (self.charset.single_shift) |key_once| blk: { - self.charset.single_shift = null; + const key = if (self.screen.charset.single_shift) |key_once| blk: { + self.screen.charset.single_shift = null; break :blk key_once; - } else self.charset.gl; - const set = self.charset.charsets.get(key); + } else self.screen.charset.gl; + const set = self.screen.charset.charsets.get(key); // UTF-8 or ASCII is used as-is if (set == .utf8 or set == .ascii) break :c unmapped_c; @@ -1749,7 +1725,7 @@ pub fn kittyGraphics( /// Full reset pub fn fullReset(self: *Terminal, alloc: Allocator) void { self.primaryScreen(alloc, .{ .clear_on_exit = true, .cursor_save = true }); - self.charset = .{}; + self.screen.charset = .{}; self.modes = .{}; self.flags = .{}; self.tabstops.reset(0); @@ -2930,14 +2906,38 @@ test "Terminal: saveCursor" { defer t.deinit(alloc); t.screen.cursor.pen.attrs.bold = true; - t.charset.gr = .G3; + t.screen.charset.gr = .G3; t.modes.set(.origin, true); t.saveCursor(); - t.charset.gr = .G0; + t.screen.charset.gr = .G0; t.screen.cursor.pen.attrs.bold = false; t.modes.set(.origin, false); t.restoreCursor(); try testing.expect(t.screen.cursor.pen.attrs.bold); - try testing.expect(t.charset.gr == .G3); + try testing.expect(t.screen.charset.gr == .G3); + try testing.expect(t.modes.get(.origin)); +} + +test "Terminal: saveCursor with screen change" { + const alloc = testing.allocator; + var t = try init(alloc, 3, 3); + defer t.deinit(alloc); + + t.screen.cursor.pen.attrs.bold = true; + t.screen.charset.gr = .G3; + t.modes.set(.origin, true); + t.alternateScreen(alloc, .{ + .cursor_save = true, + .clear_on_enter = true, + }); + t.screen.charset.gr = .G0; + t.screen.cursor.pen.attrs.bold = false; + t.modes.set(.origin, false); + t.primaryScreen(alloc, .{ + .cursor_save = true, + .clear_on_enter = true, + }); + try testing.expect(t.screen.cursor.pen.attrs.bold); + try testing.expect(t.screen.charset.gr == .G3); try testing.expect(t.modes.get(.origin)); } From b592b069c8bf7d539eee0bed4c38b0a0f8c25662 Mon Sep 17 00:00:00 2001 From: Tim Culverhouse Date: Mon, 25 Sep 2023 11:42:01 -0500 Subject: [PATCH 2/3] terminal: retain cursor and charset when entering alt screen The state of the cursor and charset should be retained when entering the alternate screen. Fixes: alacritty/save_cursor_alt --- src/terminal/Terminal.zig | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index 5187dc56a..6a32e8ffc 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -203,14 +203,22 @@ pub fn alternateScreen( self.secondary_screen = old; self.active_screen = .alternate; - // Clear our pen - self.screen.cursor = .{}; + // Bring our pen with us + self.screen.cursor = old.cursor; + + // Bring our charset state with us + self.screen.charset = old.charset; // Clear our selection self.screen.selection = null; if (options.clear_on_enter) { self.eraseDisplay(alloc, .complete); + // HACK: eraseDisplay needs to work properly here, but alters the state + // of our pen in doing so. Either way, we need to set pen state before + // erasing, use the semantics of erasing, and end up with the pen state + // where it was + self.screen.cursor = old.cursor; } } @@ -2924,12 +2932,18 @@ test "Terminal: saveCursor with screen change" { defer t.deinit(alloc); t.screen.cursor.pen.attrs.bold = true; + t.screen.cursor.x = 2; t.screen.charset.gr = .G3; t.modes.set(.origin, true); t.alternateScreen(alloc, .{ .cursor_save = true, .clear_on_enter = true, }); + // make sure our cursor and charset have come with us + try testing.expect(t.screen.cursor.pen.attrs.bold); + try testing.expect(t.screen.cursor.x == 2); + try testing.expect(t.screen.charset.gr == .G3); + try testing.expect(t.modes.get(.origin)); t.screen.charset.gr = .G0; t.screen.cursor.pen.attrs.bold = false; t.modes.set(.origin, false); From 3264e707017253cb3c238bca6fa6e43a84e9f20c Mon Sep 17 00:00:00 2001 From: Tim Culverhouse Date: Mon, 25 Sep 2023 12:25:45 -0500 Subject: [PATCH 3/3] terminal: don't modify pen state when erasing eraseDisplay Erasing the display should be done with only the background attribute of the current pen. This is the current behavior but is done by altering the current pen state. The pen state should remain after erasure. Use a new pen variable to erase the display to enable retaining the pen state. Add a test condition. --- src/terminal/Terminal.zig | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index 6a32e8ffc..eae35385a 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -214,11 +214,6 @@ pub fn alternateScreen( if (options.clear_on_enter) { self.eraseDisplay(alloc, .complete); - // HACK: eraseDisplay needs to work properly here, but alters the state - // of our pen in doing so. Either way, we need to set pen state before - // erasing, use the semantics of erasing, and end up with the pen state - // where it was - self.screen.cursor = old.cursor; } } @@ -1020,7 +1015,7 @@ pub fn eraseDisplay( defer tracy.end(); // Erasing clears all attributes / colors _except_ the background - self.screen.cursor.pen = if (!self.screen.cursor.pen.attrs.has_bg) .{} else .{ + const pen: Screen.Cell = if (!self.screen.cursor.pen.attrs.has_bg) .{} else .{ .bg = self.screen.cursor.pen.bg, .attrs = .{ .has_bg = true }, }; @@ -1031,7 +1026,7 @@ pub fn eraseDisplay( while (it.next()) |row| { row.setWrapped(false); row.setDirty(true); - row.clear(self.screen.cursor.pen); + row.clear(pen); } // Unsets pending wrap state @@ -1050,7 +1045,7 @@ pub fn eraseDisplay( for (self.screen.cursor.x..self.cols) |x| { if (row.header().flags.grapheme) row.clearGraphemes(x); const cell = row.getCellPtr(x); - cell.* = self.screen.cursor.pen; + cell.* = pen; cell.char = 0; } } @@ -1063,7 +1058,7 @@ pub fn eraseDisplay( for (0..self.cols) |x| { if (row.header().flags.grapheme) row.clearGraphemes(x); const cell = row.getCellPtr(x); - cell.* = self.screen.cursor.pen; + cell.* = pen; cell.char = 0; } } @@ -1077,7 +1072,7 @@ pub fn eraseDisplay( var x: usize = 0; while (x <= self.screen.cursor.x) : (x += 1) { const cell = self.screen.getCellPtr(.active, self.screen.cursor.y, x); - cell.* = self.screen.cursor.pen; + cell.* = pen; cell.char = 0; } @@ -1087,7 +1082,7 @@ pub fn eraseDisplay( x = 0; while (x < self.cols) : (x += 1) { const cell = self.screen.getCellPtr(.active, y, x); - cell.* = self.screen.cursor.pen; + cell.* = pen; cell.char = 0; } } @@ -1135,6 +1130,9 @@ test "Terminal: eraseDisplay above" { try testing.expect(!cell.attrs.bold); try testing.expect(cell.attrs.has_bg); + // Check that our pen hasn't changed + try testing.expect(t.screen.cursor.pen.attrs.bold); + // check that another cell got the correct bg cell = t.screen.getCell(.active, 0, 1); try testing.expect(cell.bg.eql(pink));