From d250bcda65b6fbdec4a54f543e2c1abf28ed96c6 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Tue, 19 Apr 2022 19:30:30 -0700
Subject: [PATCH] fix the baseline for font rendering

---
 shaders/cell.v.glsl    | 10 ++++++----
 src/FontAtlas.zig      |  2 +-
 src/Grid.zig           | 21 +++++++++++++++------
 src/opengl/Program.zig |  1 +
 4 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/shaders/cell.v.glsl b/shaders/cell.v.glsl
index 8dd30dfe7..59eb02f74 100644
--- a/shaders/cell.v.glsl
+++ b/shaders/cell.v.glsl
@@ -44,6 +44,7 @@ flat out uint mode;
 uniform sampler2D text;
 uniform vec2 cell_size;
 uniform mat4 projection;
+uniform float glyph_baseline;
 
 /********************************************************************
  * Modes
@@ -99,11 +100,12 @@ void main() {
         break;
 
     case MODE_FG:
-        // The glyph offset is upside down so we need to reverse it to
-        // be based on the offset of our cell. This is equivalent to
-        // "1 - value" to flip the value.
+        // The glyph_offset.y is the y bearing, a y value that when added
+        // to the baseline is the offset (+y is up). Our grid goes down.
+        // So we flip it with `cell_size.y - glyph_offset.y`. The glyph_baseline
+        // uniform sets our line baseline where characters "sit".
         vec2 glyph_offset_calc = glyph_offset;
-        glyph_offset_calc.y = cell_size.y - glyph_offset.y;
+        glyph_offset_calc.y = cell_size.y - glyph_offset.y - glyph_baseline;
 
         // Calculate the final position of the cell.
         cell_pos = cell_pos + glyph_size * position + glyph_offset_calc;
diff --git a/src/FontAtlas.zig b/src/FontAtlas.zig
index 87680bafc..e6b403578 100644
--- a/src/FontAtlas.zig
+++ b/src/FontAtlas.zig
@@ -42,7 +42,7 @@ pub const Glyph = struct {
     offset_y: i32,
 
     /// coordinates in the atlas of the top-left corner. These have to
-    /// be normalized to be between 0 and 1 prior to use.
+    /// be normalized to be between 0 and 1 prior to use in shaders.
     atlas_x: u32,
     atlas_y: u32,
 
diff --git a/src/Grid.zig b/src/Grid.zig
index f45e8ebcf..7e84d6a34 100644
--- a/src/Grid.zig
+++ b/src/Grid.zig
@@ -95,14 +95,19 @@ pub fn init(alloc: Allocator) !Grid {
     // The cell height is the vertical height required to render underscore
     // '_' which should live at the bottom of a cell.
     const cell_height: f32 = cell_height: {
-        // TODO(render): kitty does a calculation based on other font
-        // metrics that we probably want to research more. For now, this is
-        // fine.
+        // This is the height reported by the font face
+        const face_height: i32 = font.ft_face.*.height >> 6;
+
+        // Determine the height of the underscore char
         assert(font.ft_face != null);
         const glyph = font.getGlyph('_').?;
         var res: i32 = font.ft_face.*.ascender >> 6;
         res -= glyph.offset_y;
         res += @intCast(i32, glyph.height);
+
+        // We take whatever is larger to account for some fonts that
+        // put the underscore outside f the rectangle.
+        if (res < face_height) res = face_height;
         break :cell_height @intToFloat(f32, res);
     };
     log.debug("cell dimensions w={d} h={d}", .{ cell_width, cell_height });
@@ -117,6 +122,10 @@ pub fn init(alloc: Allocator) !Grid {
     const pbind = try program.use();
     defer pbind.unbind();
     try program.setUniform("cell_size", @Vector(2, f32){ cell_width, cell_height });
+    try program.setUniform(
+        "glyph_baseline",
+        cell_height - @intToFloat(f32, font.ft_face.*.ascender >> 6),
+    );
 
     // Setup our VAO
     const vao = try gl.VertexArray.create();
@@ -258,6 +267,7 @@ pub fn updateCells(self: *Grid, term: Terminal) !void {
 
             // TODO: for background colors, add another cell with mode = 1
             self.cells.appendAssumeCapacity(.{
+                .mode = 2,
                 .grid_col = @intCast(u16, x),
                 .grid_row = @intCast(u16, y),
                 .glyph_x = glyph.atlas_x,
@@ -274,13 +284,13 @@ pub fn updateCells(self: *Grid, term: Terminal) !void {
                 .bg_g = 0xA5,
                 .bg_b = 0,
                 .bg_a = 0,
-                .mode = 2,
             });
         }
     }
 
     // Draw the cursor
     self.cells.appendAssumeCapacity(.{
+        .mode = 1,
         .grid_col = @intCast(u16, term.cursor.x),
         .grid_row = @intCast(u16, term.cursor.y),
         .fg_r = 0,
@@ -291,7 +301,6 @@ pub fn updateCells(self: *Grid, term: Terminal) !void {
         .bg_g = 0xFF,
         .bg_b = 0xFF,
         .bg_a = 255,
-        .mode = 1,
     });
 }
 
@@ -316,7 +325,7 @@ pub fn setScreenSize(self: *Grid, dim: ScreenSize) !void {
     // Recalculate the rows/columns.
     self.size.update(dim, self.cell_size);
 
-    log.debug("screen size screen={} grid={}", .{ dim, self.size });
+    log.debug("screen size screen={} grid={}, cell={}", .{ dim, self.size, self.cell_size });
 }
 
 pub fn render(self: Grid) !void {
diff --git a/src/opengl/Program.zig b/src/opengl/Program.zig
index a68f0cd9b..095896f53 100644
--- a/src/opengl/Program.zig
+++ b/src/opengl/Program.zig
@@ -83,6 +83,7 @@ pub inline fn setUniform(p: Program, n: [:0]const u8, value: anytype) !void {
     // Perform the correct call depending on the type of the value.
     switch (@TypeOf(value)) {
         comptime_int => c.glUniform1i(loc, value),
+        f32 => c.glUniform1f(loc, value),
         @Vector(2, f32) => c.glUniform2f(loc, value[0], value[1]),
         @Vector(3, f32) => c.glUniform3f(loc, value[0], value[1], value[2]),
         @Vector(4, f32) => c.glUniform4f(loc, value[0], value[1], value[2], value[3]),