From 0a648cddf83b59352b409db7504a484f83a3d7c0 Mon Sep 17 00:00:00 2001 From: Jacob Sandlund Date: Tue, 30 Dec 2025 17:36:45 -0500 Subject: [PATCH 1/6] shaping: Use position.y directly for CoreText --- src/font/shaper/coretext.zig | 71 ++++++++++++++++-------------------- 1 file changed, 31 insertions(+), 40 deletions(-) diff --git a/src/font/shaper/coretext.zig b/src/font/shaper/coretext.zig index 17d7801ff..09f123b4b 100644 --- a/src/font/shaper/coretext.zig +++ b/src/font/shaper/coretext.zig @@ -103,15 +103,9 @@ pub const Shaper = struct { } }; - const RunOffset = struct { - x: f64 = 0, - y: f64 = 0, - }; - const CellOffset = struct { cluster: u32 = 0, x: f64 = 0, - y: f64 = 0, }; /// Create a CoreFoundation Dictionary suitable for @@ -388,15 +382,15 @@ pub const Shaper = struct { const line = typesetter.createLine(.{ .location = 0, .length = 0 }); self.cf_release_pool.appendAssumeCapacity(line); - // This keeps track of the current offsets within a run. - var run_offset: RunOffset = .{}; + // This keeps track of the current x offset within a run. + var run_offset_x: f64 = 0.0; - // This keeps track of the current offsets within a cell. + // This keeps track of the current x offset and cluster for a cell. var cell_offset: CellOffset = .{}; // For debugging positions, turn this on: - //var start_index: usize = 0; - //var end_index: usize = 0; + //var run_offset_y: f64 = 0.0; + //var cell_offset_y: f64 = 0.0; // Clear our cell buf and make sure we have enough room for the whole // line of glyphs, so that we can just assume capacity when appending @@ -450,39 +444,31 @@ pub const Shaper = struct { cell_offset = .{ .cluster = cluster, - .x = run_offset.x, - .y = run_offset.y, + .x = run_offset_x, }; // For debugging positions, turn this on: - // start_index = index; - // end_index = index; - //} else { - // if (index < start_index) { - // start_index = index; - // } - // if (index > end_index) { - // end_index = index; - // } + //cell_offset_y = run_offset_y; } // For debugging positions, turn this on: - //try self.debugPositions(alloc, run_offset, cell_offset, position, start_index, end_index, index); + //try self.debugPositions(alloc, run_offset_x, run_offset_y, cell_offset, cell_offset_y, position, index); const x_offset = position.x - cell_offset.x; - const y_offset = position.y - cell_offset.y; self.cell_buf.appendAssumeCapacity(.{ .x = @intCast(cluster), .x_offset = @intFromFloat(@round(x_offset)), - .y_offset = @intFromFloat(@round(y_offset)), + .y_offset = @intFromFloat(@round(position.y)), .glyph_index = glyph, }); // Add our advances to keep track of our run offsets. // Advances apply to the NEXT cell. - run_offset.x += advance.width; - run_offset.y += advance.height; + run_offset_x += advance.width; + + // For debugging positions, turn this on: + //run_offset_y += advance.height; } } @@ -655,33 +641,38 @@ pub const Shaper = struct { fn debugPositions( self: *Shaper, alloc: Allocator, - run_offset: RunOffset, + run_offset_x: f64, + run_offset_y: f64, cell_offset: CellOffset, + cell_offset_y: f64, position: macos.graphics.Point, - start_index: usize, - end_index: usize, index: usize, ) !void { const state = &self.run_state; const x_offset = position.x - cell_offset.x; - const y_offset = position.y - cell_offset.y; - const advance_x_offset = run_offset.x - cell_offset.x; - const advance_y_offset = run_offset.y - cell_offset.y; + const advance_x_offset = run_offset_x - cell_offset.x; + const advance_y_offset = run_offset_y - cell_offset_y; const x_offset_diff = x_offset - advance_x_offset; - const y_offset_diff = y_offset - advance_y_offset; + const y_offset_diff = position.y - advance_y_offset; if (@abs(x_offset_diff) > 0.0001 or @abs(y_offset_diff) > 0.0001) { var allocating = std.Io.Writer.Allocating.init(alloc); const writer = &allocating.writer; - const codepoints = state.codepoints.items[start_index .. end_index + 1]; + const codepoints = state.codepoints.items; for (codepoints) |cp| { - if (cp.codepoint == 0) continue; // Skip surrogate pair padding - try writer.print("\\u{{{x}}}", .{cp.codepoint}); + if (cp.cluster == cell_offset.cluster and + cp.codepoint != 0 // Skip surrogate pair padding + ) { + try writer.print("\\u{{{x}}}", .{cp.codepoint}); + } } try writer.writeAll(" → "); for (codepoints) |cp| { - if (cp.codepoint == 0) continue; // Skip surrogate pair padding - try writer.print("{u}", .{@as(u21, @intCast(cp.codepoint))}); + if (cp.cluster == cell_offset.cluster and + cp.codepoint != 0 // Skip surrogate pair padding + ) { + try writer.print("{u}", .{@as(u21, @intCast(cp.codepoint))}); + } } const formatted_cps = try allocating.toOwnedSlice(); @@ -698,7 +689,7 @@ pub const Shaper = struct { log.warn("position differs from advance: cluster={d} pos=({d:.2},{d:.2}) adv=({d:.2},{d:.2}) diff=({d:.2},{d:.2}) current cp={x}, cps={s}", .{ cell_offset.cluster, x_offset, - y_offset, + position.y, advance_x_offset, advance_y_offset, x_offset_diff, From 1d4a5d91e08eb09eb1b1b984278c8ec980a3b2ca Mon Sep 17 00:00:00 2001 From: Jacob Sandlund Date: Sun, 4 Jan 2026 21:15:29 -0500 Subject: [PATCH 2/6] More debugging for position.y differences --- src/font/shaper/coretext.zig | 80 ++++++++++++++++++++++-------------- 1 file changed, 49 insertions(+), 31 deletions(-) diff --git a/src/font/shaper/coretext.zig b/src/font/shaper/coretext.zig index 09f123b4b..4b8b05419 100644 --- a/src/font/shaper/coretext.zig +++ b/src/font/shaper/coretext.zig @@ -382,15 +382,16 @@ pub const Shaper = struct { const line = typesetter.createLine(.{ .location = 0, .length = 0 }); self.cf_release_pool.appendAssumeCapacity(line); - // This keeps track of the current x offset within a run. + // This keeps track of the current x offset (sum of advance.width) for + // a run. var run_offset_x: f64 = 0.0; - // This keeps track of the current x offset and cluster for a cell. + // This keeps track of the cell starting x and cluster. var cell_offset: CellOffset = .{}; // For debugging positions, turn this on: - //var run_offset_y: f64 = 0.0; - //var cell_offset_y: f64 = 0.0; + var run_offset_y: f64 = 0.0; + var cell_offset_y: f64 = 0.0; // Clear our cell buf and make sure we have enough room for the whole // line of glyphs, so that we can just assume capacity when appending @@ -448,11 +449,11 @@ pub const Shaper = struct { }; // For debugging positions, turn this on: - //cell_offset_y = run_offset_y; + cell_offset_y = run_offset_y; } // For debugging positions, turn this on: - //try self.debugPositions(alloc, run_offset_x, run_offset_y, cell_offset, cell_offset_y, position, index); + try self.debugPositions(alloc, run_offset_x, run_offset_y, cell_offset, cell_offset_y, position, index); const x_offset = position.x - cell_offset.x; @@ -468,7 +469,7 @@ pub const Shaper = struct { run_offset_x += advance.width; // For debugging positions, turn this on: - //run_offset_y += advance.height; + run_offset_y += advance.height; } } @@ -654,21 +655,31 @@ pub const Shaper = struct { const advance_y_offset = run_offset_y - cell_offset_y; const x_offset_diff = x_offset - advance_x_offset; const y_offset_diff = position.y - advance_y_offset; + const positions_differ = @abs(x_offset_diff) > 0.0001 or @abs(y_offset_diff) > 0.0001; + const old_offset_y = position.y - cell_offset_y; + const position_y_differs = @abs(cell_offset_y) > 0.0001; - if (@abs(x_offset_diff) > 0.0001 or @abs(y_offset_diff) > 0.0001) { + if (positions_differ or position_y_differs) { var allocating = std.Io.Writer.Allocating.init(alloc); const writer = &allocating.writer; const codepoints = state.codepoints.items; + var last_cluster: ?u32 = null; for (codepoints) |cp| { - if (cp.cluster == cell_offset.cluster and + if ((cp.cluster == cell_offset.cluster or cp.cluster == cell_offset.cluster - 1 or cp.cluster == cell_offset.cluster + 1) and cp.codepoint != 0 // Skip surrogate pair padding ) { + if (last_cluster) |last| { + if (cp.cluster != last) { + try writer.writeAll(" "); + } + } try writer.print("\\u{{{x}}}", .{cp.codepoint}); + last_cluster = cp.cluster; } } try writer.writeAll(" → "); for (codepoints) |cp| { - if (cp.cluster == cell_offset.cluster and + if ((cp.cluster == cell_offset.cluster or cp.cluster == cell_offset.cluster - 1 or cp.cluster == cell_offset.cluster + 1) and cp.codepoint != 0 // Skip surrogate pair padding ) { try writer.print("{u}", .{@as(u21, @intCast(cp.codepoint))}); @@ -676,27 +687,34 @@ pub const Shaper = struct { } const formatted_cps = try allocating.toOwnedSlice(); - // Note that the codepoints from `start_index .. end_index + 1` - // might not include all the codepoints being shaped. Sometimes a - // codepoint gets represented in a glyph with a later codepoint - // such that the index for the former codepoint is skipped and just - // the index for the latter codepoint is used. Additionally, this - // gets called as we iterate through the glyphs, so it won't - // include the codepoints that come later that might be affecting - // positions for the current glyph. Usually though, for that case - // the positions of the later glyphs will also be affected and show - // up in the logs. - log.warn("position differs from advance: cluster={d} pos=({d:.2},{d:.2}) adv=({d:.2},{d:.2}) diff=({d:.2},{d:.2}) current cp={x}, cps={s}", .{ - cell_offset.cluster, - x_offset, - position.y, - advance_x_offset, - advance_y_offset, - x_offset_diff, - y_offset_diff, - state.codepoints.items[index].codepoint, - formatted_cps, - }); + if (positions_differ) { + log.warn("position differs from advance: cluster={d} pos=({d:.2},{d:.2}) adv=({d:.2},{d:.2}) diff=({d:.2},{d:.2}) current cp={x}, cps={s}", .{ + cell_offset.cluster, + x_offset, + position.y, + advance_x_offset, + advance_y_offset, + x_offset_diff, + y_offset_diff, + state.codepoints.items[index].codepoint, + formatted_cps, + }); + } + + if (position_y_differs) { + log.warn("position.y differs from old offset.y: cluster={d} pos=({d:.2},{d:.2}) run_offset=({d:.2},{d:.2}) cell_offset=({d:.2},{d:.2}) old offset.y={d:.2} current cp={x}, cps={s}", .{ + cell_offset.cluster, + x_offset, + position.y, + run_offset_x, + run_offset_y, + cell_offset.x, + cell_offset_y, + old_offset_y, + state.codepoints.items[index].codepoint, + formatted_cps, + }); + } } } }; From d38558aee10fb444c6eb7e9d38b04b777035cacd Mon Sep 17 00:00:00 2001 From: Jacob Sandlund Date: Mon, 5 Jan 2026 09:34:17 -0500 Subject: [PATCH 3/6] =?UTF-8?q?Show=20current=20cp=20with=20=E2=96=B8=20in?= =?UTF-8?q?=20list=20of=20cps?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/font/shaper/coretext.zig | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/font/shaper/coretext.zig b/src/font/shaper/coretext.zig index 4b8b05419..f1c643848 100644 --- a/src/font/shaper/coretext.zig +++ b/src/font/shaper/coretext.zig @@ -663,6 +663,7 @@ pub const Shaper = struct { var allocating = std.Io.Writer.Allocating.init(alloc); const writer = &allocating.writer; const codepoints = state.codepoints.items; + const current_cp = state.codepoints.items[index].codepoint; var last_cluster: ?u32 = null; for (codepoints) |cp| { if ((cp.cluster == cell_offset.cluster or cp.cluster == cell_offset.cluster - 1 or cp.cluster == cell_offset.cluster + 1) and @@ -673,6 +674,9 @@ pub const Shaper = struct { try writer.writeAll(" "); } } + if (cp.cluster == cell_offset.cluster and cp.codepoint == current_cp) { + try writer.writeAll("▸"); + } try writer.print("\\u{{{x}}}", .{cp.codepoint}); last_cluster = cp.cluster; } @@ -688,7 +692,7 @@ pub const Shaper = struct { const formatted_cps = try allocating.toOwnedSlice(); if (positions_differ) { - log.warn("position differs from advance: cluster={d} pos=({d:.2},{d:.2}) adv=({d:.2},{d:.2}) diff=({d:.2},{d:.2}) current cp={x}, cps={s}", .{ + log.warn("position differs from advance: cluster={d} pos=({d:.2},{d:.2}) adv=({d:.2},{d:.2}) diff=({d:.2},{d:.2}) cps = {s}", .{ cell_offset.cluster, x_offset, position.y, @@ -696,13 +700,12 @@ pub const Shaper = struct { advance_y_offset, x_offset_diff, y_offset_diff, - state.codepoints.items[index].codepoint, formatted_cps, }); } if (position_y_differs) { - log.warn("position.y differs from old offset.y: cluster={d} pos=({d:.2},{d:.2}) run_offset=({d:.2},{d:.2}) cell_offset=({d:.2},{d:.2}) old offset.y={d:.2} current cp={x}, cps={s}", .{ + log.warn("position.y differs from old offset.y: cluster={d} pos=({d:.2},{d:.2}) run_offset=({d:.2},{d:.2}) cell_offset=({d:.2},{d:.2}) old offset.y={d:.2} cps = {s}", .{ cell_offset.cluster, x_offset, position.y, @@ -711,7 +714,6 @@ pub const Shaper = struct { cell_offset.x, cell_offset_y, old_offset_y, - state.codepoints.items[index].codepoint, formatted_cps, }); } From 41f63384f54c429ee873faeed41c9aba53218441 Mon Sep 17 00:00:00 2001 From: Jacob Sandlund Date: Mon, 5 Jan 2026 09:59:37 -0500 Subject: [PATCH 4/6] Turn off debugging --- src/font/shaper/coretext.zig | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/font/shaper/coretext.zig b/src/font/shaper/coretext.zig index f1c643848..a1e0c0129 100644 --- a/src/font/shaper/coretext.zig +++ b/src/font/shaper/coretext.zig @@ -390,8 +390,8 @@ pub const Shaper = struct { var cell_offset: CellOffset = .{}; // For debugging positions, turn this on: - var run_offset_y: f64 = 0.0; - var cell_offset_y: f64 = 0.0; + //var run_offset_y: f64 = 0.0; + //var cell_offset_y: f64 = 0.0; // Clear our cell buf and make sure we have enough room for the whole // line of glyphs, so that we can just assume capacity when appending @@ -449,11 +449,11 @@ pub const Shaper = struct { }; // For debugging positions, turn this on: - cell_offset_y = run_offset_y; + //cell_offset_y = run_offset_y; } // For debugging positions, turn this on: - try self.debugPositions(alloc, run_offset_x, run_offset_y, cell_offset, cell_offset_y, position, index); + //try self.debugPositions(alloc, run_offset_x, run_offset_y, cell_offset, cell_offset_y, position, index); const x_offset = position.x - cell_offset.x; @@ -469,7 +469,7 @@ pub const Shaper = struct { run_offset_x += advance.width; // For debugging positions, turn this on: - run_offset_y += advance.height; + //run_offset_y += advance.height; } } From 7aec7effea33b39bb8caabda26e6eaad0486815f Mon Sep 17 00:00:00 2001 From: Jacob Sandlund Date: Mon, 5 Jan 2026 10:12:05 -0500 Subject: [PATCH 5/6] Add test for Tai Tham letter position.y difference --- src/font/shaper/coretext.zig | 60 ++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/src/font/shaper/coretext.zig b/src/font/shaper/coretext.zig index a1e0c0129..bcf1fe879 100644 --- a/src/font/shaper/coretext.zig +++ b/src/font/shaper/coretext.zig @@ -1533,6 +1533,66 @@ test "shape Tai Tham vowels (position differs from advance)" { try testing.expectEqual(@as(usize, 1), count); } +test "shape Tai Tham letters (position.y differs from advance)" { + const testing = std.testing; + const alloc = testing.allocator; + + // We need a font that supports Tai Tham for this to work, if we can't find + // Noto Sans Tai Tham, which is a system font on macOS, we just skip the + // test. + var testdata = testShaperWithDiscoveredFont( + alloc, + "Noto Sans Tai Tham", + ) catch return error.SkipZigTest; + defer testdata.deinit(); + + var buf: [32]u8 = undefined; + var buf_idx: usize = 0; + + // First grapheme cluster: + buf_idx += try std.unicode.utf8Encode(0x1a49, buf[buf_idx..]); // HA + buf_idx += try std.unicode.utf8Encode(0x1a60, buf[buf_idx..]); // SAKOT + // Second grapheme cluster, combining with the first in a ligature: + buf_idx += try std.unicode.utf8Encode(0x1a3f, buf[buf_idx..]); // YA + buf_idx += try std.unicode.utf8Encode(0x1a69, buf[buf_idx..]); // U + + // Make a screen with some data + var t = try terminal.Terminal.init(alloc, .{ .cols = 30, .rows = 3 }); + defer t.deinit(alloc); + + // Enable grapheme clustering + t.modes.set(.grapheme_cluster, true); + + var s = t.vtStream(); + defer s.deinit(); + try s.nextSlice(buf[0..buf_idx]); + + var state: terminal.RenderState = .empty; + defer state.deinit(alloc); + try state.update(alloc, &t); + + // Get our run iterator + var shaper = &testdata.shaper; + var it = shaper.runIterator(.{ + .grid = testdata.grid, + .cells = state.row_data.get(0).cells.slice(), + }); + var count: usize = 0; + while (try it.next(alloc)) |run| { + count += 1; + + const cells = try shaper.shape(run); + try testing.expectEqual(@as(usize, 3), cells.len); + try testing.expectEqual(@as(u16, 0), cells[0].x); + try testing.expectEqual(@as(u16, 0), cells[1].x); + try testing.expectEqual(@as(u16, 1), cells[2].x); // U from second grapheme + + // The U glyph renders at a y below zero + try testing.expectEqual(@as(i16, -3), cells[2].y_offset); + } + try testing.expectEqual(@as(usize, 1), count); +} + test "shape box glyphs" { const testing = std.testing; const alloc = testing.allocator; From 15899b70fb111d5ba6206a5cb329444ed64f49c2 Mon Sep 17 00:00:00 2001 From: Jacob Sandlund Date: Mon, 5 Jan 2026 10:35:22 -0500 Subject: [PATCH 6/6] simplify run_offset_x comment --- src/font/shaper/coretext.zig | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/font/shaper/coretext.zig b/src/font/shaper/coretext.zig index bcf1fe879..c8822a373 100644 --- a/src/font/shaper/coretext.zig +++ b/src/font/shaper/coretext.zig @@ -382,8 +382,7 @@ pub const Shaper = struct { const line = typesetter.createLine(.{ .location = 0, .length = 0 }); self.cf_release_pool.appendAssumeCapacity(line); - // This keeps track of the current x offset (sum of advance.width) for - // a run. + // This keeps track of the current x offset (sum of advance.width) var run_offset_x: f64 = 0.0; // This keeps track of the cell starting x and cluster.