Fix off-by-one error & adjust overline pos in cell height mod (#8022)

I noticed that there was an off-by-one error in cell height adjustment
when the number of pixels to add/subtract is odd. The metrics measured
from the top would be shifted by one less than they should, so, for
example, the underline position would move one pixel closer to the
baseline than it had been (or one pixel further away if subtracting).

Also noticed that the overline position was missing here, so added that.
pull/8539/head
Mitchell Hashimoto 2025-09-05 13:10:17 -07:00 committed by GitHub
commit 6b21662219
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 37 additions and 19 deletions

View File

@ -281,17 +281,25 @@ pub fn apply(self: *Metrics, mods: ModifierSet) void {
// centered in the cell. // centered in the cell.
if (comptime tag == .cell_height) { if (comptime tag == .cell_height) {
// We split the difference in half because we want to // We split the difference in half because we want to
// center the baseline in the cell. // center the baseline in the cell. If the difference
// is odd, one more pixel is added/removed on top than
// on the bottom.
if (new > original) { if (new > original) {
const diff = (new - original) / 2; const diff = new - original;
self.cell_baseline +|= diff; const diff_bottom = diff / 2;
self.underline_position +|= diff; const diff_top = diff - diff_bottom;
self.strikethrough_position +|= diff; self.cell_baseline +|= diff_bottom;
self.underline_position +|= diff_top;
self.strikethrough_position +|= diff_top;
self.overline_position +|= @as(i32, @intCast(diff_top));
} else { } else {
const diff = (original - new) / 2; const diff = original - new;
self.cell_baseline -|= diff; const diff_bottom = diff / 2;
self.underline_position -|= diff; const diff_top = diff - diff_bottom;
self.strikethrough_position -|= diff; self.cell_baseline -|= diff_bottom;
self.underline_position -|= diff_top;
self.strikethrough_position -|= diff_top;
self.overline_position -|= @as(i32, @intCast(diff_top));
} }
} }
}, },
@ -509,19 +517,24 @@ test "Metrics: adjust cell height smaller" {
var set: ModifierSet = .{}; var set: ModifierSet = .{};
defer set.deinit(alloc); defer set.deinit(alloc);
try set.put(alloc, .cell_height, .{ .percent = 0.5 }); // We choose numbers such that the subtracted number of pixels is odd,
// as that's the case that could most easily have off-by-one errors.
// Here we're removing 25 pixels: 12 on the bottom, 13 on top.
try set.put(alloc, .cell_height, .{ .percent = 0.75 });
var m: Metrics = init(); var m: Metrics = init();
m.cell_baseline = 50; m.cell_baseline = 50;
m.underline_position = 55; m.underline_position = 55;
m.strikethrough_position = 30; m.strikethrough_position = 30;
m.overline_position = 0;
m.cell_height = 100; m.cell_height = 100;
m.cursor_height = 100; m.cursor_height = 100;
m.apply(set); m.apply(set);
try testing.expectEqual(@as(u32, 50), m.cell_height); try testing.expectEqual(@as(u32, 75), m.cell_height);
try testing.expectEqual(@as(u32, 25), m.cell_baseline); try testing.expectEqual(@as(u32, 38), m.cell_baseline);
try testing.expectEqual(@as(u32, 30), m.underline_position); try testing.expectEqual(@as(u32, 42), m.underline_position);
try testing.expectEqual(@as(u32, 5), m.strikethrough_position); try testing.expectEqual(@as(u32, 17), m.strikethrough_position);
try testing.expectEqual(@as(i32, -13), m.overline_position);
// Cursor height is separate from cell height and does not follow it. // Cursor height is separate from cell height and does not follow it.
try testing.expectEqual(@as(u32, 100), m.cursor_height); try testing.expectEqual(@as(u32, 100), m.cursor_height);
} }
@ -532,19 +545,24 @@ test "Metrics: adjust cell height larger" {
var set: ModifierSet = .{}; var set: ModifierSet = .{};
defer set.deinit(alloc); defer set.deinit(alloc);
try set.put(alloc, .cell_height, .{ .percent = 2 }); // We choose numbers such that the added number of pixels is odd,
// as that's the case that could most easily have off-by-one errors.
// Here we're adding 75 pixels: 37 on the bottom, 38 on top.
try set.put(alloc, .cell_height, .{ .percent = 1.75 });
var m: Metrics = init(); var m: Metrics = init();
m.cell_baseline = 50; m.cell_baseline = 50;
m.underline_position = 55; m.underline_position = 55;
m.strikethrough_position = 30; m.strikethrough_position = 30;
m.overline_position = 0;
m.cell_height = 100; m.cell_height = 100;
m.cursor_height = 100; m.cursor_height = 100;
m.apply(set); m.apply(set);
try testing.expectEqual(@as(u32, 200), m.cell_height); try testing.expectEqual(@as(u32, 175), m.cell_height);
try testing.expectEqual(@as(u32, 100), m.cell_baseline); try testing.expectEqual(@as(u32, 87), m.cell_baseline);
try testing.expectEqual(@as(u32, 105), m.underline_position); try testing.expectEqual(@as(u32, 93), m.underline_position);
try testing.expectEqual(@as(u32, 80), m.strikethrough_position); try testing.expectEqual(@as(u32, 68), m.strikethrough_position);
try testing.expectEqual(@as(i32, 38), m.overline_position);
// Cursor height is separate from cell height and does not follow it. // Cursor height is separate from cell height and does not follow it.
try testing.expectEqual(@as(u32, 100), m.cursor_height); try testing.expectEqual(@as(u32, 100), m.cursor_height);
} }