Skip to content

fix(web): erase terminal cursor fringes between blinks - #11424

Open
Lucenx9 wants to merge 1 commit into
pingdotgg:mainfrom
Lucenx9:fix/terminal-cursor-blink-fringe
Open

Lucenx9 wants to merge 1 commit into
pingdotgg:mainfrom
Lucenx9:fix/terminal-cursor-blink-fringe

Conversation

@Lucenx9

@Lucenx9 Lucenx9 commented Sep 12, 2026 •

Copy link
Copy Markdown
Contributor

What changed

Snap terminal row boundaries to device pixels and use those same boundaries for the cursor, backgrounds, selection, and text clipping. Adjacent rows share an edge, so repainting the cursor row does not touch its neighbors.

The production change is confined to the shared web/desktop renderer. Blink timing, product defaults, and the terminal protocol stay unchanged. Mobile uses its separate native renderer.

Why

At fractional display scales, repainting an antialiased row boundary blends over the cursor edge instead of fully erasing it. This leaves faint horizontal lines during the blink's off phase. Pixel-aligned boundaries fix that without repainting the entire terminal on every blink.

Closes #11402.

flowchart LR
    A[Cursor on] --> B[Blink off]
    B --> C[Repaint cursor row]
    C --> D[Shared pixel edges erase the cursor completely]
Loading

UI changes

Before/after Chromium reproduction using the actual renderer from b1e223e2b0 and this branch, with identical input. The PNGs are captured at 2x density; the inset enlarges the original canvas pixels 8x. These are isolated renderer captures, not full-app screenshots.

Before After
Cursor edges remain during blink off Cursor disappears completely

Short before/after video. Full comparison and integrated-terminal evidence.

Verification

  • 66 focused tests pass in renderer.test.ts and surface.test.ts. The new geometry tests fail at 125% and 150% on the base renderer.
  • Web package typecheck and targeted lint/format checks pass.
  • All 97 real-canvas comparisons pass after the fix, covering four cursor styles, light/dark themes, text, selections, adjacent rows, four display scales, and a fractional CSS origin.
  • Tested the full app in Chromium with isolated state and synthetic shell output. At 125%, the live cursor leaves no residual pixels, including after scrollback appears.
  • Scoped Fallow audit passes with one thread and no introduced findings. Existing findings were left untouched.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes
  • I included a video for animation/interaction changes

Model: GPT-5. Harness: OpenAI Codex.

Summary by CodeRabbit

  • Bug Fixes

    • Improved terminal rendering across different display scales by aligning row boundaries to device pixels.
    • Corrected row heights for backgrounds, selections, text, decorations, and cursor rendering to maintain consistent vertical alignment.
    • Limited cursor updates to the affected row when appropriate, improving repaint behavior.
  • Tests

    • Added coverage for pixel alignment, contiguous row boundaries, and targeted cursor repainting across multiple display scales.

Copilot AI lite review requested due to automatic review settings September 12, 2026 13:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 12, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-12T13:41:13.315318Z 48311fb PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Sep 12, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 12, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 48311fb

Macroscope's review found this PR approvable — This is a small, self-contained terminal rendering bug fix that aligns row and cursor edges to device pixels to prevent blink artifacts. Production impact is confined to existing canvas rendering, with targeted fractional-scale and cursor repaint tests added.

No code changes detected at ff04249. Prior analysis still applies.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Sep 12, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: dd4dd2e5-85bb-415f-bf81-5eae2da592c5

📥 Commits

Reviewing files that changed from the base of the PR and between b1e223e and 48311fb.

📒 Files selected for processing (3)
  • apps/web/src/terminal/ghostty/renderer.test.ts
  • apps/web/src/terminal/ghostty/renderer.ts
  • apps/web/src/terminal/ghostty/surface.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

Ghostty terminal rendering now snaps row boundaries to device pixels. Backgrounds, text clipping, decorations, and cursors use the computed row geometry. Tests cover multiple display scales and canvas transform mocks.

Changes

Terminal rendering

Layer / File(s) Summary
Transform-aware row geometry and rendering
apps/web/src/terminal/ghostty/renderer.ts, apps/web/src/terminal/ghostty/renderer.test.ts, apps/web/src/terminal/ghostty/surface.test.ts
renderGhosttySnapshot snaps row boundaries using the canvas transform. Row backgrounds, selection, text clipping, decorations, and cursors use the computed row height. Tests verify pixel alignment, contiguous rows, repaint behavior, and getTransform() mocks.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: stienswout

Merge Risk: ⚪ Minimal · up to ff042

The terminal rendering change has focused coverage for pixel alignment and blink repaint behavior, with no actionable current-head risk identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the primary fix: removing residual terminal cursor edges during blinking.
Description check ✅ Passed The description includes the required change summary, rationale, UI evidence, verification details, and completed checklist items. It is focused and aligned with the template.
Linked Issues check ✅ Passed Issue #11402 requires the cursor to disappear during the blink-off phase without residual cell edges, while preserving the background and text. renderer.ts snaps shared row boundaries with the canva…
Out of Scope Changes check ✅ Passed The changes stay within Issue #11402. They modify the shared Ghostty web renderer, add focused renderer coverage, and update canvas test doubles with getTransform(). They do not change blink timing,…
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@greatitself

Copy link
Copy Markdown

Independently validated 48311fb in Chromium using the actual renderer from that commit and base b1e223e.

  • The 125% scale reproduction leaves 26 residual pixels on the base and zero on this PR after the cursor blinks off.
  • 192 canvas comparisons passed across DPRs 1, 1.25, 1.5, and 2; integer and fractional row origins; light/dark, glyph, colored background, selection, and strikethrough cases; and all four cursor styles. Each case runs three blink cycles followed by hiding the cursor with a changed row, then verifies the final canvas exactly matches its original cursor-off pixels.

Base and PR renderer comparison at 125% scale

Blink recording · Evidence and test details

These are isolated renderer captures, not a separate Electron application test.

Model: GPT-6. Harness: Codex.

@Lucenx9
Lucenx9 force-pushed the fix/terminal-cursor-blink-fringe branch from 48311fb to ff04249 Compare September 12, 2026 18:18
@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 1, 2026 — with ChatGPT Codex Connector

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews size:M 30-99 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Terminal cursor leaves top and bottom edges visible during blink off phase

4 participants