Inactive selection background is not redrawn correctly when switching to the canvas renderer

[Tyriar]

Repro:

1. Set inactive selection background to an obvious opaque color
2. Enable webgl renderer
3. Select some text
4. Enable canvas renderer

Click to remove selection:

Scrolling redraws and fixes it

[jerch]

My first guess is that there is something off with the selection state and the events of the selection service. I stumbled over this in the DOM PR when trying to reduce the selection updates forced on the renderer (was this "there is no selection at all case - why is it creating so many events") - but then I figured the empty updates are also used to clean up selection remnants in the renderer state. The selection changed event is also fired on scrolling, guess thats the reason why it fixes things here.

Maybe we need a bit more clever event handling on selection service side - like separating selection changed from other states like focus/unfocus?

[tisilent]

Found a problem.Even if the selection exceeds the boundary, it will continue to render continuously. Canvas.

[tisilent]

I'm trying, but it may not be so fast.

[Tyriar]

@tisilent we may remove the canvas renderer soon since the dom renderer is so much better now, so this one is pretty low priority

[tisilent]

I see.

[tisilent]

Hi @Tyriar ,Do you have any comparative information as an entry point? I would like to explore the differences between these two renderers.

[Tyriar]

@tisilent do you mean where the rendering occurs? These are roughly equivalent functions:

DOM:

https://github.com/xtermjs/xterm.js/blob/cc83b9a4ca25335ccf050febb545d60e8609646c/src/browser/renderer/dom/DomRendererRowFactory.ts#L61-L73

Canvas:

https://github.com/xtermjs/xterm.js/blob/cc83b9a4ca25335ccf050febb545d60e8609646c/addons/xterm-addon-canvas/src/TextRenderLayer.ts#L69-L77

Webgl:

https://github.com/xtermjs/xterm.js/blob/master/addons/xterm-addon-webgl/src/WebglRenderer.ts#L374 https://github.com/xtermjs/xterm.js/blob/cc83b9a4ca25335ccf050febb545d60e8609646c/addons/xterm-addon-webgl/src/GlyphRenderer.ts#L239-L244

[tisilent]

we may remove the canvas renderer soon since the dom renderer is so much better now I have a component that comes in both dom and canvas versions, so I have also considered this aspect.

[Tyriar]

@tisilent some history on that is that years ago, back when the canvas renderer was built in to the core, we wanted to remove it completely to reduce maintenance overhead. We ended up keeping it pretty much exclusively for the cases where the webgl renderer would not work like some Linux setups, VMs, iOS/Safari. The case of Safari not supporting webgl2 was the biggest of these which made us instead split canvas into an addon which reduces code loading unless the user opts in or falls back to canvas.

But since https://github.com/xtermjs/xterm.js/pull/4605 and its follow ups, plus the fact that webgl2 has shipped in Safari/WebKit, I think that covers all our bases. So we could rip out the ~1-2k lines of code and also move the shared renderer code in core into the addon (which probably won't reduce core bundle size but it will simplify things).

[tisilent]

Thanks.