Render glitches (blacked out content)

[willmcgugan]

I'm working on a TUI framework for Python, which does tend to push the terminal more than a typical app.

On certain apps, after a few seconds of updates the terminal starts glitching heavily. The more animation, the quicker this seems to happen.

It's probably best to show in a video:

Screen.Recording.2023-04-18.at.14.17.53.mov

This doesn't occur on other terminals. And I'm fairly sure (although not 100%) that this is not an error in Textual itself.

The video above is with an embedded xterm.js, but this issue also occurs in VSCode.

I haven't addressed flow control yet. But from experimentation, it only takes a a few hundred K before this issue occurs.

Details

• Browser and browser version: Chrome. Version 112.0.5615.49 (Official Build) (arm64)
• OS version: MacOS 13.3.1 (22E261)
• xterm.js version: 5.1.0"

Steps to reproduce

Install Textual:

pip install textual[dev]

Then in VS code integrated terminal:

textual easing

[willmcgugan]

Possibly part of the same issue, but if I keep clicking the app in this glitched state, then a different glitch occurs.

Screen.Recording.2023-04-18.at.14.35.00_1.mp4

Which looks like some kind of texture cache has become corrupt?

[jerch]

To me both looks like texture cache issues, but at different levels - 1. looks like no invalidate/erase going on, as if nothing gets into textures anymore (works fine in the beginning). 2. looks like a cache key collision, placing stuff into wrong place. (Quickly tested in xterm.js demo under linux/chrome and firefox...)

One question regarding your screen updates - from terminal logs it seems that you do a full screen updates for every animation step. Wouldn't a diff buffer approach save a lot of bandwidth and TE work here?

[willmcgugan]

One question regarding your screen updates - from terminal logs it seems that you do a full screen updates for every animation step. Wouldn't a diff buffer approach save a lot of bandwidth and TE work here?

It does do diffs at the widget level, but not at the character level. In this particular example, the changed widgets cover most of the screen so the updates are large.

[willmcgugan]

After some investigation, I've found that it is nothing to do with the speed or volume of updates. It seems to be triggered when Textual writes a large number of different colors.

If I disable the blending effect in the video above, then I can't reproduce the issue at all.

I can reproduce this outside of Textual. I've documented this in #4484

[willmcgugan]

I've found that calling clearTextureAtlas after every write essentially fixes this issue. I'm assuming this is sub-optimal, and it does seem a little slower to update.

[jerch]

I've found that calling clearTextureAtlas after every write essentially fixes this issue. I'm assuming this is sub-optimal, and it does seem a little slower to update.

This kinda confirms that there is an issue with eviction in the cache, like it is ever-growing until an upper bound is hit (thats also what I see in the demo, the atlas pages grow really big there from your easing example).
Since I have not dealt with the atlas implementation myself this prolly has to wait until @Tyriar finds some time to look into it.

[Tyriar]

I'm assuming this is sub-optimal, and it does seem a little slower to update.

Very, especially if you use a lot of unique glyphs. You don't seem to use that much other than the fading of the yellow square.

#4351 is the only issue I was aware of with the texture atlas currently, it's possibly the same.

@willmcgugan what renderer is that using?

[willmcgugan]

It was the GL renderer.

[Tyriar]

Maybe related? #4485

[jerch]

@Tyriar It still repros with #4527, which fixes #4485. (Chrome)

Edit: Also repros in Firefox with the same weird blacked out behavior. (#4485 never happened on Firefox)

Edit2: Maybe a hint - if I return early from the microtask here skipping all code, I cannot provoke the blackout anymore:

xterm.js/src/browser/renderer/shared/TextureAtlas.ts

Lines 152 to 160 in cdec1e5

	if (TextureAtlas.maxAtlasPages && this._pages.length >= Math.max(4, TextureAtlas.maxAtlasPages / 2)) {
	queueMicrotask(() => {
	// Find the set of the largest 4 images, below the maximum size, with the highest
	// percentages used
	const pagesBySize = this._pages.filter(e => {
	return e.canvas.width * 2 <= (TextureAtlas.maxTextureSize || Constants.FORCED_MAX_TEXTURE_SIZE);
	}).sort((a, b) => {
	if (b.canvas.width !== a.canvas.width) {
	return b.canvas.width - a.canvas.width;

Yes, just tracked the invocation of the microtask - the issue occurs as soon as the page merging happened.

[jerch]

@Tyriar Did a bit more digging.

For webgl the issue gets resolved by this change:

diff --git a/addons/xterm-addon-webgl/src/WebglRenderer.ts b/addons/xterm-addon-webgl/src/WebglRenderer.ts
index fcb0dd80..2b96c7d6 100644
--- a/addons/xterm-addon-webgl/src/WebglRenderer.ts
+++ b/addons/xterm-addon-webgl/src/WebglRenderer.ts
@@ -338,10 +338,13 @@ export class WebglRenderer extends Disposable implements IRenderer {
     // Tell renderer the frame is beginning
     if (this._glyphRenderer.beginFrame()) {
       this._clearModel(true);
+      this._updateModel(0, this._terminal.rows);
+    } else {
+      this._updateModel(start, end);
     }
 
     // Update model to reflect what's drawn
-    this._updateModel(start, end);
+    //this._updateModel(start, end);
 
     // Render
     this._rectangleRenderer?.render();

which forces the model into a full refresh upon a page merge in the atlas.

For canvas the issue goes away, if comment out the bitmap usage here:

diff --git a/addons/xterm-addon-canvas/src/BaseRenderLayer.ts b/addons/xterm-addon-canvas/src/BaseRenderLayer.ts
index 88d929f1..06185528 100644
--- a/addons/xterm-addon-canvas/src/BaseRenderLayer.ts
+++ b/addons/xterm-addon-canvas/src/BaseRenderLayer.ts
@@ -388,7 +388,8 @@ export abstract class BaseRenderLayer extends Disposable implements IRenderLayer
       this._bitmapGenerator[glyph.texturePage]!.version = this._charAtlas.pages[glyph.texturePage].version;
     }
     this._ctx.drawImage(
-      this._bitmapGenerator[glyph.texturePage]?.bitmap || this._charAtlas!.pages[glyph.texturePage].canvas,
+      // this._bitmapGenerator[glyph.texturePage]?.bitmap || this._charAtlas!.pages[glyph.texturePage].canvas,
+      this._charAtlas!.pages[glyph.texturePage].canvas,
       glyph.texturePosition.x,
       glyph.texturePosition.y,
       glyph.size.x,

It looks like some issue with the bitmap used after a page merge. Also bitmap.close() is never called in the bitmap lifecycling (not sure if JS will fully cleanup bitmaps by GC, still wondering why there is an explicit ImageBitmap.close).

Since I am not sure, if these are the right fixes (not familiar with the renderer code), I'd leave it up to you.

[Tyriar]

@jerch the first one is definitely the first fix as when beginFrame returns true the atlas is cleared/merged so any pointers they have to textures become invalidated:

https://github.com/Tyriar/xterm.js/blob/eb56356133c91df39fbdab2e416d30a79d678acd/src/browser/renderer/shared/TextureAtlas.ts#L192-L194

You can PR that fix if you want, I'll look into the second one now as we don't want to lose the bitmap optimization

[jerch]

Ah ok, gonna do the webgl PR then.

[jerch]

@Tyriar Still not fully fixed - still seeing the glitch as shown here #4480 (comment)

To me this also looks like a different issue 👉 #4534

[willmcgugan]

I can still reproduce this isssue. Tried with the latest xterm and xterm-addon-webgl, and the beta versions.

Seems the same in VSCode: