Skip to content

Commit ca73c7f

Browse files
authored
fix(web): stop pty sizing loop on fractional cell widths (#84)
1 parent 5d85cde commit ca73c7f

2 files changed

Lines changed: 43 additions & 4 deletions

File tree

web/src/emulator/emulator.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,31 @@ describe('Emulator interface', () => {
277277
em.dispose()
278278
})
279279

280+
it('reports the exact laid-out size, not whole pixels', () => {
281+
// The renderer writes the screen's true size into its style attribute,
282+
// and on a Retina display that value is fractional: a cell is a whole
283+
// count of device pixels, divided by the pixel ratio. offsetWidth rounds
284+
// it to an integer, and that half-pixel, divided into a cell width and
285+
// multiplied back out across the pane, is enough to flip cellsThatFit
286+
// between N and N+1 columns depending on the dimensions the screen
287+
// happens to wear — each answer reshapes the pty, the new dimensions
288+
// re-round the other way, and the pane flickers with prompt redraws
289+
// forever.
290+
const el = document.createElement('div')
291+
document.body.appendChild(el)
292+
const em = createXtermEmulator({ cols: 100, rows: 24 })
293+
em.attachTo(el)
294+
// jsdom lays nothing out, so the renderer's write is stated by hand:
295+
// 100 columns of a 7.8125px cell, 24 rows of a 17.02083…px line.
296+
const screen = el.querySelector<HTMLElement>('.xterm-screen')!
297+
screen.style.width = '781.25px'
298+
screen.style.height = '408.5px'
299+
300+
expect(em.contentSize()).toEqual({ width: 781.25, height: 408.5 })
301+
em.dispose()
302+
el.remove()
303+
})
304+
280305
it('takes a colour palette at build time and again afterwards', () => {
281306
// Both matter. The option is what stops a terminal painting one frame in
282307
// xterm's own colours before flue's land; setTheme is what lets a running

web/src/emulator/xterm.ts

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -383,10 +383,24 @@ export function createXtermEmulator(opts: XtermOptions = {}): Emulator {
383383
if (disposed) return null
384384
const screen = term.element?.querySelector(SCREEN_SELECTOR)
385385
if (!(screen instanceof HTMLElement)) return null
386-
// offsetWidth/offsetHeight rather than getBoundingClientRect, because a
387-
// non-primary view is scaled by CSS: the rect would report the scaled
388-
// box, and dividing the pane by that converges on nothing.
389-
const size = { width: screen.offsetWidth, height: screen.offsetHeight }
386+
// The style attribute, which both renderers write the screen's exact
387+
// size into, rather than either measurement the browser offers. The
388+
// rect is out because a non-primary view is scaled by CSS: it reports
389+
// the scaled box, and dividing the pane by that converges on nothing.
390+
// And offsetWidth — unscaled, the obvious next choice — rounds to
391+
// whole pixels, when on a Retina display the true width is fractional:
392+
// a cell is a whole count of device pixels divided by the pixel ratio.
393+
// The half-pixel it drops, divided into a cell width and multiplied
394+
// back out across the pane, can flip cellsThatFit between N and N+1
395+
// columns depending on the dimensions the screen currently wears; the
396+
// pty then answers each flip with the other one — a sizing loop that
397+
// redraws the prompt several times a second for as long as the pane
398+
// keeps that width. (Style, not "the obvious CSS word for it": see the
399+
// Tailwind scanner note in src/styles.css.)
400+
const size = {
401+
width: parseFloat(screen.style.width) || screen.offsetWidth,
402+
height: parseFloat(screen.style.height) || screen.offsetHeight,
403+
}
390404
return size.width > 0 && size.height > 0 ? size : null
391405
},
392406

0 commit comments

Comments
 (0)