Center glyphs vertically when lineSpacing > 1

[Coiggahou2002]

What

When lineSpacing > 1, vertically center the glyph within the (now taller) cell, splitting the extra space above and below — instead of leaving it all above the text.

Rebased onto current main (cb3c863), after the new-io work landed. Single commit.

Why

lineSpacing scales the cell height in computeFontDimensions():

let cellHeight = ceil((lineAscent + lineDescent + lineLeading) * _lineSpacing)

but every renderer positioned the baseline with the un-scaled offset:

let yOffset = ceil(lineDescent + lineLeading)

Since the row is drawn from the bottom up, the glyph baseline stays pinned to the bottom of the cell and all the added spacing ends up above the text. At larger values (e.g. 1.4–1.6) the lines look bottom-heavy. Splitting the extra evenly matches how iTerm2 and WezTerm distribute their line spacing (and the CSS half-leading model).

Change

One definition, CellGeometry.baselineOffset, that every path reads:

/// Text geometry derived from a cell's size. Not isolated to the main actor:
/// the snapshot renderers do this arithmetic on their own threads, from the
/// fonts and cell dimensions captured for the frame.
enum CellGeometry {
    static func baselineOffset (normalFont: CTFont, cellHeight: CGFloat) -> CGFloat
    {
        let descent = CTFontGetDescent (normalFont)
        let leading = CTFontGetLeading (normalFont)
        let naturalHeight = ceil (CTFontGetAscent (normalFont) + descent + leading)
        let extra = max (0, cellHeight - naturalHeight)
        return ceil (descent + leading) + floor (extra / 2)
    }
}

It is a plain non-isolated namespace rather than a TerminalView member because the snapshot renderers do this arithmetic off the main actor — hanging it on the (now @MainActor) view fails strict-concurrency checking with sending 'normalFont' risks causing data races. SnapshotRenderContext.baselineOffset and TerminalView.baselineOffset are thin accessors over it, so a frame's text, caret, and scaled glyphs are guaranteed to share one baseline.

Every place that computed its own ceil(descent + leading) now reads it. There were eight, up from four when this PR was first opened:

site
drawTerminalContents	via SnapshotRenderContext.baselineOffset
CaretView.drawCursor	so the glyph inside the caret stays on the row's baseline
MetalTerminalRenderer.buildDrawDataPass
MetalTerminalRenderer.buildCursorDrawData	lineDescent/lineLeading parameters dropped — it already receives the SnapshotRenderContext they were derived from
GlyphSlotFit.calculate	its baselineFromBottom must agree with the draw-time offset or scaled wide glyphs stop being centered on it
TerminalView.glyphSlotFit(font:glyph:columnWidth:policy:)	new since the last revision
quickLook's definition popover	new since the last revision; it anchors on the word's baseline, so its position was already wrong under lineSpacing > 1
GlyphMetricsParityTests' scaledFit helper	see below

On rounding

The classic ceil(descent + leading) term is kept intact and the glyph's share of the extra is added as a whole number of points on top, rather than rounding the sum (ceil(descent + leading + extra/2), which is what the previous revision did).

Two reasons. It keeps the baseline on the same pixel grid the old offset landed on. And it makes lineSpacing == 1 exact rather than approximate: cellHeight is snapped to the pixel grid, so on a fractional backing scale it can sit a point or two above the font's natural height even at the default, and rounding the sum would then quietly move the baseline for someone who never touched lineSpacing.

The parity test below is what caught this: with Menlo 16pt (natural line 19pt) and its synthetic 20pt cell, extra is exactly 1.0 and ceil(3.77 + 0.5) pushed the baseline up a full point.

The test that had its own copy

GlyphMetricsParityTests.scaledFit hardcoded ceil(CTFontGetDescent(normalFont) + CTFontGetLeading(normalFont)) as its baselineFromBottom, while the reference side called GlyphSlotFit.calculate(font:...). Once the two disagreed the test failed by 41pt on the 100×100 synthetic cell. Both sides now derive the baseline from CellGeometry, which is what the test set out to compare in the first place — the scaled pixel-space path against the point-space reference, not two different baselines.

Verification

swift build clean, 990 tests pass (Xcode 26 / Swift 6.3, macOS).

New CellBaselineTests pins both halves of the invariant:

• No regression at the default. baselineOffset is equal to the old ceil(descent + leading) across eight font/size combinations (Monaco, Menlo, SF Mono, Courier; 10–24pt, including a fractional 13.5pt), and for any cell within two points of the natural line height.
• Whole-point, bounded, monotonic. Sweeping extra from 0 to 40pt in 0.5pt steps, the offset always lands on an integer point, never decreases, and never pushes the ascender out of the cell.
• Actually centered. At lineSpacing 1.2 / 1.4 / 1.6 / 2.0, the space under the descender and the space over the ascender agree to within one rounding step.
• Renderers agree. The snapshot geometry the Metal and CoreGraphics paths use matches the view's at 1.0 / 1.3 / 1.6 — the caret-vs-text divergence class from the first review, now asserted rather than eyeballed.

