Skip to content

Fix terminal content hidden under the scroller after a font change#577

Open
bones7456 wants to merge 1 commit into
migueldeicaza:mainfrom
bones7456:fix/resetfont-effective-width
Open

Fix terminal content hidden under the scroller after a font change#577
bones7456 wants to merge 1 commit into
migueldeicaza:mainfrom
bones7456:fix/resetfont-effective-width

Conversation

@bones7456

Copy link
Copy Markdown
Contributor

Problem

After changing the font size (zoom in/out), the right-most columns of the
terminal are drawn underneath the scroller and become hidden. Zooming
the font in and back out also leaves the reported column count off by a
couple of columns from what a window resize produces.

Root cause

The column/row count is computed in two places, and they disagree about
the scroller:

  • The live window-resize path, processSizeChange(newSize:), uses
    getEffectiveWidth(size:), which subtracts the reserved scroller width.

  • resetFont() (invoked when the font changes) computed the columns from
    the raw frame.width:

    let newCols = Int(frame.width / cellDimension.width)

So after a font change the terminal claims more columns than actually fit
in the visible (non-scroller) area, and the overflowing right-most columns
are rendered beneath the scroller.

Fix

Compute the columns from getEffectiveWidth(size:) in resetFont() too,
matching processSizeChange. The function already exists and is used on
both macOS and iOS, so this keeps the two paths consistent and guarantees
content stays clear of the scroller.

Tests

Added FontResizeColumnsTests (macOS) which:

  • asserts the rendered width (cols * cellDimension.width) stays within
    getEffectiveWidth(size:) after a font change — i.e. content is not
    covered by the scroller;
  • asserts the font-change path and the live-resize path report the same
    column count for the same frame.

Both tests fail on main (e.g. renderedWidth 397.5 > usableWidth 383.0,
and 53 != 51 columns) and pass with this change. The rest of the suite
is unaffected (the single pre-existing testSynchronizedOutputBlocksDisplayUntilReset
failure also reproduces on a clean main and is unrelated).

@bones7456 bones7456 force-pushed the fix/resetfont-effective-width branch from bcdb82f to 5f80ac1 Compare June 24, 2026 01:52
@bones7456

Copy link
Copy Markdown
Contributor Author

Friendly ping on this one 🙂 Now rebased on latest main (sits right on top of #578, no conflicts). It's a small fix (+56/−1, 2 files) with macOS regression tests that fail on main and pass with the change. Its sibling PR #576 was already merged — this addresses the related scroller/column-drift issue after a font-size change. Happy to adjust anything if needed. Thanks for maintaining SwiftTerm!

@bones7456

Copy link
Copy Markdown
Contributor Author

The CI failure is the pre-existing flaky testSynchronizedOutputBlocksDisplayUntilReset (SynchronizedOutputTests.swift:54), unrelated to this change. The Mac/iOS/SPM builds all pass — only that one timing-dependent test records an issue.

The exact same failure shows up on main (e.g. run 27973775699): same test, same line :54, same assertion (topLineText(...) → "NEW").hasPrefix("OLD"). A re-run should go green.

@bones7456

Copy link
Copy Markdown
Contributor Author

Update on the CI failure: it's not flaky — testSynchronizedOutputBlocksDisplayUntilReset is a stale test that fails deterministically on main too. Commit 9446f60 removed the frozen synchronizedOutputBuffer snapshot (display blocking moved to the view layer) but never updated this test, so it still asserts the old snapshot behaviour.

Opened a separate test-only fix in #581 to keep it isolated from this font/scroller change. Once that merges, this PR's CI should go green on rebase.

resetFont() recomputed the column count from the raw frame width, while
the live window-resize path (processSizeChange) uses getEffectiveWidth(),
which subtracts the reserved scroller width. After zooming the font the
terminal therefore claimed more columns than fit the visible area, so the
right-most columns were drawn underneath the scroller and that content was
hidden. Zooming in and back out also drifted the reported column count by
the scroller width.

Compute the columns from getEffectiveWidth() in resetFont() as well, so the
font-change and live-resize paths agree and content always stays clear of
the scroller.

Add macOS regression tests asserting the content stays within the usable
(non-scroller) width and that both code paths report the same column count.
@bones7456 bones7456 force-pushed the fix/resetfont-effective-width branch from 5f80ac1 to 64b5ef4 Compare June 25, 2026 02:23
@bones7456

Copy link
Copy Markdown
Contributor Author

Rebased onto latest main (now includes the #581 test fix) and CI is green ✅. The only thing previously blocking this was that stale test, so this should be ready for review/merge whenever you have a moment. Thanks!

bones7456 added a commit to bones7456/notchy that referenced this pull request Jun 29, 2026
…hange; bump to 1.3.4

Pin the SwiftTerm dependency to the fork commit bones7456/SwiftTerm@64b5ef4
(PR migueldeicaza/SwiftTerm#577) which fixes the scroller/column-drift issue
after changing font size. Revert to an upstream version tag once #577 is merged
and a new SwiftTerm release ships.
@migueldeicaza

Copy link
Copy Markdown
Owner

Apologies, let me review now

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.

2 participants