Skip to content

fix(renderer): correct scrollback/screen row mapping during smooth scroll#171

Open
brianegan wants to merge 1 commit into
coder:mainfrom
brianegan:fix/scroll-viewport-boundary
Open

fix(renderer): correct scrollback/screen row mapping during smooth scroll#171
brianegan wants to merge 1 commit into
coder:mainfrom
brianegan:fix/scroll-viewport-boundary

Conversation

@brianegan

Copy link
Copy Markdown

Problem

When scrolled up during a smooth-scroll animation, a line near the top of the viewport appears duplicated and overlapping its neighbours.

The renderer decides whether each visible row comes from scrollback or the live screen using the raw comparison y < viewportY, but computes the scrollback offset and screen row using Math.floor(viewportY). These agree only when viewportY is a whole number. During a smooth-scroll animation viewportY is fractional (e.g. 2.5), and the two disagree for the boundary row y === Math.floor(viewportY):

  • It passes y < viewportY, so it's treated as scrollback and asked for offset scrollbackLength - floor + floor === scrollbackLength — one past the last valid index (0 … length-1). The WASM call returns null, so that row is never repainted and keeps stale pixels from the previous frame.
  • Screen row 0 (screenRow = floor - floor = 0) is never fetched, so the real top line of the screen is dropped and every row below shifts up by one.

The stale, un-repainted row sitting among the shifted rows is the duplicated/overlapping line. It shows up near the top of the viewport because the boundary sits at Math.floor(viewportY), which is small when scrolled up a little.

Fix

Floor viewportY once and use that single integer for both the scrollback/screen boundary comparison and the offset / screen-row math — in the render loop and the hyperlink-hover scan. This matches every coordinate-mapping site in terminal.ts, which already floors viewportY before comparing.

Tests

Adds lib/renderer-viewport-boundary.test.ts:

  • integer viewportY never requests an out-of-range scrollback offset
  • fractional viewportY never requests an out-of-range scrollback offset
  • fractional viewportY still renders the top screen row (no dropped line)
  • fractional viewportY maps rows identically to floor(viewportY)

The fractional cases fail before the change and pass after. bun run typecheck is clean.

🤖 Generated with Claude Code

…roll

The renderer chose between scrollback and screen rows using the raw
`y < viewportY` comparison, but computed the scrollback offset and screen
row with `Math.floor(viewportY)`. When viewportY is fractional (during a
smooth-scroll animation) the two disagree for the boundary row
`y === Math.floor(viewportY)`: it is treated as scrollback and asked for
offset `scrollbackLength`, which is one past the last valid index, so the
WASM call returns null and that row is never repainted (keeping stale
pixels). At the same time the top screen row (screen row 0) is dropped and
every row below shifts up by one. Visually a line near the top of the
viewport appears duplicated and overlaps its neighbours while scrolling.

Floor viewportY once and use that single integer for both the boundary
comparison and the offset/screen-row math, in the render loop and the
hyperlink-hover scan. This matches every coordinate-mapping site in
terminal.ts, which already floors before comparing.

Adds renderer-viewport-boundary.test.ts covering the integer and
fractional cases (no out-of-range scrollback offset, top screen row not
dropped, fractional maps the same as floored).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@brianegan

Copy link
Copy Markdown
Author

@diegosouzapw Might be another one you're interested in! I was getting duplicated lines / odd artifacts without this one.

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