Also verified previously, and unchanged by the rebase: at lineSpacing == 1.6 the caret-drawn glyph sits on the same baseline as the surrounding text, and DECDHL double-height rows join cleanly at 1.5 (both halves shift by the same pre-scale offset and the scale pivots sit on the shared row boundary).

[migueldeicaza]

I like the idea in principle, but this is not in sync with the caretView, we recently had to address that divergence, should be easy to test if you use a text editor and go to that line, the cursor is likely out of place,

This also does not cover metal.

Lastly, double-height characters might need to be addressed - check glyphSlotFit for details.

[Coiggahou2002]

Thanks for the review — all three points addressed, by restructuring rather than patching each site.

I introduced a shared TerminalView.baselineOffset (documented next to computeFontDimensions) that splits the lineSpacing extra evenly above/below the glyph. Every place that previously computed its own ceil(descent + leading) now reads it: drawTerminalContents, CaretView.drawCursor, the Metal renderer (both buildDrawData and buildCursorDrawData, which no longer takes lineDescent/lineLeading parameters), and glyphSlotFit's vertical-centering dy. So the caret/text divergence class is gone at the source, not just for this change — there is no local copy of the formula left to drift.

On your specific points:

1. caretView — confirmed your prediction: with the first revision, the glyph drawn inside the caret sat extra/2 lower than the row text. Now both read baselineOffset; verified with offscreen renders at lineSpacing == 1.6 with the cursor on a descender glyph.
2. Metal — both the text and cursor paths in MetalTerminalRenderer now use the shared property.
3. double-height / glyphSlotFit — glyphSlotFit's dy is applied on top of the draw-time offset, so its old hardcoded ceil(descent + leading) would have pushed scaled glyphs extra/2 above center; it now references baselineOffset too, keeping the "center ink at cellHeight/2" invariant exact. DECDHL rows verified joining cleanly at lineSpacing == 1.5 (both halves shift by the same pre-scale offset and the 2× pivots sit on the shared row boundary).

Also rebased onto current main. At lineSpacing == 1.0 the offscreen render output is byte-identical to main, and all 452 tests pass.

[migueldeicaza]

Thanks, let me check it again.

[Coiggahou2002]

Rebased onto current main (cb3c863) — the new-io work had moved all three files out from under this, so I rewrote it against the snapshot architecture rather than resolving the conflicts mechanically. Still one commit. swift build clean, 990 tests pass.

Three things worth flagging:

The formula had spread from 4 copies to 8. Two are new code: TerminalView.glyphSlotFit(font:glyph:columnWidth:policy:), and quickLook's definition popover in MacTerminalView — that one anchors on the word's baseline, so its position is already wrong under lineSpacing > 1 independently of this PR. All eight now read one CellGeometry.baselineOffset.

It's a non-isolated namespace, not a TerminalView member. The version you reviewed hung baselineOffset on the view. That no longer compiles after the strict-concurrency work: GlyphSlotFit.calculate runs on the renderer's thread, so calling a @MainActor member fails with sending 'normalFont' risks causing data races. CellGeometry takes just the font and the cell height, which is what SnapshotRenderContext already captures per frame — it fits the snapshot data flow better than the original did.

GlyphMetricsParityTests had its own copy of the formula, and finding it changed the rounding. scaledFit hardcoded ceil(descent + leading) as its baselineFromBottom while the reference side went through GlyphSlotFit.calculate, so the two sides were comparing different baselines. Pointing both at CellGeometry then failed by exactly 1pt: with Menlo 16pt (natural line 19pt) and the test's synthetic 20pt cell, extra is exactly 1.0, and the previous revision's ceil(descent + leading + extra/2) rounded that half-point up to a whole one.

So the split is now ceil(descent + leading) + floor(extra / 2) — the old term kept intact, the glyph's share added as whole points. That keeps the baseline on the pixel grid the old offset landed on, and makes lineSpacing == 1 exact instead of approximate: cellHeight is pixel-snapped, so on a fractional backing scale it can sit a point or two above the natural line height even at the default, and rounding the sum would have moved the baseline for people who never touch lineSpacing.

Added CellBaselineTests to pin that down instead of leaving it to offscreen renders: the offset is identical to the old ceil(descent + leading) across eight font/size combinations and for any cell within two points of natural height; sweeping the extra from 0 to 40pt it stays integral, monotonic, and never pushes the ascender out of the cell; at 1.2/1.4/1.6/2.0 the space below the descender and above the ascender agree to within a rounding step; and the snapshot geometry the two renderers use matches the view's — the caret-vs-text divergence you predicted, now asserted rather than eyeballed.