Skip to content

Fix font metrics and add procedural box-drawing for U+2500..U+259F#164

Open
sauyon wants to merge 17 commits into
coder:mainfrom
sauyon:sauyon/confident-aryabhata-2878c3
Open

Fix font metrics and add procedural box-drawing for U+2500..U+259F#164
sauyon wants to merge 17 commits into
coder:mainfrom
sauyon:sauyon/confident-aryabhata-2878c3

Conversation

@sauyon
Copy link
Copy Markdown

@sauyon sauyon commented May 9, 2026

Summary

Fixes two visible rendering bugs and ports Ghostty's procedural box-drawing for U+2500..U+259F.

Before:

  • Letter descenders (g, p, y, j, q) clipped at the bottom of cells.
  • Box-drawing glyphs (, , ┌─┐, ═══, ╔╗╚╝) and block elements (, ▄▀░▒▓) had visible inter-cell gaps.

After:

  • Cell metrics fit every glyph in the font, descender-safe.
  • All 160 glyphs in U+2500..U+259F tile across cells with no gaps and no font-dependent visual variation.

What changed

Font metrics ([renderer.ts])

  • Cell width now uses the font's natural advance (measureText('M').width, no rounding). The previous Math.ceil was making cells wider than the font's advance, leaving sub-pixel gaps between adjacent box-drawing glyphs.
  • Cell height now takes max(fontBoundingBox*, actualBoundingBox*) independently for ascent and descent, ceiled. This handles fonts/browsers that under-report either metric.
  • boxThickness is measured from the font's actual glyph extent (Ghostty: box.zig derives from font's underline thickness; Canvas2D doesn't expose that, so measuring is the closest equivalent).
  • Canvas backing-store size uses Math.round(cssDim * dpr) so the resize-detection check stays stable with fractional CSS-pixel widths.
  • Fixed clear() passing device-pixel canvas.width/canvas.height to a CSS-scaled context (pre-existing bug, newly visible).
  • Removed redundant canvas resize in terminal.ts setFontSize / setFontFamily / resize that was stomping the renderer's correct DPR-scaled dimensions (pre-existing bug from feat: support dynamic font resizing #80, exposed by the fractional-width change).

Procedural box-drawing ([box-drawing.ts])

New module that renders U+2500..U+259F (160 glyphs) as canvas paths sized to the cell, instead of going through the font. This is the standard approach in Alacritty, kitty, wezterm, Ghostty native, and Windows Terminal — see #163 for why a glyph atlas isn't a win in Canvas2D.

Ports the algorithm from Ghostty's box.zig and block.zig, including:

  • Junction-aware arm endpoints (`up_bottom`, `down_top`, `left_right`, `right_left`) so heavy crossbars cover light arms cleanly and double-line corners (╔╗╚╝╠╣╦╩╬) form proper inner Ls instead of crossing parallels.
  • Cubic-Bezier arcs (╭╮╯╰) reaching the cell-edge midpoint so they join flush to neighboring /.
  • Tile-aware dashes with the horizontal-vs-vertical asymmetry from box.zig:878-881 (vertical pushes the full extra gap to the bottom, horizontal puts half-gaps on each side).
  • Diagonals (╱╲╳) with sub-pixel overshoot for clean anti-aliased tiling.
  • Heavy = 2 × light, double = 3 × light (two parallels with a 1-light gap), matching Ghostty's Thickness model.
  • Block elements via the block(alignment, wFrac, hFrac) + quadrant({tl,tr,bl,br}) helpers from block.zig.

Tests ([box-drawing.test.ts])

