fix(renderer): repaint when the screen changes size by taciturnaxolotl · Pull Request #175 · charmbracelet/ultraviolet

taciturnaxolotl · 2026-09-06T17:25:05Z

Fixes new issues discovered by fuzzing

codecov · 2026-09-06T17:25:53Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.20%. Comparing base (6c9e17d) to head (2b8b13b).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #175      +/-   ##
==========================================
+ Coverage   61.07%   61.20%   +0.12%     
==========================================
  Files          52       52              
  Lines        6860     6866       +6     
==========================================
+ Hits         4190     4202      +12     
+ Misses       2392     2388       -4     
+ Partials      278      276       -2

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

andrinoff · 2026-09-07T06:14:23Z

+      run: |
+        version=$(awk '$1 == "go.mitchellh.com/libghostty" { print $2 }' internal/conformance/go.mod)
+        test -n "$version"
+        echo "rev=${version##*-}" >> "$GITHUB_OUTPUT"


I don't love this solution. What was the intent behind taking the version after the dash?

i don't love it either; it was needed to pin the exact commit hash so that the ci was in sync with the go.mod

andrinoff · 2026-09-07T06:16:43Z

+	if s.flags.Contains(tFullscreen) && s.curbuf != nil &&
+		((width > 0 && width != s.curbuf.Width()) || (height > 0 && height != s.curbuf.Height())) {


This compares against the last rendered frame rather than the last Resize call, which is what makes the shrink-then-grow-back case work. Just confirming that's intentional and not an accident.

andrinoff · 2026-09-07T11:19:01Z

+	// only visits rows the application touched. Mark the whole scrolled range
+	// so every row it moved is compared against the model again; a row the
+	// scroll already put right costs a comparison and no output.
+	s.touchLine(newbuf, top, bot-top+1, true)


One question: does anything downstream reset these touch flags on the caller's frame, or does the frame now come back to the app with extra lines marked touched?

The width half of the rule the height already had: a screen that loses columns clips or rewraps every row it holds, and one that gains them refills from the scrollback where an earlier shrink parked the remainder. No row keeps its meaning across either, so there is nothing to diff against. Render alone cannot see it. It compares its model against the frame it is handed, so a screen that shrinks and grows back between two renders looks unchanged by the time it gets there, and the content the terminal moved in the meantime is never repainted. Resize latches the change instead, where every size the terminal passed through is visible. The fuzzer found both halves: 23 columns narrowed to 11 and widened back left the tail of a row on screen, and once that was fixed, a clear at 5 columns followed by a grow to 23 brought the clipped remainder back out of scrollback. Costs a full repaint on every resize, where before a width grow paid nothing. A resize is rare and user-driven; a wrong screen that survives every later frame is not.

Writing a grapheme cluster of more than one codepoint into the last column with autowrap off breaks it. The terminal never advances past the margin, so it reads the combining codepoints as belonging to the cell to the left: the mark moves back a column and the base rune is left alone at the margin. Ghostty in legacy width mode turns "###ééé" into "###éé́e", and no part of that is visible to the model, so it outlives every later frame. The autowrap dance was there for the opposite fear, that a terminal might let the combining codepoints wrap and land the tail on the next row. Neither reference emulator does: both hold the whole cluster on the row, and neither scrolls, because a pending wrap only flushes when something else prints and the renderer always moves the cursor first. The measured harm beats the hypothetical one. The lower right corner keeps the dance for cells that cannot be split, since there the pending wrap is a scroll waiting to happen.

The scroll moves every row in its range, including rows the application never drew into. Only the model was told: scrollBuffer touches the lines it shifted on curbuf, and the diff loop reads the touch state of the frame instead, so a row the application left alone was never compared again. Whatever the scroll carried off the screen stayed gone, and the model went on claiming it was there. Mark the scrolled range on the frame as well. A row the scroll already put in the right place costs a comparison and no output, which is the whole point of scrolling in hardware; a row it emptied gets painted again.

The build cloned go-libghostty at HEAD, whose CMakeLists pins whatever ghostty revision is current. The bindings the module actually depends on are older, and a revision that exports a different set of symbols fails the link with undefined references. It worked only because a warm cache kept an older build alive; the first cold cache after go-libghostty moved would have broken every job that touches this module. Read the commit out of go.mod instead. Bumping the dependency now moves the build with it, and there is no second place to remember. GHOSTTY_PIN is gone with it: it named the ghostty revision rather than the one that decides it, and was copied into two workflows with a comment asking that they be kept in step. The three jobs that needed all this shared twenty identical lines, so they now share a composite action instead, which also exports PKG_CONFIG_PATH rather than having each caller spell out the same path.

Taking everything after the final dash reads the commit out of a pseudo-version correctly and mangles anything else: a prerelease tag like v1.2.3-rc.1 comes out as "1", which is not a ref and fails the checkout with nothing useful to say. Match the twelve-digit commit a pseudo-version ends in instead, and leave every other version alone. A version the module actually tags is already a ref git can check out, so there is nothing to take apart.

… model The repaint a resize forces was decided by comparing the reported size against the model, which is the last frame the application handed over. Those are two different things. An application is free to draw a frame smaller than the screen, and one that does differs from the model on every call, so every Resize latched a clear and repainted the whole screen even when the size had not changed. A duplicate SIGWINCH or a defensive per-frame resize then cost an erase and a full repaint on a screen where nothing moved. Record the size Resize was told and compare against that. Consecutive reports differ exactly when the terminal changed, which is the question being asked, and a screen that shrinks and grows back between two renders still reports two changes and still latches. Caught in review by andrinoff, who asked whether comparing against the model was deliberate. It was deliberate and it was wrong.

Applications are told the terminal size on a schedule rather than only when it changes, so a resize that changed nothing is the steady state and what it costs is worth watching. The existing resize benchmark cannot see it: it always resizes to exactly the frame size, so the model and the report agree and the bug this measures cannot happen. With the latch comparing against the model rather than the last report, a frame one row shorter than the screen repainted on every call: 142795 ns/op 4372 B/op 495 allocs/op 5073 ns/op 2548 B/op 94 allocs/op

taciturnaxolotl requested a review from andrinoff September 6, 2026 17:25

taciturnaxolotl force-pushed the fix/renderer-model-drift branch from e33bd07 to 78bc347 Compare September 6, 2026 17:35

andrinoff reviewed Sep 7, 2026

View reviewed changes

taciturnaxolotl added 7 commits September 10, 2026 16:39

taciturnaxolotl force-pushed the fix/renderer-model-drift branch from d289414 to 2b8b13b Compare September 10, 2026 20:39

Navigation Menu

fix(renderer): repaint when the screen changes size - #175

fix(renderer): repaint when the screen changes size#175
taciturnaxolotl wants to merge 7 commits into
mainfrom
fix/renderer-model-drift

taciturnaxolotl commented Sep 6, 2026

Uh oh!

codecov Bot commented Sep 6, 2026 •

edited

Loading

Uh oh!

andrinoff Sep 7, 2026

Uh oh!

taciturnaxolotl Sep 10, 2026

Uh oh!

andrinoff Sep 7, 2026

Uh oh!

andrinoff Sep 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		if s.flags.Contains(tFullscreen) && s.curbuf != nil &&
		((width > 0 && width != s.curbuf.Width()) \|\| (height > 0 && height != s.curbuf.Height())) {

Conversation

taciturnaxolotl commented Sep 6, 2026

Uh oh!

codecov Bot commented Sep 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

andrinoff Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

taciturnaxolotl Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

andrinoff Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

andrinoff Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov Bot commented Sep 6, 2026 •

edited

Loading