superrob1500
/
mirror-ghostty
mirror of https://github.com/ghostty-org/ghostty
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix font handling for bitmap and non-sfnt fonts (#8512) 

Fixes #8483, fixes #2991

With this change, `font.face.getMetrics` is now infallible, and real
bitmap fonts are properly handled and can be configured as the primary
font (or used as fallbacks), as long as the backend (FreeType or
CoreText) knows how to interpret them, since we now fall back on the
backend for any metrics we can't extract from sfnt tables (which means
we don't need any to be present in the first place).

Also, doing this uncovered a double-free issue in our FreeType
`renderGlyph` code, which thankfully wasn't too hard to track down and
fix.

> [!NOTE]
> We should vendor a true bitmap font in each of the native formats
supported by each backend and add tests for the metrics being computed
right and the glyphs being rendered correctly. Idk if we wanna block
this PR on that or not.
pull/8526/head
Mitchell Hashimoto 2025-09-04 12:20:49 -07:00 committed by GitHub
parent 93744a4002 a590194cd7
commit e2504d9cbf
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 220 additions and 179 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
src/font/Collection.zig
View File

@ -92,7 +92,6 @@ pub const AddOptions = struct {
pub const AddError =
Allocator.Error ||
Face.GetMetricsError ||
error{
/// There's no more room in the collection.
CollectionFull,
@ -127,7 +126,7 @@ pub fn add(
// Scale factor to adjust the size of the added face.
const scale_factor = self.scaleFactor(
try owned_face.getMetrics(),
owned_face.getMetrics(),
opts.size_adjustment,
);
@ -225,7 +224,7 @@ fn getFaceFromEntry(
// entry now that we have a loaded face.
entry.scale_factor = .{
.scale = self.scaleFactor(
try face.getMetrics(),
face.getMetrics(),
entry.scale_factor.adjustment,
),
};
@ -592,7 +591,7 @@ fn scaleFactor(
@branchHint(.unlikely);
// If we can't load the primary face, just use 1.0 as the scale factor.
const primary_face = self.getFace(.{ .idx = 0 }) catch return 1.0;
self.primary_face_metrics = primary_face.getMetrics() catch return 1.0;
self.primary_face_metrics = primary_face.getMetrics();
}
const primary_metrics = self.primary_face_metrics.?;
@ -652,7 +651,7 @@ fn scaleFactor(
return primary_metric / face_metric;
}
const UpdateMetricsError = font.Face.GetMetricsError || error{
const UpdateMetricsError = error{
CannotLoadPrimaryFont,
};
@ -663,7 +662,7 @@ const UpdateMetricsError = font.Face.GetMetricsError || error{
pub fn updateMetrics(self: *Collection) UpdateMetricsError!void {
const primary_face = self.getFace(.{ .idx = 0 }) catch return error.CannotLoadPrimaryFont;
self.primary_face_metrics = try primary_face.getMetrics();
self.primary_face_metrics = primary_face.getMetrics();
var metrics = Metrics.calc(self.primary_face_metrics.?);
@ -1288,8 +1287,8 @@ test "adjusted sizes" {
// The chosen metric should match.
{
const primary_metrics = try (try c.getFace(.{ .idx = 0 })).getMetrics();
const fallback_metrics = try (try c.getFace(fallback_idx)).getMetrics();
const primary_metrics = (try c.getFace(.{ .idx = 0 })).getMetrics();
const fallback_metrics = (try c.getFace(fallback_idx)).getMetrics();
try std.testing.expectApproxEqAbs(
@field(primary_metrics, metric).?,
@ -1302,8 +1301,8 @@ test "adjusted sizes" {
// Resize should keep that relationship.
try c.setSize(.{ .points = 37, .xdpi = 96, .ydpi = 96 });
{
const primary_metrics = try (try c.getFace(.{ .idx = 0 })).getMetrics();
const fallback_metrics = try (try c.getFace(fallback_idx)).getMetrics();
const primary_metrics = (try c.getFace(.{ .idx = 0 })).getMetrics();
const fallback_metrics = (try c.getFace(fallback_idx)).getMetrics();
try std.testing.expectApproxEqAbs(
@field(primary_metrics, metric).?,
@ -1359,8 +1358,8 @@ test "adjusted sizes" {
// Test fallback to lineHeight() (ex_height and cap_height not defined in symbols font).
{
const primary_metrics = try (try c.getFace(.{ .idx = 0 })).getMetrics();
const symbol_metrics = try (try c.getFace(symbol_idx)).getMetrics();
const primary_metrics = (try c.getFace(.{ .idx = 0 })).getMetrics();
const symbol_metrics = (try c.getFace(symbol_idx)).getMetrics();
try std.testing.expectApproxEqAbs(
primary_metrics.lineHeight(),

86
src/font/face/coretext.zig
View File

@ -574,19 +574,12 @@ pub const Face = struct {
};
}
pub const GetMetricsError = error{
CopyTableError,
InvalidHeadTable,
InvalidPostTable,
InvalidHheaTable,
};
/// Get the `FaceMetrics` for this face.
pub fn getMetrics(self: *Face) GetMetricsError!font.Metrics.FaceMetrics {
pub fn getMetrics(self: *Face) font.Metrics.FaceMetrics {
const ct_font = self.font;
// Read the 'head' table out of the font data.
const head: opentype.Head = head: {
const head_: ?opentype.Head = head: {
// macOS bitmap-only fonts use a 'bhed' tag rather than 'head', but
// the table format is byte-identical to the 'head' table, so if we
// can't find 'head' we try 'bhed' instead before failing.
@ -597,29 +590,26 @@ pub const Face = struct {
const data =
ct_font.copyTable(head_tag) orelse
ct_font.copyTable(bhed_tag) orelse
return error.CopyTableError;
break :head null;
defer data.release();
const ptr = data.getPointer();
const len = data.getLength();
break :head opentype.Head.init(ptr[0..len]) catch |err| {
return switch (err) {
error.EndOfStream,
=> error.InvalidHeadTable,
};
log.warn("error parsing head table: {}", .{err});
break :head null;
};
};
// Read the 'post' table out of the font data.
const post: opentype.Post = post: {
const post_: ?opentype.Post = post: {
const tag = macos.text.FontTableTag.init("post");
const data = ct_font.copyTable(tag) orelse return error.CopyTableError;
const data = ct_font.copyTable(tag) orelse break :post null;
defer data.release();
const ptr = data.getPointer();
const len = data.getLength();
break :post opentype.Post.init(ptr[0..len]) catch |err| {
return switch (err) {
error.EndOfStream => error.InvalidPostTable,
};
log.warn("error parsing post table: {}", .{err});
break :post null;
};
};
@ -637,29 +627,46 @@ pub const Face = struct {
};
// Read the 'hhea' table out of the font data.
const hhea: opentype.Hhea = hhea: {
const hhea_: ?opentype.Hhea = hhea: {
const tag = macos.text.FontTableTag.init("hhea");
const data = ct_font.copyTable(tag) orelse return error.CopyTableError;
const data = ct_font.copyTable(tag) orelse break :hhea null;
defer data.release();
const ptr = data.getPointer();
const len = data.getLength();
break :hhea opentype.Hhea.init(ptr[0..len]) catch |err| {
return switch (err) {
error.EndOfStream => error.InvalidHheaTable,
};
log.warn("error parsing hhea table: {}", .{err});
break :hhea null;
};
};
const units_per_em: f64 = @floatFromInt(head.unitsPerEm);
const units_per_em: f64 =
if (head_) |head|
@floatFromInt(head.unitsPerEm)
else
@floatFromInt(self.font.getUnitsPerEm());
const px_per_em: f64 = ct_font.getSize();
const px_per_unit: f64 = px_per_em / units_per_em;
const ascent: f64, const descent: f64, const line_gap: f64 = vertical_metrics: {
// If we couldn't get the hhea table, rely on metrics from CoreText.
const hhea = hhea_ orelse break :vertical_metrics .{
self.font.getAscent(),
-self.font.getDescent(),
self.font.getLeading(),
};
const hhea_ascent: f64 = @floatFromInt(hhea.ascender);
const hhea_descent: f64 = @floatFromInt(hhea.descender);
const hhea_line_gap: f64 = @floatFromInt(hhea.lineGap);
if (os2_) |os2| {
// If our font has no OS/2 table, then we just
// blindly use the metrics from the hhea table.
const os2 = os2_ orelse break :vertical_metrics .{
hhea_ascent * px_per_unit,
hhea_descent * px_per_unit,
hhea_line_gap * px_per_unit,
};
const os2_ascent: f64 = @floatFromInt(os2.sTypoAscender);
const os2_descent: f64 = @floatFromInt(os2.sTypoDescender);
const os2_line_gap: f64 = @floatFromInt(os2.sTypoLineGap);
@ -700,16 +707,10 @@ pub const Face = struct {
-win_descent * px_per_unit,
0.0,
};
}
};
// If our font has no OS/2 table, then we just
// blindly use the metrics from the hhea table.
break :vertical_metrics .{
hhea_ascent * px_per_unit,
hhea_descent * px_per_unit,
hhea_line_gap * px_per_unit,
};
};
const underline_position, const underline_thickness = ul: {
const post = post_ orelse break :ul .{ null, null };
// Some fonts have degenerate 'post' tables where the underline
// thickness (and often position) are 0. We consider them null
@ -718,16 +719,19 @@ pub const Face = struct {
// If the underline position isn't 0 then we do use it,
// even if the thickness is't properly specified.
const underline_position: ?f64 = if (has_broken_underline and post.underlinePosition == 0)
const pos: ?f64 = if (has_broken_underline and post.underlinePosition == 0)
null
else
@as(f64, @floatFromInt(post.underlinePosition)) * px_per_unit;
const underline_thickness = if (has_broken_underline)
const thick: ?f64 = if (has_broken_underline)
null
else
@as(f64, @floatFromInt(post.underlineThickness)) * px_per_unit;
break :ul .{ pos, thick };
};
// Similar logic to the underline above.
const strikethrough_position, const strikethrough_thickness = st: {
const os2 = os2_ orelse break :st .{ null, null };
@ -989,7 +993,7 @@ test {
alloc,
&atlas,
face.glyphIndex(i).?,
.{ .grid_metrics = font.Metrics.calc(try face.getMetrics()) },
.{ .grid_metrics = font.Metrics.calc(face.getMetrics()) },
);
}
}
@ -1054,7 +1058,7 @@ test "in-memory" {
alloc,
&atlas,
face.glyphIndex(i).?,
.{ .grid_metrics = font.Metrics.calc(try face.getMetrics()) },
.{ .grid_metrics = font.Metrics.calc(face.getMetrics()) },
);
}
}
@ -1081,7 +1085,7 @@ test "variable" {
alloc,
&atlas,
face.glyphIndex(i).?,
.{ .grid_metrics = font.Metrics.calc(try face.getMetrics()) },
.{ .grid_metrics = font.Metrics.calc(face.getMetrics()) },
);
}
}
@ -1112,7 +1116,7 @@ test "variable set variation" {
alloc,
&atlas,
face.glyphIndex(i).?,
.{ .grid_metrics = font.Metrics.calc(try face.getMetrics()) },
.{ .grid_metrics = font.Metrics.calc(face.getMetrics()) },
);
}
}

92
src/font/face/freetype.zig
View File

@ -594,6 +594,12 @@ pub const Face = struct {
freetype.c.FT_PIXEL_MODE_GRAY,
=> {},
else => {
// Make sure the slot owns its bitmap,
// since we'll be modifying it here.
if (freetype.c.FT_GlyphSlot_Own_Bitmap(glyph) != 0) {
return error.BitmapHandlingError;
}
var converted: freetype.c.FT_Bitmap = undefined;
freetype.c.FT_Bitmap_Init(&converted);
if (freetype.c.FT_Bitmap_Convert(
@ -784,12 +790,8 @@ pub const Face = struct {
return @as(F26Dot6, @bitCast(@as(i32, @intCast(v)))).to(f64);
}
pub const GetMetricsError = error{
CopyTableError,
};
/// Get the `FaceMetrics` for this face.
pub fn getMetrics(self: *Face) GetMetricsError!font.Metrics.FaceMetrics {
pub fn getMetrics(self: *Face) font.Metrics.FaceMetrics {
const face = self.face;
const size_metrics = face.handle.*.size.*.metrics;
@ -799,10 +801,10 @@ pub const Face = struct {
assert(size_metrics.x_ppem == size_metrics.y_ppem);
// Read the 'head' table out of the font data.
const head = face.getSfntTable(.head) orelse return error.CopyTableError;
const head_ = face.getSfntTable(.head);
// Read the 'post' table out of the font data.
const post = face.getSfntTable(.post) orelse return error.CopyTableError;
const post_ = face.getSfntTable(.post);
// Read the 'OS/2' table out of the font data.
const os2_: ?*freetype.c.TT_OS2 = os2: {
@ -812,18 +814,57 @@ pub const Face = struct {
};
// Read the 'hhea' table out of the font data.
const hhea = face.getSfntTable(.hhea) orelse return error.CopyTableError;
const hhea_ = face.getSfntTable(.hhea);
const units_per_em = head.Units_Per_EM;
// Whether the font is in a scalable format. We need to know this
// because many of the metrics provided by FreeType are invalid for
// non-scalable fonts.
const is_scalable = face.handle.*.face_flags & freetype.c.FT_FACE_FLAG_SCALABLE != 0;
// We get the UPM from the head table.
//
// If we have no head, but it is a scalable face, take the UPM from
// FreeType's units_per_EM, otherwise we'll assume that UPM == PPEM.
const units_per_em: freetype.c.FT_UShort =
if (head_) |head|
head.Units_Per_EM
else if (is_scalable)
face.handle.*.units_per_EM
else
size_metrics.y_ppem;
const px_per_em: f64 = @floatFromInt(size_metrics.y_ppem);
const px_per_unit = px_per_em / @as(f64, @floatFromInt(units_per_em));
const ascent: f64, const descent: f64, const line_gap: f64 = vertical_metrics: {
const hhea = hhea_ orelse {
// If we couldn't get the hhea table, rely on metrics from FreeType.
const ascender = f26dot6ToF64(size_metrics.ascender);
const descender = f26dot6ToF64(size_metrics.descender);
const height = f26dot6ToF64(size_metrics.height);
break :vertical_metrics .{
ascender,
descender,
// We compute the line gap by adding the (negative) descender
// and subtracting the (positive) ascender from the line height
// to get the remaining gap size.
//
// NOTE: This might always be 0... but it doesn't hurt to do.
height + descender - ascender,
};
};
const hhea_ascent: f64 = @floatFromInt(hhea.Ascender);
const hhea_descent: f64 = @floatFromInt(hhea.Descender);
const hhea_line_gap: f64 = @floatFromInt(hhea.Line_Gap);
if (os2_) |os2| {
// If our font has no OS/2 table, then we just
// blindly use the metrics from the hhea table.
const os2 = os2_ orelse break :vertical_metrics .{
hhea_ascent * px_per_unit,
hhea_descent * px_per_unit,
hhea_line_gap * px_per_unit,
};
const os2_ascent: f64 = @floatFromInt(os2.sTypoAscender);
const os2_descent: f64 = @floatFromInt(os2.sTypoDescender);
const os2_line_gap: f64 = @floatFromInt(os2.sTypoLineGap);
@ -871,16 +912,10 @@ pub const Face = struct {
-win_descent * px_per_unit,
0.0,
};
}
};
// If our font has no OS/2 table, then we just
// blindly use the metrics from the hhea table.
break :vertical_metrics .{
hhea_ascent * px_per_unit,
hhea_descent * px_per_unit,
hhea_line_gap * px_per_unit,
};
};
const underline_position: ?f64, const underline_thickness: ?f64 = ul: {
const post = post_ orelse break :ul .{ null, null };
// Some fonts have degenerate 'post' tables where the underline
// thickness (and often position) are 0. We consider them null
@ -889,16 +924,19 @@ pub const Face = struct {
// If the underline position isn't 0 then we do use it,
// even if the thickness is't properly specified.
const underline_position = if (has_broken_underline and post.underlinePosition == 0)
const pos: ?f64 = if (has_broken_underline and post.underlinePosition == 0)
null
else
@as(f64, @floatFromInt(post.underlinePosition)) * px_per_unit;
const underline_thickness = if (has_broken_underline)
const thick: ?f64 = if (has_broken_underline)
null
else
@as(f64, @floatFromInt(post.underlineThickness)) * px_per_unit;
break :ul .{ pos, thick };
};
// Similar logic to the underline above.
const strikethrough_position, const strikethrough_thickness = st: {
const os2 = os2_ orelse break :st .{ null, null };
@ -1085,7 +1123,7 @@ test {
alloc,
&atlas,
ft_font.glyphIndex(i).?,
.{ .grid_metrics = font.Metrics.calc(try ft_font.getMetrics()) },
.{ .grid_metrics = font.Metrics.calc(ft_font.getMetrics()) },
);
}
@ -1095,7 +1133,7 @@ test {
alloc,
&atlas,
ft_font.glyphIndex('A').?,
.{ .grid_metrics = font.Metrics.calc(try ft_font.getMetrics()) },
.{ .grid_metrics = font.Metrics.calc(ft_font.getMetrics()) },
);
try testing.expectEqual(@as(u32, 11), g1.height);
@ -1104,7 +1142,7 @@ test {
alloc,
&atlas,
ft_font.glyphIndex('A').?,
.{ .grid_metrics = font.Metrics.calc(try ft_font.getMetrics()) },
.{ .grid_metrics = font.Metrics.calc(ft_font.getMetrics()) },
);
try testing.expectEqual(@as(u32, 20), g2.height);
}
@ -1131,7 +1169,7 @@ test "color emoji" {
alloc,
&atlas,
ft_font.glyphIndex('🥸').?,
.{ .grid_metrics = font.Metrics.calc(try ft_font.getMetrics()) },
.{ .grid_metrics = font.Metrics.calc(ft_font.getMetrics()) },
);
// Make sure this glyph has color
@ -1191,7 +1229,7 @@ test "mono to bgra" {
alloc,
&atlas,
3,
.{ .grid_metrics = font.Metrics.calc(try ft_font.getMetrics()) },
.{ .grid_metrics = font.Metrics.calc(ft_font.getMetrics()) },
);
}
@ -1255,7 +1293,7 @@ test "bitmap glyph" {
alloc,
&atlas,
77,
.{ .grid_metrics = font.Metrics.calc(try ft_font.getMetrics()) },
.{ .grid_metrics = font.Metrics.calc(ft_font.getMetrics()) },
);
// should render crisp