Skip to content

Commit

Permalink
font/shaper: split text runs on common bad ligature pairs
Browse files Browse the repository at this point in the history
Fixes #2081

Many fonts have a bad ligature for "fl", "fi", or "st". We previously
maintained a list of such fonts in quirks.zig. However, these are so
common that it was suggested we do something more systematic and this
commit is that.

This commit changes our text run splitting algorithm to always split on
`fl`, `fi`, and `st`. This will cause some more runs for well behaved
fonts but the combination of those characters is rare enough and our
caching algorithm is good enough that it should be minimal overhead.

This commit renders our existing quirks fonts obsolete but I kept that
logic around so we can add to it if/when we find other quirky font
behaviors.
  • Loading branch information
mitchellh committed Aug 11, 2024
1 parent de9ea2d commit e385e0f
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 16 deletions.
21 changes: 21 additions & 0 deletions src/font/shaper/coretext.zig
Original file line number Diff line number Diff line change
Expand Up @@ -698,6 +698,27 @@ test "run iterator" {
while (try it.next(alloc)) |_| count += 1;
try testing.expectEqual(@as(usize, 3), count);
}

// Bad ligatures
for (&[_][]const u8{ "fl", "fi", "st" }) |bad| {
// Make a screen with some data
var screen = try terminal.Screen.init(alloc, 5, 3, 0);
defer screen.deinit();
try screen.testWriteString(bad);

// Get our run iterator
var shaper = &testdata.shaper;
var it = shaper.runIterator(
testdata.grid,
&screen,
screen.pages.pin(.{ .screen = .{ .y = 0 } }).?,
null,
null,
);
var count: usize = 0;
while (try it.next(alloc)) |_| count += 1;
try testing.expectEqual(@as(usize, 2), count);
}
}

test "run iterator: empty cells with background set" {
Expand Down
24 changes: 24 additions & 0 deletions src/font/shaper/run.zig
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,30 @@ pub const RunIterator = struct {
if (j > self.i) style: {
const prev_cell = cells[j - 1];

// If the prev cell and this cell are both plain
// codepoints then we check if they are commonly "bad"
// ligatures and spit the run if they are.
if (prev_cell.content_tag == .codepoint and
cell.content_tag == .codepoint)
{
const prev_cp = prev_cell.codepoint();
switch (prev_cp) {
// fl, fi
'f' => {
const cp = cell.codepoint();
if (cp == 'l' or cp == 'i') break;
},

// st
's' => {
const cp = cell.codepoint();
if (cp == 't') break;
},

else => {},
}
}

// If the style is exactly the change then fast path out.
if (prev_cell.style_id == cell.style_id) break :style;

Expand Down
29 changes: 13 additions & 16 deletions src/quirks.zig
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,18 @@ const font = @import("font/main.zig");

/// If true, the default font features should be disabled for the given face.
pub fn disableDefaultFontFeatures(face: *const font.Face) bool {
var buf: [64]u8 = undefined;
const name = face.name(&buf) catch |err| switch (err) {
// If the name doesn't fit in buf we know this will be false
// because we have no quirks fonts that are longer than buf!
error.OutOfMemory => return false,
};
_ = face;

// CodeNewRoman, Menlo and Monaco both have a default ligature of "fi" that
// looks really bad in terminal grids, so we want to disable ligatures
// by default for these faces.
//
// JuliaMono has a default ligature of "st" that looks bad.
return std.mem.eql(u8, name, "CodeNewRoman") or
std.mem.eql(u8, name, "CodeNewRoman Nerd Font") or
std.mem.eql(u8, name, "JuliaMono") or
std.mem.eql(u8, name, "Menlo") or
std.mem.eql(u8, name, "Monaco");
// This function used to do something, but we integrated the logic
// we checked for directly into our shaping algorithm. It's likely
// there are other broken fonts for other reasons so I'm keeping this
// around so its easy to add more checks in the future.
return false;

// var buf: [64]u8 = undefined;
// const name = face.name(&buf) catch |err| switch (err) {
// // If the name doesn't fit in buf we know this will be false
// // because we have no quirks fonts that are longer than buf!
// error.OutOfMemory => return false,
// };
}

0 comments on commit e385e0f

Please sign in to comment.