From cd6f370c2855ff0d4ed99030a5a4494323858cb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Wed, 28 Sep 2022 20:03:55 +0200 Subject: [PATCH 1/5] unbuffered delivery for userInput response only --- demo/server.js | 35 ++++++++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/demo/server.js b/demo/server.js index 8bb684a2..c594da3e 100644 --- a/demo/server.js +++ b/demo/server.js @@ -79,14 +79,25 @@ function startServer() { console.log('Connected to terminal ' + term.pid); ws.send(logs[term.pid]); + // unbuffered delivery after user input + let userInput = false; + // string message buffering - function buffer(socket, timeout) { + function buffer(socket, timeout, maxSize) { let s = ''; let sender = null; return (data) => { s += data; - if (!sender) { - sender = queueMicrotask(() => { + if (s.length > maxSize || userInput) { + userInput = false; + socket.send(s); + s = ''; + if (sender) { + clearTimeout(sender); + sender = null; + } + } else if (!sender) { + sender = setTimeout(() => { socket.send(s); s = ''; sender = null; @@ -95,15 +106,24 @@ function startServer() { }; } // binary message buffering - function bufferUtf8(socket, timeout) { + function bufferUtf8(socket, timeout, maxSize) { let buffer = []; let sender = null; let length = 0; return (data) => { buffer.push(data); length += data.length; - if (!sender) { - sender = queueMicrotask(() => { + if (length > maxSize || userInput) { + userInput = false; + socket.send(Buffer.concat(buffer, length)); + buffer = []; + length = 0; + if (sender) { + clearTimeout(sender); + sender = null; + } + } else if (!sender) { + sender = setTimeout(() => { socket.send(Buffer.concat(buffer, length)); buffer = []; sender = null; @@ -112,7 +132,7 @@ function startServer() { } }; } - const send = USE_BINARY ? bufferUtf8(ws, 5) : buffer(ws, 5); + const send = (USE_BINARY ? bufferUtf8 : buffer)(ws, 2, 262144); // WARNING: This is a naive implementation that will not throttle the flow of data. This means // it could flood the communication channel and make the terminal unresponsive. Learn more about @@ -126,6 +146,7 @@ function startServer() { }); ws.on('message', function(msg) { term.write(msg); + userInput = true; }); ws.on('close', function () { term.kill(); From 193d305dcd37537aa47758e580059ffd220b8723 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Thu, 29 Sep 2022 10:36:08 +0200 Subject: [PATCH 2/5] revert timeout to 5ms, remove log in server.js --- demo/server.js | 17 +++-------------- src/common/input/WriteBuffer.ts | 2 +- 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/demo/server.js b/demo/server.js index c594da3e..0e82f9e9 100644 --- a/demo/server.js +++ b/demo/server.js @@ -16,8 +16,7 @@ function startServer() { var app = express(); expressWs(app); - var terminals = {}, - logs = {}; + var terminals = {}; app.use('/xterm.css', express.static(__dirname + '/../css/xterm.css')); app.get('/logo.png', (req, res) => { @@ -55,10 +54,6 @@ function startServer() { console.log('Created terminal with PID: ' + term.pid); terminals[term.pid] = term; - logs[term.pid] = ''; - term.on('data', function(data) { - logs[term.pid] += data; - }); res.send(term.pid.toString()); res.end(); }); @@ -77,7 +72,6 @@ function startServer() { app.ws('/terminals/:pid', function (ws, req) { var term = terminals[parseInt(req.params.pid)]; console.log('Connected to terminal ' + term.pid); - ws.send(logs[term.pid]); // unbuffered delivery after user input let userInput = false; @@ -132,17 +126,13 @@ function startServer() { } }; } - const send = (USE_BINARY ? bufferUtf8 : buffer)(ws, 2, 262144); + const send = (USE_BINARY ? bufferUtf8 : buffer)(ws, 5, 262144); // WARNING: This is a naive implementation that will not throttle the flow of data. This means // it could flood the communication channel and make the terminal unresponsive. Learn more about // the problem and how to implement flow control at https://xtermjs.org/docs/guides/flowcontrol/ term.on('data', function(data) { - try { - send(data); - } catch (ex) { - // The WebSocket is not open, ignore - } + send(data); }); ws.on('message', function(msg) { term.write(msg); @@ -153,7 +143,6 @@ function startServer() { console.log('Closed terminal ' + term.pid); // Clean things up delete terminals[term.pid]; - delete logs[term.pid]; }); }); diff --git a/src/common/input/WriteBuffer.ts b/src/common/input/WriteBuffer.ts index 4f316f24..8cb1edf4 100644 --- a/src/common/input/WriteBuffer.ts +++ b/src/common/input/WriteBuffer.ts @@ -106,7 +106,7 @@ export class WriteBuffer { this._bufferOffset = 0; // If this is the first write call after the user has done some input, - // parse it immediately in an upcoming microtask to minimize reduce input, + // parse it immediately to minimize reduce input, // otherwise schedule for the next event if (this._didUserInput) { this._didUserInput = false; From 150499b7e5ee56fa4bc0985f0099ce739ff6f802 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Thu, 29 Sep 2022 10:46:25 +0200 Subject: [PATCH 3/5] comment fix --- src/common/input/WriteBuffer.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/input/WriteBuffer.ts b/src/common/input/WriteBuffer.ts index 8cb1edf4..5e19105d 100644 --- a/src/common/input/WriteBuffer.ts +++ b/src/common/input/WriteBuffer.ts @@ -106,7 +106,7 @@ export class WriteBuffer { this._bufferOffset = 0; // If this is the first write call after the user has done some input, - // parse it immediately to minimize reduce input, + // parse it immediately to minimize input latency, // otherwise schedule for the next event if (this._didUserInput) { this._didUserInput = false; From 2a602ef231abcbb431d240679a6d02f44a99c83b Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 1 Oct 2022 10:20:37 -0700 Subject: [PATCH 4/5] Ensure texture atlas comparison uses rgba not object Fixes #4128 --- addons/xterm-addon-canvas/src/atlas/CharAtlasUtils.ts | 4 ++-- addons/xterm-addon-webgl/src/atlas/CharAtlasUtils.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/addons/xterm-addon-canvas/src/atlas/CharAtlasUtils.ts b/addons/xterm-addon-canvas/src/atlas/CharAtlasUtils.ts index b0151e30..fa4be1e5 100644 --- a/addons/xterm-addon-canvas/src/atlas/CharAtlasUtils.ts +++ b/addons/xterm-addon-canvas/src/atlas/CharAtlasUtils.ts @@ -45,8 +45,8 @@ export function configEquals(a: ICharAtlasConfig, b: ICharAtlasConfig): boolean a.allowTransparency === b.allowTransparency && a.scaledCharWidth === b.scaledCharWidth && a.scaledCharHeight === b.scaledCharHeight && - a.colors.foreground === b.colors.foreground && - a.colors.background === b.colors.background; + a.colors.foreground.rgba === b.colors.foreground.rgba && + a.colors.background.rgba === b.colors.background.rgba; } export function is256Color(colorCode: number): boolean { diff --git a/addons/xterm-addon-webgl/src/atlas/CharAtlasUtils.ts b/addons/xterm-addon-webgl/src/atlas/CharAtlasUtils.ts index 83f82fa7..dc503d75 100644 --- a/addons/xterm-addon-webgl/src/atlas/CharAtlasUtils.ts +++ b/addons/xterm-addon-webgl/src/atlas/CharAtlasUtils.ts @@ -70,8 +70,8 @@ export function configEquals(a: ICharAtlasConfig, b: ICharAtlasConfig): boolean a.scaledCharHeight === b.scaledCharHeight && a.drawBoldTextInBrightColors === b.drawBoldTextInBrightColors && a.minimumContrastRatio === b.minimumContrastRatio && - a.colors.foreground === b.colors.foreground && - a.colors.background === b.colors.background; + a.colors.foreground.rgba === b.colors.foreground.rgba && + a.colors.background.rgba === b.colors.background.rgba; } export function is256Color(colorCode: number): boolean { From f087bd121ea7f1006bad7564eb4e95cf06490d87 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 1 Oct 2022 21:38:01 -0700 Subject: [PATCH 5/5] Add willReadFrequently to canvas renderer too --- addons/xterm-addon-canvas/src/atlas/DynamicCharAtlas.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/addons/xterm-addon-canvas/src/atlas/DynamicCharAtlas.ts b/addons/xterm-addon-canvas/src/atlas/DynamicCharAtlas.ts index 3098538d..b809378e 100644 --- a/addons/xterm-addon-canvas/src/atlas/DynamicCharAtlas.ts +++ b/addons/xterm-addon-canvas/src/atlas/DynamicCharAtlas.ts @@ -96,7 +96,10 @@ export class DynamicCharAtlas extends BaseCharAtlas { const tmpCanvas = document.createElement('canvas'); tmpCanvas.width = this._config.scaledCharWidth; tmpCanvas.height = this._config.scaledCharHeight; - this._tmpCtx = throwIfFalsy(tmpCanvas.getContext('2d', { alpha: this._config.allowTransparency })); + this._tmpCtx = throwIfFalsy(tmpCanvas.getContext('2d', { + alpha: this._config.allowTransparency, + willReadFrequently: true + })); this._width = Math.floor(TEXTURE_WIDTH / this._config.scaledCharWidth); this._height = Math.floor(TEXTURE_HEIGHT / this._config.scaledCharHeight);