Change font metrics to all be integers, not floats.

Font metrics realistically should be integral. Cell widths, cell
heights, etc. do not make sense to be floats, since our grid is
integral. There is no such thing as a "half cell" (or any point).

The reason we historically had these all as f32 is simplicity mixed
with history. OpenGL APIs and shaders all use f32 for their values, we
originally only supported OpenGL, and all the font rendering used to be
directly in the renderer code (like... a year+ ago).

When we refactored the font metrics calculation to its own system and
also added additional renderers like Metal (which use f64, not f32), we
never updated anything. We just kept metrics as f32 and casted
everywhere.

With CoreText and #177 this finally reared its ugly head. By forgetting
a simple rounding on cell metric calculation, our integral renderers
(sprite fonts) were off by 1 pixel compared to the GPU renderers.
Insidious.

Let's represent font metrics with the types that actually make sense: a
cell width/height, etc. is _integral_. When we get to the GPU, we now
cast to floats. We also cast to floats whenever we're doing more precise
math (i.e. mouse offset calculation). In this case, we're only
converting to floats from a integral type which is going to be much
safer and less prone to uncertain rounding than converting to an int
from a float type.

Fixes #177

src/Surface.zig

@@ -429,8 +429,8 @@ pub fn init(
     // Set a minimum size that is cols=10 h=4. This matches Mac's Terminal.app
     // but is otherwise somewhat arbitrary.
     try rt_surface.setSizeLimits(.{
-        .width = @intFromFloat(cell_size.width * 10),
+        .width = cell_size.width * 10,
-        .height = @intFromFloat(cell_size.height * 4),
+        .height = cell_size.height * 4,
     }, null);
 
     // Call our size callback which handles all our retina setup
@@ -662,10 +662,10 @@ pub fn imePoint(self: *const Surface) apprt.IMEPos {
 
     const x: f64 = x: {
         // Simple x * cell width gives the top-left corner
-        var x: f64 = @floatCast(@as(f32, @floatFromInt(cursor.x)) * self.cell_size.width);
+        var x: f64 = @floatFromInt(cursor.x * self.cell_size.width);
 
         // We want the midpoint
-        x += self.cell_size.width / 2;
+        x += @as(f64, @floatFromInt(self.cell_size.width)) / 2;
 
         // And scale it
         x /= content_scale.x;
@@ -675,10 +675,10 @@ pub fn imePoint(self: *const Surface) apprt.IMEPos {
 
     const y: f64 = y: {
         // Simple x * cell width gives the top-left corner
-        var y: f64 = @floatCast(@as(f32, @floatFromInt(cursor.y)) * self.cell_size.height);
+        var y: f64 = @floatFromInt(cursor.y * self.cell_size.height);
 
         // We want the bottom
-        y += self.cell_size.height;
+        y += @floatFromInt(self.cell_size.height);
 
         // And scale it
         y /= content_scale.y;
@@ -1332,7 +1332,7 @@ pub fn scrollCallback(
 
         // If the new offset is less than a single unit of scroll, we save
         // the new pending value and do not scroll yet.
-        const cell_size = self.cell_size.height;
+        const cell_size: f64 = @floatFromInt(self.cell_size.height);
         if (@fabs(poff) < cell_size) {
             self.mouse.pending_scroll_y = poff;
             break :y .{};
@@ -1359,7 +1359,7 @@ pub fn scrollCallback(
         }
 
         const poff = self.mouse.pending_scroll_x + (xoff * -1);
-        const cell_size = self.cell_size.width;
+        const cell_size: f64 = @floatFromInt(self.cell_size.width);
         if (@fabs(poff) < cell_size) {
             self.mouse.pending_scroll_x = poff;
             break :x .{};
@@ -1724,7 +1724,7 @@ pub fn mouseButtonCallback(
         // If we move our cursor too much between clicks then we reset
         // the multi-click state.
         if (self.mouse.left_click_count > 0) {
-            const max_distance = self.cell_size.width;
+            const max_distance: f64 = @floatFromInt(self.cell_size.width);
             const distance = @sqrt(
                 std.math.pow(f64, pos.x - self.mouse.left_click_xpos, 2) +
                     std.math.pow(f64, pos.y - self.mouse.left_click_ypos, 2),
@@ -1951,10 +1951,13 @@ fn dragLeftClickSingle(
     //
 
     // the boundary point at which we consider selection or non-selection
-    const cell_xboundary = self.cell_size.width * 0.6;
+    const cell_xboundary = @as(f32, @floatFromInt(self.cell_size.width)) * 0.6;
 
     // first xpos of the clicked cell
-    const cell_xstart = @as(f32, @floatFromInt(self.mouse.left_click_point.x)) * self.cell_size.width;
+    const cell_xstart = @as(
+        f32,
+        @floatFromInt(self.mouse.left_click_point.x),
+    ) * @as(f32, @floatFromInt(self.cell_size.width));
     const cell_start_xpos = self.mouse.left_click_xpos - cell_xstart;
 
     // If this is the same cell, then we only start the selection if weve
@@ -2038,7 +2041,7 @@ fn posToViewport(self: Surface, xpos: f64, ypos: f64) terminal.point.Viewport {
     return .{
         .x = if (xpos_adjusted < 0) 0 else x: {
             // Our cell is the mouse divided by cell width
-            const cell_width: f64 = @floatCast(self.cell_size.width);
+            const cell_width: f64 = @floatFromInt(self.cell_size.width);
             const x: usize = @intFromFloat(xpos_adjusted / cell_width);
 
             // Can be off the screen if the user drags it out, so max
@@ -2047,7 +2050,7 @@ fn posToViewport(self: Surface, xpos: f64, ypos: f64) terminal.point.Viewport {
         },
 
         .y = if (ypos_adjusted < 0) 0 else y: {
-            const cell_height: f64 = @floatCast(self.cell_size.height);
+            const cell_height: f64 = @floatFromInt(self.cell_size.height);
             const y: usize = @intFromFloat(ypos_adjusted / cell_height);
             break :y @min(y, self.grid_size.rows - 1);
         },

src/font/Group.zig

@@ -329,10 +329,10 @@ pub const Wasm = struct {
 
         // Set details for our sprite font
         self.sprite = font.sprite.Face{
-            .width = @intFromFloat(metrics.cell_width),
+            .width = metrics.cell_width,
-            .height = @intFromFloat(metrics.cell_height),
+            .height = metrics.cell_height,
             .thickness = 2,
-            .underline_position = @intFromFloat(metrics.underline_position),
+            .underline_position = metrics.underline_position,
         };
     }
 

src/font/face.zig

@@ -39,23 +39,23 @@ pub const DesiredSize = struct {
 /// Metrics associated with the font that are useful for renderers to know.
 pub const Metrics = struct {
     /// Recommended cell width and height for a monospace grid using this font.
-    cell_width: f32,
+    cell_width: u32,
-    cell_height: f32,
+    cell_height: u32,
 
     /// For monospace grids, the recommended y-value from the bottom to set
     /// the baseline for font rendering. This is chosen so that things such
     /// as the bottom of a "g" or "y" do not drop below the cell.
-    cell_baseline: f32,
+    cell_baseline: u32,
 
     /// The position of the underline from the top of the cell and the
     /// thickness in pixels.
-    underline_position: f32,
+    underline_position: u32,
-    underline_thickness: f32,
+    underline_thickness: u32,
 
     /// The position and thickness of a strikethrough. Same units/style
     /// as the underline fields.
-    strikethrough_position: f32,
+    strikethrough_position: u32,
-    strikethrough_thickness: f32,
+    strikethrough_thickness: u32,
 };
 
 /// Additional options for rendering glyphs.

src/font/face/coretext.zig

@@ -261,7 +261,7 @@ pub const Face = struct {
         const offset_y: i32 = offset_y: {
             // Our Y coordinate in 3D is (0, 0) bottom left, +y is UP.
             // We need to calculate our baseline from the bottom of a cell.
-            const baseline_from_bottom = self.metrics.cell_height - self.metrics.cell_baseline;
+            const baseline_from_bottom: f64 = @floatFromInt(self.metrics.cell_height - self.metrics.cell_baseline);
 
             // Next we offset our baseline by the bearing in the font. We
             // ADD here because CoreText y is UP.
@@ -328,7 +328,7 @@ pub const Face = struct {
                 max = @max(advances[i].width, max);
             }
 
-            break :cell_width @floatCast(max);
+            break :cell_width @floatCast(@ceil(max));
         };
 
         // Calculate the cell height by using CoreText's layout engine
@@ -424,8 +424,8 @@ pub const Face = struct {
         };
 
         // All of these metrics are based on our layout above.
-        const cell_height = layout_metrics.height;
+        const cell_height = @ceil(layout_metrics.height);
-        const cell_baseline = layout_metrics.ascent;
+        const cell_baseline = @ceil(layout_metrics.ascent);
         const underline_thickness = @ceil(@as(f32, @floatCast(ct_font.getUnderlineThickness())));
         const strikethrough_position = cell_baseline * 0.6;
         const strikethrough_thickness = underline_thickness;
@@ -441,24 +441,20 @@ pub const Face = struct {
         // const units_per_em = ct_font.getUnitsPerEm();
         // const units_per_point = @intToFloat(f64, units_per_em) / ct_font.getSize();
 
-        // std.log.warn("font size size={d}", .{ct_font.getSize()});
+        const result = font.face.Metrics{
-        // std.log.warn("font metrics width={d}, height={d} baseline={d} underline_pos={d} underline_thickness={d}", .{
+            .cell_width = @intFromFloat(cell_width),
-        //     cell_width,
+            .cell_height = @intFromFloat(cell_height),
-        //     cell_height,
+            .cell_baseline = @intFromFloat(cell_baseline),
-        //     cell_baseline,
+            .underline_position = @intFromFloat(underline_position),
-        //     underline_position,
+            .underline_thickness = @intFromFloat(underline_thickness),
-        //     underline_thickness,
+            .strikethrough_position = @intFromFloat(strikethrough_position),
-        // });
+            .strikethrough_thickness = @intFromFloat(strikethrough_thickness),
-
-        return font.face.Metrics{
-            .cell_width = cell_width,
-            .cell_height = cell_height,
-            .cell_baseline = cell_baseline,
-            .underline_position = underline_position,
-            .underline_thickness = underline_thickness,
-            .strikethrough_position = strikethrough_position,
-            .strikethrough_thickness = strikethrough_thickness,
         };
+
+        // std.log.warn("font size size={d}", .{ct_font.getSize()});
+        // std.log.warn("font metrics={}", .{result});
+
+        return result;
     }
 };
 

src/font/face/freetype.zig

@@ -325,7 +325,7 @@ pub const Face = struct {
             // baseline calculation. The baseline calculation is so that everything
             // is properly centered when we render it out into a monospace grid.
             // Note: we add here because our X/Y is actually reversed, adding goes UP.
-            break :offset_y glyph_metrics.bitmap_top + @as(c_int, @intFromFloat(self.metrics.cell_baseline));
+            break :offset_y glyph_metrics.bitmap_top + @as(c_int, @intCast(self.metrics.cell_baseline));
         };
 
         // log.warn("renderGlyph width={} height={} offset_x={} offset_y={} glyph_metrics={}", .{
@@ -475,25 +475,20 @@ pub const Face = struct {
             .thickness = underline_thickness,
         };
 
-        // log.warn("METRICS={} width={d} height={d} baseline={d} underline_pos={d} underline_thickness={d} strikethrough={}", .{
+        const result = font.face.Metrics{
-        //     size_metrics,
+            .cell_width = @intFromFloat(cell_width),
-        //     cell_width,
+            .cell_height = @intFromFloat(cell_height),
-        //     cell_height,
+            .cell_baseline = @intFromFloat(cell_baseline),
-        //     cell_height - cell_baseline,
+            .underline_position = @intFromFloat(underline_position),
-        //     underline_position,
+            .underline_thickness = @intFromFloat(underline_thickness),
-        //     underline_thickness,
+            .strikethrough_position = @intFromFloat(strikethrough.pos),
-        //     strikethrough,
+            .strikethrough_thickness = @intFromFloat(strikethrough.thickness),
-        // });
-
-        return .{
-            .cell_width = cell_width,
-            .cell_height = cell_height,
-            .cell_baseline = cell_baseline,
-            .underline_position = underline_position,
-            .underline_thickness = underline_thickness,
-            .strikethrough_position = strikethrough.pos,
-            .strikethrough_thickness = strikethrough.thickness,
         };
+
+        // std.log.warn("font size size={d}", .{ct_font.getSize()});
+        // std.log.warn("font metrics={}", .{result});
+
+        return result;
     }
 
     /// Convert freetype "font units" to pixels using the Y scale.

src/font/face/web_canvas.zig

@@ -273,16 +273,17 @@ pub const Face = struct {
         const underline_position = cell_height - 1;
         const underline_thickness: f32 = 1;
 
-        self.metrics = .{
+        const result = font.face.Metrics{
-            .cell_width = cell_width,
+            .cell_width = @intFromFloat(cell_width),
-            .cell_height = cell_height,
+            .cell_height = @intFromFloat(cell_height),
-            .cell_baseline = cell_baseline,
+            .cell_baseline = @intFromFloat(cell_baseline),
-            .underline_position = underline_position,
+            .underline_position = @intFromFloat(underline_position),
-            .underline_thickness = underline_thickness,
+            .underline_thickness = @intFromFloat(underline_thickness),
-            .strikethrough_position = underline_position,
+            .strikethrough_position = @intFromFloat(underline_position),
-            .strikethrough_thickness = underline_thickness,
+            .strikethrough_thickness = @intFromFloat(underline_thickness),
         };
 
+        self.metrics = result;
         log.debug("metrics font={s} value={}", .{ self.font_str, self.metrics });
     }
 

src/renderer/Metal.zig

@@ -216,10 +216,10 @@ pub fn init(alloc: Allocator, options: renderer.Options) !Metal {
 
     // Set the sprite font up
     options.font_group.group.sprite = font.sprite.Face{
-        .width = @intFromFloat(metrics.cell_width),
+        .width = metrics.cell_width,
-        .height = @intFromFloat(metrics.cell_height),
+        .height = metrics.cell_height,
         .thickness = 2,
-        .underline_position = @intFromFloat(metrics.underline_position),
+        .underline_position = metrics.underline_position,
     };
 
     // Create the font shaper. We initially create a shaper that can support
@@ -301,8 +301,8 @@ pub fn init(alloc: Allocator, options: renderer.Options) !Metal {
         .uniforms = .{
             .projection_matrix = undefined,
             .cell_size = undefined,
-            .strikethrough_position = metrics.strikethrough_position,
+            .strikethrough_position = @floatFromInt(metrics.strikethrough_position),
-            .strikethrough_thickness = metrics.strikethrough_thickness,
+            .strikethrough_thickness = @floatFromInt(metrics.strikethrough_thickness),
         },
 
         // Fonts
@@ -460,9 +460,12 @@ pub fn setFontSize(self: *Metal, size: font.face.DesiredSize) !void {
     // Update our uniforms
     self.uniforms = .{
         .projection_matrix = self.uniforms.projection_matrix,
-        .cell_size = .{ new_cell_size.width, new_cell_size.height },
+        .cell_size = .{
-        .strikethrough_position = metrics.strikethrough_position,
+            @floatFromInt(new_cell_size.width),
-        .strikethrough_thickness = metrics.strikethrough_thickness,
+            @floatFromInt(new_cell_size.height),
+        },
+        .strikethrough_position = @floatFromInt(metrics.strikethrough_position),
+        .strikethrough_thickness = @floatFromInt(metrics.strikethrough_thickness),
     };
 
     // Recalculate our cell size. If it is the same as before, then we do
@@ -480,10 +483,10 @@ pub fn setFontSize(self: *Metal, size: font.face.DesiredSize) !void {
 
     // Set the sprite font up
     self.font_group.group.sprite = font.sprite.Face{
-        .width = @intFromFloat(self.cell_size.width),
+        .width = self.cell_size.width,
-        .height = @intFromFloat(self.cell_size.height),
+        .height = self.cell_size.height,
         .thickness = 2,
-        .underline_position = @intFromFloat(metrics.underline_position),
+        .underline_position = metrics.underline_position,
     };
 
     // Notify the window that the cell size changed.
@@ -792,7 +795,10 @@ pub fn setScreenSize(self: *Metal, dim: renderer.ScreenSize) !void {
             @as(f32, @floatFromInt(padded_dim.height)) + padding.bottom,
             -1 * padding.top,
         ),
-        .cell_size = .{ self.cell_size.width, self.cell_size.height },
+        .cell_size = .{
+            @floatFromInt(self.cell_size.width),
+            @floatFromInt(self.cell_size.height),
+        },
         .strikethrough_position = old.strikethrough_position,
         .strikethrough_thickness = old.strikethrough_thickness,
     };
@@ -1008,7 +1014,7 @@ pub fn updateCell(
             shaper_run.font_index,
             shaper_cell.glyph_index,
             .{
-                .max_height = @intFromFloat(@ceil(self.cell_size.height)),
+                .max_height = @intCast(self.cell_size.height),
                 .thicken = self.config.font_thicken,
             },
         );

src/renderer/OpenGL.zig

@@ -161,10 +161,19 @@ const SetFontSize = struct {
     fn apply(self: SetFontSize, r: *const OpenGL) !void {
         try r.program.setUniform(
             "cell_size",
-            @Vector(2, f32){ self.metrics.cell_width, self.metrics.cell_height },
+            @Vector(2, f32){
+                @floatFromInt(self.metrics.cell_width),
+                @floatFromInt(self.metrics.cell_height),
+            },
+        );
+        try r.program.setUniform(
+            "strikethrough_position",
+            @as(f32, @floatFromInt(self.metrics.strikethrough_position)),
+        );
+        try r.program.setUniform(
+            "strikethrough_thickness",
+            @as(f32, @floatFromInt(self.metrics.strikethrough_thickness)),
         );
-        try r.program.setUniform("strikethrough_position", self.metrics.strikethrough_position);
-        try r.program.setUniform("strikethrough_thickness", self.metrics.strikethrough_thickness);
     }
 };
 
@@ -661,10 +670,10 @@ fn resetFontMetrics(
 
     // Set details for our sprite font
     font_group.group.sprite = font.sprite.Face{
-        .width = @intFromFloat(metrics.cell_width),
+        .width = metrics.cell_width,
-        .height = @intFromFloat(metrics.cell_height),
+        .height = metrics.cell_height,
         .thickness = 2,
-        .underline_position = @intFromFloat(metrics.underline_position),
+        .underline_position = metrics.underline_position,
     };
 
     return metrics;
@@ -1148,7 +1157,7 @@ pub fn updateCell(
             shaper_run.font_index,
             shaper_cell.glyph_index,
             .{
-                .max_height = @intFromFloat(@ceil(self.cell_size.height)),
+                .max_height = @intCast(self.cell_size.height),
                 .thicken = self.config.font_thicken,
             },
         );

src/renderer/opengl/Program.zig

@@ -101,7 +101,10 @@ pub inline fn setUniform(
             c.GL_FALSE,
             @ptrCast(&value),
         ),
-        else => unreachable,
+        else => {
+            log.warn("unsupported uniform type {}", .{@TypeOf(value)});
+            unreachable;
+        },
     }
     try errors.getError();
 }

src/renderer/size.zig

@@ -14,8 +14,8 @@ const log = std.log.scoped(.renderer_size);
 /// The units for the width and height are in world space. They have to
 /// be normalized for any renderer implementation.
 pub const CellSize = struct {
-    width: f32,
+    width: u32,
-    height: f32,
+    height: u32,
 
     /// Initialize the cell size information from a font group. This ensures
     /// that all renderers use the same cell sizing information for the same
@@ -71,8 +71,14 @@ pub const GridSize = struct {
     /// Update the columns/rows for the grid based on the given screen and
     /// cell size.
     pub fn update(self: *GridSize, screen: ScreenSize, cell: CellSize) void {
-        self.columns = @max(1, @as(Unit, @intFromFloat(@as(f32, @floatFromInt(screen.width)) / cell.width)));
+        const cell_width: f32 = @floatFromInt(cell.width);
-        self.rows = @max(1, @as(Unit, @intFromFloat(@as(f32, @floatFromInt(screen.height)) / cell.height)));
+        const cell_height: f32 = @floatFromInt(cell.height);
+        const screen_width: f32 = @floatFromInt(screen.width);
+        const screen_height: f32 = @floatFromInt(screen.height);
+        const calc_cols: Unit = @intFromFloat(screen_width / cell_width);
+        const calc_rows: Unit = @intFromFloat(screen_height / cell_height);
+        self.columns = @max(1, calc_cols);
+        self.rows = @max(1, calc_rows);
     }
 };
 
@@ -86,9 +92,13 @@ pub const Padding = struct {
     /// Returns padding that balances the whitespace around the screen
     /// for the given grid and cell sizes.
     pub fn balanced(screen: ScreenSize, grid: GridSize, cell: CellSize) Padding {
+        // Turn our cell sizes into floats for the math
+        const cell_width: f32 = @floatFromInt(cell.width);
+        const cell_height: f32 = @floatFromInt(cell.height);
+
         // The size of our full grid
-        const grid_width = @as(f32, @floatFromInt(grid.columns)) * cell.width;
+        const grid_width = @as(f32, @floatFromInt(grid.columns)) * cell_width;
-        const grid_height = @as(f32, @floatFromInt(grid.rows)) * cell.height;
+        const grid_height = @as(f32, @floatFromInt(grid.rows)) * cell_height;
 
         // The empty space to the right of a line and bottom of the last row
         const space_right = @as(f32, @floatFromInt(screen.width)) - grid_width;