From 6917bcacad036e89d9efa666b0e607d840067d2c Mon Sep 17 00:00:00 2001
From: Qwerasd <qwerasd205@users.noreply.github.com>
Date: Thu, 8 Aug 2024 11:13:30 -0400
Subject: [PATCH 01/13] font/sprite: fix 1px gap at right edge of dotted and
 dashed underlines

---
 src/font/sprite/underline.zig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/font/sprite/underline.zig b/src/font/sprite/underline.zig
index b22db4f52..c60fd71b7 100644
--- a/src/font/sprite/underline.zig
+++ b/src/font/sprite/underline.zig
@@ -133,7 +133,7 @@ const Draw = struct {
         while (i < dot_count) : (i += 2) {
             // Ensure we never go out of bounds for the rect
             const x = @min(i * dot_width, self.width - 1);
-            const width = @min(self.width - 1 - x, dot_width);
+            const width = @min(self.width - x, dot_width);
             canvas.rect(.{
                 .x = @intCast(i * dot_width),
                 .y = @intCast(y),
@@ -154,7 +154,7 @@ const Draw = struct {
         while (i < dash_count) : (i += 2) {
             // Ensure we never go out of bounds for the rect
             const x = @min(i * dash_width, self.width - 1);
-            const width = @min(self.width - 1 - x, dash_width);
+            const width = @min(self.width - x, dash_width);
             canvas.rect(.{
                 .x = @intCast(x),
                 .y = @intCast(y),

From b3a7901b791b5639f3c68954c67a6046470a5012 Mon Sep 17 00:00:00 2001
From: Qwerasd <qwerasd205@users.noreply.github.com>
Date: Mon, 23 Sep 2024 18:12:56 -0600
Subject: [PATCH 02/13] font/sprite: rework underline rendering, adjust
 positioning

---
 src/font/sprite/underline.zig | 281 ++++++++++++++++------------------
 1 file changed, 129 insertions(+), 152 deletions(-)

diff --git a/src/font/sprite/underline.zig b/src/font/sprite/underline.zig
index c60fd71b7..af0dd739a 100644
--- a/src/font/sprite/underline.zig
+++ b/src/font/sprite/underline.zig
@@ -27,189 +27,166 @@ pub fn renderGlyph(
     line_pos: u32,
     line_thickness: u32,
 ) !font.Glyph {
-    // Create the canvas we'll use to draw. We draw the underline in
-    // a full cell size and position it according to "pos".
-    var canvas = try font.sprite.Canvas.init(alloc, width, height);
-    defer canvas.deinit(alloc);
+    // _ = height;
 
-    // Perform the actual drawing
-    (Draw{
-        .width = width,
-        .height = height,
-        .pos = line_pos,
-        .thickness = line_thickness,
-    }).draw(&canvas, sprite);
+    // Draw the appropriate sprite
+    var canvas: font.sprite.Canvas, const offset_y: i32 = switch (sprite) {
+        .underline => try drawSingle(alloc, width, line_thickness),
+        .underline_double => try drawDouble(alloc, width, line_thickness),
+        .underline_dotted => try drawDotted(alloc, width, line_thickness),
+        .underline_dashed => try drawDashed(alloc, width, line_thickness),
+        .underline_curly => try drawCurly(alloc, width, line_thickness),
+        .strikethrough => try drawSingle(alloc, width, line_thickness),
+        else => unreachable,
+    };
+    defer canvas.deinit(alloc);
 
     // Write the drawing to the atlas
     const region = try canvas.writeAtlas(alloc, atlas);
 
-    // Our coordinates start at the BOTTOM for our renderers so we have to
-    // specify an offset of the full height because we rendered a full size
-    // cell.
-    const offset_y = @as(i32, @intCast(height));
-
     return font.Glyph{
         .width = width,
-        .height = height,
+        .height = @intCast(region.height),
         .offset_x = 0,
-        .offset_y = offset_y,
+        .offset_y = @as(i32, @intCast(height + region.height)) - @as(i32, @intCast(line_pos)) + offset_y + 1,
         .atlas_x = region.x,
         .atlas_y = region.y,
         .advance_x = @floatFromInt(width),
     };
 }
 
-/// Stores drawing state.
-const Draw = struct {
-    width: u32,
-    height: u32,
-    pos: u32,
-    thickness: u32,
+/// Draw a single underline.
+fn drawSingle(alloc: Allocator, width: u32, thickness: u32) !struct { font.sprite.Canvas, i32 } {
+    const height: u32 = thickness;
+    var canvas = try font.sprite.Canvas.init(alloc, width, height);
 
-    /// Draw a specific underline sprite to the canvas.
-    fn draw(self: Draw, canvas: *font.sprite.Canvas, sprite: Sprite) void {
-        switch (sprite) {
-            .underline => self.drawSingle(canvas),
-            .underline_double => self.drawDouble(canvas),
-            .underline_dotted => self.drawDotted(canvas),
-            .underline_dashed => self.drawDashed(canvas),
-            .underline_curly => self.drawCurly(canvas),
-            .strikethrough => self.drawSingle(canvas),
-            else => unreachable,
-        }
-    }
+    canvas.rect(.{
+        .x = 0,
+        .y = 0,
+        .width = width,
+        .height = thickness,
+    }, .on);
 
-    /// Draw a single underline.
-    fn drawSingle(self: Draw, canvas: *font.sprite.Canvas) void {
-        // Ensure we never overflow out of bounds on the canvas
-        const y_max = self.height -| 1;
-        const bottom = @min(self.pos + self.thickness, y_max);
-        const y = bottom -| self.thickness;
-        const max_height = self.height - y;
+    const offset_y: i32 = 0;
 
+    return .{ canvas, offset_y };
+}
+
+/// Draw a double underline.
+fn drawDouble(alloc: Allocator, width: u32, thickness: u32) !struct { font.sprite.Canvas, i32 } {
+    const height: u32 = thickness * 3;
+    var canvas = try font.sprite.Canvas.init(alloc, width, height);
+
+    canvas.rect(.{
+        .x = 0,
+        .y = 0,
+        .width = width,
+        .height = thickness,
+    }, .on);
+
+    canvas.rect(.{
+        .x = 0,
+        .y = @intCast(thickness * 2),
+        .width = width,
+        .height = thickness,
+    }, .on);
+
+    const offset_y: i32 = -@as(i32, @intCast(thickness));
+
+    return .{ canvas, offset_y };
+}
+
+/// Draw a dotted underline.
+fn drawDotted(alloc: Allocator, width: u32, thickness: u32) !struct { font.sprite.Canvas, i32 } {
+    const height: u32 = thickness;
+    var canvas = try font.sprite.Canvas.init(alloc, width, height);
+
+    const dot_width = @max(thickness, 3);
+    const dot_count = width / dot_width;
+    var i: u32 = 0;
+    while (i < dot_count) : (i += 2) {
+        // Ensure we never go out of bounds for the rect
+        const x = @min(i * dot_width, width - 1);
+        const rect_width = @min(width - x, dot_width);
         canvas.rect(.{
-            .x = 0,
-            .y = @intCast(y),
-            .width = self.width,
-            .height = @min(self.thickness, max_height),
+            .x = @intCast(i * dot_width),
+            .y = 0,
+            .width = rect_width,
+            .height = thickness,
         }, .on);
     }
 
-    /// Draw a double underline.
-    fn drawDouble(self: Draw, canvas: *font.sprite.Canvas) void {
-        // The maximum y value has to have space for the bottom underline.
-        // If we underflow (saturated) to 0, then we don't draw. This should
-        // never happen but we don't want to draw something undefined.
-        const y_max = self.height -| 1 -| self.thickness;
-        if (y_max == 0) return;
+    const offset_y: i32 = 0;
 
-        const space = self.thickness * 2;
-        const bottom = @min(self.pos + space, y_max);
-        const top = bottom - space;
+    return .{ canvas, offset_y };
+}
 
+/// Draw a dashed underline.
+fn drawDashed(alloc: Allocator, width: u32, thickness: u32) !struct { font.sprite.Canvas, i32 } {
+    const height: u32 = thickness;
+    var canvas = try font.sprite.Canvas.init(alloc, width, height);
+
+    const dash_width = width / 3 + 1;
+    const dash_count = (width / dash_width) + 1;
+    var i: u32 = 0;
+    while (i < dash_count) : (i += 2) {
+        // Ensure we never go out of bounds for the rect
+        const x = @min(i * dash_width, width - 1);
+        const rect_width = @min(width - x, dash_width);
         canvas.rect(.{
-            .x = 0,
-            .y = @intCast(top),
-            .width = self.width,
-            .height = self.thickness,
-        }, .on);
-
-        canvas.rect(.{
-            .x = 0,
-            .y = @intCast(bottom),
-            .width = self.width,
-            .height = self.thickness,
+            .x = @intCast(x),
+            .y = 0,
+            .width = rect_width,
+            .height = thickness,
         }, .on);
     }
 
-    /// Draw a dotted underline.
-    fn drawDotted(self: Draw, canvas: *font.sprite.Canvas) void {
-        const y_max = self.height -| 1 -| self.thickness;
-        if (y_max == 0) return;
-        const y = @min(self.pos, y_max);
-        const dot_width = @max(self.thickness, 3);
-        const dot_count = self.width / dot_width;
-        var i: u32 = 0;
-        while (i < dot_count) : (i += 2) {
-            // Ensure we never go out of bounds for the rect
-            const x = @min(i * dot_width, self.width - 1);
-            const width = @min(self.width - x, dot_width);
-            canvas.rect(.{
-                .x = @intCast(i * dot_width),
-                .y = @intCast(y),
-                .width = width,
-                .height = self.thickness,
-            }, .on);
+    const offset_y: i32 = 0;
+
+    return .{ canvas, offset_y };
+}
+
+/// Draw a curly underline. Thanks to Wez Furlong for providing
+/// the basic math structure for this since I was lazy with the
+/// geometry.
+fn drawCurly(alloc: Allocator, width: u32, thickness: u32) !struct { font.sprite.Canvas, i32 } {
+    const height: u32 = thickness * 4;
+    var canvas = try font.sprite.Canvas.init(alloc, width, height);
+
+    // Calculate the wave period for a single character
+    //   `2 * pi...` = 1 peak per character
+    //   `4 * pi...` = 2 peaks per character
+    const wave_period = 2 * std.math.pi / @as(f64, @floatFromInt(width - 1));
+
+    // The full amplitude of the wave can be from the bottom to the
+    // underline position. We also calculate our mid y point of the wave
+    const half_amplitude: f64 = @as(f64, @floatFromInt(thickness));
+    const y_mid: f64 = half_amplitude + 1;
+
+    // follow Xiaolin Wu's antialias algorithm to draw the curve
+    var x: u32 = 0;
+    while (x < width) : (x += 1) {
+        const cosx: f64 = @cos(@as(f64, @floatFromInt(x)) * wave_period);
+        const y: f64 = y_mid + half_amplitude * cosx;
+        const y_upper: u32 = @intFromFloat(@floor(y));
+        const y_lower: u32 = y_upper + thickness + (thickness >> 1);
+        const alpha: u8 = @intFromFloat(255 * @abs(y - @floor(y)));
+
+        // upper and lower bounds
+        canvas.pixel(x, @min(y_upper, height), @enumFromInt(255 - alpha));
+        canvas.pixel(x, @min(y_lower, height), @enumFromInt(alpha));
+
+        // fill between upper and lower bound
+        var y_fill: u32 = y_upper + 1;
+        while (y_fill < y_lower) : (y_fill += 1) {
+            canvas.pixel(x, @min(y_fill, height), .on);
         }
     }
 
-    /// Draw a dashed underline.
-    fn drawDashed(self: Draw, canvas: *font.sprite.Canvas) void {
-        const y_max = self.height -| 1 -| self.thickness;
-        if (y_max == 0) return;
-        const y = @min(self.pos, y_max);
-        const dash_width = self.width / 3 + 1;
-        const dash_count = (self.width / dash_width) + 1;
-        var i: u32 = 0;
-        while (i < dash_count) : (i += 2) {
-            // Ensure we never go out of bounds for the rect
-            const x = @min(i * dash_width, self.width - 1);
-            const width = @min(self.width - x, dash_width);
-            canvas.rect(.{
-                .x = @intCast(x),
-                .y = @intCast(y),
-                .width = width,
-                .height = self.thickness,
-            }, .on);
-        }
-    }
+    const offset_y: i32 = -@as(i32, @intCast(thickness * 2));
 
-    /// Draw a curly underline. Thanks to Wez Furlong for providing
-    /// the basic math structure for this since I was lazy with the
-    /// geometry.
-    fn drawCurly(self: Draw, canvas: *font.sprite.Canvas) void {
-        // This is the lowest that the curl can go.
-        const y_max = self.height - 1;
-
-        // Calculate the wave period for a single character
-        //   `2 * pi...` = 1 peak per character
-        //   `4 * pi...` = 2 peaks per character
-        const wave_period = 2 * std.math.pi / @as(f64, @floatFromInt(self.width - 1));
-
-        // Some fonts put the underline too close to the bottom of the
-        // cell height and this doesn't allow us to make a high enough
-        // wave. This constant is arbitrary, change it for aesthetics.
-        const pos: u32 = pos: {
-            const MIN_AMPLITUDE: u32 = @max(self.height / 9, 2);
-            break :pos y_max - (MIN_AMPLITUDE * 2);
-        };
-
-        // The full amplitude of the wave can be from the bottom to the
-        // underline position. We also calculate our mid y point of the wave
-        const double_amplitude: f64 = @floatFromInt(y_max - pos);
-        const half_amplitude: f64 = @max(1, double_amplitude / 4);
-        const y_mid: u32 = pos + @as(u32, @intFromFloat(2 * half_amplitude));
-
-        // follow Xiaolin Wu's antialias algorithm to draw the curve
-        var x: u32 = 0;
-        while (x < self.width) : (x += 1) {
-            const y: f64 = @as(f64, @floatFromInt(y_mid)) + (half_amplitude * @cos(@as(f64, @floatFromInt(x)) * wave_period));
-            const y_upper: u32 = @intFromFloat(@floor(y));
-            const y_lower: u32 = y_upper + self.thickness;
-            const alpha: u8 = @intFromFloat(255 * @abs(y - @floor(y)));
-
-            // upper and lower bounds
-            canvas.pixel(x, @min(y_upper, y_max), @enumFromInt(255 - alpha));
-            canvas.pixel(x, @min(y_lower, y_max), @enumFromInt(alpha));
-
-            // fill between upper and lower bound
-            var y_fill: u32 = y_upper + 1;
-            while (y_fill < y_lower) : (y_fill += 1) {
-                canvas.pixel(x, @min(y_fill, y_max), .on);
-            }
-        }
-    }
-};
+    return .{ canvas, offset_y };
+}
 
 test "single" {
     const testing = std.testing;

From bf2794f90fff5fd3c37047048022bdeb677f88db Mon Sep 17 00:00:00 2001
From: Qwerasd <qwerasd205@users.noreply.github.com>
Date: Mon, 23 Sep 2024 18:31:39 -0600
Subject: [PATCH 03/13] renderer: draw underlines below text to improve
 legibility

---
 src/renderer/Metal.zig  | 75 +++++++++++++++++----------------
 src/renderer/OpenGL.zig | 91 +++++++++++++++++++++--------------------
 2 files changed, 86 insertions(+), 80 deletions(-)

diff --git a/src/renderer/Metal.zig b/src/renderer/Metal.zig
index bb2a27f44..1790711c8 100644
--- a/src/renderer/Metal.zig
+++ b/src/renderer/Metal.zig
@@ -2523,6 +2523,45 @@ fn updateCell(
         }
     }
 
+    // If the cell has an underline, draw it before the character glyph,
+    // so that it layers underneath instead of overtop, since that can
+    // make text difficult to read.
+    if (underline != .none) {
+        const sprite: font.Sprite = switch (underline) {
+            .none => unreachable,
+            .single => .underline,
+            .double => .underline_double,
+            .dotted => .underline_dotted,
+            .dashed => .underline_dashed,
+            .curly => .underline_curly,
+        };
+
+        const render = try self.font_grid.renderGlyph(
+            self.alloc,
+            font.sprite_index,
+            @intFromEnum(sprite),
+            .{
+                .cell_width = if (cell.wide == .wide) 2 else 1,
+                .grid_metrics = self.grid_metrics,
+            },
+        );
+
+        const color = style.underlineColor(palette) orelse colors.fg;
+
+        try self.cells.add(self.alloc, .underline, .{
+            .mode = .fg,
+            .grid_pos = .{ @intCast(coord.x), @intCast(coord.y) },
+            .constraint_width = cell.gridWidth(),
+            .color = .{ color.r, color.g, color.b, alpha },
+            .glyph_pos = .{ render.glyph.atlas_x, render.glyph.atlas_y },
+            .glyph_size = .{ render.glyph.width, render.glyph.height },
+            .bearings = .{
+                @intCast(render.glyph.offset_x),
+                @intCast(render.glyph.offset_y),
+            },
+        });
+    }
+
     // If the shaper cell has a glyph, draw it.
     if (shaper_cell.glyph_index) |glyph_index| glyph: {
         // Render
@@ -2566,42 +2605,6 @@ fn updateCell(
         });
     }
 
-    if (underline != .none) {
-        const sprite: font.Sprite = switch (underline) {
-            .none => unreachable,
-            .single => .underline,
-            .double => .underline_double,
-            .dotted => .underline_dotted,
-            .dashed => .underline_dashed,
-            .curly => .underline_curly,
-        };
-
-        const render = try self.font_grid.renderGlyph(
-            self.alloc,
-            font.sprite_index,
-            @intFromEnum(sprite),
-            .{
-                .cell_width = if (cell.wide == .wide) 2 else 1,
-                .grid_metrics = self.grid_metrics,
-            },
-        );
-
-        const color = style.underlineColor(palette) orelse colors.fg;
-
-        try self.cells.add(self.alloc, .underline, .{
-            .mode = .fg,
-            .grid_pos = .{ @intCast(coord.x), @intCast(coord.y) },
-            .constraint_width = cell.gridWidth(),
-            .color = .{ color.r, color.g, color.b, alpha },
-            .glyph_pos = .{ render.glyph.atlas_x, render.glyph.atlas_y },
-            .glyph_size = .{ render.glyph.width, render.glyph.height },
-            .bearings = .{
-                @intCast(render.glyph.offset_x),
-                @intCast(render.glyph.offset_y),
-            },
-        });
-    }
-
     if (style.flags.strikethrough) {
         const render = try self.font_grid.renderGlyph(
             self.alloc,
diff --git a/src/renderer/OpenGL.zig b/src/renderer/OpenGL.zig
index a8f7c385c..082f330c5 100644
--- a/src/renderer/OpenGL.zig
+++ b/src/renderer/OpenGL.zig
@@ -1761,6 +1761,53 @@ fn updateCell(
         @intFromFloat(@max(0, @min(255, @round(self.config.background_opacity * 255)))),
     };
 
+    // If the cell has an underline, draw it before the character glyph,
+    // so that it layers underneath instead of overtop, since that can
+    // make text difficult to read.
+    if (underline != .none) {
+        const sprite: font.Sprite = switch (underline) {
+            .none => unreachable,
+            .single => .underline,
+            .double => .underline_double,
+            .dotted => .underline_dotted,
+            .dashed => .underline_dashed,
+            .curly => .underline_curly,
+        };
+
+        const render = try self.font_grid.renderGlyph(
+            self.alloc,
+            font.sprite_index,
+            @intFromEnum(sprite),
+            .{
+                .cell_width = if (cell.wide == .wide) 2 else 1,
+                .grid_metrics = self.grid_metrics,
+            },
+        );
+
+        const color = style.underlineColor(palette) orelse colors.fg;
+
+        try self.cells.append(self.alloc, .{
+            .mode = .fg,
+            .grid_col = @intCast(x),
+            .grid_row = @intCast(y),
+            .grid_width = cell.gridWidth(),
+            .glyph_x = render.glyph.atlas_x,
+            .glyph_y = render.glyph.atlas_y,
+            .glyph_width = render.glyph.width,
+            .glyph_height = render.glyph.height,
+            .glyph_offset_x = render.glyph.offset_x,
+            .glyph_offset_y = render.glyph.offset_y,
+            .r = color.r,
+            .g = color.g,
+            .b = color.b,
+            .a = alpha,
+            .bg_r = bg[0],
+            .bg_g = bg[1],
+            .bg_b = bg[2],
+            .bg_a = bg[3],
+        });
+    }
+
     // If the cell has a character, draw it
     if (cell.hasText()) fg: {
         // Render
@@ -1807,50 +1854,6 @@ fn updateCell(
         });
     }
 
-    if (underline != .none) {
-        const sprite: font.Sprite = switch (underline) {
-            .none => unreachable,
-            .single => .underline,
-            .double => .underline_double,
-            .dotted => .underline_dotted,
-            .dashed => .underline_dashed,
-            .curly => .underline_curly,
-        };
-
-        const render = try self.font_grid.renderGlyph(
-            self.alloc,
-            font.sprite_index,
-            @intFromEnum(sprite),
-            .{
-                .cell_width = if (cell.wide == .wide) 2 else 1,
-                .grid_metrics = self.grid_metrics,
-            },
-        );
-
-        const color = style.underlineColor(palette) orelse colors.fg;
-
-        try self.cells.append(self.alloc, .{
-            .mode = .fg,
-            .grid_col = @intCast(x),
-            .grid_row = @intCast(y),
-            .grid_width = cell.gridWidth(),
-            .glyph_x = render.glyph.atlas_x,
-            .glyph_y = render.glyph.atlas_y,
-            .glyph_width = render.glyph.width,
-            .glyph_height = render.glyph.height,
-            .glyph_offset_x = render.glyph.offset_x,
-            .glyph_offset_y = render.glyph.offset_y,
-            .r = color.r,
-            .g = color.g,
-            .b = color.b,
-            .a = alpha,
-            .bg_r = bg[0],
-            .bg_g = bg[1],
-            .bg_b = bg[2],
-            .bg_a = bg[3],
-        });
-    }
-
     if (style.flags.strikethrough) {
         const render = try self.font_grid.renderGlyph(
             self.alloc,

From ecb3c543b39a848fa58cce0780e5e73400f993d3 Mon Sep 17 00:00:00 2001
From: Qwerasd <qwerasd205@users.noreply.github.com>
Date: Mon, 23 Sep 2024 18:35:20 -0600
Subject: [PATCH 04/13] renderer/OpenGL: use better logic for whether to render
 glyph

Metal already had this change made, so I copied it over from there. This
logic is more straightforward. Also copied the check to skip 0-sized
glyphs, since sometimes, for example, spaces are emitted as glyphs by
the shaper for some reason, even though they have no actual content, and
we want to avoid sending a bunch of useless stuff to the GPU.
---
 src/renderer/OpenGL.zig | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/renderer/OpenGL.zig b/src/renderer/OpenGL.zig
index 082f330c5..760721af3 100644
--- a/src/renderer/OpenGL.zig
+++ b/src/renderer/OpenGL.zig
@@ -1808,19 +1808,25 @@ fn updateCell(
         });
     }
 
-    // If the cell has a character, draw it
-    if (cell.hasText()) fg: {
+    // If the shaper cell has a glyph, draw it.
+    if (shaper_cell.glyph_index) |glyph_index| glyph: {
         // Render
         const render = try self.font_grid.renderGlyph(
             self.alloc,
             shaper_run.font_index,
-            shaper_cell.glyph_index orelse break :fg,
+            glyph_index,
             .{
                 .grid_metrics = self.grid_metrics,
                 .thicken = self.config.font_thicken,
             },
         );
 
+        // If the glyph is 0 width or height, it will be invisible
+        // when drawn, so don't bother adding it to the buffer.
+        if (render.glyph.width == 0 or render.glyph.height == 0) {
+            break :glyph;
+        }
+
         // If we're rendering a color font, we use the color atlas
         const mode: CellProgram.CellMode = switch (try fgMode(
             render.presentation,

From 3ec36e4d239d71b52ef7820e179367f9e2b97c28 Mon Sep 17 00:00:00 2001
From: Qwerasd <qwerasd205@users.noreply.github.com>
Date: Mon, 23 Sep 2024 19:01:15 -0600
Subject: [PATCH 05/13] coretext: improve strikethrough position calculation

---
 pkg/macos/text/font.zig    |  8 ++++++++
 src/font/face/coretext.zig | 20 ++++++++++++--------
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/pkg/macos/text/font.zig b/pkg/macos/text/font.zig
index c08e8ee14..85f7de47e 100644
--- a/pkg/macos/text/font.zig
+++ b/pkg/macos/text/font.zig
@@ -188,6 +188,14 @@ pub const Font = opaque {
         return c.CTFontGetUnderlineThickness(@ptrCast(self));
     }
 
+    pub fn getCapHeight(self: *Font) f64 {
+        return c.CTFontGetCapHeight(@ptrCast(self));
+    }
+
+    pub fn getXHeight(self: *Font) f64 {
+        return c.CTFontGetXHeight(@ptrCast(self));
+    }
+
     pub fn getUnitsPerEm(self: *Font) u32 {
         return c.CTFontGetUnitsPerEm(@ptrCast(self));
     }
diff --git a/src/font/face/coretext.zig b/src/font/face/coretext.zig
index 5e141e053..3a69ef95b 100644
--- a/src/font/face/coretext.zig
+++ b/src/font/face/coretext.zig
@@ -596,15 +596,19 @@ pub const Face = struct {
         const cell_baseline = @ceil(layout_metrics.height - layout_metrics.ascent);
         const underline_thickness = @ceil(@as(f32, @floatCast(ct_font.getUnderlineThickness())));
         const strikethrough_position = strikethrough_position: {
-            // This is the height above baseline consumed by text. We must take
-            // into account that our cell height splits the leading between two
-            // rows so we subtract leading space (blank space).
-            const above_baseline = layout_metrics.ascent - (layout_metrics.leading / 2);
+            // This is the height of lower case letters in our font.
+            const ex_height = ct_font.getXHeight();
 
-            // We want to position the strikethrough at 65% of the height.
-            // This generally gives a nice visual appearance. The number 65%
-            // is somewhat arbitrary but is a common value across terminals.
-            const pos = above_baseline * 0.65;
+            // We want to position the strikethrough so that it's
+            // vertically centered on any lower case text. This is
+            // a fairly standard choice for strikethrough positioning.
+            //
+            // Because our `strikethrough_position` is relative to the
+            // top of the cell we start with the ascent metric, which
+            // is the distance from the top down to the baseline, then
+            // we subtract half of the ex height to go back up to the
+            // correct height that should evenly split lowercase text.
+            const pos = layout_metrics.ascent - ex_height * 0.5 + 1;
 
             break :strikethrough_position @ceil(pos);
         };

From c7d6227befef09a607b4b1c3858902c3a02eaf85 Mon Sep 17 00:00:00 2001
From: Qwerasd <qwerasd205@users.noreply.github.com>
Date: Mon, 23 Sep 2024 19:18:44 -0600
Subject: [PATCH 06/13] freetype: improve strikethrough position guess by using
 ex height

---
 src/font/face/freetype.zig | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/src/font/face/freetype.zig b/src/font/face/freetype.zig
index 5004a040a..04f037c85 100644
--- a/src/font/face/freetype.zig
+++ b/src/font/face/freetype.zig
@@ -607,6 +607,20 @@ pub const Face = struct {
             break :cell_width f26dot6ToFloat(size_metrics.max_advance);
         };
 
+        // Ex height is calculated by measuring the height of the `x` glyph.
+        // If that fails then we just pretend it's 65% of the ascent height.
+        const ex_height: f32 = ex_height: {
+            if (face.getCharIndex('x')) |glyph_index| {
+                if (face.loadGlyph(glyph_index, .{ .render = true })) {
+                    break :ex_height f26dot6ToFloat(face.handle.*.glyph.*.metrics.height);
+                } else |_| {
+                    // Ignore the error since we just fall back to 65% of the ascent below
+                }
+            }
+
+            break :ex_height f26dot6ToFloat(size_metrics.ascender) * 0.65;
+        };
+
         // Cell height is calculated as the maximum of multiple things in order
         // to handle edge cases in fonts: (1) the height as reported in metadata
         // by the font designer (2) the maximum glyph height as measured in the
@@ -689,7 +703,9 @@ pub const Face = struct {
             },
             .thickness = @max(@as(f32, 1), fontUnitsToPxY(face, os2.yStrikeoutSize)),
         } else .{
-            .pos = cell_baseline * 0.6,
+            // Exactly 50% of the ex height so that our strikethrough is
+            // centered through lowercase text. This is a common choice.
+            .pos = cell_baseline - ex_height * 0.5 + 1,
             .thickness = underline_thickness,
         };
 

From 49a3008919378f2dde829941a5fb0e33defbf39c Mon Sep 17 00:00:00 2001
From: Qwerasd <qwerasd205@users.noreply.github.com>
Date: Mon, 23 Sep 2024 19:41:49 -0600
Subject: [PATCH 07/13] font/sprite: reduce uneven gaps in dotted underline

---
 src/font/sprite/underline.zig | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/font/sprite/underline.zig b/src/font/sprite/underline.zig
index af0dd739a..f38bc50e6 100644
--- a/src/font/sprite/underline.zig
+++ b/src/font/sprite/underline.zig
@@ -102,14 +102,15 @@ fn drawDotted(alloc: Allocator, width: u32, thickness: u32) !struct { font.sprit
     var canvas = try font.sprite.Canvas.init(alloc, width, height);
 
     const dot_width = @max(thickness, 3);
-    const dot_count = width / dot_width;
+    const dot_count = @max((width / dot_width) / 2, 1);
+    const gap_width = try std.math.divCeil(u32, width -| (dot_count * dot_width), dot_count);
     var i: u32 = 0;
-    while (i < dot_count) : (i += 2) {
+    while (i < dot_count) : (i += 1) {
         // Ensure we never go out of bounds for the rect
-        const x = @min(i * dot_width, width - 1);
+        const x = @min(i * (dot_width + gap_width), width - 1);
         const rect_width = @min(width - x, dot_width);
         canvas.rect(.{
-            .x = @intCast(i * dot_width),
+            .x = @intCast(x),
             .y = 0,
             .width = rect_width,
             .height = thickness,

From 7a1d304fa91ddd6961c0973be3a6ea2d99fb6538 Mon Sep 17 00:00:00 2001
From: Qwerasd <qwerasd205@users.noreply.github.com>
Date: Mon, 23 Sep 2024 22:10:43 -0600
Subject: [PATCH 08/13] font: further improve ul/st position calculations

---
 src/font/face/coretext.zig | 14 +++++++-----
 src/font/face/freetype.zig | 44 ++++++++++++++++++--------------------
 2 files changed, 30 insertions(+), 28 deletions(-)

diff --git a/src/font/face/coretext.zig b/src/font/face/coretext.zig
index 3a69ef95b..ee2460572 100644
--- a/src/font/face/coretext.zig
+++ b/src/font/face/coretext.zig
@@ -594,7 +594,10 @@ pub const Face = struct {
         // All of these metrics are based on our layout above.
         const cell_height = @ceil(layout_metrics.height);
         const cell_baseline = @ceil(layout_metrics.height - layout_metrics.ascent);
+
         const underline_thickness = @ceil(@as(f32, @floatCast(ct_font.getUnderlineThickness())));
+        const strikethrough_thickness = underline_thickness;
+
         const strikethrough_position = strikethrough_position: {
             // This is the height of lower case letters in our font.
             const ex_height = ct_font.getXHeight();
@@ -608,20 +611,21 @@ pub const Face = struct {
             // is the distance from the top down to the baseline, then
             // we subtract half of the ex height to go back up to the
             // correct height that should evenly split lowercase text.
-            const pos = layout_metrics.ascent - ex_height * 0.5 + 1;
+            const pos = layout_metrics.ascent -
+                ex_height * 0.5 +
+                strikethrough_thickness * 0.5 +
+                1;
 
             break :strikethrough_position @ceil(pos);
         };
-        const strikethrough_thickness = underline_thickness;
 
         // Underline position reported is usually something like "-1" to
         // represent the amount under the baseline. We add this to our real
         // baseline to get the actual value from the bottom (+y is up).
         // The final underline position is +y from the TOP (confusing)
         // so we have to subtract from the cell height.
-        const underline_position = cell_height -
-            (cell_baseline + @ceil(@as(f32, @floatCast(ct_font.getUnderlinePosition())))) +
-            1;
+        const underline_position = @ceil(layout_metrics.ascent -
+            @as(f32, @floatCast(ct_font.getUnderlinePosition())) + 1);
 
         // Note: is this useful?
         // const units_per_em = ct_font.getUnitsPerEm();
diff --git a/src/font/face/freetype.zig b/src/font/face/freetype.zig
index 04f037c85..32664a1fd 100644
--- a/src/font/face/freetype.zig
+++ b/src/font/face/freetype.zig
@@ -660,52 +660,50 @@ pub const Face = struct {
         // is reversed.
         const cell_baseline = -1 * f26dot6ToFloat(size_metrics.descender);
 
+        const underline_thickness = @max(@as(f32, 1), fontUnitsToPxY(
+            face,
+            face.handle.*.underline_thickness,
+        ));
+
         // The underline position. This is a value from the top where the
         // underline should go.
         const underline_position: f32 = underline_pos: {
-            // The ascender is already scaled for scalable fonts, but the
-            // underline position is not.
-            const ascender_px = @as(i32, @intCast(size_metrics.ascender)) >> 6;
-            const declared_px = freetype.mulFix(
+            const declared_px = @as(f32, @floatFromInt(freetype.mulFix(
                 face.handle.*.underline_position,
                 @intCast(face.handle.*.size.*.metrics.y_scale),
-            ) >> 6;
+            ))) / 64;
 
             // We use the declared underline position if its available
-            const declared = ascender_px - declared_px;
+            const declared = cell_height - cell_baseline - declared_px;
             if (declared > 0)
-                break :underline_pos @floatFromInt(declared);
+                break :underline_pos declared;
 
             // If we have no declared underline position, we go slightly under the
             // cell height (mainly: non-scalable fonts, i.e. emoji)
             break :underline_pos cell_height - 1;
         };
-        const underline_thickness = @max(@as(f32, 1), fontUnitsToPxY(
-            face,
-            face.handle.*.underline_thickness,
-        ));
 
         // The strikethrough position. We use the position provided by the
         // font if it exists otherwise we calculate a best guess.
         const strikethrough: struct {
             pos: f32,
             thickness: f32,
-        } = if (face.getSfntTable(.os2)) |os2| .{
-            .pos = pos: {
-                // Ascender is scaled, strikeout pos is not
-                const ascender_px = @as(i32, @intCast(size_metrics.ascender)) >> 6;
-                const declared_px = freetype.mulFix(
-                    os2.yStrikeoutPosition,
-                    @as(i32, @intCast(face.handle.*.size.*.metrics.y_scale)),
-                ) >> 6;
+        } = if (face.getSfntTable(.os2)) |os2| st: {
+            const thickness = @max(@as(f32, 1), fontUnitsToPxY(face, os2.yStrikeoutSize));
 
-                break :pos @floatFromInt(ascender_px - declared_px);
-            },
-            .thickness = @max(@as(f32, 1), fontUnitsToPxY(face, os2.yStrikeoutSize)),
+            const pos = @as(f32, @floatFromInt(freetype.mulFix(
+                os2.yStrikeoutPosition,
+                @as(i32, @intCast(face.handle.*.size.*.metrics.y_scale)),
+            ))) / 64;
+
+            break :st .{
+                .pos = @ceil(cell_height - cell_baseline - pos + thickness + 1),
+                .thickness = thickness,
+            };
         } else .{
             // Exactly 50% of the ex height so that our strikethrough is
             // centered through lowercase text. This is a common choice.
-            .pos = cell_baseline - ex_height * 0.5 + 1,
+            .pos = @ceil(cell_height - cell_baseline - ex_height * 0.5 + underline_thickness),
             .thickness = underline_thickness,
         };
 

From ac68686036012ad851367946baa884b8fd87be48 Mon Sep 17 00:00:00 2001
From: Qwerasd <qwerasd205@users.noreply.github.com>
Date: Mon, 23 Sep 2024 22:30:59 -0600
Subject: [PATCH 09/13] freetype: fix underline position calculation

---
 src/font/face/freetype.zig | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/font/face/freetype.zig b/src/font/face/freetype.zig
index 32664a1fd..f764ac61d 100644
--- a/src/font/face/freetype.zig
+++ b/src/font/face/freetype.zig
@@ -668,13 +668,18 @@ pub const Face = struct {
         // The underline position. This is a value from the top where the
         // underline should go.
         const underline_position: f32 = underline_pos: {
+            // From the FreeType docs:
+            // > `underline_position`
+            // > The position, in font units, of the underline line for
+            // > this face. It is the center of the underlining stem.
+
             const declared_px = @as(f32, @floatFromInt(freetype.mulFix(
                 face.handle.*.underline_position,
                 @intCast(face.handle.*.size.*.metrics.y_scale),
             ))) / 64;
 
             // We use the declared underline position if its available
-            const declared = cell_height - cell_baseline - declared_px;
+            const declared = @ceil(cell_height - cell_baseline - declared_px - underline_thickness * 0.5 + 1);
             if (declared > 0)
                 break :underline_pos declared;
 

From 74d24a53ab5f2d09417250c5cbb703534fb3a36d Mon Sep 17 00:00:00 2001
From: Qwerasd <qwerasd205@users.noreply.github.com>
Date: Wed, 25 Sep 2024 11:41:12 -0600
Subject: [PATCH 10/13] correct diagram/description (+Y is down not up)

---
 src/renderer/shaders/cell.metal | 44 ++++++++++++++++-----------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/src/renderer/shaders/cell.metal b/src/renderer/shaders/cell.metal
index 734608c76..b6af33824 100644
--- a/src/renderer/shaders/cell.metal
+++ b/src/renderer/shaders/cell.metal
@@ -224,30 +224,30 @@ vertex CellTextVertexOut cell_text_vertex(
   out.color = float4(in.color) / 255.0f;
 
   //              === Grid Cell ===
-  //
-  //    offset.x = bearings.x
-  //                  .|.
-  //                  | |
-  //                  +-------+_.
-  //                ._|       | |
-  //                | | .###. | |
-  //                | | #...# | +- bearings.y
-  //  glyph_size.y -+ | ##### | |
-  //                | | #.... | |
-  //   ^            |_| .#### |_| _.
-  //   |              |       |    +- offset.y = cell_size.y - bearings.y
-  //   .  cell_pos -> +-------+   -'
-  // +Y.                |_._|
-  //   .                  |
-  //   |             glyph_size.x
-  // 0,0--...->
   //      +X
+  // 0,0--...->
+  //   |
+  //   . offset.x = bearings.x
+  // +Y.               .|.
+  //   .               | |
+  //   |   cell_pos -> +-------+   _.
+  //   v             ._|       |_. _|- offset.y = cell_size.y - bearings.y
+  //                 | | .###. | |
+  //                 | | #...# | |
+  //   glyph_size.y -+ | ##### | |
+  //                 | | #.... | +- bearings.y
+  //                 |_| .#### | |
+  //                   |       |_|
+  //                   +-------+
+  //                     |_._|
+  //                       |
+  //                  glyph_size.x
   //
-  // In order to get the bottom left of the glyph, we compute an offset based
-  // on the bearings. The Y bearing is the distance from the top of the cell
-  // to the bottom of the glyph, so we subtract it from the cell height to get
-  // the y offset. The X bearing is the distance from the left of the cell to
-  // the left of the glyph, so it works as the x offset directly.
+  // In order to get the top left of the glyph, we compute an offset based on
+  // the bearings. The Y bearing is the distance from the bottom of the cell
+  // to the top of the glyph, so we subtract it from the cell height to get
+  // the y offset. The X bearing is the distance from the left of the cell
+  // to the left of the glyph, so it works as the x offset directly.
 
   float2 size = float2(in.glyph_size);
   float2 offset = float2(in.bearings);

From 9a87001fa649eb5a8ee2ff9afc31a943963d912c Mon Sep 17 00:00:00 2001
From: Qwerasd <qwerasd205@users.noreply.github.com>
Date: Wed, 25 Sep 2024 11:46:43 -0600
Subject: [PATCH 11/13] font/sprite: correct underline placement calculation

---
 src/font/sprite/underline.zig | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/src/font/sprite/underline.zig b/src/font/sprite/underline.zig
index f38bc50e6..08e74c781 100644
--- a/src/font/sprite/underline.zig
+++ b/src/font/sprite/underline.zig
@@ -27,9 +27,7 @@ pub fn renderGlyph(
     line_pos: u32,
     line_thickness: u32,
 ) !font.Glyph {
-    // _ = height;
-
-    // Draw the appropriate sprite
+   // Draw the appropriate sprite
     var canvas: font.sprite.Canvas, const offset_y: i32 = switch (sprite) {
         .underline => try drawSingle(alloc, width, line_thickness),
         .underline_double => try drawDouble(alloc, width, line_thickness),
@@ -48,15 +46,24 @@ pub fn renderGlyph(
         .width = width,
         .height = @intCast(region.height),
         .offset_x = 0,
-        .offset_y = @as(i32, @intCast(height + region.height)) - @as(i32, @intCast(line_pos)) + offset_y + 1,
+        // Glyph.offset_y is the distance between the top of the glyph and the
+        // bottom of the cell. We want the top of the glyph to be at line_pos
+        // from the TOP of the cell, and then offset by the offset_y from the
+        // draw function.
+        .offset_y = @as(i32, @intCast(height - line_pos)) - offset_y,
         .atlas_x = region.x,
         .atlas_y = region.y,
         .advance_x = @floatFromInt(width),
     };
 }
 
+/// A tuple with the canvas that the desired sprite was drawn on and
+/// a recommended offset (+Y = down) to shift its Y position by, to
+/// correct for underline styles with additional thickness.
+const CanvasAndOffset = struct { font.sprite.Canvas, i32 };
+
 /// Draw a single underline.
-fn drawSingle(alloc: Allocator, width: u32, thickness: u32) !struct { font.sprite.Canvas, i32 } {
+fn drawSingle(alloc: Allocator, width: u32, thickness: u32) !CanvasAndOffset {
     const height: u32 = thickness;
     var canvas = try font.sprite.Canvas.init(alloc, width, height);
 
@@ -73,7 +80,7 @@ fn drawSingle(alloc: Allocator, width: u32, thickness: u32) !struct { font.sprit
 }
 
 /// Draw a double underline.
-fn drawDouble(alloc: Allocator, width: u32, thickness: u32) !struct { font.sprite.Canvas, i32 } {
+fn drawDouble(alloc: Allocator, width: u32, thickness: u32) !CanvasAndOffset {
     const height: u32 = thickness * 3;
     var canvas = try font.sprite.Canvas.init(alloc, width, height);
 
@@ -97,7 +104,7 @@ fn drawDouble(alloc: Allocator, width: u32, thickness: u32) !struct { font.sprit
 }
 
 /// Draw a dotted underline.
-fn drawDotted(alloc: Allocator, width: u32, thickness: u32) !struct { font.sprite.Canvas, i32 } {
+fn drawDotted(alloc: Allocator, width: u32, thickness: u32) !CanvasAndOffset {
     const height: u32 = thickness;
     var canvas = try font.sprite.Canvas.init(alloc, width, height);
 
@@ -123,7 +130,7 @@ fn drawDotted(alloc: Allocator, width: u32, thickness: u32) !struct { font.sprit
 }
 
 /// Draw a dashed underline.
-fn drawDashed(alloc: Allocator, width: u32, thickness: u32) !struct { font.sprite.Canvas, i32 } {
+fn drawDashed(alloc: Allocator, width: u32, thickness: u32) !CanvasAndOffset {
     const height: u32 = thickness;
     var canvas = try font.sprite.Canvas.init(alloc, width, height);
 
@@ -150,7 +157,7 @@ fn drawDashed(alloc: Allocator, width: u32, thickness: u32) !struct { font.sprit
 /// Draw a curly underline. Thanks to Wez Furlong for providing
 /// the basic math structure for this since I was lazy with the
 /// geometry.
-fn drawCurly(alloc: Allocator, width: u32, thickness: u32) !struct { font.sprite.Canvas, i32 } {
+fn drawCurly(alloc: Allocator, width: u32, thickness: u32) !CanvasAndOffset {
     const height: u32 = thickness * 4;
     var canvas = try font.sprite.Canvas.init(alloc, width, height);
 

From 003b100707a7125f1e84ce44b42e0b1bd8b3484d Mon Sep 17 00:00:00 2001
From: Qwerasd <qwerasd205@users.noreply.github.com>
Date: Wed, 25 Sep 2024 12:01:24 -0600
Subject: [PATCH 12/13] font: remove fudge factors in ul and st position
 calculations

These were present because of an incorrect calculation in the underline
sprite renderer, and are no longer necessary.
---
 src/font/face/coretext.zig | 7 +++----
 src/font/face/freetype.zig | 8 ++++----
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/src/font/face/coretext.zig b/src/font/face/coretext.zig
index ee2460572..dacb79476 100644
--- a/src/font/face/coretext.zig
+++ b/src/font/face/coretext.zig
@@ -612,9 +612,8 @@ pub const Face = struct {
             // we subtract half of the ex height to go back up to the
             // correct height that should evenly split lowercase text.
             const pos = layout_metrics.ascent -
-                ex_height * 0.5 +
-                strikethrough_thickness * 0.5 +
-                1;
+                ex_height * 0.5 -
+                strikethrough_thickness * 0.5;
 
             break :strikethrough_position @ceil(pos);
         };
@@ -625,7 +624,7 @@ pub const Face = struct {
         // The final underline position is +y from the TOP (confusing)
         // so we have to subtract from the cell height.
         const underline_position = @ceil(layout_metrics.ascent -
-            @as(f32, @floatCast(ct_font.getUnderlinePosition())) + 1);
+            @as(f32, @floatCast(ct_font.getUnderlinePosition())));
 
         // Note: is this useful?
         // const units_per_em = ct_font.getUnitsPerEm();
diff --git a/src/font/face/freetype.zig b/src/font/face/freetype.zig
index f764ac61d..dae46d6d6 100644
--- a/src/font/face/freetype.zig
+++ b/src/font/face/freetype.zig
@@ -678,8 +678,8 @@ pub const Face = struct {
                 @intCast(face.handle.*.size.*.metrics.y_scale),
             ))) / 64;
 
-            // We use the declared underline position if its available
-            const declared = @ceil(cell_height - cell_baseline - declared_px - underline_thickness * 0.5 + 1);
+            // We use the declared underline position if its available.
+            const declared = @ceil(cell_height - cell_baseline - declared_px - underline_thickness * 0.5);
             if (declared > 0)
                 break :underline_pos declared;
 
@@ -702,13 +702,13 @@ pub const Face = struct {
             ))) / 64;
 
             break :st .{
-                .pos = @ceil(cell_height - cell_baseline - pos + thickness + 1),
+                .pos = @ceil(cell_height - cell_baseline - pos),
                 .thickness = thickness,
             };
         } else .{
             // Exactly 50% of the ex height so that our strikethrough is
             // centered through lowercase text. This is a common choice.
-            .pos = @ceil(cell_height - cell_baseline - ex_height * 0.5 + underline_thickness),
+            .pos = @ceil(cell_height - cell_baseline - ex_height * 0.5 - underline_thickness * 0.5),
             .thickness = underline_thickness,
         };
 

From a0f017d6fdad3cf5ec4952498d7f09b12b997132 Mon Sep 17 00:00:00 2001
From: Qwerasd <qwerasd205@users.noreply.github.com>
Date: Wed, 25 Sep 2024 12:11:04 -0600
Subject: [PATCH 13/13] freetype: update expected ul pos in tests to account
 for removed fudge factor

---
 src/font/face/freetype.zig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/font/face/freetype.zig b/src/font/face/freetype.zig
index dae46d6d6..3ff9e9ffa 100644
--- a/src/font/face/freetype.zig
+++ b/src/font/face/freetype.zig
@@ -851,7 +851,7 @@ test "metrics" {
         .cell_width = 16,
         .cell_height = 35,
         .cell_baseline = 7,
-        .underline_position = 36,
+        .underline_position = 35,
         .underline_thickness = 2,
         .strikethrough_position = 20,
         .strikethrough_thickness = 2,