+34 new tests using a recording-context stub (Canvas2D in happy-dom doesn't rasterize, so we capture and assert the structured op stream):

  • Exhaustive coverage check for all 160 codepoints in the range.
  • Per-glyph shape assertions for every quadrant glyph (catches dispatch swaps).
  • Concrete coordinate assertions for tricky cases (╔ junction-aware corner, ┄ horizontal half-gap layout, ┆ vertical bottom-gap layout).
  • Heavy-dash degenerate fallback uses light thickness, matching Ghostty.

Why procedural?

Three rounds of independent code review verified the port branch-by-branch against box.zig/block.zig. Net code is ~950 lines of TypeScript covering 160 codepoints, all sharing the cell metrics already measured for text rendering. Tradeoff vs the simpler "just use the font" approach is captured in #163 — Canvas2D fillRect/stroke is GPU-accelerated; the cache-and-drawImage shortcut native renderers use buys nothing here, so direct rendering is the right model.

Test plan

  • `bun run typecheck` passes
  • `bun run lint` passes
  • `bun test` — 379 tests pass (was 345; +34 new)
  • `bun run build:lib` produces a working bundle
  • Visual verification at 12pt, 14pt, 20pt, 28pt: box drawing tiles seamlessly, descenders not clipped, selection highlighting preserves descenders, italic/bold/underline/strikethrough render correctly
  • Verified against Ghostty source for U+2500..U+259F, including all junction edge cases

Related

🤖 Generated with Claude Code

sauyon and others added 17 commits March 22, 2026 06:27
…oder#141)

PageList only zeroed page buffers in debug/safe builds, relying on the
OS to guarantee zeroed memory in release builds. On WASM there is no OS
guarantee — the allocator reuses freed memory as-is. This caused two bugs:

1. Stale cell data from freed terminals appearing in newly created ones
2. WASM memory corruption after freeing terminals that processed
   multi-codepoint grapheme clusters (flag emoji, skin tones, ZWJ
   sequences), crashing all subsequent terminal writes

The fix makes @Memset(page_buf, 0) unconditional on WASM in both
initPages and createPageExt.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Callers that want focus after opening should call terminal.focus()
explicitly. Auto-focusing unconditionally causes focus to be stolen
from other elements in multi-terminal UIs (e.g. when a background
output terminal is opened while the user is typing in an input bar).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When the user has scrolled up to review earlier output, keep the
viewport locked on the same content as new lines arrive instead of
snapping back to the bottom.

Save the scrollback length before each write and adjust `viewportY`
by the delta afterward.  Clamp to the current scrollback length in
case old lines are dropped by the buffer limit.

When the user is already at the bottom (`viewportY === 0`), the
adjustment is skipped and the viewport naturally follows new output.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
scrollback_limit is passed to ghostty's Terminal.max_scrollback, which
is in bytes. The low-level config types described it as "number of
scrollback lines", which is misleading — a caller passing 10,000
expecting lines gets 10,000 bytes and falls below the 2-page PageList
floor.

Only the low-level GhosttyTerminalConfig / TerminalConfig docs are
corrected here. The xterm.js-compat ITerminalOptions.scrollback field
still inherits xterm.js-compat framing and a misleadingly xterm.js-
shaped default (1000) despite plumbing directly to a bytes-valued
field; fixing that properly requires a lines-to-bytes conversion in
buildWasmConfig, which belongs in a separate change.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add ITerminalOptions.wasmTerm to let callers keep a wasm terminal alive
independently of the Terminal wrapper's lifetime. When provided:

- The Terminal constructor adopts the injected wasm terminal instead of
  allocating a fresh one; cols/rows default to the injected dims.
- Terminal.dispose() skips the free() call — the caller retains ownership.
- Terminal.reset() throws to preserve the ownership contract (reset would
  otherwise free a buffer the caller still holds).

Motivates a terminal-emulator client that needs to preserve buffer state
across view-layer mount cycles (e.g. reparenting into a different DOM
tree without losing scrollback, cursor, alt-screen mode, or hyperlinks).

Tests cover: adoption identity, buffer preservation across dispose +
second-wrapper adoption, writes reaching the injected buffer, reset
throwing, dimension defaulting / explicit resize, and regression of the
normal owned-wasmTerm path.

Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
Deletes the "printable character" and "simple special keys" fast paths
from InputHandler.handleKeyDown and routes every keydown through the
Ghostty WASM key encoder.

The old fast paths were a simplified model that diverged from both
xterm.js and Ghostty's encoder for several keys:

- Home and End ignored DECCKM (application cursor mode). xterm.js
  emits \x1b[H in normal mode and \x1bOH in application mode; the
  fast path emitted \x1b[H always.
- Shift+Home / Shift+End / Shift+PageUp / Shift+PageDown /
  Shift+F1..F12 dropped the Shift modifier. xterm.js encodes it into
  the CSI sequence (e.g. \x1b[1;2H for Shift+Home); the fast path
  emitted the plain unmodified sequence.
- Non-BMP characters (surrogate-pair emoji) were dropped entirely
  because of a length === 1 filter in the fallback utf8 path.

Routing through the encoder brings ghostty-web into line with xterm.js
on these. It also picks up three behaviors that xterm.js doesn't
implement:

- Shift+Enter distinguishable from Enter (\x1b[27;2;13~ instead of
  bare \r). This is a deliberate divergence from xterm.js, which
  emits \r for both. Modern line editors and REPLs use fixterms-style
  encoding and expect Shift+Enter to be distinguishable for
  multi-line input.
- Kitty keyboard protocol flags affect every key when enabled.
- xterm modifyOtherKeys state 2 affects every key when enabled.

Consumers who need byte-for-byte xterm.js behavior on specific keys
can intercept in attachCustomKeyEventHandler and emit the desired
bytes via Terminal.input(data, /*wasUserInput*/ true). README.md's
"Keyboard encoding" note shows the pattern.

Note: preventDefault / stopPropagation fire on any key mapKeyCode
recognizes, before the encode attempt — so a failed or empty encode
drops the keystroke silently rather than letting browser defaults
fire (F11 fullscreen, Ctrl+W close tab, etc.). Deliberate divergence
from native Ghostty's keyCallback policy of returning .ignored on
empty encode; documented in a code comment.

Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
…00..U+259F

Cell metrics:
- Width: use the font's natural advance (no Math.ceil) so glyphs designed
  to tile across cells don't leave seams. Canvas backing-store dimensions
  are rounded only when sizing the canvas.
- Height: take max(fontBoundingBox*, actualBoundingBox*) for ascent and
  descent independently from a probe string with descenders ('Mgjpqy│'),
  then ceil. Canvas2D has no API for "design metrics excluding leading,"
  so combining both metric families is the most reliable way to fit
  every glyph regardless of which source a given font under-reports.

Box drawing (U+2500..U+257F) and block elements (U+2580..U+259F) are
now drawn as canvas paths instead of through the font. The font path
left visible gaps because:
  - the font's advance often differs from our cell width,
  - the cell height we need for descender safety rarely matches the
    proportions a font designer chose for U+2502 / U+2588.

Procedural rendering is the standard approach in Alacritty, kitty,
wezterm, Ghostty native, and Windows Terminal; this implementation
ports the algorithms from Ghostty's box.zig:
  - junction-aware arm endpoints (up_bottom, down_top, left_right,
    right_left) so heavy crossbars cover light arms cleanly and double
    lines form proper inner-L corners (╔╗╚╝╠╣╦╩╬, all heavy/light
    mixed junctions ┡┹╆╃ etc.),
  - quarter-circle arcs (╭╮╯╰) as cubic Beziers reaching the cell-edge
    midpoint so they join flush to neighboring straight cells,
  - dashed/dotted lines (┄┅┈┉┆┇┊┋╌╍╎╏) with integer-pixel gap
    distribution so multi-cell dashed runs tile cleanly,
  - diagonals (╱╲╳) with sub-pixel overshoot so anti-aliasing covers
    cell corners exactly,
  - heavy = 2 × light thickness, double = three light strokes wide
    with a one-light gap between the parallels.

Code is split into lib/box-drawing/{index,common,blocks,lines}.ts,
mirroring Ghostty's box.zig / block.zig / common.zig layout.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previously light/heavy thickness were a fixed fraction of cell height
(0.07h for light), which doesn't match the font designer's intent. The
font's own U+2500 '─' glyph already encodes the chosen box-stroke
weight; measuring its `actualBoundingBox*` extent recovers it directly.

Per-font measurements at 14pt confirm the variation:
  Monaco       1.27 px
  Menlo        1.18 px
  Consolas     1.18 px
  Courier New  1.00 px

`measureFont` now stores `boxThickness` on `FontMetrics`, and the
box-drawing/block-element renderer takes it as `lightPx`. Heavy is
2 × light; double lines are two parallel light strokes separated by
one light gap (3 × light total) — same model Ghostty uses.

Falls back to ~7% of font size when the font lacks U+2500 (some
browsers report 0 width for missing glyphs).

Note on sprite-atlas caching (the other "skip" item in the prior
review): a Canvas2D microbench across 10K mixed-glyph renders showed
direct rendering is 2-9% *faster* than an offscreen-canvas atlas with
`drawImage`. Canvas2D `fillRect`/`stroke` are already GPU-accelerated;
the cache lookup + `drawImage` path adds overhead without buying
batching. Native renderers (Ghostty, Alacritty) need atlases because
they render through their own pipelines without GPU primitives at the
call level. Not implementing the atlas.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace per-codepoint inline `fillRect` calls with the same helper
trio Ghostty's block.zig uses:

- Named fraction constants (ONE_EIGHTH, ONE_QUARTER, …, SEVEN_EIGHTHS)
  matching block.zig:19-28.
- `block(alignment, wFrac, hFrac)` for axis-aligned partial-cell fills,
  matching `blockShade` (block.zig:121-152). The `Alignment` is a
  string union ('upper' | 'lower' | 'left' | 'right') mirroring
  Ghostty's named Alignment constants (common.zig:92-95).
- `quadrant({tl, tr, bl, br})` for the ▖▗▘▙▚▛▜▝▞▟ family, matching
  Ghostty's `quadrant` (block.zig:168-177).
- `fullShade(color, alpha)` for ░▒▓.

Switch arms become one-liners that read as the family they belong to
(lower-eighths progression, left-eighths progression, single-quadrant,
multi-quadrant), instead of 32 hand-rolled fillRect calls with
scattered (h*5)/8 / (h*3)/4 literals.

Pure refactor, no behavior change. All 32 codepoints produce the same
shape they did before; verified visually against the previous output.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Code review caught two real bugs against Ghostty's `box.zig`:

1. **Vertical dashes started at the wrong offset.** Ghostty's
   `dashVertical` (box.zig:907-909) starts at y=0 and pushes the full
   extra gap to the bottom, with the explicit comment that "a single
   full-sized extra gap is preferred to two half-sized ones for
   vertical to allow better joining to solid characters". Our port
   used `Math.floor(gap_width / 2)` for both axes, which mis-tiled
   vertical dashed lines (┆ ┇ ┊ ┋ ╎ ╏) against neighboring solid
   `│`/`┃` cells.

2. **`desired_gap` didn't match Ghostty.** Every dispatch site in
   `box.zig` (lines 73, 81, 89, 97, …) passes `@max(4, light)` as the
   desired gap. Our port passed plain `light`, producing tighter and
   visually different dashes at small light thicknesses.

Both fixed in `drawDashed` / `drawDashRun`.

Also adds `lib/box-drawing/box-drawing.test.ts` (24 tests) covering:
- coverage: every codepoint in U+2500..U+259F produces drawing ops
  (catches silent fallback-to-font regressions for the whole range),
- block elements: shape and edge alignment for halves, eighths,
  quadrants, and shades,
- lines: union-of-arms covers the cell, light/heavy/double thickness
  ratios, junction-aware corners (regression check that ╔ doesn't
  draw crossing parallels into the upper-left quadrant),
- arcs: stroked Bezier path, no fillRect,
- dashes: dash count per glyph + the vertical/horizontal asymmetry
  fixed in #1,
- diagonals: stroke-based, no fillRect.

Tests use a recording stub that captures every `ctx` call as a
structured op, since happy-dom's CanvasRenderingContext2D doesn't
rasterize. This catches behavioral changes without needing a real
canvas raster comparison.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collapse `lib/box-drawing/{index,common,blocks,lines}.ts` into one
`lib/box-drawing.ts`. The split-file layout was over-structuring for
the size of the module — the code is one self-contained renderer for
one Unicode range, and the cross-file imports were noise.

The single file is organized in clearly-delimited sections:
  1. Common types (Weight, N/L/H/D, heavyThickness)
  2. Public API (isBoxOrBlock, drawBoxOrBlock)
  3. Block elements U+2580..U+259F (block/quadrant/fullShade helpers
     + drawBlockElement dispatch)
  4. Box-drawing lines U+2500..U+257F (drawBoxLine dispatch + EDGES
     /DASHED/ARC tables + drawEdges/drawArc/drawDashed/drawDiagonal)

No behavior change. The renderer's import path (`./box-drawing`) still
resolves correctly. The test file moved from
`lib/box-drawing/box-drawing.test.ts` to `lib/box-drawing.test.ts`
and updated its import. All 369 tests still pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… tests

Second-pass review caught five issues:

1. **HIGH — `terminal.ts` stomps the canvas backing store** (lines 259-262
   and 722-725, pre-existing in `e879eef`). After `renderer.resize(...)`
   correctly sets both the CSS size and the device-pixel-scaled
   backing-store size, terminal.ts redundantly re-sets them with
   `metrics.width * cols` (no DPR scaling, fractional values truncated).
   With the new fractional `metrics.width` from the font-metrics fix,
   this drops sub-pixel and breaks high-DPI rendering after font/size
   changes. Fix: delete the redundant assignments — the renderer
   already handles canvas sizing.

2. **MEDIUM — vacuous regression check for ╔.** The previous "no
   crossing parallels" assertion checked whether any rect was entirely
   contained in the cell's upper-left ninth — which can never be true
   given the actual rect extents. Replaced with concrete coordinate
   assertions on all four expected rects, plus a positive check that
   the inner-corner test point isn't covered by any rect.

3. **MEDIUM — dash tests re-derived the implementation.** The horizontal
   dash assertion recomputed `min(desired_gap, floor(span/(2*count)))`
   in the test and compared it to the implementation's output, so any
   shared bug would pass. Replaced with hand-computed expected
   coordinates for a known cell size, plus invariant checks
   (half-gap-on-each-side for horizontal, full-gap-pushed-to-bottom
   for vertical per box.zig:878-881).

4. **MEDIUM — coverage test wouldn't catch dispatch swaps.** "Every
   codepoint draws something" passes if ▖ and ▗ get swapped in the
   case list. Added `test.each` per-codepoint quadrant assertions for
   all 9 quadrant glyphs, so any swap surfaces.

5. **LOW — degenerate-dash fallback used dash weight, not light.** When
   the cell is too small to hold the dash run, the fallback should
   draw a LIGHT line (Ghostty's `vlineMiddle(.light)` /
   `hlineMiddle(.light)`, box.zig:812/891), not a heavy bar for heavy
   dashes. Fixed and added a test.

Also updated the doc comment claiming `0x40/0x80/0xc0 = 0.251/0.502/
0.753` matches our `0.25/0.5/0.75` "exactly" — they differ by under
0.003. Visually indistinguishable but the comment was overstated.

379 tests pass (was 369; +10 new).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Third-pass review caught:

1. **HIGH — `renderer.clear()` mixes coordinate spaces.** After
   `resize()` calls `ctx.scale(dpr, dpr)`, all context coordinates
   are CSS pixels, but `clearRect`/`fillRect` were being passed
   `canvas.width`/`canvas.height` (device pixels). Internally clamped
   so it didn't visibly misbehave, but conceptually the same bug
   pattern as the canvas-stomp fix in `ef45ab1`. Pre-existing, but
   worth fixing in the same pass. Now divides by DPR.

2. **MEDIUM — `boxThickness` doc was misleading at typical sizes.**
   The comment listed pre-rounding measurements (Monaco @14pt → 1.27,
   Menlo → 1.18, Courier → 1.00) implying meaningful per-font
   variation, but `Math.round` collapses all three to 1 at 14pt.
   Variation does come through at larger sizes (28pt: Monaco → 3,
   Menlo → 2, Courier → 2), so the comment now uses 28pt examples
   and notes the small-font rounding behaviour explicitly.

3. **MEDIUM — quadrant `test.each` list could mislead a future
   editor.** ▙ (U+2599) is intentionally tested separately above and
   omitted from the list; added a comment so a future contributor
   doesn't add it back as a duplicate.

4. **LOW — shade-test alpha tolerance was 1e-9.** A future change to
   match Ghostty's exact `0x40/255 = 0.2509…` alpha would have
   broken the test for no good reason. Relaxed to `toBeCloseTo(0.25,
   2)` which covers both values.

Other findings from the review (gap_width edge case, saturating
subtraction in drawEdges, comment about font being set per-cell)
were either non-issues at realistic sizes, defensive-only, or
taste-level and not worth code churn.

379 tests still pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Address remaining findings from third-pass review:

- **drawDashRun gap_width lower bound** (Finding #2): Math.floor on
  fractional `span/(2*count)` could theoretically return 0 at the
  exact boundary, though the early-return guarantees `span >=
  2*count` mathematically. Added `Math.max(1, ...)` to make the
  invariant explicit, matching Ghostty's `assert(dash_width >= 1)`
  on integer arithmetic (box.zig:824).

- **Degenerate-dash fractional-threshold note** (Finding #3): Ghostty
  compares integer `cell_width < count + count`; we compare
  fractional `span`. Added a comment noting the boundary behaviour
  may differ for fractional values just above the threshold.

- **Saturating subtraction in drawEdges** (Finding #6): Ghostty uses
  `-|` so values can't go negative when light_px > cell. Our `-` can
  produce negative `h_double_top` etc. at degenerate-tiny cells.
  Added `Math.max(0, ...)` clamps to all six h_*/v_* coordinates
  that subtract.

- **resize() font-property comment** (Finding #7): Noted that
  `ctx.font` is intentionally not re-set after the canvas-width
  reset, because `renderCellText` sets it per-cell to handle italic
  /bold.

- **Coverage-test docstring** (Finding #9): Spelled out that the
  "every codepoint draws something" check is exhaustive but
  intentionally weak — it catches missing dispatch entries, not
  wrong ones; per-glyph shape assertions later in the file do the
  correctness checking.

- **renderCellText box-or-block dispatch** (Finding #10): Hoisted
  the `cell.grapheme_len === 0 && cell.codepoint && isBoxOrBlock(...)`
  predicate into a named local and collapsed the grapheme/codepoint
  fallback into a single ternary.

379 tests still pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fourth-pass review caught:

- **HIGH — `FontMetrics.boxThickness` was a TypeScript breaking
  change.** The exported interface gained `boxThickness: number` as a
  required field, which would break any downstream consumer that
  constructs or implements `FontMetrics` (test mocks, custom
  renderers). Marked the field optional and added a fallback at the
  call site (`fontSize * 0.07`) so external code that doesn't supply
  a value still gets a reasonable default. The built-in `measureFont`
  always populates it.

- **LOW — Misleading saturating-subtraction comment.** The
  `Math.max(0, ...)` clamp prevents negative coordinates but doesn't
  prevent overdraw past the cell edge when `lt > h` — same as
  Ghostty's `-|`. Updated the comment to spell that out.

- **LOW — `drawArc` center expression looked like it had an
  invariant it doesn't.** `(w - lt) / 2 + lt / 2` is verbatim from
  `box.zig:704-705` where Ghostty's integer division can differ from
  `w/2` for odd values; in JS it's mathematically equivalent.
  Annotated the line so a future reader doesn't read significance
  into the formulation.

- **LOW — `drawDashRun` widened `count` to `number`.** Restored the
  `2 | 3 | 4` narrow type from `Dashed.count` for type safety on the
  inner loop.

Other fourth-pass findings (sub-pixel leakage in Math.floor for
fractional cell heights, JSON.stringify hover-range comparison) were
either non-issues at our integer-ceiled metrics or pre-existing code
not in this PR's scope.

The reviewer also independently verified 17 random EDGES entries
against `box.zig` switch dispatch (all match), and confirmed all the
prior-round fixes (vertical-dash asymmetry, desired_gap, junction
endpoints, clear() coord space, terminal.ts canvas-stomp removal)
are correct.

379 tests still pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fifth-pass review caught:

- **`drawBoxOrBlock` / `isBoxOrBlock` weren't exported**, contradicting
  the FontMetrics doc that referenced "external code that calls
  `drawBoxOrBlock` directly". Added them to `lib/index.ts`.

- **`drawBoxOrBlock` had no input hardening for public callers.** A
  0×0 cell fed `0/0 = NaN` into `drawArc`'s slope math; a fractional
  `lightPx` produced sub-pixel dash positions that don't tile.
  Added `if (w <= 0 || h <= 0) return false` and
  `Math.max(1, Math.round(lightPx))` at the entry. Internal callers
  already passed valid values; this just hardens the public API.

- **┼ U+253C cross test was too weak**: only checked
  `rects.length >= 2` and that the cell-center pixel was covered. A
  refactor that dropped the LEFT or DOWN arm would still satisfy
  both. Now asserts exactly 4 rects and that vertical strokes
  collectively span y=0..CH at the horizontal center, and horizontal
  strokes collectively span x=0..CW at the vertical center — so a
  missing arm fails immediately.

- **Coverage test predicate accepted degenerate fillRects.** A bug
  that emitted a 0×0 fillRect for some glyph would have passed the
  "drewSomething" check. Tightened to require `w > 0 && h > 0` (or
  a stroke).

379 tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sixth review pass surfaced two LOW + one INFO:

- ┼ test's "verticalCoverage"/"horizontalCoverage" filter accepted both
  axes because the predicate `r.x <= cy && cy <= r.x + r.w` matches
  any rect that crosses the cell's horizontal centerline — including
  the horizontal arms. Test still failed correctly (length assertion
  + min-y = 0 still uniquely required the up-arm), but the variable
  names were misleading. Filter now uses the rect's narrow dimension
  (`r.w <= 2*LT` for vertical arms, `r.h <= 2*LT` for horizontal),
  which unambiguously partitions the four arms.

- `drawBoxOrBlock` JSDoc didn't mention input clamping. Now documents
  that `lightPx` is silently rounded to the nearest integer ≥ 1, and
  that `w`/`h` ≤ 0 returns false without drawing.

- `cell.codepoint && isBoxOrBlock(cell.codepoint)` had inferred type
  `0 | true | false`. Tightened to `cell.codepoint > 0 && ...` for
  clean boolean inference.

379 tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant