fix(terminal): fit the terminal only once the terminal font is measurable - #396
Conversation
…able Opening a session could render its frame with characters spliced into each other, as though two frames were overlaid — a status-line fragment landing in the middle of a file path, for instance. Resizing the browser window cleared it. The first fit runs while the browser is still painting with a fallback font. A cell measured against that font has a different width and height from one measured against the terminal font, so the fit produces the wrong column and row count. Codeman sizes the pane to it and replays the capture. When the font finishes loading the measurement changes, the pane is resized a second time, and the CLI repaints for a shape that does not match the frame already on screen. Its later partial updates then land on the wrong rows. selectSession now waits for the font before it measures, so the pane is sized once, at the size that sticks, and the capture is taken at that size. The wait always resolves, so a font that never loads cannot block a terminal, and it resolves immediately once the font is in, so a tab switch pays nothing after the first load. document.fonts.ready alone is not enough: it can resolve before the stylesheet declaring @font-face has been parsed. document.fonts.load for each family in the stack is what actually requests the faces. Measured on a session opening at 2328px wide: the cell went from 8.43x16.00 to 8.00x21.00 roughly 900ms in, moving the grid from 112x36 to 118x28 after the replay had already been painted.
8d8f1bd to
0e82443
Compare
Review of the previous commit found that waiting for the font does not, on its
own, do anything.
`FitAddon.proposeDimensions()` measures nothing — it divides the container by a
CACHED cell size, and xterm refreshes that cache only from `open()`, from a
resize that actually changed the grid, and on a device-pixel-ratio change.
Nothing in it listens for font loading. So a fit that runs after the font
arrives can still divide by the fallback cell, propose the grid it already has,
and short-circuit before anything re-measures. The wait now ends by calling
`_charSizeService.measure()` itself, which is the step that makes the following
fit see the real font. Private API, as FitAddon's own dependency on `_core` is,
and guarded because a terminal can be disposed mid-wait.
The wait was also unbounded, and it sat behind the buffer-load gate. Neither
`FontFaceSet.load()` nor `FontFaceSet.ready` has a deadline, so a font request
that never settled left the tab spinning with live output queued behind it —
permanently, and on every session, since they share one promise. The comment
claimed the opposite ("a font that never loads must not block the terminal, so
this always resolves"), which was true of the per-face loads and false of
`ready`. It is now raced against TERMINAL_FONT_WAIT_MS, and the await moved
ahead of `_beginBufferLoad` so a slow font cannot hold output back at all —
which also removes the stale-select interaction with `_restoringFlushedState`,
since that flag is not yet set when the wait runs.
The awaited set no longer includes faces that cannot move the measured cell.
The bundled symbols font is ~1.2MB of private-use-area glyphs and xterm
measures `W`, so awaiting it put a megabyte in front of the first frame for
nothing; the generic families match no FontFace at all.
A runtime font change had the same race the boot-time one did:
applyTerminalFontFamily wrote the new family and fit on the next line, against
a family the browser might not have loaded. It now re-arms the wait and fits
again when it settles.
The claim that this could not be tested was wrong: the repo's vm harness
reaches both halves. The new suite pins the family filter, the forced
re-measure, the deadline, a rejecting load, a browser with no font API, and a
terminal disposed mid-wait — plus the four ordering properties in
selectSession, including that iOS Safari's synchronous focus still precedes the
first await. Each assertion was checked by reverting its fix.
Also corrects the docstring's reason for calling `document.fonts.load` (the
stylesheet is render-blocking and long parsed by then; the real reason is that
the WebGL renderer rasterises through a canvas atlas, and canvas text never
triggers a CSS font fetch), restores the JSDoc block the previous commit
displaced from getTerminalDimensions, and fixes a comment that described the
first fit as already having run when the mobile-Safari branch defers it.
|
Merged and shipped in v1.26.2, together with #395 and #397. Thank you. The finding that makes this PR worth more than its diff is that waiting is not sufficient on its own. The Both are now in the code where the next person will hit them. Excluding the ~1.2MB symbols face from the wait (xterm measures |
Live terminal events are queued while a buffer load runs, and the load discards that queue when it ends. That is right when the loaded buffer is the server's accumulated byte history. The history is current up to the response, so the queued events already appear in it and replaying them would duplicate output, most visibly Ink's cursor-up redraws. A tmux pane capture is current only up to CAPTURE time, which is part-way through the fetch. Everything arriving between the capture and the end of the chunked write was dropped, and nothing scheduled a re-fetch to recover it: `_onSessionNeedsRefresh` is wired only to the 128KB overflow path. The CLI's next partial redraw then landed on a frame the terminal never received. The window covers the whole load, not a sliver of it — a 400ms redraw settle after a real resize, the fetch, and the chunked write after that. Queue entries now carry their arrival time, and `_finishBufferLoad` takes a `since` cutoff, so a capture load replays exactly the tail that arrived after the response headers. The pre-capture events stay dropped, because the capture does hold those. Two further things had to change for that tail to still exist when the load ends, and a browser test is what found both. `chunkedTerminalWrite` is what ends the load for every non-empty buffer, so the flush policy travels to its own finish calls; the call in `selectSession` runs only when the write was skipped. `_beginBufferLoad` no longer empties the queue when one load re-enters it, which it does on every write, because that reset discarded the whole fetch window before anything could replay it. The response already distinguishes the two cases. `source` reads `mux-visible` or `mux-full-history` for a capture and `history` for the byte stream. Follows Ark0N#395, Ark0N#396 and Ark0N#397, which fixed the ways the replayed frame itself could disagree with the terminal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Live terminal events are queued while a buffer load runs, and the load discards that queue when it ends. That is right when the loaded buffer is the server's accumulated byte history. The route appends to that history right up to the moment it serializes the response, so a queued event already appears in it and replaying it would duplicate output, most visibly Ink's cursor-up redraws. A tmux pane capture is a photograph, current only as of the instant `capture-pane` ran. Output printed afterwards was queued and then dropped, and nothing scheduled a re-fetch to recover it: `_onSessionNeedsRefresh` is wired only to the 128KB overflow path. The CLI's next partial redraw then landed on a frame the terminal never received. How much went missing depended on which capture the route served. A `?full=1` load returns the capture alone, with no history in front of it, so it lost everything from the capture to the end of the chunked write. A `?tail=` load returns history, a clear, and then the capture, and the route reads that history after the capture, so it lost everything from the response to the end of that write. The chunked write dominates either way. An agent CLI hides the loss on its next full redraw; a shell session does not, because its output is linear and nothing repaints it. Queue entries now carry their arrival time, and `_finishBufferLoad` takes a `since` cutoff, so a capture load replays exactly the tail that arrived after the response headers. The earlier events stay dropped, because a payload that carries history does hold those. Two further things had to change for that tail to still exist when the load ends, and a browser test is what found both. `chunkedTerminalWrite` is what ends the load for every non-empty buffer, so the flush policy travels to its own finish calls; the call in `selectSession` runs only when the write was skipped. `_beginBufferLoad` no longer empties the queue when one load re-enters it, which it does on every write, because that reset discarded the whole fetch window before anything could replay it. The response already distinguishes the sources. `source` reads `mux-visible` or `mux-full-history` for a capture and `history` for the byte stream. Follows Ark0N#395, Ark0N#396 and Ark0N#397, which fixed the ways the replayed frame itself could disagree with the terminal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A visible-frame capture repaints each row at an absolute position, counting up to the pane's height. A terminal shorter than that clamps every address past its own height onto its last line. The overflow rows then overwrite one another, and the rows underneath are lost. Replaying a real 50-row capture into a 30-row terminal rendered 28 lines of a 45-line command and drew the frame twice. Nothing in the response said what height the frame was built for, so the client could not detect this. A capture now reports the geometry it was really taken at through `capturedGeometry` on `PaneCaptureOptions`, and the terminal response carries it as `captureCols` and `captureRows`. When the captured pane is taller than the terminal, or the size that produced the capture did not survive the load, `selectSession` replays once at the size that stuck. `resizeRetry` caps that at one attempt, so two competing fits cannot trade replays forever. The retry re-arms the full-history flag only when the pass that ran had consumed it. A tab switch takes the bounded tail, so its retry takes the tail too: clearing the flag unconditionally would upgrade that switch into a fresh scrollback capture the user never asked for, which the route's own comments put at tens of megabytes. What this repairs is a capture that won a race against the resize meant to precede it. It does not repair a capture whose pane was too tall because `Session.resize` declined the resize outright, which it does for a small viewport while a desktop viewport's size claim is live. The retry re-sends the same declined resize and captures the same pane, and `resizeRetry` then stops it. Repairing that means changing who owns the pane size, which is a policy question this does not touch. The reported geometry still helps there, because the client can see the mismatch at all rather than being blind to it. Follows Ark0N#395, Ark0N#396 and Ark0N#397, which fixed the other ways the replayed frame and the terminal could disagree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Live terminal events are queued while a buffer load runs, and the load discards that queue when it ends. That is right when the loaded buffer is the server's accumulated byte history. The route appends to that history right up to the moment it serializes the response, so a queued event already appears in it and replaying it would duplicate output, most visibly Ink's cursor-up redraws. A tmux pane capture is a photograph, current only as of the instant `capture-pane` ran. Output printed afterwards was queued and then dropped, and nothing scheduled a re-fetch to recover it: `_onSessionNeedsRefresh` is wired only to the 128KB overflow path. The CLI's next partial redraw then landed on a frame the terminal never received. How much went missing depended on which capture the route served. A `?full=1` load returns the capture alone, with no history in front of it, so it lost everything from the capture to the end of the chunked write. A `?tail=` load returns history, a clear, and then the capture, and the route reads that history after the capture, so it lost everything from the response to the end of that write. The chunked write dominates either way. An agent CLI hides the loss on its next full redraw; a shell session does not, because its output is linear and nothing repaints it. Queue entries now carry their arrival time, and `_finishBufferLoad` takes a `since` cutoff, so a capture load replays exactly the tail that arrived after the response headers. The earlier events stay dropped, because a payload that carries history does hold those. All four paths that fetch a terminal buffer and write it now decide this the same way, through one `_bufferLoadFinishOpts` helper, so they cannot drift apart: `selectSession`, `_onSessionNeedsRefresh`, `_onSessionClearTerminal` and `_maybeRefetchFullHistory`. The second of those is the one that stings. It exists to restore output the client already dropped once under backpressure, and it was dropping more output while performing that recovery. The cache-hit write inside `selectSession` stays on discard deliberately: it runs before the fetch, so its queue holds only events the capture that follows already contains. Two further things had to change for that tail to still exist when the load ends, and a browser test is what found both. `chunkedTerminalWrite` is what ends the load for every non-empty buffer, so the flush policy travels to its own finish calls; the call in `selectSession` runs only when the write was skipped. `_beginBufferLoad` no longer empties the queue when one load re-enters it, which it does on every write, because that reset discarded the whole fetch window before anything could replay it. The response already distinguishes the sources. `source` reads `mux-visible` or `mux-full-history` for a capture and `history` for the byte stream. Follows Ark0N#395, Ark0N#396 and Ark0N#397, which fixed the ways the replayed frame itself could disagree with the terminal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A visible-frame capture repaints each row at an absolute position, counting up to the pane's height. A terminal shorter than that clamps every address past its own height onto its last line. The overflow rows then overwrite one another, and the rows underneath are lost. Replaying a real 50-row capture into a 30-row terminal rendered 28 lines of a 45-line command and drew the frame twice. Nothing in the response said what height the frame was built for, so the client could not detect this. A capture now reports the geometry it was really taken at through `capturedGeometry` on `PaneCaptureOptions`, and the terminal response carries it as `captureCols` and `captureRows`. When the captured pane is taller than the terminal, or the size that produced the capture did not survive the load, `selectSession` replays once at the size that stuck. `resizeRetry` caps that at one attempt, so two competing fits cannot trade replays forever. The retry re-arms the full-history flag only when the pass that ran had consumed it. A tab switch takes the bounded tail, so its retry takes the tail too: clearing the flag unconditionally would upgrade that switch into a fresh scrollback capture the user never asked for, which the route's own comments put at tens of megabytes. What this repairs is a capture that won a race against the resize meant to precede it. It does not repair a capture whose pane was too tall because `Session.resize` declined the resize outright, which it does for a small viewport while a desktop viewport's size claim is live. The retry re-sends the same declined resize and captures the same pane, and `resizeRetry` then stops it. Repairing that means changing who owns the pane size, which is a policy question this does not touch. The reported geometry still helps there, because the client can see the mismatch at all rather than being blind to it. Follows Ark0N#395, Ark0N#396 and Ark0N#397, which fixed the other ways the replayed frame and the terminal could disagree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Part of #398, which describes the symptom and the other causes found with it.
Opening a session could render its frame with characters spliced into each
other, as though two frames were overlaid — a status-line fragment landing in
the middle of a file path, for instance. Resizing the browser window cleared it.
The first fit runs while the browser is still painting with a fallback font. A
cell measured against that font has a different width and height from one
measured against the terminal font, so the fit produces the wrong column and row
count. Codeman sizes the pane to it and replays the capture. When the font
finishes loading the measurement changes, the pane is resized a second time, and
the CLI repaints for a shape that does not match the frame already on screen.
Its later partial updates then land on the wrong rows.
selectSessionnow waits for the font before it measures, so the pane is sizedonce, at the size that sticks, and the capture is taken at that size.
Waiting is not enough on its own
FitAddon.proposeDimensions()measures nothing. It divides the container by aCACHED cell size, and xterm refreshes that cache only from
open(), from aresize that actually changed the grid, and on a device-pixel-ratio change —
nothing in it listens for font loading. A fit that runs after the font arrives
can therefore still divide by the fallback cell, propose the grid it already
has, and short-circuit before anything re-measures.
So the wait ends by calling
_charSizeService.measure()itself. That is thestep that makes the following fit see the real font. It reaches through
_core,as
FitAddonitself does and as seven existing call sites in this repo do, andit is guarded because a terminal can be disposed mid-wait.
Why
document.fonts.loadand not justreadyThe WebGL renderer rasterises glyphs through a canvas texture atlas, and canvas
text never triggers a CSS font fetch — so
document.fonts.readycan resolvewith a face never having been requested at all.
load()per family is whatactually asks for them.
Only families that can move the measured cell are awaited. The bundled symbols
font is ~1.2MB of private-use-area glyphs and xterm measures
W, so awaiting itput a megabyte in front of the first frame for nothing; the generic families
match no
FontFace.Bounded, and out of the way of live output
Neither
FontFaceSet.load()norFontFaceSet.readyhas a deadline, and theawait sits in front of the buffer replay, so a font request that never settled
would leave the tab spinning with output queued behind it — permanently, and on
every session, since they share one promise. The wait is raced against
TERMINAL_FONT_WAIT_MS, and it runs BEFORE_beginBufferLoadso a slow fontcannot hold live output back at all. Past the deadline the terminal fits against
whatever is painted, which is today's behaviour rather than a new failure.
Verification
Measured on a session opening at 2328px wide: the cell went from 8.43×16.00 to
8.00×21.00 roughly 900ms in, moving the grid from 112×36 to 118×28 after the
replay had already been painted. Comparing the browser's rendered rows against
tmux capture-paneon a busy session, five mismatched rows per run became zeroor one, the remaining one being the spinner's elapsed time advancing between the
two samples.
npm testis green (6768 passing), along with typecheck, lint, format:check andcheck:frontend-syntax.
Cross-browser, including one case this does NOT cover
Measured on the running build at 2328px wide, watching the grid settle after a
session loads. The wait resolves on all three engines and
document.fonts.loadexists on all three.
WebKit still moves once more, about two seconds after the fonts have settled. It
is width-only with the row count unchanged, roughly 110px appearing late, which
makes it a layout change rather than a font one — waiting for the font cannot
catch it and this PR does not claim to. I have not identified what moves it.
The repo's browser testing is Chromium-only in practice (every command in
docs/browser-testing-guide.md installs chromium), so this is a gap the project
does not currently cover either way.
Tests
An earlier revision of this PR shipped without tests, on the claim that a
browser font-loading race had no seam the vm harness could reach. That was
wrong. The suite pins the family filter, the forced re-measure, the deadline, a
rejecting load, a browser with no font API, and a terminal disposed mid-wait —
plus four ordering properties in
selectSession, including that iOS Safari'ssynchronous focus still precedes the first await. Each assertion was checked by
reverting the fix it covers and confirming it fails.
One of three PRs for the same report; the others cover a full-history replay
whose rows and cursor disagreed, and two windows sizing one pane. This one
stands alone.