From 7f4088174718781727031d9b4d55f9c8fdd90e26 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 2 Dec 2023 09:39:45 -0800 Subject: [PATCH] font: faces use primary grid metrics to better line up glyphs Fixes #895 Every loaded font face calculates metrics for itself. One of the important metrics is the baseline to "sit" the glyph on top of. Prior to this commit, each rasterized glyph would sit on its own calculated baseline. However, this leads to off-center rendering when the font being rasterized isn't the font that defines the terminal grid. This commit passes in the font metrics for the font defining the terminal grid to all font rasterization requests. This can then be used by non-primary fonts to sit the glyph according to the primary grid. --- src/font/face.zig | 9 +++--- src/font/face/coretext.zig | 4 ++- src/font/face/freetype.zig | 16 ++++++++-- src/renderer/Metal.zig | 64 ++++++++++++++++++++++++-------------- src/renderer/OpenGL.zig | 60 ++++++++++++++++++++++++----------- 5 files changed, 104 insertions(+), 49 deletions(-) diff --git a/src/font/face.zig b/src/font/face.zig index b8afee685..640f7a5ce 100644 --- a/src/font/face.zig +++ b/src/font/face.zig @@ -71,10 +71,11 @@ pub const Variation = struct { /// Additional options for rendering glyphs. pub const RenderOptions = struct { - /// The maximum height of the glyph. If this is set, then any glyph - /// larger than this height will be shrunk to this height. The scaling - /// is typically naive, but ultimately up to the rasterizer. - max_height: ?u16 = null, + /// The metrics that are defining the grid layout. These are usually + /// the metrics of the primary font face. The grid metrics are used + /// by the font face to better layout the glyph in situations where + /// the font is not exactly the same size as the grid. + grid_metrics: ?Metrics = null, /// The number of grid cells this glyph will take up. This can be used /// optionally by the rasterizer to better layout the glyph. diff --git a/src/font/face/coretext.zig b/src/font/face/coretext.zig index eb83354b5..0ce09d2c7 100644 --- a/src/font/face/coretext.zig +++ b/src/font/face/coretext.zig @@ -386,7 +386,9 @@ 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: f64 = @floatFromInt(self.metrics.cell_baseline); + //const baseline_from_bottom: f64 = @floatFromInt(self.metrics.cell_baseline); + const metrics = opts.grid_metrics orelse self.metrics; + const baseline_from_bottom: f64 = @floatFromInt(metrics.cell_baseline); // Next we offset our baseline by the bearing in the font. We // ADD here because CoreText y is UP. diff --git a/src/font/face/freetype.zig b/src/font/face/freetype.zig index c8cc38067..362588e38 100644 --- a/src/font/face/freetype.zig +++ b/src/font/face/freetype.zig @@ -226,6 +226,8 @@ pub const Face = struct { glyph_index: u32, opts: font.face.RenderOptions, ) !Glyph { + const metrics = opts.grid_metrics orelse self.metrics; + // If our glyph has color, we want to render the color try self.face.loadGlyph(glyph_index, .{ .render = true, @@ -288,7 +290,7 @@ pub const Face = struct { // and copy the atlas. const bitmap_original = bitmap_converted orelse bitmap_ft; const bitmap_resized: ?freetype.c.struct_FT_Bitmap_ = resized: { - const max = opts.max_height orelse break :resized null; + const max = metrics.cell_height; const bm = bitmap_original; if (bm.rows <= max) break :resized null; @@ -425,7 +427,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, @intCast(self.metrics.cell_baseline)); + break :offset_y glyph_metrics.bitmap_top + @as(c_int, @intCast(metrics.cell_baseline)); }; // log.warn("renderGlyph width={} height={} offset_x={} offset_y={} glyph_metrics={}", .{ @@ -662,7 +664,15 @@ test "color emoji" { // resize { const glyph = try ft_font.renderGlyph(alloc, &atlas, ft_font.glyphIndex('🥸').?, .{ - .max_height = 24, + .grid_metrics = .{ + .cell_width = 10, + .cell_height = 24, + .cell_baseline = 0, + .underline_position = 0, + .underline_thickness = 0, + .strikethrough_position = 0, + .strikethrough_thickness = 0, + }, }); try testing.expectEqual(@as(u32, 24), glyph.height); } diff --git a/src/renderer/Metal.zig b/src/renderer/Metal.zig index d230c6d74..9c87149c6 100644 --- a/src/renderer/Metal.zig +++ b/src/renderer/Metal.zig @@ -56,8 +56,8 @@ config: DerivedConfig, /// The mailbox for communicating with the window. surface_mailbox: apprt.surface.Mailbox, -/// Current cell dimensions for this grid. -cell_size: renderer.CellSize, +/// Current font metrics defining our grid. +grid_metrics: font.face.Metrics, /// Current screen size dimensions for this grid. This is set on the first /// resize event, and is not immediately available. @@ -359,7 +359,7 @@ pub fn init(alloc: Allocator, options: renderer.Options) !Metal { .alloc = alloc, .config = options.config, .surface_mailbox = options.surface_mailbox, - .cell_size = .{ .width = metrics.cell_width, .height = metrics.cell_height }, + .grid_metrics = metrics, .screen_size = null, .padding = options.padding, .focused = true, @@ -497,7 +497,10 @@ fn gridSize(self: *Metal) ?renderer.GridSize { const screen_size = self.screen_size orelse return null; return renderer.GridSize.init( screen_size.subPadding(self.padding.explicit), - self.cell_size, + .{ + .width = self.grid_metrics.cell_width, + .height = self.grid_metrics.cell_height, + }, ); } @@ -523,14 +526,13 @@ pub fn setFontSize(self: *Metal, size: font.face.DesiredSize) !void { const face = try self.font_group.group.faceFromIndex(index); break :metrics face.metrics; }; - const new_cell_size = .{ .width = metrics.cell_width, .height = metrics.cell_height }; // Update our uniforms self.uniforms = .{ .projection_matrix = self.uniforms.projection_matrix, .cell_size = .{ - @floatFromInt(new_cell_size.width), - @floatFromInt(new_cell_size.height), + @floatFromInt(metrics.cell_width), + @floatFromInt(metrics.cell_height), }, .strikethrough_position = @floatFromInt(metrics.strikethrough_position), .strikethrough_thickness = @floatFromInt(metrics.strikethrough_thickness), @@ -539,20 +541,23 @@ pub fn setFontSize(self: *Metal, size: font.face.DesiredSize) !void { // Recalculate our cell size. If it is the same as before, then we do // nothing since the grid size couldn't have possibly changed. - if (std.meta.eql(self.cell_size, new_cell_size)) return; - self.cell_size = new_cell_size; + if (std.meta.eql(self.grid_metrics, metrics)) return; + self.grid_metrics = metrics; // Set the sprite font up self.font_group.group.sprite = font.sprite.Face{ - .width = self.cell_size.width, - .height = self.cell_size.height, + .width = metrics.cell_width, + .height = metrics.cell_height, .thickness = metrics.underline_thickness * @as(u32, if (self.config.font_thicken) 2 else 1), .underline_position = metrics.underline_position, }; // Notify the window that the cell size changed. _ = self.surface_mailbox.push(.{ - .cell_size = new_cell_size, + .cell_size = .{ + .width = metrics.cell_width, + .height = metrics.cell_height, + }, }, .{ .forever = {} }); } @@ -1140,7 +1145,7 @@ fn prepKittyGraphics( // so that we render (0, 0) with some offset for the texture. const offset_y: u32 = if (rect.top_left.y < t.screen.viewport) offset_y: { const offset_cells = t.screen.viewport - rect.top_left.y; - const offset_pixels = offset_cells * self.cell_size.height; + const offset_pixels = offset_cells * self.grid_metrics.cell_height; break :offset_y @intCast(offset_pixels); } else 0; @@ -1181,8 +1186,8 @@ fn prepKittyGraphics( image.height -| offset_y; // Calculate the width/height of our image. - const dest_width = if (p.columns > 0) p.columns * self.cell_size.width else source_width; - const dest_height = if (p.rows > 0) p.rows * self.cell_size.height else source_height; + const dest_width = if (p.columns > 0) p.columns * self.grid_metrics.cell_width else source_width; + const dest_height = if (p.rows > 0) p.rows * self.grid_metrics.cell_height else source_height; // Accumulate the placement if (image.width > 0 and image.height > 0) { @@ -1286,7 +1291,14 @@ pub fn setScreenSize( // the leftover amounts on the right/bottom that don't fit a full grid cell // and we split them equal across all boundaries. const padding = if (self.padding.balance) - renderer.Padding.balanced(dim, grid_size, self.cell_size) + renderer.Padding.balanced( + dim, + grid_size, + .{ + .width = self.grid_metrics.cell_width, + .height = self.grid_metrics.cell_height, + }, + ) else self.padding.explicit; const padded_dim = dim.subPadding(padding); @@ -1307,8 +1319,8 @@ pub fn setScreenSize( -1 * @as(f32, @floatFromInt(padding.top)), ), .cell_size = .{ - @floatFromInt(self.cell_size.width), - @floatFromInt(self.cell_size.height), + @floatFromInt(self.grid_metrics.cell_width), + @floatFromInt(self.grid_metrics.cell_height), }, .strikethrough_position = old.strikethrough_position, .strikethrough_thickness = old.strikethrough_thickness, @@ -1366,7 +1378,7 @@ pub fn setScreenSize( }; } - log.debug("screen size screen={} grid={}, cell={}", .{ dim, grid_size, self.cell_size }); + log.debug("screen size screen={} grid={}, cell_width={} cell_height={}", .{ dim, grid_size, self.grid_metrics.cell_width, self.grid_metrics.cell_height }); } /// Sync all the CPU cells with the GPU state (but still on the CPU here). @@ -1728,7 +1740,7 @@ pub fn updateCell( shaper_run.font_index, shaper_cell.glyph_index, .{ - .max_height = @intCast(self.cell_size.height), + .grid_metrics = self.grid_metrics, .thicken = self.config.font_thicken, }, ); @@ -1766,7 +1778,10 @@ pub fn updateCell( self.alloc, font.sprite_index, @intFromEnum(sprite), - .{ .cell_width = if (cell.attrs.wide) 2 else 1 }, + .{ + .cell_width = if (cell.attrs.wide) 2 else 1, + .grid_metrics = self.grid_metrics, + }, ); const color = if (cell.attrs.underline_color) cell.underline_fg else colors.fg; @@ -1839,7 +1854,10 @@ fn addCursor( self.alloc, font.sprite_index, @intFromEnum(sprite), - .{ .cell_width = if (wide) 2 else 1 }, + .{ + .cell_width = if (wide) 2 else 1, + .grid_metrics = self.grid_metrics, + }, ) catch |err| { log.warn("error rendering cursor glyph err={}", .{err}); return null; @@ -1894,7 +1912,7 @@ fn addPreeditCell( self.alloc, font_index, glyph_index, - .{}, + .{ .grid_metrics = self.grid_metrics }, ) catch |err| { log.warn("error rendering preedit glyph err={}", .{err}); return; diff --git a/src/renderer/OpenGL.zig b/src/renderer/OpenGL.zig index 8e144b832..aa7936c03 100644 --- a/src/renderer/OpenGL.zig +++ b/src/renderer/OpenGL.zig @@ -47,8 +47,8 @@ alloc: std.mem.Allocator, /// The configuration we need derived from the main config. config: DerivedConfig, -/// Current cell dimensions for this grid. -cell_size: renderer.CellSize, +/// Current font metrics defining our grid. +grid_metrics: font.face.Metrics, /// Current screen size dimensions for this grid. This is set on the first /// resize event, and is not immediately available. @@ -128,7 +128,14 @@ const SetScreenSize = struct { // Apply our padding const padding = if (r.padding.balance) - renderer.Padding.balanced(self.size, r.gridSize(self.size), r.cell_size) + renderer.Padding.balanced( + self.size, + r.gridSize(self.size), + .{ + .width = r.grid_metrics.cell_width, + .height = r.grid_metrics.cell_height, + }, + ) else r.padding.explicit; const padded_size = self.size.subPadding(padding); @@ -137,7 +144,10 @@ const SetScreenSize = struct { padded_size, self.size, r.gridSize(self.size), - r.cell_size, + renderer.CellSize{ + .width = r.grid_metrics.cell_width, + .height = r.grid_metrics.cell_height, + }, r.padding.explicit, }); @@ -340,7 +350,7 @@ pub fn init(alloc: Allocator, options: renderer.Options) !OpenGL { .config = options.config, .cells_bg = .{}, .cells = .{}, - .cell_size = .{ .width = metrics.cell_width, .height = metrics.cell_height }, + .grid_metrics = metrics, .screen_size = null, .gl_state = gl_state, .font_group = options.font_group, @@ -576,13 +586,15 @@ pub fn setFontSize(self: *OpenGL, size: font.face.DesiredSize) !void { // Recalculate our cell size. If it is the same as before, then we do // nothing since the grid size couldn't have possibly changed. - const new_cell_size = .{ .width = metrics.cell_width, .height = metrics.cell_height }; - if (std.meta.eql(self.cell_size, new_cell_size)) return; - self.cell_size = new_cell_size; + if (std.meta.eql(self.grid_metrics, metrics)) return; + self.grid_metrics = metrics; // Notify the window that the cell size changed. _ = self.surface_mailbox.push(.{ - .cell_size = new_cell_size, + .cell_size = .{ + .width = metrics.cell_width, + .height = metrics.cell_height, + }, }, .{ .forever = {} }); } @@ -780,7 +792,7 @@ fn prepKittyGraphics( // so that we render (0, 0) with some offset for the texture. const offset_y: u32 = if (rect.top_left.y < t.screen.viewport) offset_y: { const offset_cells = t.screen.viewport - rect.top_left.y; - const offset_pixels = offset_cells * self.cell_size.height; + const offset_pixels = offset_cells * self.grid_metrics.cell_height; break :offset_y @intCast(offset_pixels); } else 0; @@ -821,8 +833,8 @@ fn prepKittyGraphics( image.height -| offset_y; // Calculate the width/height of our image. - const dest_width = if (p.columns > 0) p.columns * self.cell_size.width else source_width; - const dest_height = if (p.rows > 0) p.rows * self.cell_size.height else source_height; + const dest_width = if (p.columns > 0) p.columns * self.grid_metrics.cell_width else source_width; + const dest_height = if (p.rows > 0) p.rows * self.grid_metrics.cell_height else source_height; // Accumulate the placement if (image.width > 0 and image.height > 0) { @@ -1145,7 +1157,7 @@ fn addPreeditCell( self.alloc, font_index, glyph_index, - .{}, + .{ .grid_metrics = self.grid_metrics }, ) catch |err| { log.warn("error rendering preedit glyph err={}", .{err}); return; @@ -1239,7 +1251,10 @@ fn addCursor( self.alloc, font.sprite_index, @intFromEnum(sprite), - .{ .cell_width = if (wide) 2 else 1 }, + .{ + .grid_metrics = self.grid_metrics, + .cell_width = if (wide) 2 else 1, + }, ) catch |err| { log.warn("error rendering cursor glyph err={}", .{err}); return null; @@ -1431,7 +1446,7 @@ pub fn updateCell( shaper_run.font_index, shaper_cell.glyph_index, .{ - .max_height = @intCast(self.cell_size.height), + .grid_metrics = self.grid_metrics, .thicken = self.config.font_thicken, }, ); @@ -1479,7 +1494,10 @@ pub fn updateCell( self.alloc, font.sprite_index, @intFromEnum(sprite), - .{ .cell_width = if (cell.attrs.wide) 2 else 1 }, + .{ + .grid_metrics = self.grid_metrics, + .cell_width = if (cell.attrs.wide) 2 else 1, + }, ); const color = if (cell.attrs.underline_color) cell.underline_fg else colors.fg; @@ -1537,7 +1555,10 @@ pub fn updateCell( fn gridSize(self: *const OpenGL, screen_size: renderer.ScreenSize) renderer.GridSize { return renderer.GridSize.init( screen_size.subPadding(self.padding.explicit), - self.cell_size, + .{ + .width = self.grid_metrics.cell_width, + .height = self.grid_metrics.cell_height, + }, ); } @@ -1598,7 +1619,10 @@ pub fn setScreenSize( log.debug("screen size screen={} grid={} cell={} padding={}", .{ dim, grid_size, - self.cell_size, + renderer.CellSize{ + .width = self.grid_metrics.cell_width, + .height = self.grid_metrics.cell_height, + }, self.padding.explicit, });