From 5abc63193ec515b73d7150c0f37f05c4da29f4b4 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Mon, 1 Apr 2024 18:35:34 -0400 Subject: [PATCH 1/3] font/sprite: improve rendering of dashed lines Previous implementation would draw dashes to the edge of the character cell, which would result in double-wide dashes at the point where they tiled. This fixes that, and also generally implements it in a cleaner way than before. --- src/font/sprite/Box.zig | 234 +++++++++++++++++++++------------------- 1 file changed, 121 insertions(+), 113 deletions(-) diff --git a/src/font/sprite/Box.zig b/src/font/sprite/Box.zig index a38c4b76c..03237c68d 100644 --- a/src/font/sprite/Box.zig +++ b/src/font/sprite/Box.zig @@ -2425,145 +2425,153 @@ fn draw_light_arc( fn draw_dash_horizontal( self: Box, canvas: *font.sprite.Canvas, - count: u8, + comptime count: u8, thick_px: u32, desired_gap: u32, ) void { assert(count >= 2 and count <= 4); - // The number of gaps we have is one less than the number of dashes. - // "- - -" => 2 gaps - const gap_count = count - 1; + // +------------+ + // | | + // | | + // | | + // | | + // | -- -- -- | + // | | + // | | + // | | + // | | + // +------------+ + // Our dashed line should be made such that when tiled horizontally + // it creates one consistent line with no uneven gap or segment sizes. + // In order to make sure this is the case, we should have half-sized + // gaps on the left and right so that it is centered properly. - // Determine the width of each dash and the gap between them. We try - // to have gap match desired_gap but if our cell is too small then we - // have to bring it down. - const adjusted: struct { - dash_width: u32, - gap: u32, - } = adjusted: { - for (0..desired_gap) |i| { - const gap_width: u32 = desired_gap - @as(u32, @intCast(i)); - const total_gap_width: u32 = gap_count * gap_width; + // For N dashes, there are N - 1 gaps between them, but we also have + // half-sized gaps on either side, adding up to N total gaps. + const gap_count = count; - // This would make a negative and overflow our u32. A negative - // dash width is not allowed so we keep trying to fit it. - if (total_gap_width >= self.width) continue; - - break :adjusted .{ - .dash_width = (self.width - total_gap_width) / count, - .gap = gap_width, - }; - } - - // In this case, there is no combination of gap width and dash - // width that would fit our desired number of dashes, so we just - // draw a horizontal line. + // We need at least 1 pixel for each gap and each dash, if we don't + // have that then we can't draw our dashed line correctly so we just + // draw a solid line and return. + if (self.width < count + gap_count) { self.hline_middle(canvas, .light); return; - }; - const dash_width = adjusted.dash_width; - const gap = adjusted.gap; - - // Our total width should be less than our real width - assert(count * dash_width + gap_count * gap <= self.width); - const remaining = self.width - count * dash_width - gap_count * gap; - - var x: [4]u32 = .{0} ** 4; - var w: [4]u32 = .{dash_width} ** 4; - x[1] = x[0] + w[0] + gap; - if (count == 2) - w[1] = self.width - x[1] - else if (count == 3) - w[1] += remaining - else - w[1] += remaining / 2; - - if (count >= 3) { - x[2] = x[1] + w[1] + gap; - if (count == 3) - w[2] = self.width - x[2] - else - w[2] += remaining - remaining / 2; } - if (count >= 4) { - x[3] = x[2] + w[2] + gap; - w[3] = self.width - x[3]; - } + // We never want the gaps to take up more than 50% of the space, + // because if they do the dashes are too small and look wrong. + const gap_width = @min(desired_gap, self.width / (2 * count)); + const total_gap_width = gap_count * gap_width; + const total_dash_width = self.width - total_gap_width; + const dash_width = total_dash_width / count; + const remaining = total_dash_width % count; - self.hline(canvas, x[0], x[0] + w[0], (self.height -| thick_px) / 2, thick_px); - self.hline(canvas, x[1], x[1] + w[1], (self.height -| thick_px) / 2, thick_px); - if (count >= 3) - self.hline(canvas, x[2], x[2] + w[2], (self.height -| thick_px) / 2, thick_px); - if (count >= 4) - self.hline(canvas, x[3], x[3] + w[3], (self.height -| thick_px) / 2, thick_px); + assert(dash_width * count + gap_width * gap_count + remaining == self.width); + + // Our dashes should be centered vertically. + const y: u32 = (self.height -| thick_px) / 2; + + // We start at half a gap from the left edge, in order to center + // our dashes properly. + var x: u32 = gap_width / 2; + + // We'll distribute the extra space in to dash widths, 1px at a + // time. We prefer this to making gaps larger since that is much + // more visually obvious. + var extra: u32 = remaining; + + inline for (0..count) |_| { + var x1 = x + dash_width; + // We distribute left-over size in to dash widths, + // since it's less obvious there than in the gaps. + if (extra > 0) { + extra -= 1; + x1 += 1; + } + self.hline(canvas, x, x1, y, thick_px); + // Advance by the width of the dash we drew and the width + // of a gap to get the the start of the next dash. + x = x1 + gap_width; + } } fn draw_dash_vertical( self: Box, canvas: *font.sprite.Canvas, - count: u8, + comptime count: u8, thick_px: u32, - gap: u32, + desired_gap: u32, ) void { assert(count >= 2 and count <= 4); - // The number of gaps we have is one less than the number of dashes. - // "- - -" => 2 gaps - const gap_count = count - 1; + // +-----------+ + // | | | + // | | | + // | | + // | | | + // | | | + // | | + // | | | + // | | | + // | | + // +-----------+ + // Our dashed line should be made such that when tiled verically it + // it creates one consistent line with no uneven gap or segment sizes. + // In order to make sure this is the case, we should have an extra gap + // gap at the bottom. + // + // A single full-sized extra gap is preferred to two half-sized ones for + // vertical to allow better joining to solid characters without creating + // visible half-sized gaps. Unlike horizontal, centering is a lot less + // important, visually. - // Determine the height of our dashes - const dash_height = dash_height: { - var gap_i = gap; - var dash_height = (self.height - (gap_count * gap_i)) / count; - while (dash_height <= 0 and gap_i > 1) { - gap_i -= 1; - dash_height = (self.height - (gap_count * gap_i)) / count; - } + // Because of the extra gap at the bottom, there are as many gaps as + // there are dashes. + const gap_count = count; - // If we can't fit any dashes then we just render a horizontal line. - if (dash_height <= 0) { - self.vline_middle(canvas, .light); - return; - } - - break :dash_height dash_height; - }; - - // Our total height should be less than our real height - assert(count * dash_height + gap_count * gap <= self.height); - const remaining = self.height - count * dash_height - gap_count * gap; - - var y: [4]u32 = .{0} ** 4; - var h: [4]u32 = .{dash_height} ** 4; - y[1] = y[0] + h[0] + gap; - if (count == 2) - h[1] = self.height - y[1] - else if (count == 3) - h[1] += remaining - else - h[1] += remaining / 2; - - if (count >= 3) { - y[2] = y[1] + h[1] + gap; - if (count == 3) - h[2] = self.height - y[2] - else - h[2] += remaining - remaining / 2; + // We need at least 1 pixel for each gap and each dash, if we don't + // have that then we can't draw our dashed line correctly so we just + // draw a solid line and return. + if (self.height < count + gap_count) { + self.vline_middle(canvas, .light); + return; } - if (count >= 4) { - y[3] = y[2] + h[2] + gap; - h[3] = self.height - y[3]; - } + // We never want the gaps to take up more than 50% of the space, + // because if they do the dashes are too small and look wrong. + const gap_height = @min(desired_gap, self.height / (2 * count)); + const total_gap_height = gap_count * gap_height; + const total_dash_height = self.height - total_gap_height; + const dash_height = total_dash_height / count; + const remaining = total_dash_height % count; - self.vline(canvas, y[0], y[0] + h[0], (self.width -| thick_px) / 2, thick_px); - self.vline(canvas, y[1], y[1] + h[1], (self.width -| thick_px) / 2, thick_px); - if (count >= 3) - self.vline(canvas, y[2], y[2] + h[2], (self.width -| thick_px) / 2, thick_px); - if (count >= 4) - self.vline(canvas, y[3], y[3] + h[3], (self.width -| thick_px) / 2, thick_px); + assert(dash_height * count + gap_height * gap_count + remaining == self.height); + + // Our dashes should be centered horizontally. + const x: u32 = (self.width -| thick_px) / 2; + + // We start at the top of the cell. + var y: u32 = 0; + + // We'll distribute the extra space in to dash heights, 1px at a + // time. We prefer this to making gaps larger since that is much + // more visually obvious. + var extra: u32 = remaining; + + inline for (0..count) |_| { + var y1 = y + dash_height; + // We distribute left-over size in to dash widths, + // since it's less obvious there than in the gaps. + if (extra > 0) { + extra -= 1; + y1 += 1; + } + self.vline(canvas, y, y1, x, thick_px); + // Advance by the height of the dash we drew and the height + // of a gap to get the the start of the next dash. + y = y1 + gap_height; + } } fn draw_cursor_rect(self: Box, canvas: *font.sprite.Canvas) void { From 555f6e159ff69c862c48057785ba718e658e2d6d Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 2 Apr 2024 08:38:36 -0700 Subject: [PATCH 2/3] font/sprite: remove comptime arg for box drawing --- src/font/sprite/Box.zig | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/font/sprite/Box.zig b/src/font/sprite/Box.zig index 03237c68d..ae991f4b5 100644 --- a/src/font/sprite/Box.zig +++ b/src/font/sprite/Box.zig @@ -2425,7 +2425,7 @@ fn draw_light_arc( fn draw_dash_horizontal( self: Box, canvas: *font.sprite.Canvas, - comptime count: u8, + count: u8, thick_px: u32, desired_gap: u32, ) void { @@ -2461,11 +2461,11 @@ fn draw_dash_horizontal( // We never want the gaps to take up more than 50% of the space, // because if they do the dashes are too small and look wrong. - const gap_width = @min(desired_gap, self.width / (2 * count)); - const total_gap_width = gap_count * gap_width; + const gap_width = @min(desired_gap, self.width / (2 * count)); + const total_gap_width = gap_count * gap_width; const total_dash_width = self.width - total_gap_width; - const dash_width = total_dash_width / count; - const remaining = total_dash_width % count; + const dash_width = total_dash_width / count; + const remaining = total_dash_width % count; assert(dash_width * count + gap_width * gap_count + remaining == self.width); @@ -2481,7 +2481,7 @@ fn draw_dash_horizontal( // more visually obvious. var extra: u32 = remaining; - inline for (0..count) |_| { + for (0..count) |_| { var x1 = x + dash_width; // We distribute left-over size in to dash widths, // since it's less obvious there than in the gaps. @@ -2540,11 +2540,11 @@ fn draw_dash_vertical( // We never want the gaps to take up more than 50% of the space, // because if they do the dashes are too small and look wrong. - const gap_height = @min(desired_gap, self.height / (2 * count)); - const total_gap_height = gap_count * gap_height; + const gap_height = @min(desired_gap, self.height / (2 * count)); + const total_gap_height = gap_count * gap_height; const total_dash_height = self.height - total_gap_height; - const dash_height = total_dash_height / count; - const remaining = total_dash_height % count; + const dash_height = total_dash_height / count; + const remaining = total_dash_height % count; assert(dash_height * count + gap_height * gap_count + remaining == self.height); From eb2a2e39317529d41055a952bee7c462fa2dc6e5 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 2 Apr 2024 08:38:51 -0700 Subject: [PATCH 3/3] fmt --- src/fastmem.zig | 2 +- src/main_ghostty.zig | 2 +- src/terminal/Terminal.zig | 10 +++++----- src/terminal/page.zig | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/fastmem.zig b/src/fastmem.zig index 8f32bc3c8..53c9e1122 100644 --- a/src/fastmem.zig +++ b/src/fastmem.zig @@ -26,7 +26,7 @@ pub inline fn copy(comptime T: type, dest: []T, source: []const T) void { /// and a tmp var for the single rotated item instead of 3 calls to reverse. pub inline fn rotateOnce(comptime T: type, items: []T) void { const tmp = items[0]; - move(T, items[0..items.len - 1], items[1..items.len]); + move(T, items[0 .. items.len - 1], items[1..items.len]); items[items.len - 1] = tmp; } diff --git a/src/main_ghostty.zig b/src/main_ghostty.zig index 32458ada9..17c521d1e 100644 --- a/src/main_ghostty.zig +++ b/src/main_ghostty.zig @@ -91,7 +91,7 @@ pub fn main() !MainReturn { \\ \\We don't have proper help output yet, sorry! Please refer to the \\source code or Discord community for help for now. We'll fix this in time. - \\ + \\ , .{}, ); diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index d002b47ec..e29a4003d 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -1331,8 +1331,8 @@ fn rowWillBeShifted( // spacer head will be either moved or cleared, so we also need // to turn the spacer heads in to empty cells in that case. if (self.scrolling_region.right == self.cols - 1 or - self.scrolling_region.left < 2 - ) { + self.scrolling_region.left < 2) + { const end_cell: *Cell = &cells[page.size.cols - 1]; if (end_cell.wide == .spacer_head) { end_cell.wide = .narrow; @@ -6318,7 +6318,7 @@ test "Terminal: deleteLines wide character spacer head left and right scroll mar try t.printString("AAAAABBBB\u{1F600}CCC"); t.scrolling_region.right = 3; - t.scrolling_region.left = 2; + t.scrolling_region.left = 2; // Delete the top line // ## <- scrolling region @@ -6360,7 +6360,7 @@ test "Terminal: deleteLines wide character spacer head left (< 2) and right scro try t.printString("AAAAABBBB\u{1F600}CCC"); t.scrolling_region.right = 3; - t.scrolling_region.left = 1; + t.scrolling_region.left = 1; // Delete the top line // ### <- scrolling region @@ -6400,7 +6400,7 @@ test "Terminal: deleteLines wide characters split by left/right scroll region bo try t.printString("AAAAA\n\u{1F600}B\u{1F600}"); t.scrolling_region.right = 3; - t.scrolling_region.left = 1; + t.scrolling_region.left = 1; // Delete the top line // ### <- scrolling region diff --git a/src/terminal/page.zig b/src/terminal/page.zig index d100acc89..e70695213 100644 --- a/src/terminal/page.zig +++ b/src/terminal/page.zig @@ -184,7 +184,7 @@ pub const Page = struct { pub fn reinit(self: *Page) void { // We zero the page memory as u64 instead of u8 because // we can and it's empirically quite a bit faster. - @memset(@as([*]u64, @ptrCast(self.memory))[0..self.memory.len / 8], 0); + @memset(@as([*]u64, @ptrCast(self.memory))[0 .. self.memory.len / 8], 0); self.* = initBuf(OffsetBuf.init(self.memory), layout(self.capacity)); }