fix(renderer): correct scrollback/screen row mapping during smooth scroll#171
Open
brianegan wants to merge 1 commit into
Open
fix(renderer): correct scrollback/screen row mapping during smooth scroll#171brianegan wants to merge 1 commit into
brianegan wants to merge 1 commit into
Conversation
…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>
Author
|
@diegosouzapw Might be another one you're interested in! I was getting duplicated lines / odd artifacts without this one. |
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
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.
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 usingMath.floor(viewportY). These agree only whenviewportYis a whole number. During a smooth-scroll animationviewportYis fractional (e.g.2.5), and the two disagree for the boundary rowy === Math.floor(viewportY):y < viewportY, so it's treated as scrollback and asked for offsetscrollbackLength - floor + floor === scrollbackLength— one past the last valid index (0 … length-1). The WASM call returnsnull, so that row is never repainted and keeps stale pixels from the previous frame.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
viewportYonce 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 interminal.ts, which already floorsviewportYbefore comparing.Tests
Adds
lib/renderer-viewport-boundary.test.ts:viewportYnever requests an out-of-range scrollback offsetviewportYnever requests an out-of-range scrollback offsetviewportYstill renders the top screen row (no dropped line)viewportYmaps rows identically tofloor(viewportY)The fractional cases fail before the change and pass after.
bun run typecheckis clean.🤖 Generated with Claude Code