feat(renderer): powerline glyph rendering + fold-in of PR #128#24
Merged
Conversation
sauyon
commented
May 18, 2026
sauyon
commented
Owner
- lib/powerline.ts: port Ghostty's src/font/sprite/draw/powerline.zig for U+E0B0..U+E0BF and U+E0D2/U+E0D4 (18 glyphs total). Drawn as Canvas2D paths sized to the cell; structurally mirrors box-drawing.ts. Diagonals (E0B9/BB/BD/BF) delegate to drawBoxOrBlock for clean tiling against U+2571/U+2572. Includes lib/powerline.test.ts with 35 structural + coverage tests.
- lib/renderer.ts: wire powerline dispatch into the cell text path. Add optional style? to IRenderable.getCursor() so the buffer's reported cursor style (from CSI q / DECSCUSR via WASM) wins over the renderer-level default, matching Ghostty native's precedence rule (renderer/cursor.zig:36-68). cursorStyle option now documents itself as a fallback for non-WASM IRenderable implementations.
- demo/render-test.html, demo/bin/render-test.ts, demo/baselines/: headless visual-regression suite folded as-is from PR feat: powerline and block character rendering with visual tests coder/ghostty-web#128.
- package.json: test:render{,:update,:web} scripts + puppeteer devDependency.
- .gitignore: ignore demo/baselines/*.fail.png artifacts.
- lib/powerline.ts: port Ghostty's src/font/sprite/draw/powerline.zig for U+E0B0..U+E0BF and U+E0D2/U+E0D4 (18 glyphs total). Drawn as Canvas2D paths sized to the cell; structurally mirrors box-drawing.ts. Diagonals (E0B9/BB/BD/BF) delegate to drawBoxOrBlock for clean tiling against U+2571/U+2572. Includes lib/powerline.test.ts with 35 structural + coverage tests. - lib/renderer.ts: wire powerline dispatch into the cell text path. Add optional style? to IRenderable.getCursor() so the buffer's reported cursor style (from CSI q / DECSCUSR via WASM) wins over the renderer-level default, matching Ghostty native's precedence rule (renderer/cursor.zig:36-68). cursorStyle option now documents itself as a fallback for non-WASM IRenderable implementations. - demo/render-test.html, demo/bin/render-test.ts, demo/baselines/: headless visual-regression suite folded as-is from PR coder/ghostty-web#128. - package.json: test:render{,:update,:web} scripts + puppeteer devDependency. - .gitignore: ignore demo/baselines/*.fail.png artifacts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements a native Powerline glyph renderer using Canvas2D and introduces a visual regression testing suite powered by Puppeteer. It also updates the renderer to support per-cursor style overrides, facilitating better testing. Feedback recommends using a pixel-level comparison library like pixelmatch in the test runner to improve the reliability of visual checks against baseline images.
There was a problem hiding this comment.
Code Review
This pull request introduces a visual regression testing framework for the renderer, including a headless test runner using Puppeteer and a browser-based test page. It also adds a new Powerline glyph renderer to improve terminal rendering accuracy. The review comment correctly identifies that the current byte-for-byte image comparison logic is fragile and suggests a more robust pixel-based comparison using the existing fast-png dependency.
Local gemini-review findings (off-PR):
- Extract Op/RecordingCtx/makeCtx into lib/canvas-recorder.ts; both
box-drawing.test.ts and powerline.test.ts now consume the shared
helper, dropping ~250 lines of duplicated mock-context scaffolding.
- Single `as unknown as` cast lives once inside makeRecordingCtx;
test files import a value already typed as CanvasRenderingContext2D
and pass it straight to draw functions, no per-test cast jump.
- Remove the closure allocated by withMirror() on every call to
fillStadium/innerStrokeStadium/fillBisector — replaced with an
inline `if (mirror) { save; ... }` guard around a no-allocation
mirrorAroundCellCenter helper that just does the translate/scale.
Gemini-bot inline review on PR #24:
- demo/bin/render-test.ts:280 calculateDiffPercent now decodes both
PNGs via fast-png (already a project dep) and compares RGBA
pixel-by-pixel with a ±2/channel tolerance for AA jitter, instead
of byte-comparing the compressed PNG buffers. Dimension mismatch
reports 100% diff. Fixes the flakiness the bot flagged: PNG
metadata or compression-level changes no longer count as visual
regressions.
All 69 lib tests still pass; typecheck, fmt, lint clean (the two
pre-existing biome warnings in url-detection.test.ts are unchanged).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two follow-ups on the previous gemini-cleanup commit: - vite.config.js: add lib/canvas-recorder.ts to the vite-plugin-dts `exclude` list. The recorder isn't imported from lib/index.ts, so its JS gets tree-shaken by Rollup, but the dts plugin's exclude only filtered *.test.ts and was happily emitting RecordingOp / RecordingCanvas / makeRecordingCtx into the published .d.ts bundle. Now they stay internal. Updated the file's doc to spell out both protection mechanisms (entry-point tree-shaking for JS, explicit dts exclude for types). - demo/bin/render-test.ts: calculateDiffPercent now treats a channel-layout mismatch (RGB vs RGBA) as 100% diff, the same way it already treats a dimension mismatch. Previously it took `Math.min(a.channels, b.channels)`, which would silently ignore alpha for an RGB-vs-RGBA pair and falsely match an opaque baseline against a transparent current. Canvas2D's toDataURL always emits RGBA so this doesn't fire in practice, but the earlier comment claimed robustness the code didn't deliver. All 69 lib tests still pass; typecheck/fmt/lint clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- lib/canvas-recorder.ts: use `CanvasLineCap` / `CanvasLineJoin` for the lineCap / lineJoin RecordingOp variants and the corresponding setter signatures, instead of bare `string`. Now matches the DOM lib's own type for `CanvasRenderingContext2D.lineCap`/`.lineJoin`. - demo/bin/render-test.ts: drop the `as number` casts on fast-png `data[...]` accesses in calculateDiffPercent. PngDataArray is `Uint8Array | Uint8ClampedArray | Uint16Array` and TypeScript already narrows indexed access to `number` for that union. A third gemini suggestion — collapsing innerStrokeStadium's two save/restore pairs (outer for the mirror transform, inner for the clip) into a single pair — is *not* taken. The two-pair structure is load-bearing for the mirror-pair test in lib/powerline.test.ts: stripMirrorEnvelope(E0B7) must equal E0B5, and that equality only holds when the mirror envelope is fully separable from the clip's own save/restore. Merging would buy at most one fewer save/restore per E0B6/E0B7 cell — uncommon glyphs, negligible cost — at the price of weakening that structural invariant. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sauyon
added a commit
that referenced
this pull request
Jun 8, 2026
Local gemini-review findings (off-PR):
- Extract Op/RecordingCtx/makeCtx into lib/canvas-recorder.ts; both
box-drawing.test.ts and powerline.test.ts now consume the shared
helper, dropping ~250 lines of duplicated mock-context scaffolding.
- Single `as unknown as` cast lives once inside makeRecordingCtx;
test files import a value already typed as CanvasRenderingContext2D
and pass it straight to draw functions, no per-test cast jump.
- Remove the closure allocated by withMirror() on every call to
fillStadium/innerStrokeStadium/fillBisector — replaced with an
inline `if (mirror) { save; ... }` guard around a no-allocation
mirrorAroundCellCenter helper that just does the translate/scale.
Gemini-bot inline review on PR #24:
- demo/bin/render-test.ts:280 calculateDiffPercent now decodes both
PNGs via fast-png (already a project dep) and compares RGBA
pixel-by-pixel with a ±2/channel tolerance for AA jitter, instead
of byte-comparing the compressed PNG buffers. Dimension mismatch
reports 100% diff. Fixes the flakiness the bot flagged: PNG
metadata or compression-level changes no longer count as visual
regressions.
All 69 lib tests still pass; typecheck, fmt, lint clean (the two
pre-existing biome warnings in url-detection.test.ts are unchanged).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.