Skip to content

Skip a flush that races between resize() and render() - #1785

Open
afonsojanu wants to merge 1 commit into
charmbracelet:mainfrom
afonsojanu:fix/resize-render-race-stale-flush-1780
Open

Skip a flush that races between resize() and render()#1785
afonsojanu wants to merge 1 commit into
charmbracelet:mainfrom
afonsojanu:fix/resize-render-race-stale-flush-1780

Conversation

@afonsojanu

Copy link
Copy Markdown

Fixes #1780

resize() updates the renderer's width and height and arms a redraw, but it never touches the stored view — that only happens afterward, when render() runs with a fresh View laid out for the new size. Both calls happen back to back in the same event loop iteration, but the ticker goroutine calls flush() independently at 60Hz, and if a tick lands in the gap between them, flush() sees the new size paired with the previous size's view. It draws that stale content into a buffer already resized for the new dimensions, and the next tick then corrects it a frame later — which is exactly the "ghost frame" the issue describes and traces in detail.

flush() now checks a viewStale flag that resize() sets and render() clears, and returns immediately without drawing while it's set. The pending erase and the width/height change stay in place, so once render() catches up (at most one tick later, since it always follows resize() in the same event loop iteration), the very next flush() draws the correct view at the correct size in one pass instead of two.

Added a direct test on the renderer driving the exact sequence the race produces: resize, flush before render, render, flush again — checking that nothing reaches the output writer during the gap and that the cell buffer only resizes once the correct view is available. Confirmed via git stash that this test fails (drawing the stale "old" view) without the fix and passes with it.

  • go test ./...: all passing.
  • go test -race ./... on the affected tests: clean.
  • go vet ./... and gofmt -l: clean.
  • golangci-lint run ./...: 0 issues.

resize() updates the renderer's width/height and arms a redraw, but
it never touches the stored view — that only happens afterward, when
render() runs with a fresh View laid out for the new size. Both calls
happen back to back in the same event loop iteration, but the ticker
goroutine calls flush() independently at 60Hz, and if a tick lands in
the gap between them, flush sees the new size paired with the
previous size's view. It draws that stale content into a buffer
already resized for the new dimensions, and the next tick then
corrects it a frame later, which shows up as a brief ghost of the old
layout at the new size.

flush() now checks a flag that resize() sets and render() clears, and
returns immediately without drawing while it's set. The pending erase
and the width/height change stay in place, so once render() catches
up (at most one tick later, since it always follows resize() in the
same iteration), the very next flush draws the correct view at the
correct size in one pass instead of two.

Added a direct test on the renderer driving the exact sequence a race
would produce: resize, then flush before render, then render, then
flush again, checking that nothing reaches the output writer until
render has actually caught up.
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.

v2: cursed_renderer emits a one-tick stale frame on resize (race between resize() and the 60Hz ticker flush())

1 participant