From 2783702c45d27df5756892613f9dd2e8c4b10bca Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 5 Jun 2026 21:00:08 -0700 Subject: [PATCH] terminal: honor glyph protocol widths Registered Glyph Protocol codepoints declare an authoritative terminal cell width, but printing still used the Unicode width table for all codepoints. This meant a registered wide PUA glyph could render across two cells while the terminal buffer, cursor advance, and wrapping logic treated it as narrow. Route print width calculation through a small helper that preserves the ASCII fast path and only checks the glossary when registrations exist and the codepoint is in a Private Use Area. Add glossary accessors for registered width and cover wide, narrow-overwrite, and edge-wrapping layout behavior with terminal tests. --- src/terminal/Terminal.zig | 127 +++++++++++++++++++++++++++- src/terminal/apc/glyph/Glossary.zig | 48 ++++++++++- 2 files changed, 172 insertions(+), 3 deletions(-) diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index 9350c5549..57d59f093 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -567,7 +567,7 @@ pub fn print(self: *Terminal, c: u21) !void { // non-single-width characters properly. We have a fast-path for // byte-sized characters since they're so common. We can ignore // control characters because they're always filtered prior. - const width: usize = if (c <= 0xFF) 1 else @intCast(unicode.table.get(c).width); + const width = self.printWidth(c); // Note: it is possible to have a width of "3" and a width of "-1" from // uucode.x's wcwidth. We should look into those cases and handle them @@ -700,6 +700,28 @@ pub fn print(self: *Terminal, c: u21) !void { self.screens.active.cursorRight(1); } +/// Return the terminal layout width for a codepoint. +/// +/// This is on the print hot path. Glyph Protocol widths are authoritative for +/// registered codepoints, but registrations are PUA-only, so keep the common +/// paths free of glossary hash-map lookups. +inline fn printWidth(self: *const Terminal, c: u21) usize { + if (c <= 0xFF) return 1; + + if (!self.glyph_glossary.isEmpty() and glyph.Glossary.isPrivateUse(c)) { + @branchHint(.unlikely); + + if (self.glyph_glossary.registeredWidth(c)) |width| { + return switch (width) { + .narrow => 1, + .wide => 2, + }; + } + } + + return @intCast(unicode.table.get(c).width); +} + fn printCell( self: *Terminal, unmapped_c: u21, @@ -3377,6 +3399,29 @@ test "Terminal: print single very long line" { for (0..1000) |_| try t.print('x'); } +fn testRegisterGlyphWidth( + t: *Terminal, + alloc: Allocator, + cp: u21, + width: glyph.request.Width, +) !void { + const data = try std.fmt.allocPrint( + alloc, + "r;cp={x};width={d};AAAAAAAAAAAAAA==", + .{ cp, @intFromEnum(width) }, + ); + defer alloc.free(data); + + var parser = glyph.CommandParser.init(alloc, 1024 * 1024); + defer parser.deinit(); + for (data) |byte| try parser.feed(byte); + + var req = try parser.complete(alloc); + defer req.deinit(alloc); + + _ = t.glyphProtocol(alloc, &req); +} + test "Terminal: print wide char" { var t = try init(testing.allocator, .{ .cols = 80, .rows = 80 }); defer t.deinit(testing.allocator); @@ -3400,6 +3445,56 @@ test "Terminal: print wide char" { try testing.expect(t.isDirty(.{ .screen = .{ .x = 0, .y = 0 } })); } +test "Terminal: registered wide glyph protocol PUA prints as wide" { + const alloc = testing.allocator; + var t = try init(alloc, .{ .cols = 80, .rows = 80 }); + defer t.deinit(alloc); + + try testRegisterGlyphWidth(&t, alloc, 0xE0A0, .wide); + try t.print(0xE0A0); + + try testing.expectEqual(@as(usize, 0), t.screens.active.cursor.y); + try testing.expectEqual(@as(usize, 2), t.screens.active.cursor.x); + + { + const list_cell = t.screens.active.pages.getCell(.{ .screen = .{ .x = 0, .y = 0 } }).?; + const cell = list_cell.cell; + try testing.expectEqual(@as(u21, 0xE0A0), cell.content.codepoint); + try testing.expectEqual(Cell.Wide.wide, cell.wide); + } + { + const list_cell = t.screens.active.pages.getCell(.{ .screen = .{ .x = 1, .y = 0 } }).?; + const cell = list_cell.cell; + try testing.expectEqual(Cell.Wide.spacer_tail, cell.wide); + } +} + +test "Terminal: registered narrow glyph protocol PUA overrides previous wide registration" { + const alloc = testing.allocator; + var t = try init(alloc, .{ .cols = 80, .rows = 80 }); + defer t.deinit(alloc); + + try testRegisterGlyphWidth(&t, alloc, 0xE0A0, .wide); + try testRegisterGlyphWidth(&t, alloc, 0xE0A0, .narrow); + try t.print(0xE0A0); + + try testing.expectEqual(@as(usize, 0), t.screens.active.cursor.y); + try testing.expectEqual(@as(usize, 1), t.screens.active.cursor.x); + + { + const list_cell = t.screens.active.pages.getCell(.{ .screen = .{ .x = 0, .y = 0 } }).?; + const cell = list_cell.cell; + try testing.expectEqual(@as(u21, 0xE0A0), cell.content.codepoint); + try testing.expectEqual(Cell.Wide.narrow, cell.wide); + } + { + const list_cell = t.screens.active.pages.getCell(.{ .screen = .{ .x = 1, .y = 0 } }).?; + const cell = list_cell.cell; + try testing.expectEqual(@as(u21, 0), cell.content.codepoint); + try testing.expectEqual(Cell.Wide.narrow, cell.wide); + } +} + test "Terminal: print wide char at edge creates spacer head" { var t = try init(testing.allocator, .{ .cols = 10, .rows = 10 }); defer t.deinit(testing.allocator); @@ -3434,6 +3529,36 @@ test "Terminal: print wide char at edge creates spacer head" { try testing.expect(t.isDirty(.{ .screen = .{ .x = 0, .y = 1 } })); } +test "Terminal: registered wide glyph protocol PUA at edge creates spacer head" { + const alloc = testing.allocator; + var t = try init(alloc, .{ .cols = 10, .rows = 10 }); + defer t.deinit(alloc); + + try testRegisterGlyphWidth(&t, alloc, 0xE0A0, .wide); + + t.setCursorPos(1, 10); + try t.print(0xE0A0); + try testing.expectEqual(@as(usize, 1), t.screens.active.cursor.y); + try testing.expectEqual(@as(usize, 2), t.screens.active.cursor.x); + + { + const list_cell = t.screens.active.pages.getCell(.{ .screen = .{ .x = 9, .y = 0 } }).?; + const cell = list_cell.cell; + try testing.expectEqual(Cell.Wide.spacer_head, cell.wide); + } + { + const list_cell = t.screens.active.pages.getCell(.{ .screen = .{ .x = 0, .y = 1 } }).?; + const cell = list_cell.cell; + try testing.expectEqual(@as(u21, 0xE0A0), cell.content.codepoint); + try testing.expectEqual(Cell.Wide.wide, cell.wide); + } + { + const list_cell = t.screens.active.pages.getCell(.{ .screen = .{ .x = 1, .y = 1 } }).?; + const cell = list_cell.cell; + try testing.expectEqual(Cell.Wide.spacer_tail, cell.wide); + } +} + test "Terminal: print wide char with 1-column width" { const alloc = testing.allocator; var t = try init(alloc, .{ .cols = 1, .rows = 2 }); diff --git a/src/terminal/apc/glyph/Glossary.zig b/src/terminal/apc/glyph/Glossary.zig index e82d9e8e8..4174e28ee 100644 --- a/src/terminal/apc/glyph/Glossary.zig +++ b/src/terminal/apc/glyph/Glossary.zig @@ -108,10 +108,25 @@ pub fn clearAndFree(self: *Glossary, alloc: Allocator) void { } /// Contains returns true if the codepoint is covered by the glossary. -pub fn contains(self: *Glossary, cp: u21) bool { +pub fn contains(self: *const Glossary, cp: u21) bool { return self.entries.contains(cp); } +/// Return true if the glossary has no registered glyphs. +pub fn isEmpty(self: *const Glossary) bool { + return self.entries.count() == 0; +} + +/// Return the registered layout width for `cp`, if any. +/// +/// Glyph Protocol `width` is authoritative for terminal layout decisions for +/// registered codepoints. Callers should still avoid this map lookup unless +/// `cp` is in a Private Use Area, because registrations are PUA-only. +pub fn registeredWidth(self: *const Glossary, cp: u21) ?request.Width { + const entry = self.entries.get(cp) orelse return null; + return entry.width; +} + /// A single glyph registration entry. pub const Entry = struct { /// Stored glyph payload variants. @@ -268,7 +283,7 @@ pub const Entry = struct { }; /// Return true if `cp` is in one of the Unicode Private Use Areas. -fn isPrivateUse(cp: u21) bool { +pub inline fn isPrivateUse(cp: u21) bool { return (cp >= 0xE000 and cp <= 0xF8FF) or (cp >= 0xF0000 and cp <= 0xFFFFD) or (cp >= 0x100000 and cp <= 0x10FFFD); @@ -479,3 +494,32 @@ test "Glossary contains reports registered slots" { try testing.expect(glossary.contains(0xE000)); try testing.expect(!glossary.contains(0xE001)); } + +test "Glossary registeredWidth tracks register overwrite delete and clear" { + const testing = std.testing; + const alloc = testing.allocator; + + var glossary: Glossary = .empty; + defer glossary.deinit(alloc); + + try testing.expect(glossary.isEmpty()); + try testing.expect(glossary.registeredWidth(0xE000) == null); + + try glossary.register(alloc, 0xE000, try testRegisterEntry(alloc, 0xE000)); + try testing.expect(!glossary.isEmpty()); + try testing.expectEqual(request.Width.wide, glossary.registeredWidth(0xE000).?); + + var entry = try testRegisterEntry(alloc, 0xE000); + entry.width = .narrow; + try glossary.register(alloc, 0xE000, entry); + try testing.expectEqual(request.Width.narrow, glossary.registeredWidth(0xE000).?); + + try glossary.delete(alloc, 0xE000); + try testing.expect(glossary.isEmpty()); + try testing.expect(glossary.registeredWidth(0xE000) == null); + + try glossary.register(alloc, 0xE000, try testRegisterEntry(alloc, 0xE000)); + glossary.clearAndFree(alloc); + try testing.expect(glossary.isEmpty()); + try testing.expect(glossary.registeredWidth(0xE000) == null); +}