Xterm.js: Display density changes lead to corrupted characters

[Eugeny]

When switching between a built-in Retina display and an external normal-density display, some characters will be drawn with incorrect size by the canvas renderer.

Now, I can't confirm whether it's possibly Electron's fault, but I suspect that Chrome would behave the same.

xterm 3.13.2

[Tyriar]

Might be rel;ated to ligature problems in https://github.com/xtermjs/xterm.js/issues/2135?

[Eugeny]

Might be, but I don't think so. Looks like some chars might be cached in the renderer atlas with a smaller resolution?

[Tyriar]

@Eugeny seems to me like the cached parts are -r, -x and --.

[Eugeny]

Good point - these are the ones that are incorrectly rendered in #2135

[Eugeny]

Weirdly enough, switching to a higher density makes these characters larger, and switching to a smaller density display makes them smaller.

[Tyriar]

The APIs that https://github.com/xtermjs/xterm-addon-ligatures use probably just need some updating for the recent updates to renderers.

[jerch]

Cannot repro this and the other issue #2135 (on Ubuntu though, with Fira Mono and ligatures enabled).
Kinda sounds as the dpr change gets not applied? @Eugeny Could you test, if the demo shows the same wrongly scaled glyphs when you switch the screens?

[Eugeny]

Still reliably reproducible in master with the webgl renderer:

The font doesn't get corrupted immediately after the DPI switch, but rather after font-family and font-size are updated (re-set to the same values) in xterm.

[yuezk]

@jerch @Tyriar
I came from the VS Code issue https://github.com/microsoft/vscode/issues/94656
It can be reliably reproduced reliably on my macOS 10.15.3 with VS Code 1.44.2 and enabled the experimentalWebgl renderer type.

I have two displays show below

1. Keep the external display connected.
2. Open the Terminal if it hasn't been.
3. Move the VS Code to the external display if it hasn't been.
   
4. Disconnect the external display.
5. The VS Code will be moved to the builtin display automatically.
6. The Terminal font corrupted.
   

[Tyriar]

I've seen this happen a bunch, we're probably just missing a case in https://github.com/xtermjs/xterm.js/blob/master/src/browser/ScreenDprMonitor.ts, or something is not listening to the monitor.

[Eugeny]

@Tyriar in case this helps, it can be reliably reproduced with the demo page, but only for unicode characters. ASCII will get occasionally corrupted, but not always.

Changing font size to something else and back immediately fixes it.

[jerch]

@Eugeny Could you test if this can be repro'ed for ASCII as well, if the fg color is set to RGB?

I suspect that _drawUncachedChars does not get the DPR changes for some reason.

[Eugeny]

@jerch I've just pushed a PR with the fix. The listener wasn't being called at all.