fix(renderer): repaint unchanged rows inside hard-scrolled regions - #143
Open
dmccaffery wants to merge 1 commit into
Open
fix(renderer): repaint unchanged rows inside hard-scrolled regions#143dmccaffery wants to merge 1 commit into
dmccaffery wants to merge 1 commit into
Conversation
After a successful hardware scroll, scrolln shifted the model of the physical screen on curbuf only, while the repaint loop in Render skips lines by newbuf.Touched. A row inside the scrolled region whose newbuf content is identical to the previous frame at the same index was therefore never repainted and kept displaying the scrolled-in content, leaving stale duplicate rows on screen. Touch the scrolled region on newbuf as well, so every row whose physical cells moved is re-diffed against curbuf. Rows that already match emit nothing, so the optimization stays intact: benchmarks show no measurable regression (sec/op and bytes/frame unchanged within noise on Apple M2, benchstat n=10). Adds a regression test asserting that curbuf matches the rendered frame after every render, plus scroll-path benchmarks for the unique- and repeated-row cases. Fixes: charmbracelet#137 Signed-off-by: Deavon M. McCaffery <dmccaffery@users.noreply.github.com>
tiffanywang3
approved these changes
Jul 15, 2026
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.
After a successful hardware scroll, scrolln shifted the model of the physical screen on curbuf only, while the repaint loop in Render skips lines by newbuf.Touched. A row inside the scrolled region whose newbuf content is identical to the previous frame at the same index was therefore never repainted and kept displaying the scrolled-in content, leaving stale duplicate rows on screen.
Touch the scrolled region on newbuf as well, so every row whose physical cells moved is re-diffed against curbuf. Rows that already match emit nothing, so the optimization stays intact: benchmarks show no measurable regression (sec/op and bytes/frame unchanged within noise on Apple M2, benchstat n=10).
Adds a regression test asserting that curbuf matches the rendered frame after every render, plus scroll-path benchmarks for the unique- and repeated-row cases.
Benchmark results — issue #137 fix (
touchLine(newbuf, …)inscrolln)Renderer hard-scroll benchmarks (
BenchmarkRendererScroll*, new interminal_renderer_hardscroll_test.go):an 80×24 fullscreen viewport with scroll optimization enabled, scrolled up one line per frame.
Compared with
benchstat, n=10 runs each. goos: darwin, goarch: arm64, cpu: Apple M2.Time per frame (sec/op)
Bytes written to the terminal (bytes/frame)
Allocations (B/op)
Allocations (allocs/op)
Summary
No performance regression from the fix. Touched-but-unchanged rows re-diff against
curbufand emitnothing, so time per frame is unchanged within noise. The repeated-rows case (the bug scenario) writes
slightly fewer bytes after the fix because keeping
curbufaccurate avoids spurious repaints onlater frames. Correctness: the issue's fuzz harness went from 5/38 corrupted trials to 0/4143.
Fixes: #137
CONTRIBUTING.md.