From 480d262bc1afac02738c2a61ceaa5ea64b8d7851 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 30 Aug 2022 10:06:26 -0700 Subject: [PATCH] Calculate grid_width properly, use that instead of wide mask in shader --- shaders/cell.v.glsl | 13 ++---- src/Grid.zig | 20 +++----- src/font/Shaper.zig | 110 +++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 113 insertions(+), 30 deletions(-) diff --git a/shaders/cell.v.glsl b/shaders/cell.v.glsl index 549f39eba..cc6414346 100644 --- a/shaders/cell.v.glsl +++ b/shaders/cell.v.glsl @@ -11,7 +11,6 @@ const uint MODE_CURSOR_RECT = 3u; const uint MODE_CURSOR_RECT_HOLLOW = 4u; const uint MODE_CURSOR_BAR = 5u; const uint MODE_UNDERLINE = 6u; -const uint MODE_WIDE_MASK = 128u; // 0b1000_0000 // The grid coordinates (x, y) where x < columns and y < rows layout (location = 0) in vec2 grid_coord; @@ -81,12 +80,9 @@ uniform float glyph_baseline; */ void main() { - // Remove any masks from our mode - uint mode_unmasked = mode_in & ~MODE_WIDE_MASK; - // We always forward our mode unmasked because the fragment // shader doesn't use any of the masks. - mode = mode_unmasked; + mode = mode_in; // Top-left cell coordinates converted to world space // Example: (1,0) with a 30 wide cell is converted to (30,0) @@ -113,9 +109,7 @@ void main() { // Scaled for wide chars vec2 cell_size_scaled = cell_size; - if ((mode_in & MODE_WIDE_MASK) == MODE_WIDE_MASK) { - cell_size_scaled.x = cell_size_scaled.x * 2; - } + cell_size_scaled.x = cell_size_scaled.x * grid_width; switch (mode) { case MODE_BG: @@ -136,7 +130,8 @@ void main() { // The "+ 3" here is to give some wiggle room for fonts that are // BARELY over it. vec2 glyph_size_downsampled = glyph_size; - if (glyph_size.y > cell_size.y + 2) { + if (glyph_size.y > cell_size.y + 2 || + glyph_size.x > cell_size_scaled.x + 2) { glyph_size_downsampled.x = cell_size_scaled.x; glyph_size_downsampled.y = glyph_size.y * (glyph_size_downsampled.x / glyph_size.x); glyph_offset_calc.y = glyph_offset.y * (glyph_size_downsampled.x / glyph_size.x); diff --git a/src/Grid.zig b/src/Grid.zig index 22b9f8222..daaa2e930 100644 --- a/src/Grid.zig +++ b/src/Grid.zig @@ -78,7 +78,9 @@ pub const CursorStyle = enum(u8) { }; /// The raw structure that maps directly to the buffer sent to the vertex shader. -const GPUCell = struct { +/// This must be "extern" so that the field order is not reordered by the +/// Zig compiler. +const GPUCell = extern struct { /// vec2 grid_coord grid_col: u16, grid_row: u16, @@ -111,7 +113,7 @@ const GPUCell = struct { mode: GPUCellMode, /// The width in grid cells that a rendering takes. - grid_width: u16 = 1, + grid_width: u8, }; const GPUCellMode = enum(u8) { @@ -123,8 +125,6 @@ const GPUCellMode = enum(u8) { cursor_bar = 5, underline = 6, - wide_mask = 0b1000_0000, - // Non-exhaustive because masks change it _, @@ -228,7 +228,7 @@ pub fn init( offset += 4 * @sizeOf(u8); try vbobind.attributeIAdvanced(6, 1, gl.c.GL_UNSIGNED_BYTE, @sizeOf(GPUCell), offset); offset += 1 * @sizeOf(u8); - try vbobind.attributeAdvanced(7, 1, gl.c.GL_UNSIGNED_SHORT, false, @sizeOf(GPUCell), offset); + try vbobind.attributeIAdvanced(7, 1, gl.c.GL_UNSIGNED_BYTE, @sizeOf(GPUCell), offset); try vbobind.enableAttribArray(0); try vbobind.enableAttribArray(1); try vbobind.enableAttribArray(2); @@ -413,7 +413,6 @@ fn addCursor(self: *Grid, term: *Terminal) void { GPUCellMode, @enumToInt(self.cursor_style), ); - if (cell.attrs.wide) mode = mode.mask(.wide_mask); self.cells.appendAssumeCapacity(.{ .mode = mode, @@ -511,7 +510,6 @@ pub fn updateCell( // If the cell has a background, we always draw it. if (colors.bg) |rgb| { var mode: GPUCellMode = .bg; - if (cell.attrs.wide) mode = mode.mask(.wide_mask); self.cells.appendAssumeCapacity(.{ .mode = mode, @@ -549,9 +547,6 @@ pub fn updateCell( var mode: GPUCellMode = .fg; if (face.hasColor()) mode = .fg_color; - // If the cell is wide, we need to note that in the mode - if (cell.attrs.wide) mode = mode.mask(.wide_mask); - self.cells.appendAssumeCapacity(.{ .mode = mode, .grid_col = @intCast(u16, x), @@ -575,11 +570,8 @@ pub fn updateCell( } if (cell.attrs.underline) { - var mode: GPUCellMode = .underline; - if (cell.attrs.wide) mode = mode.mask(.wide_mask); - self.cells.appendAssumeCapacity(.{ - .mode = mode, + .mode = .underline, .grid_col = @intCast(u16, x), .grid_row = @intCast(u16, y), .grid_width = shaper_cell.width, diff --git a/src/font/Shaper.zig b/src/font/Shaper.zig index 4e6788d01..8f809f1a5 100644 --- a/src/font/Shaper.zig +++ b/src/font/Shaper.zig @@ -54,8 +54,14 @@ pub fn runIterator(self: *Shaper, row: terminal.Screen.Row) RunIterator { /// /// If there is not enough space in the cell buffer, an error is returned. pub fn shape(self: *Shaper, run: TextRun) ![]Cell { + // TODO: we do not want to hardcode these + const hb_feats = &[_]harfbuzz.Feature{ + harfbuzz.Feature.fromString("dlig").?, + harfbuzz.Feature.fromString("liga").?, + }; + const face = self.group.group.faceFromIndex(run.font_index); - harfbuzz.shape(face.hb_font, self.hb_buf, null); + harfbuzz.shape(face.hb_font, self.hb_buf, hb_feats); // If our buffer is empty, we short-circuit the rest of the work // return nothing. @@ -69,7 +75,7 @@ pub fn shape(self: *Shaper, run: TextRun) ![]Cell { // Convert all our info/pos to cells and set it. if (info.len > self.cell_buf.len) return error.OutOfMemory; - // log.debug("info={} pos={}", .{ info.len, pos.len }); + //log.warn("info={} pos={} run={}", .{ info.len, pos.len, run }); // x is the column that we currently occupy. We start at the offset. var x: u16 = run.offset; @@ -80,7 +86,7 @@ pub fn shape(self: *Shaper, run: TextRun) ![]Cell { // to detect since we set the cluster number to the column it // originated. const cp_width = if (i == info.len - 1) - run.max_cluster - v.cluster + (run.max_cluster - v.cluster) + 1 // + 1 because we're zero indexed else width: { const next_cluster = info[i + 1].cluster; break :width next_cluster - v.cluster; @@ -89,14 +95,14 @@ pub fn shape(self: *Shaper, run: TextRun) ![]Cell { self.cell_buf[i] = .{ .x = x, .glyph_index = v.codepoint, - .width = if (cp_width > 2) 2 else @intCast(u8, cp_width), + .width = @intCast(u8, cp_width), }; // Increase x by the amount of codepoints we replaced so that // we retain the grid. x += @intCast(u16, cp_width); - // log.debug("i={} info={} pos={} cell={}", .{ i, v, pos[i], self.cell_buf[i] }); + //log.warn("i={} info={} pos={} cell={}", .{ i, v, pos[i], self.cell_buf[i] }); } return self.cell_buf[0..info.len]; @@ -153,8 +159,15 @@ pub const RunIterator = struct { while (j < self.row.lenCells()) : (j += 1) { const cell = self.row.getCell(j); - // Ignore tailing wide spacers, this will get fixed up by the shaper - if (cell.empty() or cell.attrs.wide_spacer_tail) continue; + // Ignore empty cells + if (cell.empty()) continue; + + // If we're a spacer, then we ignore it but increase the max cluster + // size so that the width calculation is correct. + if (cell.attrs.wide_spacer_tail) { + max_cluster = j; + continue; + } const style: Style = if (cell.attrs.bold) .bold @@ -232,6 +245,19 @@ test "run iterator" { try testing.expectEqual(@as(usize, 1), count); } + // Spaces should be part of a run + { + var screen = try terminal.Screen.init(alloc, 3, 10, 0); + defer screen.deinit(alloc); + screen.testWriteString("ABCD EFG"); + + var shaper = testdata.shaper; + var it = shaper.runIterator(screen.getRow(.{ .screen = 0 })); + var count: usize = 0; + while (try it.next(alloc)) |_| count += 1; + try testing.expectEqual(@as(usize, 1), count); + } + { // Make a screen with some data var screen = try terminal.Screen.init(alloc, 3, 5, 0); @@ -282,6 +308,76 @@ test "shape" { try testing.expectEqual(@as(usize, 1), count); } +test "shape inconsolata ligs" { + const testing = std.testing; + const alloc = testing.allocator; + + var testdata = try testShaper(alloc); + defer testdata.deinit(); + + { + var screen = try terminal.Screen.init(alloc, 3, 5, 0); + defer screen.deinit(alloc); + screen.testWriteString(">="); + + var shaper = testdata.shaper; + var it = shaper.runIterator(screen.getRow(.{ .screen = 0 })); + var count: usize = 0; + while (try it.next(alloc)) |run| { + count += 1; + + const cells = try shaper.shape(run); + try testing.expectEqual(@as(usize, 1), cells.len); + try testing.expectEqual(@as(u8, 2), cells[0].width); + } + try testing.expectEqual(@as(usize, 1), count); + } + + { + var screen = try terminal.Screen.init(alloc, 3, 5, 0); + defer screen.deinit(alloc); + screen.testWriteString("==="); + + var shaper = testdata.shaper; + var it = shaper.runIterator(screen.getRow(.{ .screen = 0 })); + var count: usize = 0; + while (try it.next(alloc)) |run| { + count += 1; + + const cells = try shaper.shape(run); + try testing.expectEqual(@as(usize, 1), cells.len); + try testing.expectEqual(@as(u8, 3), cells[0].width); + } + try testing.expectEqual(@as(usize, 1), count); + } +} + +test "shape emoji width" { + const testing = std.testing; + const alloc = testing.allocator; + + var testdata = try testShaper(alloc); + defer testdata.deinit(); + + { + var screen = try terminal.Screen.init(alloc, 3, 5, 0); + defer screen.deinit(alloc); + screen.testWriteString("👍"); + + var shaper = testdata.shaper; + var it = shaper.runIterator(screen.getRow(.{ .screen = 0 })); + var count: usize = 0; + while (try it.next(alloc)) |run| { + count += 1; + + const cells = try shaper.shape(run); + try testing.expectEqual(@as(usize, 1), cells.len); + try testing.expectEqual(@as(u8, 2), cells[0].width); + } + try testing.expectEqual(@as(usize, 1), count); + } +} + const TestShaper = struct { alloc: Allocator, shaper: Shaper,