From 81a62306afebe463a192e7d4b85f345ab007c9f7 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Wed, 10 Aug 2022 05:59:29 -0700 Subject: [PATCH 01/28] Ensure canvas device dims are > 0x0 when firing callback See microsoft/vscode#157444 --- src/browser/renderer/DevicePixelObserver.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/browser/renderer/DevicePixelObserver.ts b/src/browser/renderer/DevicePixelObserver.ts index 611eb91023..caf1b21dfa 100644 --- a/src/browser/renderer/DevicePixelObserver.ts +++ b/src/browser/renderer/DevicePixelObserver.ts @@ -24,10 +24,12 @@ export function observeDevicePixelDimensions(element: HTMLElement, callback: (de return; } - callback( - entry.devicePixelContentBoxSize[0].inlineSize, - entry.devicePixelContentBoxSize[0].blockSize - ); + // Fire the callback, ignore events where the dimensions are 0x0 as the canvas is likely hidden + const width = entry.devicePixelContentBoxSize[0].inlineSize; + const height = entry.devicePixelContentBoxSize[0].blockSize; + if (width > 0 && height > 0) { + callback(width, height); + } }); observer.observe(element, { box: ['device-pixel-content-box'] } as any); return toDisposable(() => observer?.disconnect()); From 9a119a7292d43a18118f2fbb727c5729609e8f54 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Wed, 10 Aug 2022 06:27:32 -0700 Subject: [PATCH 02/28] Whitespace change to trigger release --- addons/xterm-addon-webgl/README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/addons/xterm-addon-webgl/README.md b/addons/xterm-addon-webgl/README.md index a431170f5d..5dee6b622c 100644 --- a/addons/xterm-addon-webgl/README.md +++ b/addons/xterm-addon-webgl/README.md @@ -2,7 +2,6 @@ An addon for [xterm.js](https://github.com/xtermjs/xterm.js) that enables a WebGL2-based renderer. This addon requires xterm.js v4+. - ### Install ```bash From 79d47893df3e734b25225318a0d0a45d4020d02f Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 19 Aug 2022 13:19:08 -0700 Subject: [PATCH 03/28] Move webgl underline rendering down See microsoft/vscode#158326 --- addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts index 13dceae67c..72421b2d15 100644 --- a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts +++ b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts @@ -439,8 +439,7 @@ export class WebglCharAtlas implements IDisposable { if (underline) { this._tmpCtx.save(); const lineWidth = Math.max(1, Math.floor(this._config.fontSize * window.devicePixelRatio / 15)); - // When the width is odd, draw at 0.5 position. Offset by an additional 1 dpr to bring the - // underline closer to the character + // When the line width is odd, draw at a 0.5 position const yOffset = (lineWidth % 2 === 1 ? 0.5 : 0) + window.devicePixelRatio; this._tmpCtx.lineWidth = lineWidth; @@ -463,9 +462,9 @@ export class WebglCharAtlas implements IDisposable { this._tmpCtx.beginPath(); const xLeft = padding; const xRight = padding + this._config.scaledCellWidth; - const yTop = Math.ceil(padding + this._config.scaledCharHeight - lineWidth) - yOffset; - const yMid = padding + this._config.scaledCharHeight - yOffset; - const yBot = Math.ceil(padding + this._config.scaledCharHeight + lineWidth) - yOffset; + const yTop = Math.ceil(padding + this._config.scaledCharHeight) - yOffset; + const yMid = padding + this._config.scaledCharHeight + lineWidth - yOffset; + const yBot = Math.ceil(padding + this._config.scaledCharHeight + lineWidth * 2) - yOffset; switch (this._workAttributeData.extended.underlineStyle) { case UnderlineStyle.DOUBLE: this._tmpCtx.moveTo(xLeft, yTop); From 0698afaf12c89df34b6c72edca9a7e229b4ff0fe Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 19 Aug 2022 13:24:39 -0700 Subject: [PATCH 04/28] Only stroke glyph when font size >= 12 --- addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts index 72421b2d15..a7e92f9418 100644 --- a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts +++ b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts @@ -440,7 +440,7 @@ export class WebglCharAtlas implements IDisposable { this._tmpCtx.save(); const lineWidth = Math.max(1, Math.floor(this._config.fontSize * window.devicePixelRatio / 15)); // When the line width is odd, draw at a 0.5 position - const yOffset = (lineWidth % 2 === 1 ? 0.5 : 0) + window.devicePixelRatio; + const yOffset = lineWidth % 2 === 1 ? 0.5 : 0; this._tmpCtx.lineWidth = lineWidth; // Underline color @@ -527,8 +527,9 @@ export class WebglCharAtlas implements IDisposable { this._tmpCtx.restore(); // Draw stroke in the background color for non custom characters in order to give an outline - // between the text and the underline - if (!customGlyph) { + // between the text and the underline. Only do this when font size is >= 12 as the underline + // looks odd when the font size is too small + if (!customGlyph && this._config.fontSize >= 12) { // This only works when transparency is disabled because it's not clear how to clear stroked // text if (!this._config.allowTransparency && chars !== ' ') { From b59de22c36ac376ff83bec563c0cb2a2dcb5429c Mon Sep 17 00:00:00 2001 From: greenmashimaro Date: Tue, 23 Aug 2022 15:09:43 +0800 Subject: [PATCH 05/28] fix typo --- src/common/buffer/BufferSet.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/common/buffer/BufferSet.ts b/src/common/buffer/BufferSet.ts index 7d07cec8d2..f940bb8f39 100644 --- a/src/common/buffer/BufferSet.ts +++ b/src/common/buffer/BufferSet.ts @@ -58,14 +58,14 @@ export class BufferSet extends Disposable implements IBufferSet { } /** - * Returns the normal Buffer of the BufferSet + * Returns the currently active Buffer of the BufferSet */ public get active(): Buffer { return this._activeBuffer; } /** - * Returns the currently active Buffer of the BufferSet + * Returns the normal Buffer of the BufferSet */ public get normal(): Buffer { return this._normal; From 559b6950b2ba4318ca44948350779e5f364a5f58 Mon Sep 17 00:00:00 2001 From: greenmashimaro Date: Tue, 23 Aug 2022 15:09:52 +0800 Subject: [PATCH 06/28] remove unnecessary eslint comments --- src/browser/LocalizableStrings.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/browser/LocalizableStrings.ts b/src/browser/LocalizableStrings.ts index c0a904cbce..e34eaa482c 100644 --- a/src/browser/LocalizableStrings.ts +++ b/src/browser/LocalizableStrings.ts @@ -3,8 +3,5 @@ * @license MIT */ -// eslint-disable-next-line prefer-const -export let promptLabel = 'Terminal input'; - -// eslint-disable-next-line prefer-const -export let tooMuchOutput = 'Too much output to announce, navigate to rows manually to read'; +export const promptLabel = 'Terminal input'; +export const tooMuchOutput = 'Too much output to announce, navigate to rows manually to read'; From ac81d3f72dd6a57e169dbf57b2942a7d1f87eadf Mon Sep 17 00:00:00 2001 From: greenmashimaro Date: Wed, 24 Aug 2022 22:41:45 +0800 Subject: [PATCH 07/28] feat: restore LocalizableStrings.ts const export --- src/browser/LocalizableStrings.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/browser/LocalizableStrings.ts b/src/browser/LocalizableStrings.ts index e34eaa482c..fcf677c0cf 100644 --- a/src/browser/LocalizableStrings.ts +++ b/src/browser/LocalizableStrings.ts @@ -3,5 +3,12 @@ * @license MIT */ -export const promptLabel = 'Terminal input'; -export const tooMuchOutput = 'Too much output to announce, navigate to rows manually to read'; +/* + * note: intentional design, it's exposed to embedders of xterm.js + * so they can change the strings to localize xterm.js. #4055 + */ +// eslint-disable-next-line prefer-const +export let promptLabel = 'Terminal input'; + +// eslint-disable-next-line prefer-const +export let tooMuchOutput = 'Too much output to announce, navigate to rows manually to read'; From 86ba6cd2882fb0dfff2ac151b6f75f0a2bac76d5 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Wed, 24 Aug 2022 08:55:11 -0700 Subject: [PATCH 08/28] Tweak comment --- src/browser/LocalizableStrings.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/browser/LocalizableStrings.ts b/src/browser/LocalizableStrings.ts index fcf677c0cf..d8bcc2c619 100644 --- a/src/browser/LocalizableStrings.ts +++ b/src/browser/LocalizableStrings.ts @@ -3,10 +3,8 @@ * @license MIT */ -/* - * note: intentional design, it's exposed to embedders of xterm.js - * so they can change the strings to localize xterm.js. #4055 - */ +// This file contains strings that get exported in the API so they can be localized + // eslint-disable-next-line prefer-const export let promptLabel = 'Terminal input'; From e40cf0684eba2314af92467768b6546255760438 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Wed, 24 Aug 2022 15:36:03 -0700 Subject: [PATCH 09/28] Properly clear underscore glyph's background See microsoft/vscode#158497 --- addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts index 13dceae67c..33b514b9ab 100644 --- a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts +++ b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts @@ -560,7 +560,10 @@ export class WebglCharAtlas implements IDisposable { let isBeyondCellBounds = clearColor(this._tmpCtx.getImageData(padding, padding, this._config.scaledCellWidth, this._config.scaledCellHeight), backgroundColor, foregroundColor, enableClearThresholdCheck); if (isBeyondCellBounds) { for (let offset = 1; offset <= 5; offset++) { - this._tmpCtx.clearRect(0, 0, this._tmpCanvas.width, this._tmpCanvas.height); + this._tmpCtx.save(); + this._tmpCtx.fillStyle = backgroundColor.css; + this._tmpCtx.fillRect(0, 0, this._tmpCanvas.width, this._tmpCanvas.height); + this._tmpCtx.restore(); this._tmpCtx.fillText(chars, padding, padding + this._config.scaledCharHeight - offset); isBeyondCellBounds = clearColor(this._tmpCtx.getImageData(padding, padding, this._config.scaledCellWidth, this._config.scaledCellHeight), backgroundColor, foregroundColor, enableClearThresholdCheck); if (!isBeyondCellBounds) { From 774ebe40e6b91cf1e81e0834876f7932266502c8 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 26 Aug 2022 07:26:36 -0700 Subject: [PATCH 10/28] Improve glyph padding, especially in large font sizes Fixes #4066 --- .../xterm-addon-canvas/src/BaseRenderLayer.ts | 4 +- .../src/atlas/WebglCharAtlas.ts | 2 +- src/browser/renderer/CustomGlyphs.ts | 49 ++++++++++++------- 3 files changed, 35 insertions(+), 20 deletions(-) diff --git a/addons/xterm-addon-canvas/src/BaseRenderLayer.ts b/addons/xterm-addon-canvas/src/BaseRenderLayer.ts index a69968cc53..dfd814e333 100644 --- a/addons/xterm-addon-canvas/src/BaseRenderLayer.ts +++ b/addons/xterm-addon-canvas/src/BaseRenderLayer.ts @@ -344,7 +344,7 @@ export abstract class BaseRenderLayer implements IRenderLayer { // Draw custom characters if applicable let drawSuccess = false; if (this._optionsService.rawOptions.customGlyphs !== false) { - drawSuccess = tryDrawCustomChar(this._ctx, cell.getChars(), x * this._scaledCellWidth, y * this._scaledCellHeight, this._scaledCellWidth, this._scaledCellHeight); + drawSuccess = tryDrawCustomChar(this._ctx, cell.getChars(), x * this._scaledCellWidth, y * this._scaledCellHeight, this._scaledCellWidth, this._scaledCellHeight, this._optionsService.rawOptions.fontSize); } // Draw the character @@ -473,7 +473,7 @@ export abstract class BaseRenderLayer implements IRenderLayer { // Draw custom characters if applicable let drawSuccess = false; if (this._optionsService.rawOptions.customGlyphs !== false) { - drawSuccess = tryDrawCustomChar(this._ctx, cell.getChars(), x * this._scaledCellWidth, y * this._scaledCellHeight, this._scaledCellWidth, this._scaledCellHeight); + drawSuccess = tryDrawCustomChar(this._ctx, cell.getChars(), x * this._scaledCellWidth, y * this._scaledCellHeight, this._scaledCellWidth, this._scaledCellHeight, this._optionsService.rawOptions.fontSize); } // Draw the character diff --git a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts index 90d13e5547..2321e5469b 100644 --- a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts +++ b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts @@ -427,7 +427,7 @@ export class WebglCharAtlas implements IDisposable { // Draw custom characters if applicable let customGlyph = false; if (this._config.customGlyphs !== false) { - customGlyph = tryDrawCustomChar(this._tmpCtx, chars, padding, padding, this._config.scaledCellWidth, this._config.scaledCellHeight); + customGlyph = tryDrawCustomChar(this._tmpCtx, chars, padding, padding, this._config.scaledCellWidth, this._config.scaledCellHeight, this._config.fontSize); } // Whether to clear pixels based on a threshold difference between the glyph color and the diff --git a/src/browser/renderer/CustomGlyphs.ts b/src/browser/renderer/CustomGlyphs.ts index 883b4d5dc8..2f32f98122 100644 --- a/src/browser/renderer/CustomGlyphs.ts +++ b/src/browser/renderer/CustomGlyphs.ts @@ -349,25 +349,27 @@ const enum VectorType { * not been patched with powerline characters and also to get pixel perfect rendering as rendering * issues can occur around AA/SPAA. * + * The line variants draw beyond the cell and get clipped to ensure the end of the line is not visible. + * * Original symbols defined in https://github.com/powerline/fontpatcher */ export const powerlineDefinitions: { [index: string]: IVectorShape } = { // Right triangle solid - '\u{E0B0}': { d: 'M0,0 L1,.5 L0,1', type: VectorType.FILL }, + '\u{E0B0}': { d: 'M0,0 L1,.5 L0,1', type: VectorType.FILL, rightPadding: 2 }, // Right triangle line - '\u{E0B1}': { d: 'M0,0 L1,.5 L0,1', type: VectorType.STROKE, leftPadding: window.devicePixelRatio / 2, rightPadding: window.devicePixelRatio / 2 }, + '\u{E0B1}': { d: 'M-1,-.5 L1,.5 L-1,1.5', type: VectorType.STROKE, leftPadding: 1, rightPadding: 1 }, // Left triangle solid - '\u{E0B2}': { d: 'M1,0 L0,.5 L1,1', type: VectorType.FILL }, + '\u{E0B2}': { d: 'M1,0 L0,.5 L1,1', type: VectorType.FILL, leftPadding: 2 }, // Left triangle line - '\u{E0B3}': { d: 'M1,0 L0,.5 L1,1', type: VectorType.STROKE, leftPadding: window.devicePixelRatio / 2, rightPadding: window.devicePixelRatio / 2 }, + '\u{E0B3}': { d: 'M2,-.5 L0,.5 L2,1.5', type: VectorType.STROKE, leftPadding: 1, rightPadding: 1 }, // Right semi-circle solid, - '\u{E0B4}': { d: 'M0,0 L0,1 C0.552,1,1,0.776,1,.5 C1,0.224,0.552,0,0,0', type: VectorType.FILL }, + '\u{E0B4}': { d: 'M0,0 L0,1 C0.552,1,1,0.776,1,.5 C1,0.224,0.552,0,0,0', type: VectorType.FILL, rightPadding: 1 }, // Right semi-circle line, - '\u{E0B5}': { d: 'M0,1 C0.552,1,1,0.776,1,.5 C1,0.224,0.552,0,0,0', type: VectorType.STROKE, rightPadding: window.devicePixelRatio / 2 }, + '\u{E0B5}': { d: 'M0,1 C0.552,1,1,0.776,1,.5 C1,0.224,0.552,0,0,0', type: VectorType.STROKE, rightPadding: 1 }, // Left semi-circle solid, - '\u{E0B6}': { d: 'M1,0 L1,1 C0.448,1,0,0.776,0,.5 C0,0.224,0.448,0,1,0', type: VectorType.FILL }, + '\u{E0B6}': { d: 'M1,0 L1,1 C0.448,1,0,0.776,0,.5 C0,0.224,0.448,0,1,0', type: VectorType.FILL, leftPadding: 1 }, // Left semi-circle line, - '\u{E0B7}': { d: 'M1,1 C0.448,1,0,0.776,0,.5 C0,0.224,0.448,0,1,0', type: VectorType.STROKE, leftPadding: window.devicePixelRatio / 2 } + '\u{E0B7}': { d: 'M1,1 C0.448,1,0,0.776,0,.5 C0,0.224,0.448,0,1,0', type: VectorType.STROKE, leftPadding: 1 } }; /** @@ -380,7 +382,8 @@ export function tryDrawCustomChar( xOffset: number, yOffset: number, scaledCellWidth: number, - scaledCellHeight: number + scaledCellHeight: number, + fontSize: number ): boolean { const blockElementDefinition = blockElementDefinitions[c]; if (blockElementDefinition) { @@ -402,7 +405,7 @@ export function tryDrawCustomChar( const powerlineDefinition = powerlineDefinitions[c]; if (powerlineDefinition) { - drawPowerlineChar(ctx, powerlineDefinition, xOffset, yOffset, scaledCellWidth, scaledCellHeight); + drawPowerlineChar(ctx, powerlineDefinition, xOffset, yOffset, scaledCellWidth, scaledCellHeight, fontSize); return true; } @@ -562,7 +565,7 @@ function drawBoxDrawingChar( if (!args[0] || !args[1]) { continue; } - f(ctx, translateArgs(args, scaledCellWidth, scaledCellHeight, xOffset, yOffset)); + f(ctx, translateArgs(args, scaledCellWidth, scaledCellHeight, xOffset, yOffset, true)); } ctx.stroke(); ctx.closePath(); @@ -575,10 +578,13 @@ function drawPowerlineChar( xOffset: number, yOffset: number, scaledCellWidth: number, - scaledCellHeight: number + scaledCellHeight: number, + fontSize: number ): void { ctx.beginPath(); - ctx.lineWidth = window.devicePixelRatio; + // Scale the stroke with DPR and font size + const cssLineWidth = fontSize / 12; + ctx.lineWidth = window.devicePixelRatio * cssLineWidth; for (const instruction of charDefinition.d.split(' ')) { const type = instruction[0]; const f = svgToCanvasInstructionMap[type]; @@ -590,7 +596,16 @@ function drawPowerlineChar( if (!args[0] || !args[1]) { continue; } - f(ctx, translateArgs(args, scaledCellWidth, scaledCellHeight, xOffset, yOffset, charDefinition.leftPadding, charDefinition.rightPadding)); + f(ctx, translateArgs( + args, + scaledCellWidth, + scaledCellHeight, + xOffset, + yOffset, + false, + (charDefinition.leftPadding ?? 0) * (cssLineWidth / 2), + (charDefinition.rightPadding ?? 0) * (cssLineWidth / 2) + )); } if (charDefinition.type === VectorType.STROKE) { ctx.strokeStyle = ctx.fillStyle; @@ -611,7 +626,7 @@ const svgToCanvasInstructionMap: { [index: string]: any } = { 'M': (ctx: CanvasRenderingContext2D, args: number[]) => ctx.moveTo(args[0], args[1]) }; -function translateArgs(args: string[], cellWidth: number, cellHeight: number, xOffset: number, yOffset: number, leftPadding: number = 0, rightPadding: number = 0): number[] { +function translateArgs(args: string[], cellWidth: number, cellHeight: number, xOffset: number, yOffset: number, doClamp: boolean, leftPadding: number = 0, rightPadding: number = 0): number[] { const result = args.map(e => parseFloat(e) || parseInt(e)); if (result.length < 2) { @@ -623,7 +638,7 @@ function translateArgs(args: string[], cellWidth: number, cellHeight: number, xO result[x] *= cellWidth - (leftPadding * window.devicePixelRatio) - (rightPadding * window.devicePixelRatio); // Ensure coordinate doesn't escape cell bounds and round to the nearest 0.5 to ensure a crisp // line at 100% devicePixelRatio - if (result[x] !== 0) { + if (doClamp && result[x] !== 0) { result[x] = clamp(Math.round(result[x] + 0.5) - 0.5, cellWidth, 0); } // Apply the cell's offset (ie. x*cellWidth) @@ -635,7 +650,7 @@ function translateArgs(args: string[], cellWidth: number, cellHeight: number, xO result[y] *= cellHeight; // Ensure coordinate doesn't escape cell bounds and round to the nearest 0.5 to ensure a crisp // line at 100% devicePixelRatio - if (result[y] !== 0) { + if (doClamp && result[y] !== 0) { result[y] = clamp(Math.round(result[y] + 0.5) - 0.5, cellHeight, 0); } // Apply the cell's offset (ie. x*cellHeight) From 4382a551d146a806a896634d25c53210c3e0035b Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 26 Aug 2022 07:53:20 -0700 Subject: [PATCH 11/28] Use powerline extra symbols in demo font family --- demo/client.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/demo/client.ts b/demo/client.ts index 64c694f5a5..bcc79a4550 100644 --- a/demo/client.ts +++ b/demo/client.ts @@ -203,7 +203,7 @@ function createTerminal(): void { term = new Terminal({ allowProposedApi: true, windowsMode: isWindows, - fontFamily: 'Fira Code, courier-new, courier, monospace', + fontFamily: '"Fira Code", courier-new, courier, monospace, "Powerline Extra Symbols"', theme: xtermjsTheme } as ITerminalOptions); From f6860442fb259e0c33b88b20d8d0911da372d782 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 26 Aug 2022 08:12:00 -0700 Subject: [PATCH 12/28] Show entire glyph for extra powerline symbols Fixes #4072 --- addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts | 5 +++-- src/browser/renderer/RendererUtils.ts | 8 ++++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts index 90d13e5547..4be02aab8e 100644 --- a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts +++ b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts @@ -13,7 +13,7 @@ import { IDisposable } from 'xterm'; import { AttributeData } from 'common/buffer/AttributeData'; import { color, rgba } from 'common/Color'; import { tryDrawCustomChar } from 'browser/renderer/CustomGlyphs'; -import { excludeFromContrastRatioDemands, isPowerlineGlyph } from 'browser/renderer/RendererUtils'; +import { excludeFromContrastRatioDemands, isPowerlineGlyph, isRestrictedPowerlineGlyph } from 'browser/renderer/RendererUtils'; // For debugging purposes, it can be useful to set this to a really tiny value, // to verify that LRU eviction works. @@ -418,6 +418,7 @@ export class WebglCharAtlas implements IDisposable { this._tmpCtx.textBaseline = TEXT_BASELINE; const powerlineGlyph = chars.length === 1 && isPowerlineGlyph(chars.charCodeAt(0)); + const restrictedPowerlineGlyph = chars.length === 1 && isRestrictedPowerlineGlyph(chars.charCodeAt(0)); const foregroundColor = this._getForegroundColor(bg, bgColorMode, bgColor, fg, fgColorMode, fgColor, inverse, dim, bold, excludeFromContrastRatioDemands(chars.charCodeAt(0))); this._tmpCtx.fillStyle = foregroundColor.css; @@ -606,7 +607,7 @@ export class WebglCharAtlas implements IDisposable { return NULL_RASTERIZED_GLYPH; } - const rasterizedGlyph = this._findGlyphBoundingBox(imageData, this._workBoundingBox, allowedWidth, powerlineGlyph, customGlyph, padding); + const rasterizedGlyph = this._findGlyphBoundingBox(imageData, this._workBoundingBox, allowedWidth, restrictedPowerlineGlyph, customGlyph, padding); const clippedImageData = this._clipImageData(imageData, this._workBoundingBox); // Find the best atlas row to use diff --git a/src/browser/renderer/RendererUtils.ts b/src/browser/renderer/RendererUtils.ts index 0a4a77e8ad..0f60dc2995 100644 --- a/src/browser/renderer/RendererUtils.ts +++ b/src/browser/renderer/RendererUtils.ts @@ -14,11 +14,15 @@ export function isPowerlineGlyph(codepoint: number): boolean { // Only return true for Powerline symbols which require // different padding and should be excluded from minimum contrast // ratio standards - return 0xE0A4 <= codepoint && codepoint <= 0xE0D6; + return 0xE0A4 <= codepoint && codepoint <= 0xE0D6; +} + +export function isRestrictedPowerlineGlyph(codepoint: number): boolean { + return 0xE0B0 <= codepoint && codepoint <= 0xE0B7; } function isBoxOrBlockGlyph(codepoint: number): boolean { - return (0x2500 <= codepoint && codepoint <= 0x259F); + return 0x2500 <= codepoint && codepoint <= 0x259F; } export function excludeFromContrastRatioDemands(codepoint: number): boolean { From 4979e9d2be746ff526647726e8615cba87c5c4bf Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 26 Aug 2022 13:26:00 -0700 Subject: [PATCH 13/28] Show test characters in underline test --- demo/client.ts | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/demo/client.ts b/demo/client.ts index 64c694f5a5..a053b59edd 100644 --- a/demo/client.ts +++ b/demo/client.ts @@ -766,12 +766,23 @@ function underlineTest() { term.write('\n\n\r'); term.writeln('Underline styles:'); term.writeln(''); - term.writeln(`${u(0)}4:0m - No underline`); - term.writeln(`${u(1)}4:1m - Straight`); - term.writeln(`${u(2)}4:2m - Double`); - term.writeln(`${u(3)}4:3m - Curly`); - term.writeln(`${u(4)}4:4m - Dotted`); - term.writeln(`${u(5)}4:5m - Dashed\x1b[0m`); + function showSequence(id: number, name: string) { + let alphabet = ''; + for (let i = 97; i < 123; i++) { + alphabet += String.fromCharCode(i); + } + let numbers = ''; + for (let i = 0; i < 10; i++) { + numbers += i.toString(); + } + return `${u(id)}4:${id}m - ${name}\x1b[4:0m`.padEnd(33, ' ') + `${u(id)}${alphabet} ${numbers}\x1b[4:0m`; + } + term.writeln(showSequence(0, 'No underline')); + term.writeln(showSequence(1, 'Straight')); + term.writeln(showSequence(2, 'Double')); + term.writeln(showSequence(3, 'Curly')); + term.writeln(showSequence(4, 'Dotted')); + term.writeln(showSequence(5, 'Dashed')); term.writeln(''); term.writeln(`Underline colors (256 color mode):`); term.writeln(''); From 7ffd9e1f7f24e35d79796118535177d59cc96f97 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 26 Aug 2022 14:16:02 -0700 Subject: [PATCH 14/28] Only draw character outline if it has a descent Fixes #4059 --- .../src/atlas/WebglCharAtlas.ts | 28 ++++++++++++------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts index 90d13e5547..2af88b9217 100644 --- a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts +++ b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts @@ -533,18 +533,26 @@ export class WebglCharAtlas implements IDisposable { // This only works when transparency is disabled because it's not clear how to clear stroked // text if (!this._config.allowTransparency && chars !== ' ') { - // This translates to 1/2 the line width in either direction + // Measure the text, only draw the stroke if there is a descent beyond an alphabetic text + // baseline this._tmpCtx.save(); - // Clip the region to only draw in valid pixels near the underline to avoid a slight - // outline around the whole glyph, as well as additional pixels in the glyph at the top - // which would increase GPU memory demands - const clipRegion = new Path2D(); - clipRegion.rect(xLeft, yTop - Math.ceil(lineWidth / 2), this._config.scaledCellWidth, yBot - yTop + Math.ceil(lineWidth / 2)); - this._tmpCtx.clip(clipRegion); - this._tmpCtx.lineWidth = window.devicePixelRatio * 3; - this._tmpCtx.strokeStyle = backgroundColor.css; - this._tmpCtx.strokeText(chars, padding, padding + this._config.scaledCharHeight); + this._tmpCtx.textBaseline = 'alphabetic'; + const metrics = this._tmpCtx.measureText(chars); this._tmpCtx.restore(); + if ('actualBoundingBoxDescent' in metrics && metrics.actualBoundingBoxDescent > 0) { + // This translates to 1/2 the line width in either direction + this._tmpCtx.save(); + // Clip the region to only draw in valid pixels near the underline to avoid a slight + // outline around the whole glyph, as well as additional pixels in the glyph at the top + // which would increase GPU memory demands + const clipRegion = new Path2D(); + clipRegion.rect(xLeft, yTop - Math.ceil(lineWidth / 2), this._config.scaledCellWidth, yBot - yTop + Math.ceil(lineWidth / 2)); + this._tmpCtx.clip(clipRegion); + this._tmpCtx.lineWidth = window.devicePixelRatio * 3; + this._tmpCtx.strokeStyle = backgroundColor.css; + this._tmpCtx.strokeText(chars, padding, padding + this._config.scaledCharHeight); + this._tmpCtx.restore(); + } } } } From 903830ce1f29f9f3a6842d45a3e8b6180af353aa Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 27 Aug 2022 06:06:17 -0700 Subject: [PATCH 15/28] Add CJK chars to underline test --- demo/client.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/demo/client.ts b/demo/client.ts index e06eb0a2a5..cd39b8dd64 100644 --- a/demo/client.ts +++ b/demo/client.ts @@ -775,7 +775,7 @@ function underlineTest() { for (let i = 0; i < 10; i++) { numbers += i.toString(); } - return `${u(id)}4:${id}m - ${name}\x1b[4:0m`.padEnd(33, ' ') + `${u(id)}${alphabet} ${numbers}\x1b[4:0m`; + return `${u(id)}4:${id}m - ${name}\x1b[4:0m`.padEnd(33, ' ') + `${u(id)}${alphabet} ${numbers} 汉语 한국어\x1b[4:0m`; } term.writeln(showSequence(0, 'No underline')); term.writeln(showSequence(1, 'Straight')); From e76918eddc824054fc15182123e5550f98cf11a7 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 27 Aug 2022 06:34:38 -0700 Subject: [PATCH 16/28] Take char width into account when drawing underlines/strokethrough Fixes #4063 --- .../src/atlas/CharAtlasCache.ts | 5 +- .../src/atlas/WebglCharAtlas.ts | 136 ++++++++++-------- 2 files changed, 78 insertions(+), 63 deletions(-) diff --git a/addons/xterm-addon-webgl/src/atlas/CharAtlasCache.ts b/addons/xterm-addon-webgl/src/atlas/CharAtlasCache.ts index 41114de0f4..6aba2125bc 100644 --- a/addons/xterm-addon-webgl/src/atlas/CharAtlasCache.ts +++ b/addons/xterm-addon-webgl/src/atlas/CharAtlasCache.ts @@ -7,7 +7,7 @@ import { generateConfig, configEquals } from './CharAtlasUtils'; import { WebglCharAtlas } from './WebglCharAtlas'; import { ICharAtlasConfig } from './Types'; import { Terminal } from 'xterm'; -import { IColorSet } from 'browser/Types'; +import { IColorSet, ITerminal } from 'browser/Types'; interface ICharAtlasCacheEntry { atlas: WebglCharAtlas; @@ -64,8 +64,9 @@ export function acquireCharAtlas( } } + const core: ITerminal = (terminal as any)._core; const newEntry: ICharAtlasCacheEntry = { - atlas: new WebglCharAtlas(document, newConfig), + atlas: new WebglCharAtlas(document, newConfig, core.unicodeService), config: newConfig, ownedBy: [terminal] }; diff --git a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts index 9ede81c504..5685012d47 100644 --- a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts +++ b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts @@ -14,6 +14,7 @@ import { AttributeData } from 'common/buffer/AttributeData'; import { color, rgba } from 'common/Color'; import { tryDrawCustomChar } from 'browser/renderer/CustomGlyphs'; import { excludeFromContrastRatioDemands, isPowerlineGlyph, isRestrictedPowerlineGlyph } from 'browser/renderer/RendererUtils'; +import { IUnicodeService } from 'common/services/Services'; // For debugging purposes, it can be useful to set this to a really tiny value, // to verify that LRU eviction works. @@ -89,7 +90,8 @@ export class WebglCharAtlas implements IDisposable { constructor( document: Document, - private _config: ICharAtlasConfig + private readonly _config: ICharAtlasConfig, + private readonly _unicodeService: IUnicodeService ) { this.cacheCanvas = document.createElement('canvas'); this.cacheCanvas.width = TEXTURE_WIDTH; @@ -436,6 +438,11 @@ export class WebglCharAtlas implements IDisposable { // underline colors to prevent important colors could get cleared. let enableClearThresholdCheck = !powerlineGlyph; + let chWidth: number = 1; + if (typeof codeOrChars === 'number' && (underline || strikethrough)) { + chWidth = this._unicodeService.wcwidth(codeOrChars); + } + // Draw underline if (underline) { this._tmpCtx.save(); @@ -462,69 +469,76 @@ export class WebglCharAtlas implements IDisposable { // Underline style/stroke this._tmpCtx.beginPath(); const xLeft = padding; - const xRight = padding + this._config.scaledCellWidth; + const xRight = padding + this._config.scaledCellWidth * chWidth; const yTop = Math.ceil(padding + this._config.scaledCharHeight) - yOffset; const yMid = padding + this._config.scaledCharHeight + lineWidth - yOffset; const yBot = Math.ceil(padding + this._config.scaledCharHeight + lineWidth * 2) - yOffset; - switch (this._workAttributeData.extended.underlineStyle) { - case UnderlineStyle.DOUBLE: - this._tmpCtx.moveTo(xLeft, yTop); - this._tmpCtx.lineTo(xRight, yTop); - this._tmpCtx.moveTo(xLeft, yBot); - this._tmpCtx.lineTo(xRight, yBot); - break; - case UnderlineStyle.CURLY: - const xMid = padding + this._config.scaledCellWidth / 2; - // Choose the bezier top and bottom based on the device pixel ratio, the curly line is - // made taller when the line width is as otherwise it's not very clear otherwise. - const yCurlyBot = lineWidth <= 1 ? yBot : Math.ceil(padding + this._config.scaledCharHeight - lineWidth / 2) - yOffset; - const yCurlyTop = lineWidth <= 1 ? yTop : Math.ceil(padding + this._config.scaledCharHeight + lineWidth / 2) - yOffset; - // Clip the left and right edges of the underline such that it can be drawn just outside - // the edge of the cell to ensure a continuous stroke when there are multiple underlined - // glyphs adjacent to one another. - const clipRegion = new Path2D(); - clipRegion.rect(xLeft, yTop, this._config.scaledCellWidth, yBot - yTop); - this._tmpCtx.clip(clipRegion); - // Start 1/2 cell before and end 1/2 cells after to ensure a smooth curve with other cells - this._tmpCtx.moveTo(xLeft - this._config.scaledCellWidth / 2, yMid); - this._tmpCtx.bezierCurveTo( - xLeft - this._config.scaledCellWidth / 2, yCurlyTop, - xLeft, yCurlyTop, - xLeft, yMid - ); - this._tmpCtx.bezierCurveTo( - xLeft, yCurlyBot, - xMid, yCurlyBot, - xMid, yMid - ); - this._tmpCtx.bezierCurveTo( - xMid, yCurlyTop, - xRight, yCurlyTop, - xRight, yMid - ); - this._tmpCtx.bezierCurveTo( - xRight, yCurlyBot, - xRight + this._config.scaledCellWidth / 2, yCurlyBot, - xRight + this._config.scaledCellWidth / 2, yMid - ); - break; - case UnderlineStyle.DOTTED: - this._tmpCtx.setLineDash([window.devicePixelRatio * 2, window.devicePixelRatio]); - this._tmpCtx.moveTo(xLeft, yTop); - this._tmpCtx.lineTo(xRight, yTop); - break; - case UnderlineStyle.DASHED: - this._tmpCtx.setLineDash([window.devicePixelRatio * 4, window.devicePixelRatio * 3]); - this._tmpCtx.moveTo(xLeft, yTop); - this._tmpCtx.lineTo(xRight, yTop); - break; - case UnderlineStyle.SINGLE: - default: - this._tmpCtx.moveTo(xLeft, yTop); - this._tmpCtx.lineTo(xRight, yTop); - break; + + for (let i = 0; i < chWidth; i++) { + this._tmpCtx.save(); + const xChLeft = xLeft + i * this._config.scaledCellWidth; + const xChRight = xLeft + (i + 1) * this._config.scaledCellWidth; + const xChMid = xChLeft + this._config.scaledCellWidth / 2; + switch (this._workAttributeData.extended.underlineStyle) { + case UnderlineStyle.DOUBLE: + this._tmpCtx.moveTo(xChLeft, yTop); + this._tmpCtx.lineTo(xChRight, yTop); + this._tmpCtx.moveTo(xChLeft, yBot); + this._tmpCtx.lineTo(xChRight, yBot); + break; + case UnderlineStyle.CURLY: + // Choose the bezier top and bottom based on the device pixel ratio, the curly line is + // made taller when the line width is as otherwise it's not very clear otherwise. + const yCurlyBot = lineWidth <= 1 ? yBot : Math.ceil(padding + this._config.scaledCharHeight - lineWidth / 2) - yOffset; + const yCurlyTop = lineWidth <= 1 ? yTop : Math.ceil(padding + this._config.scaledCharHeight + lineWidth / 2) - yOffset; + // Clip the left and right edges of the underline such that it can be drawn just outside + // the edge of the cell to ensure a continuous stroke when there are multiple underlined + // glyphs adjacent to one another. + const clipRegion = new Path2D(); + clipRegion.rect(xChLeft, yTop, this._config.scaledCellWidth, yBot - yTop); + this._tmpCtx.clip(clipRegion); + // Start 1/2 cell before and end 1/2 cells after to ensure a smooth curve with other cells + this._tmpCtx.moveTo(xChLeft - this._config.scaledCellWidth / 2, yMid); + this._tmpCtx.bezierCurveTo( + xChLeft - this._config.scaledCellWidth / 2, yCurlyTop, + xChLeft, yCurlyTop, + xChLeft, yMid + ); + this._tmpCtx.bezierCurveTo( + xChLeft, yCurlyBot, + xChMid, yCurlyBot, + xChMid, yMid + ); + this._tmpCtx.bezierCurveTo( + xChMid, yCurlyTop, + xChRight, yCurlyTop, + xChRight, yMid + ); + this._tmpCtx.bezierCurveTo( + xChRight, yCurlyBot, + xChRight + this._config.scaledCellWidth / 2, yCurlyBot, + xChRight + this._config.scaledCellWidth / 2, yMid + ); + break; + case UnderlineStyle.DOTTED: + this._tmpCtx.setLineDash([window.devicePixelRatio * 2, window.devicePixelRatio]); + this._tmpCtx.moveTo(xChLeft, yTop); + this._tmpCtx.lineTo(xChRight, yTop); + break; + case UnderlineStyle.DASHED: + this._tmpCtx.setLineDash([window.devicePixelRatio * 4, window.devicePixelRatio * 3]); + this._tmpCtx.moveTo(xChLeft, yTop); + this._tmpCtx.lineTo(xChRight, yTop); + break; + case UnderlineStyle.SINGLE: + default: + this._tmpCtx.moveTo(xChLeft, yTop); + this._tmpCtx.lineTo(xChRight, yTop); + break; + } + this._tmpCtx.stroke(); + this._tmpCtx.restore(); } - this._tmpCtx.stroke(); this._tmpCtx.restore(); // Draw stroke in the background color for non custom characters in order to give an outline @@ -590,7 +604,7 @@ export class WebglCharAtlas implements IDisposable { this._tmpCtx.strokeStyle = this._tmpCtx.fillStyle; this._tmpCtx.beginPath(); this._tmpCtx.moveTo(padding, padding + Math.floor(this._config.scaledCharHeight / 2) - yOffset); - this._tmpCtx.lineTo(padding + this._config.scaledCharWidth, padding + Math.floor(this._config.scaledCharHeight / 2) - yOffset); + this._tmpCtx.lineTo(padding + this._config.scaledCharWidth * chWidth, padding + Math.floor(this._config.scaledCharHeight / 2) - yOffset); this._tmpCtx.stroke(); } From 4ecc098de42dcb5f07e5d96696dccc6e70a5b727 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 27 Aug 2022 06:38:15 -0700 Subject: [PATCH 17/28] Get width of emoji/combined chars --- addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts index 5685012d47..e1d56b7867 100644 --- a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts +++ b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts @@ -438,9 +438,11 @@ export class WebglCharAtlas implements IDisposable { // underline colors to prevent important colors could get cleared. let enableClearThresholdCheck = !powerlineGlyph; - let chWidth: number = 1; - if (typeof codeOrChars === 'number' && (underline || strikethrough)) { + let chWidth: number; + if (typeof codeOrChars === 'number') { chWidth = this._unicodeService.wcwidth(codeOrChars); + } else { + chWidth = this._unicodeService.getStringCellWidth(codeOrChars); } // Draw underline From 4d837e280222c3cf101fbfc31b25bb895681ecc5 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 27 Aug 2022 06:39:07 -0700 Subject: [PATCH 18/28] Add an emoji to underline test --- demo/client.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/demo/client.ts b/demo/client.ts index cd39b8dd64..8e03ec3c1b 100644 --- a/demo/client.ts +++ b/demo/client.ts @@ -775,7 +775,7 @@ function underlineTest() { for (let i = 0; i < 10; i++) { numbers += i.toString(); } - return `${u(id)}4:${id}m - ${name}\x1b[4:0m`.padEnd(33, ' ') + `${u(id)}${alphabet} ${numbers} 汉语 한국어\x1b[4:0m`; + return `${u(id)}4:${id}m - ${name}\x1b[4:0m`.padEnd(33, ' ') + `${u(id)}${alphabet} ${numbers} 汉语 한국어 👽\x1b[4:0m`; } term.writeln(showSequence(0, 'No underline')); term.writeln(showSequence(1, 'Straight')); From 0237b2965e8b5831bdb31aeefc2b45d97e95cc75 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 27 Aug 2022 19:28:20 -0700 Subject: [PATCH 19/28] Fix resize on demo --- demo/client.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/demo/client.ts b/demo/client.ts index 8e03ec3c1b..c8fcfb79b2 100644 --- a/demo/client.ts +++ b/demo/client.ts @@ -406,7 +406,11 @@ function initOptions(term: TerminalType): void { const input = document.getElementById(`opt-${o}`); addDomListener(input, 'change', () => { console.log('change', o, input.value); - if (o === 'lineHeight') { + if (o === 'rows') { + term.resize(term.cols, parseInt(input.value)); + } else if (o === 'cols') { + term.resize(parseInt(input.value), term.rows); + } else if (o === 'lineHeight') { term.options.lineHeight = parseFloat(input.value); } else if (o === 'scrollSensitivity') { term.options.scrollSensitivity = parseFloat(input.value); From 5e3f63e876dcf875b7509c951c01818699a42370 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 27 Aug 2022 20:06:58 -0700 Subject: [PATCH 20/28] Avoid garbage collection in webgl decoration hot code Part of #4079 --- addons/xterm-addon-webgl/src/WebglRenderer.ts | 81 ++++++++++++------- src/common/SortedList.ts | 25 +++++- src/common/TestUtils.test.ts | 1 + src/common/services/DecorationService.ts | 16 ++++ src/common/services/Services.ts | 5 ++ 5 files changed, 94 insertions(+), 34 deletions(-) diff --git a/addons/xterm-addon-webgl/src/WebglRenderer.ts b/addons/xterm-addon-webgl/src/WebglRenderer.ts index 9e90df0240..efb80f662d 100644 --- a/addons/xterm-addon-webgl/src/WebglRenderer.ts +++ b/addons/xterm-addon-webgl/src/WebglRenderer.ts @@ -11,7 +11,7 @@ import { WebglCharAtlas } from './atlas/WebglCharAtlas'; import { RectangleRenderer } from './RectangleRenderer'; import { IWebGL2RenderingContext } from './Types'; import { RenderModel, COMBINED_CHAR_BIT_MASK, RENDER_MODEL_BG_OFFSET, RENDER_MODEL_FG_OFFSET, RENDER_MODEL_EXT_OFFSET, RENDER_MODEL_INDICIES_PER_CELL } from './RenderModel'; -import { Disposable, toDisposable } from 'common/Lifecycle'; +import { Disposable } from 'common/Lifecycle'; import { Attributes, BgFlags, Content, FgFlags, NULL_CELL_CHAR, NULL_CELL_CODE } from 'common/buffer/Constants'; import { Terminal, IEvent } from 'xterm'; import { IRenderLayer } from './renderLayer/Types'; @@ -26,6 +26,15 @@ import { CharData, ICellData } from 'common/Types'; import { AttributeData } from 'common/buffer/AttributeData'; import { ICoreService, IDecorationService } from 'common/services/Services'; +/** Work variables to avoid garbage collection. */ +const w: { fg: number, bg: number, hasFg: boolean, hasBg: boolean, isSelected: boolean } = { + fg: 0, + bg: 0, + hasFg: false, + hasBg: false, + isSelected: false +}; + export class WebglRenderer extends Disposable implements IRenderer { private _renderLayers: IRenderLayer[]; private _charAtlas: WebglCharAtlas | undefined; @@ -404,79 +413,89 @@ export class WebglRenderer extends Disposable implements IRenderer { this._workColors.ext = this._workCell.bg & BgFlags.HAS_EXTENDED ? this._workCell.extended.ext : 0; // Get any foreground/background overrides, this happens on the model to avoid spreading // override logic throughout the different sub-renderers - let bgOverride: number | undefined; - let fgOverride: number | undefined; - let isSelected: boolean = false; + + // Reset overrides work variables + w.bg = 0; + w.fg = 0; + w.hasBg = false; + w.hasFg = false; + w.isSelected = false; // Apply decorations on the bottom layer - for (const d of this._decorationService.getDecorationsAtCell(x, y, 'bottom')) { + this._decorationService.forEachDecorationAtCell(x, y, 'bottom', d => { if (d.backgroundColorRGB) { - bgOverride = d.backgroundColorRGB.rgba >> 8 & 0xFFFFFF; + w.bg = d.backgroundColorRGB.rgba >> 8 & 0xFFFFFF; + w.hasBg = true; } if (d.foregroundColorRGB) { - fgOverride = d.foregroundColorRGB.rgba >> 8 & 0xFFFFFF; + w.fg = d.foregroundColorRGB.rgba >> 8 & 0xFFFFFF; + w.hasFg = true; } - } + }); // Apply the selection color if needed - isSelected = this._isCellSelected(x, y); - if (isSelected) { - bgOverride = (this._coreBrowserService.isFocused ? this._colors.selectionBackgroundOpaque : this._colors.selectionInactiveBackgroundOpaque).rgba >> 8 & 0xFFFFFF; + w.isSelected = this._isCellSelected(x, y); + if (w.isSelected) { + w.bg = (this._coreBrowserService.isFocused ? this._colors.selectionBackgroundOpaque : this._colors.selectionInactiveBackgroundOpaque).rgba >> 8 & 0xFFFFFF; + w.hasBg = true; if (this._colors.selectionForeground) { - fgOverride = this._colors.selectionForeground.rgba >> 8 & 0xFFFFFF; + w.fg = this._colors.selectionForeground.rgba >> 8 & 0xFFFFFF; + w.hasFg = true; } } // Apply decorations on the top layer - for (const d of this._decorationService.getDecorationsAtCell(x, y, 'top')) { + this._decorationService.forEachDecorationAtCell(x, y, 'top', d => { if (d.backgroundColorRGB) { - bgOverride = d.backgroundColorRGB.rgba >> 8 & 0xFFFFFF; + w.bg = d.backgroundColorRGB.rgba >> 8 & 0xFFFFFF; + w.hasBg = true; } if (d.foregroundColorRGB) { - fgOverride = d.foregroundColorRGB.rgba >> 8 & 0xFFFFFF; + w.fg = d.foregroundColorRGB.rgba >> 8 & 0xFFFFFF; + w.hasFg = true; } - } + }); // Convert any overrides from rgba to the fg/bg packed format. This resolves the inverse flag // ahead of time in order to use the correct cache key - if (bgOverride !== undefined) { - if (isSelected) { + if (w.hasBg) { + if (w.isSelected) { // Non-RGB attributes from model + force non-dim + override + force RGB color mode - bgOverride = (this._workCell.bg & ~Attributes.RGB_MASK & ~BgFlags.DIM) | bgOverride | Attributes.CM_RGB; + w.bg = (this._workCell.bg & ~Attributes.RGB_MASK & ~BgFlags.DIM) | w.bg | Attributes.CM_RGB; } else { // Non-RGB attributes from model + override + force RGB color mode - bgOverride = (this._workCell.bg & ~Attributes.RGB_MASK) | bgOverride | Attributes.CM_RGB; + w.bg = (this._workCell.bg & ~Attributes.RGB_MASK) | w.bg | Attributes.CM_RGB; } } - if (fgOverride !== undefined) { + if (w.hasFg) { // Non-RGB attributes from model + force disable inverse + override + force RGB color mode - fgOverride = (this._workCell.fg & ~Attributes.RGB_MASK & ~FgFlags.INVERSE) | fgOverride | Attributes.CM_RGB; + w.fg = (this._workCell.fg & ~Attributes.RGB_MASK & ~FgFlags.INVERSE) | w.fg | Attributes.CM_RGB; } - // Handle case where inverse was specified by only one of bgOverride or fgOverride was set, + // Handle case where inverse was specified by only one of bg override or fg override was set, // resolving the other inverse color and setting the inverse flag if needed. if (this._workColors.fg & FgFlags.INVERSE) { - if (bgOverride !== undefined && fgOverride === undefined) { + if (w.hasBg && w.hasFg) { // Resolve bg color type (default color has a different meaning in fg vs bg) if ((this._workColors.bg & Attributes.CM_MASK) === Attributes.CM_DEFAULT) { - fgOverride = (this._workColors.fg & ~(Attributes.RGB_MASK | FgFlags.INVERSE | Attributes.CM_MASK)) | ((this._colors.background.rgba >> 8 & 0xFFFFFF) & Attributes.RGB_MASK) | Attributes.CM_RGB; + w.fg = (this._workColors.fg & ~(Attributes.RGB_MASK | FgFlags.INVERSE | Attributes.CM_MASK)) | ((this._colors.background.rgba >> 8 & 0xFFFFFF) & Attributes.RGB_MASK) | Attributes.CM_RGB; } else { - fgOverride = (this._workColors.fg & ~(Attributes.RGB_MASK | FgFlags.INVERSE | Attributes.CM_MASK)) | this._workColors.bg & (Attributes.RGB_MASK | Attributes.CM_MASK); + w.fg = (this._workColors.fg & ~(Attributes.RGB_MASK | FgFlags.INVERSE | Attributes.CM_MASK)) | this._workColors.bg & (Attributes.RGB_MASK | Attributes.CM_MASK); } } - if (bgOverride === undefined && fgOverride !== undefined) { + if (w.hasBg && w.hasFg) { // Resolve bg color type (default color has a different meaning in fg vs bg) if ((this._workColors.fg & Attributes.CM_MASK) === Attributes.CM_DEFAULT) { - bgOverride = (this._workColors.bg & ~(Attributes.RGB_MASK | Attributes.CM_MASK)) | ((this._colors.foreground.rgba >> 8 & 0xFFFFFF) & Attributes.RGB_MASK) | Attributes.CM_RGB; + w.bg = (this._workColors.bg & ~(Attributes.RGB_MASK | Attributes.CM_MASK)) | ((this._colors.foreground.rgba >> 8 & 0xFFFFFF) & Attributes.RGB_MASK) | Attributes.CM_RGB; } else { - bgOverride = (this._workColors.bg & ~(Attributes.RGB_MASK | Attributes.CM_MASK)) | this._workColors.fg & (Attributes.RGB_MASK | Attributes.CM_MASK); + w.bg = (this._workColors.bg & ~(Attributes.RGB_MASK | Attributes.CM_MASK)) | this._workColors.fg & (Attributes.RGB_MASK | Attributes.CM_MASK); } } } // Use the override if it exists - this._workColors.bg = bgOverride ?? this._workColors.bg; - this._workColors.fg = fgOverride ?? this._workColors.fg; + this._workColors.bg = w.bg ?? this._workColors.bg; + this._workColors.fg = w.fg ?? this._workColors.fg; } private _isCellSelected(x: number, y: number): boolean { diff --git a/src/common/SortedList.ts b/src/common/SortedList.ts index 9c819959ed..737b9acb32 100644 --- a/src/common/SortedList.ts +++ b/src/common/SortedList.ts @@ -3,6 +3,9 @@ * @license MIT */ +// Work variables to avoid garbage collection. +let i = 0; + /** * A generic list that is maintained in sorted order and allows values with duplicate keys. This * list is based on binary search and as such locating a key will take O(log n) amortized, this @@ -25,7 +28,7 @@ export class SortedList { this._array.push(value); return; } - const i = this._search(this._getKey(value), 0, this._array.length - 1); + i = this._search(this._getKey(value), 0, this._array.length - 1); this._array.splice(i, 0, value); } @@ -37,7 +40,7 @@ export class SortedList { if (key === undefined) { return false; } - let i = this._search(key, 0, this._array.length - 1); + i = this._search(key, 0, this._array.length - 1); if (i === -1) { return false; } @@ -57,7 +60,7 @@ export class SortedList { if (this._array.length === 0) { return; } - let i = this._search(key, 0, this._array.length - 1); + i = this._search(key, 0, this._array.length - 1); if (i < 0 || i >= this._array.length) { return; } @@ -69,6 +72,22 @@ export class SortedList { } while (++i < this._array.length && this._getKey(this._array[i]) === key); } + public forEachByKey(key: number, callback: (value: T) => void): void { + if (this._array.length === 0) { + return; + } + i = this._search(key, 0, this._array.length - 1); + if (i < 0 || i >= this._array.length) { + return; + } + if (this._getKey(this._array[i]) !== key) { + return; + } + do { + callback(this._array[i]); + } while (++i < this._array.length && this._getKey(this._array[i]) === key); + } + public values(): IterableIterator { return this._array.values(); } diff --git a/src/common/TestUtils.test.ts b/src/common/TestUtils.test.ts index 48f3a69e02..cbcdd36e41 100644 --- a/src/common/TestUtils.test.ts +++ b/src/common/TestUtils.test.ts @@ -163,5 +163,6 @@ export class MockDecorationService implements IDecorationService { public reset(): void { } public *getDecorationsAtLine(line: number): IterableIterator { } public *getDecorationsAtCell(x: number, line: number): IterableIterator { } + public forEachDecorationAtCell(x: number, line: number, layer: 'bottom' | 'top' | undefined, callback: (decoration: IInternalDecoration) => void): void { } public dispose(): void { } } diff --git a/src/common/services/DecorationService.ts b/src/common/services/DecorationService.ts index 58bff7298a..8957136367 100644 --- a/src/common/services/DecorationService.ts +++ b/src/common/services/DecorationService.ts @@ -11,6 +11,12 @@ import { SortedList } from 'common/SortedList'; import { IColor } from 'common/Types'; import { IDecorationOptions, IDecoration, IMarker, IEvent } from 'xterm'; +/** Work variables to avoid garbage collection. */ +const w = { + xmin: 0, + xmax: 0 +}; + export class DecorationService extends Disposable implements IDecorationService { public serviceBrand: any; @@ -72,6 +78,16 @@ export class DecorationService extends Disposable implements IDecorationService } } + public forEachDecorationAtCell(x: number, line: number, layer: 'bottom' | 'top' | undefined, callback: (decoration: IInternalDecoration) => void): void { + this._decorations.forEachByKey(line, d => { + w.xmin = d.options.x ?? 0; + w.xmax = w.xmin + (d.options.width ?? 1); + if (x >= w.xmin && x < w.xmax && (!layer || (d.options.layer ?? 'bottom') === layer)) { + callback(d); + } + }); + } + public dispose(): void { for (const d of this._decorations.values()) { this._onDecorationRemoved.fire(d); diff --git a/src/common/services/Services.ts b/src/common/services/Services.ts index 585b29ac28..b8e227b347 100644 --- a/src/common/services/Services.ts +++ b/src/common/services/Services.ts @@ -308,6 +308,11 @@ export interface IDecorationService extends IDisposable { getDecorationsAtLine(line: number): IterableIterator; /** Iterates over the decorations at a cell (in no particular order). */ getDecorationsAtCell(x: number, line: number, layer?: 'bottom' | 'top'): IterableIterator; + /** + * Trigger a callback over the decoration at a cell (in no particular order). This is a high + * performance, but less ergonomic, version of {@link getDecorationsAtCell}. + */ + forEachDecorationAtCell(x: number, line: number, layer: 'bottom' | 'top' | undefined, callback: (decoration: IInternalDecoration) => void): void; } export interface IInternalDecoration extends IDecoration { readonly options: IDecorationOptions; From 0727741ca1f061ca7d3fd8b94c6f290872c6d236 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 27 Aug 2022 20:50:16 -0700 Subject: [PATCH 21/28] Remove iterators from IDecorationService They're too low performance for typical use cases Fixes #4079 --- addons/xterm-addon-canvas/src/BaseRenderLayer.ts | 11 +++++------ addons/xterm-addon-canvas/src/TextRenderLayer.ts | 6 +++--- src/browser/renderer/dom/DomRendererRowFactory.ts | 6 +++--- src/common/TestUtils.test.ts | 2 -- src/common/services/DecorationService.ts | 4 ---- src/common/services/Services.ts | 8 ++------ 6 files changed, 13 insertions(+), 24 deletions(-) diff --git a/addons/xterm-addon-canvas/src/BaseRenderLayer.ts b/addons/xterm-addon-canvas/src/BaseRenderLayer.ts index dfd814e333..ae39601693 100644 --- a/addons/xterm-addon-canvas/src/BaseRenderLayer.ts +++ b/addons/xterm-addon-canvas/src/BaseRenderLayer.ts @@ -404,12 +404,11 @@ export abstract class BaseRenderLayer implements IRenderLayer { // Don't try cache the glyph if it uses any decoration foreground/background override. let hasOverrides = false; - for (const d of this._decorationService.getDecorationsAtCell(x, y)) { + this._decorationService.forEachDecorationAtCell(x, y, undefined, d => { if (d.backgroundColorRGB || d.foregroundColorRGB) { hasOverrides = true; - break; } - } + }); const atlasDidDraw = hasOverrides ? false : this._charAtlas?.draw(this._ctx, this._currentGlyphIdentifier, x * this._scaledCellWidth + this._scaledCharLeft, y * this._scaledCellHeight + this._scaledCharTop); @@ -519,9 +518,9 @@ export abstract class BaseRenderLayer implements IRenderLayer { let bgOverride: number | undefined; let fgOverride: number | undefined; let isTop = false; - for (const d of this._decorationService.getDecorationsAtCell(x, y)) { + this._decorationService.forEachDecorationAtCell(x, y, undefined, d => { if (d.options.layer !== 'top' && isTop) { - continue; + return; } if (d.backgroundColorRGB) { bgOverride = d.backgroundColorRGB.rgba; @@ -530,7 +529,7 @@ export abstract class BaseRenderLayer implements IRenderLayer { fgOverride = d.foregroundColorRGB.rgba; } isTop = d.options.layer === 'top'; - } + }); // Apply selection foreground if applicable if (!isTop) { diff --git a/addons/xterm-addon-canvas/src/TextRenderLayer.ts b/addons/xterm-addon-canvas/src/TextRenderLayer.ts index 0308f12560..86e21ad5c8 100644 --- a/addons/xterm-addon-canvas/src/TextRenderLayer.ts +++ b/addons/xterm-addon-canvas/src/TextRenderLayer.ts @@ -187,15 +187,15 @@ export class TextRenderLayer extends BaseRenderLayer { // Get any decoration foreground/background overrides, this must be fetched before the early // exist but applied after inverse let isTop = false; - for (const d of this._decorationService.getDecorationsAtCell(x, this._bufferService.buffer.ydisp + y)) { + this._decorationService.forEachDecorationAtCell(x, this._bufferService.buffer.ydisp + y, undefined, d => { if (d.options.layer !== 'top' && isTop) { - continue; + return; } if (d.backgroundColorRGB) { nextFillStyle = d.backgroundColorRGB.css; } isTop = d.options.layer === 'top'; - } + }); if (prevFillStyle === null) { // This is either the first iteration, or the default background was set. Either way, we diff --git a/src/browser/renderer/dom/DomRendererRowFactory.ts b/src/browser/renderer/dom/DomRendererRowFactory.ts index cf4b36802b..d3eb9e8e6b 100644 --- a/src/browser/renderer/dom/DomRendererRowFactory.ts +++ b/src/browser/renderer/dom/DomRendererRowFactory.ts @@ -204,9 +204,9 @@ export class DomRendererRowFactory { let bgOverride: IColor | undefined; let fgOverride: IColor | undefined; let isTop = false; - for (const d of this._decorationService.getDecorationsAtCell(x, row)) { + this._decorationService.forEachDecorationAtCell(x, row, undefined, d => { if (d.options.layer !== 'top' && isTop) { - continue; + return; } if (d.backgroundColorRGB) { bgColorMode = Attributes.CM_RGB; @@ -219,7 +219,7 @@ export class DomRendererRowFactory { fgOverride = d.foregroundColorRGB; } isTop = d.options.layer === 'top'; - } + }); // Apply selection foreground if applicable const isInSelection = this._isCellInSelection(x, row); diff --git a/src/common/TestUtils.test.ts b/src/common/TestUtils.test.ts index cbcdd36e41..ff1e1b4691 100644 --- a/src/common/TestUtils.test.ts +++ b/src/common/TestUtils.test.ts @@ -161,8 +161,6 @@ export class MockDecorationService implements IDecorationService { public onDecorationRemoved = new EventEmitter().event; public registerDecoration(decorationOptions: IDecorationOptions): IDecoration | undefined { return undefined; } public reset(): void { } - public *getDecorationsAtLine(line: number): IterableIterator { } - public *getDecorationsAtCell(x: number, line: number): IterableIterator { } public forEachDecorationAtCell(x: number, line: number, layer: 'bottom' | 'top' | undefined, callback: (decoration: IInternalDecoration) => void): void { } public dispose(): void { } } diff --git a/src/common/services/DecorationService.ts b/src/common/services/DecorationService.ts index 8957136367..e5d115a14a 100644 --- a/src/common/services/DecorationService.ts +++ b/src/common/services/DecorationService.ts @@ -62,10 +62,6 @@ export class DecorationService extends Disposable implements IDecorationService this._decorations.clear(); } - public *getDecorationsAtLine(line: number): IterableIterator { - return this._decorations.getKeyIterator(line); - } - public *getDecorationsAtCell(x: number, line: number, layer?: 'bottom' | 'top'): IterableIterator { let xmin = 0; let xmax = 0; diff --git a/src/common/services/Services.ts b/src/common/services/Services.ts index b8e227b347..817a7680c1 100644 --- a/src/common/services/Services.ts +++ b/src/common/services/Services.ts @@ -304,13 +304,9 @@ export interface IDecorationService extends IDisposable { readonly onDecorationRemoved: IEvent; registerDecoration(decorationOptions: IDecorationOptions): IDecoration | undefined; reset(): void; - /** Iterates over the decorations at a line (in no particular order). */ - getDecorationsAtLine(line: number): IterableIterator; - /** Iterates over the decorations at a cell (in no particular order). */ - getDecorationsAtCell(x: number, line: number, layer?: 'bottom' | 'top'): IterableIterator; /** - * Trigger a callback over the decoration at a cell (in no particular order). This is a high - * performance, but less ergonomic, version of {@link getDecorationsAtCell}. + * Trigger a callback over the decoration at a cell (in no particular order). This uses a callback + * instead of an iterator as it's typically used in hot code paths. */ forEachDecorationAtCell(x: number, line: number, layer: 'bottom' | 'top' | undefined, callback: (decoration: IInternalDecoration) => void): void; } From 1b64a09f50bcc1edda5dc16e2ff7fcd3de7602d5 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 27 Aug 2022 21:35:56 -0700 Subject: [PATCH 22/28] Correct conditions after moving to work object --- addons/xterm-addon-webgl/src/WebglRenderer.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/addons/xterm-addon-webgl/src/WebglRenderer.ts b/addons/xterm-addon-webgl/src/WebglRenderer.ts index efb80f662d..ea10b36833 100644 --- a/addons/xterm-addon-webgl/src/WebglRenderer.ts +++ b/addons/xterm-addon-webgl/src/WebglRenderer.ts @@ -475,27 +475,29 @@ export class WebglRenderer extends Disposable implements IRenderer { // Handle case where inverse was specified by only one of bg override or fg override was set, // resolving the other inverse color and setting the inverse flag if needed. if (this._workColors.fg & FgFlags.INVERSE) { - if (w.hasBg && w.hasFg) { + if (w.hasBg && !w.hasFg) { // Resolve bg color type (default color has a different meaning in fg vs bg) if ((this._workColors.bg & Attributes.CM_MASK) === Attributes.CM_DEFAULT) { w.fg = (this._workColors.fg & ~(Attributes.RGB_MASK | FgFlags.INVERSE | Attributes.CM_MASK)) | ((this._colors.background.rgba >> 8 & 0xFFFFFF) & Attributes.RGB_MASK) | Attributes.CM_RGB; } else { w.fg = (this._workColors.fg & ~(Attributes.RGB_MASK | FgFlags.INVERSE | Attributes.CM_MASK)) | this._workColors.bg & (Attributes.RGB_MASK | Attributes.CM_MASK); } + w.hasFg = true; } - if (w.hasBg && w.hasFg) { + if (!w.hasBg && w.hasFg) { // Resolve bg color type (default color has a different meaning in fg vs bg) if ((this._workColors.fg & Attributes.CM_MASK) === Attributes.CM_DEFAULT) { w.bg = (this._workColors.bg & ~(Attributes.RGB_MASK | Attributes.CM_MASK)) | ((this._colors.foreground.rgba >> 8 & 0xFFFFFF) & Attributes.RGB_MASK) | Attributes.CM_RGB; } else { w.bg = (this._workColors.bg & ~(Attributes.RGB_MASK | Attributes.CM_MASK)) | this._workColors.fg & (Attributes.RGB_MASK | Attributes.CM_MASK); } + w.hasBg = true; } } // Use the override if it exists - this._workColors.bg = w.bg ?? this._workColors.bg; - this._workColors.fg = w.fg ?? this._workColors.fg; + this._workColors.bg = w.hasBg ? w.bg : this._workColors.bg; + this._workColors.fg = w.hasFg ? w.fg : this._workColors.fg; } private _isCellSelected(x: number, y: number): boolean { From 23ee719f6ed399e01f9abac7a50146936abbba3f Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 28 Aug 2022 08:02:35 -0700 Subject: [PATCH 23/28] Remove more garbage collection issues from cell render loop Part of #4079 --- addons/xterm-addon-webgl/src/GlyphRenderer.ts | 67 ++++++++++--------- .../src/atlas/WebglCharAtlas.ts | 49 +++++++++----- 2 files changed, 66 insertions(+), 50 deletions(-) diff --git a/addons/xterm-addon-webgl/src/GlyphRenderer.ts b/addons/xterm-addon-webgl/src/GlyphRenderer.ts index 6817cb21a9..6c8df65c68 100644 --- a/addons/xterm-addon-webgl/src/GlyphRenderer.ts +++ b/addons/xterm-addon-webgl/src/GlyphRenderer.ts @@ -70,6 +70,14 @@ const INDICES_PER_CELL = 10; const BYTES_PER_CELL = INDICES_PER_CELL * Float32Array.BYTES_PER_ELEMENT; const CELL_POSITION_INDICES = 2; +/** Work variables to avoid garbage collection. */ +const w: { i: number, glyph: IRasterizedGlyph | undefined, leftCellPadding: number, clippedPixels: number } = { + i: 0, + glyph: undefined, + leftCellPadding: 0, + clippedPixels: 0 +}; + export class GlyphRenderer extends Disposable { private _atlas: WebglCharAtlas | undefined; @@ -170,18 +178,20 @@ export class GlyphRenderer extends Disposable { } public updateCell(x: number, y: number, code: number, bg: number, fg: number, ext: number, chars: string, lastBg: number): void { + // Since this function is called for every cell (`rows*cols`), it must be very optimized. It + // should not instantiate any variables unless a new glyph is drawn to the cache where the + // slight slowdown is acceptable for the developer ergonomics provided as it's a once of for + // each glyph. this._updateCell(this._vertices.attributes, x, y, code, bg, fg, ext, chars, lastBg); } private _updateCell(array: Float32Array, x: number, y: number, code: number | undefined, bg: number, fg: number, ext: number, chars: string, lastBg: number): void { - const terminal = this._terminal; - - const i = (y * terminal.cols + x) * INDICES_PER_CELL; + w.i = (y * this._terminal.cols + x) * INDICES_PER_CELL; // Exit early if this is a null character, allow space character to continue as it may have // underline/strikethrough styles if (code === NULL_CELL_CODE || code === undefined/* This is used for the right side of wide chars */) { - fill(array, 0, i, i + INDICES_PER_CELL - 1 - CELL_POSITION_INDICES); + fill(array, 0, w.i, w.i + INDICES_PER_CELL - 1 - CELL_POSITION_INDICES); return; } @@ -190,47 +200,40 @@ export class GlyphRenderer extends Disposable { } // Get the glyph - let rasterizedGlyph: IRasterizedGlyph; if (chars && chars.length > 1) { - rasterizedGlyph = this._atlas.getRasterizedGlyphCombinedChar(chars, bg, fg, ext); + w.glyph = this._atlas.getRasterizedGlyphCombinedChar(chars, bg, fg, ext); } else { - rasterizedGlyph = this._atlas.getRasterizedGlyph(code, bg, fg, ext); - } - - // Fill empty if no glyph was found - if (!rasterizedGlyph) { - fill(array, 0, i, i + INDICES_PER_CELL - 1 - CELL_POSITION_INDICES); - return; + w.glyph = this._atlas.getRasterizedGlyph(code, bg, fg, ext); } - const leftCellPadding = Math.floor((this._dimensions.scaledCellWidth - this._dimensions.scaledCharWidth) / 2); - if (bg !== lastBg && rasterizedGlyph.offset.x > leftCellPadding) { - const clippedPixels = rasterizedGlyph.offset.x - leftCellPadding; + w.leftCellPadding = Math.floor((this._dimensions.scaledCellWidth - this._dimensions.scaledCharWidth) / 2); + if (bg !== lastBg && w.glyph.offset.x > w.leftCellPadding) { + w.clippedPixels = w.glyph.offset.x - w.leftCellPadding; // a_origin - array[i ] = -(rasterizedGlyph.offset.x - clippedPixels) + this._dimensions.scaledCharLeft; - array[i + 1] = -rasterizedGlyph.offset.y + this._dimensions.scaledCharTop; + array[w.i ] = -(w.glyph.offset.x - w.clippedPixels) + this._dimensions.scaledCharLeft; + array[w.i + 1] = -w.glyph.offset.y + this._dimensions.scaledCharTop; // a_size - array[i + 2] = (rasterizedGlyph.size.x - clippedPixels) / this._dimensions.scaledCanvasWidth; - array[i + 3] = rasterizedGlyph.size.y / this._dimensions.scaledCanvasHeight; + array[w.i + 2] = (w.glyph.size.x - w.clippedPixels) / this._dimensions.scaledCanvasWidth; + array[w.i + 3] = w.glyph.size.y / this._dimensions.scaledCanvasHeight; // a_texcoord - array[i + 4] = rasterizedGlyph.texturePositionClipSpace.x + clippedPixels / this._atlas.cacheCanvas.width; - array[i + 5] = rasterizedGlyph.texturePositionClipSpace.y; + array[w.i + 4] = w.glyph.texturePositionClipSpace.x + w.clippedPixels / this._atlas.cacheCanvas.width; + array[w.i + 5] = w.glyph.texturePositionClipSpace.y; // a_texsize - array[i + 6] = rasterizedGlyph.sizeClipSpace.x - clippedPixels / this._atlas.cacheCanvas.width; - array[i + 7] = rasterizedGlyph.sizeClipSpace.y; + array[w.i + 6] = w.glyph.sizeClipSpace.x - w.clippedPixels / this._atlas.cacheCanvas.width; + array[w.i + 7] = w.glyph.sizeClipSpace.y; } else { // a_origin - array[i ] = -rasterizedGlyph.offset.x + this._dimensions.scaledCharLeft; - array[i + 1] = -rasterizedGlyph.offset.y + this._dimensions.scaledCharTop; + array[w.i ] = -w.glyph.offset.x + this._dimensions.scaledCharLeft; + array[w.i + 1] = -w.glyph.offset.y + this._dimensions.scaledCharTop; // a_size - array[i + 2] = rasterizedGlyph.size.x / this._dimensions.scaledCanvasWidth; - array[i + 3] = rasterizedGlyph.size.y / this._dimensions.scaledCanvasHeight; + array[w.i + 2] = w.glyph.size.x / this._dimensions.scaledCanvasWidth; + array[w.i + 3] = w.glyph.size.y / this._dimensions.scaledCanvasHeight; // a_texcoord - array[i + 4] = rasterizedGlyph.texturePositionClipSpace.x; - array[i + 5] = rasterizedGlyph.texturePositionClipSpace.y; + array[w.i + 4] = w.glyph.texturePositionClipSpace.x; + array[w.i + 5] = w.glyph.texturePositionClipSpace.y; // a_texsize - array[i + 6] = rasterizedGlyph.sizeClipSpace.x; - array[i + 7] = rasterizedGlyph.sizeClipSpace.y; + array[w.i + 6] = w.glyph.sizeClipSpace.x; + array[w.i + 7] = w.glyph.sizeClipSpace.y; } // a_cellpos only changes on resize } diff --git a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts index e1d56b7867..dda4c2fc8e 100644 --- a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts +++ b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts @@ -52,6 +52,19 @@ interface ICharAtlasActiveRow { height: number; } +/** Work variables to avoid garbage collection. */ +const w: { + glyphSet: IRasterizedGlyphSet | undefined; + glyphSetBg: { [fg: number]: { [ext: number]: IRasterizedGlyph } } | undefined; + glyphSetFg: { [ext: number]: IRasterizedGlyph } | undefined; + glyph: IRasterizedGlyph | undefined; +} = { + glyphSet: undefined, + glyphSetBg: undefined, + glyphSetFg: undefined, + glyph: undefined +}; + export class WebglCharAtlas implements IDisposable { private _didWarmUp: boolean = false; @@ -175,33 +188,33 @@ export class WebglCharAtlas implements IDisposable { fg: number, ext: number ): IRasterizedGlyph { - let rasterizedGlyphSet = cacheMap[key]; - if (!rasterizedGlyphSet) { - rasterizedGlyphSet = {}; - cacheMap[key] = rasterizedGlyphSet; + w.glyphSet = cacheMap[key]; + if (!w.glyphSet) { + w.glyphSet = {}; + cacheMap[key] = w.glyphSet; } - let rasterizedGlyphSetBg = rasterizedGlyphSet[bg]; - if (!rasterizedGlyphSetBg) { - rasterizedGlyphSetBg = {}; - rasterizedGlyphSet[bg] = rasterizedGlyphSetBg; + w.glyphSetBg = w.glyphSet[bg]; + if (!w.glyphSetBg) { + w.glyphSetBg = {}; + w.glyphSet[bg] = w.glyphSetBg; } - let rasterizedGlyph: IRasterizedGlyph | undefined; - let rasterizedGlyphSetFg = rasterizedGlyphSetBg[fg]; - if (!rasterizedGlyphSetFg) { - rasterizedGlyphSetFg = {}; - rasterizedGlyphSetBg[fg] = rasterizedGlyphSetFg; + w.glyph = undefined; + w.glyphSetFg = w.glyphSetBg[fg]; + if (!w.glyphSetFg) { + w.glyphSetFg = {}; + w.glyphSetBg[fg] = w.glyphSetFg; } else { - rasterizedGlyph = rasterizedGlyphSetFg[ext]; + w.glyph = w.glyphSetFg[ext]; } - if (!rasterizedGlyph) { - rasterizedGlyph = this._drawToCache(key, bg, fg, ext); - rasterizedGlyphSetFg[ext] = rasterizedGlyph; + if (!w.glyph) { + w.glyph = this._drawToCache(key, bg, fg, ext); + w.glyphSetFg[ext] = w.glyph; } - return rasterizedGlyph; + return w.glyph; } private _getColorFromAnsiIndex(idx: number): IColor { From 1d945fdffce1d86dcaf9acf4b5538ef6aa10a80f Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 28 Aug 2022 08:34:51 -0700 Subject: [PATCH 24/28] Create 2 and 4 key map objects, use in color and glyph caches --- .../src/atlas/WebglCharAtlas.ts | 54 ++++--------------- src/browser/ColorContrastCache.ts | 29 +++++----- src/common/MultiKeyMap.ts | 42 +++++++++++++++ 3 files changed, 64 insertions(+), 61 deletions(-) create mode 100644 src/common/MultiKeyMap.ts diff --git a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts index dda4c2fc8e..c17c54a527 100644 --- a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts +++ b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts @@ -15,6 +15,7 @@ import { color, rgba } from 'common/Color'; import { tryDrawCustomChar } from 'browser/renderer/CustomGlyphs'; import { excludeFromContrastRatioDemands, isPowerlineGlyph, isRestrictedPowerlineGlyph } from 'browser/renderer/RendererUtils'; import { IUnicodeService } from 'common/services/Services'; +import { FourKeyMap } from 'common/MultiKeyMap'; // For debugging purposes, it can be useful to set this to a really tiny value, // to verify that LRU eviction works. @@ -53,23 +54,15 @@ interface ICharAtlasActiveRow { } /** Work variables to avoid garbage collection. */ -const w: { - glyphSet: IRasterizedGlyphSet | undefined; - glyphSetBg: { [fg: number]: { [ext: number]: IRasterizedGlyph } } | undefined; - glyphSetFg: { [ext: number]: IRasterizedGlyph } | undefined; - glyph: IRasterizedGlyph | undefined; -} = { - glyphSet: undefined, - glyphSetBg: undefined, - glyphSetFg: undefined, +const w: { glyph: IRasterizedGlyph | undefined } = { glyph: undefined }; export class WebglCharAtlas implements IDisposable { private _didWarmUp: boolean = false; - private _cacheMap: { [code: number]: IRasterizedGlyphSet } = {}; - private _cacheMapCombined: { [chars: string]: IRasterizedGlyphSet } = {}; + private _cacheMap: FourKeyMap = new FourKeyMap(); + private _cacheMapCombined: FourKeyMap = new FourKeyMap(); // The texture that the atlas is drawn to public cacheCanvas: HTMLCanvasElement; @@ -137,13 +130,7 @@ export class WebglCharAtlas implements IDisposable { // Pre-fill with ASCII 33-126 for (let i = 33; i < 126; i++) { const rasterizedGlyph = this._drawToCache(i, DEFAULT_COLOR, DEFAULT_COLOR, DEFAULT_EXT); - this._cacheMap[i] = { - [DEFAULT_COLOR]: { - [DEFAULT_COLOR]: { - [DEFAULT_EXT]: rasterizedGlyph - } - } - }; + this._cacheMap.set(i, DEFAULT_COLOR, DEFAULT_COLOR, DEFAULT_EXT, rasterizedGlyph); } } @@ -161,8 +148,8 @@ export class WebglCharAtlas implements IDisposable { return; } this._cacheCtx.clearRect(0, 0, TEXTURE_WIDTH, TEXTURE_HEIGHT); - this._cacheMap = {}; - this._cacheMapCombined = {}; + this._cacheMap.clear(); + this._cacheMapCombined.clear(); this._currentRow.x = 0; this._currentRow.y = 0; this._currentRow.height = 0; @@ -182,38 +169,17 @@ export class WebglCharAtlas implements IDisposable { * Gets the glyphs texture coords, drawing the texture if it's not already */ private _getFromCacheMap( - cacheMap: { [key: string | number]: IRasterizedGlyphSet }, + cacheMap: FourKeyMap, key: string | number, bg: number, fg: number, ext: number ): IRasterizedGlyph { - w.glyphSet = cacheMap[key]; - if (!w.glyphSet) { - w.glyphSet = {}; - cacheMap[key] = w.glyphSet; - } - - w.glyphSetBg = w.glyphSet[bg]; - if (!w.glyphSetBg) { - w.glyphSetBg = {}; - w.glyphSet[bg] = w.glyphSetBg; - } - - w.glyph = undefined; - w.glyphSetFg = w.glyphSetBg[fg]; - if (!w.glyphSetFg) { - w.glyphSetFg = {}; - w.glyphSetBg[fg] = w.glyphSetFg; - } else { - w.glyph = w.glyphSetFg[ext]; - } - + w.glyph = cacheMap.get(key, bg, fg, ext); if (!w.glyph) { w.glyph = this._drawToCache(key, bg, fg, ext); - w.glyphSetFg[ext] = w.glyph; + cacheMap.set(key, bg, fg, ext, w.glyph); } - return w.glyph; } diff --git a/src/browser/ColorContrastCache.ts b/src/browser/ColorContrastCache.ts index 73b7a0b755..0c60e8db45 100644 --- a/src/browser/ColorContrastCache.ts +++ b/src/browser/ColorContrastCache.ts @@ -5,35 +5,30 @@ import { IColorContrastCache } from 'browser/Types'; import { IColor } from 'common/Types'; +import { TwoKeyMap } from 'common/MultiKeyMap'; export class ColorContrastCache implements IColorContrastCache { - private _color: { [bg: number]: { [fg: number]: IColor | null | undefined } | undefined } = {}; - private _rgba: { [bg: number]: { [fg: number]: string | null | undefined } | undefined } = {}; - - public clear(): void { - this._color = {}; - this._rgba = {}; - } + private _color: TwoKeyMap = new TwoKeyMap(); + private _css: TwoKeyMap = new TwoKeyMap(); public setCss(bg: number, fg: number, value: string | null): void { - if (!this._rgba[bg]) { - this._rgba[bg] = {}; - } - this._rgba[bg]![fg] = value; + this._css.set(bg, fg, value); } public getCss(bg: number, fg: number): string | null | undefined { - return this._rgba[bg] ? this._rgba[bg]![fg] : undefined; + return this._css.get(bg, fg); } public setColor(bg: number, fg: number, value: IColor | null): void { - if (!this._color[bg]) { - this._color[bg] = {}; - } - this._color[bg]![fg] = value; + this._color.set(bg, fg, value); } public getColor(bg: number, fg: number): IColor | null | undefined { - return this._color[bg] ? this._color[bg]![fg] : undefined; + return this._color.get(bg, fg); + } + + public clear(): void { + this._color.clear(); + this._css.clear(); } } diff --git a/src/common/MultiKeyMap.ts b/src/common/MultiKeyMap.ts new file mode 100644 index 0000000000..6287a8f243 --- /dev/null +++ b/src/common/MultiKeyMap.ts @@ -0,0 +1,42 @@ +/** + * Copyright (c) 2022 The xterm.js authors. All rights reserved. + * @license MIT + */ + +export class TwoKeyMap { + private _data: { [bg: string | number]: { [fg: string | number]: TValue | undefined } | undefined } = {}; + + public set(first: TFirst, second: TSecond, value: TValue): void { + if (!this._data[first]) { + this._data[first] = {}; + } + this._data[first as string | number]![second] = value; + } + + public get(first: TFirst, second: TSecond): TValue | undefined { + return this._data[first as string | number] ? this._data[first as string | number]![second] : undefined; + } + + public clear(): void { + this._data = {}; + } +} + +export class FourKeyMap { + private _data: TwoKeyMap> = new TwoKeyMap(); + + public set(first: TFirst, second: TSecond, third: TThird, fourth: TFourth, value: TValue): void { + if (!this._data.get(first, second)) { + this._data.set(first, second, new TwoKeyMap()); + } + this._data.get(first, second)!.set(third, fourth, value); + } + + public get(first: TFirst, second: TSecond, third: TThird, fourth: TFourth): TValue | undefined { + return this._data.get(first, second)?.get(third, fourth); + } + + public clear(): void { + this._data.clear(); + } +} From caaca169eb6c8c56f009b671b170ccac709d1374 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 28 Aug 2022 08:45:36 -0700 Subject: [PATCH 25/28] Add multi key map tests --- src/common/MultiKeyMap.test.ts | 69 ++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 src/common/MultiKeyMap.test.ts diff --git a/src/common/MultiKeyMap.test.ts b/src/common/MultiKeyMap.test.ts new file mode 100644 index 0000000000..b908321c32 --- /dev/null +++ b/src/common/MultiKeyMap.test.ts @@ -0,0 +1,69 @@ +/** + * Copyright (c) 2018 The xterm.js authors. All rights reserved. + * @license MIT + */ + +import { assert } from 'chai'; +import { FourKeyMap, TwoKeyMap } from 'common/MultiKeyMap'; + +const strictEqual = assert.strictEqual; + +describe('TwoKeyMap', () => { + let map: TwoKeyMap; + + beforeEach(() => { + map = new TwoKeyMap(); + }); + + it('set, get', () => { + strictEqual(map.get(1, 2), undefined); + map.set(1, 2, 'foo'); + strictEqual(map.get(1, 2), 'foo'); + map.set(1, 3, 'bar'); + strictEqual(map.get(1, 2), 'foo'); + strictEqual(map.get(1, 3), 'bar'); + map.set(2, 2, 'foo2'); + map.set(2, 3, 'bar2'); + strictEqual(map.get(1, 2), 'foo'); + strictEqual(map.get(1, 3), 'bar'); + strictEqual(map.get(2, 2), 'foo2'); + strictEqual(map.get(2, 3), 'bar2'); + }); + it('clear', () => { + strictEqual(map.get(1, 2), undefined); + map.set(1, 2, 'foo'); + strictEqual(map.get(1, 2), 'foo'); + map.clear(); + strictEqual(map.get(1, 2), undefined); + }); +}); + +describe('FourKeyMap', () => { + let map: FourKeyMap; + + beforeEach(() => { + map = new FourKeyMap(); + }); + + it('set, get', () => { + strictEqual(map.get(1, 2, 3, 4), undefined); + map.set(1, 2, 3, 4, 'foo'); + strictEqual(map.get(1, 2, 3, 4), 'foo'); + map.set(1, 3, 3, 4, 'bar'); + strictEqual(map.get(1, 2, 3, 4), 'foo'); + strictEqual(map.get(1, 3, 3, 4), 'bar'); + map.set(2, 2, 3, 4, 'foo2'); + map.set(2, 3, 3, 4, 'bar2'); + strictEqual(map.get(1, 2, 3, 4), 'foo'); + strictEqual(map.get(1, 3, 3, 4), 'bar'); + strictEqual(map.get(2, 2, 3, 4), 'foo2'); + strictEqual(map.get(2, 3, 3, 4), 'bar2'); + }); + it('clear', () => { + strictEqual(map.get(1, 2, 3, 4), undefined); + map.set(1, 2, 3, 4, 'foo'); + strictEqual(map.get(1, 2, 3, 4), 'foo'); + map.clear(); + strictEqual(map.get(1, 2, 3, 4), undefined); + }); +}); From 4a50ca81c62528e4171ba8654a08184eb846e4bf Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 28 Aug 2022 09:06:42 -0700 Subject: [PATCH 26/28] Optimize gc for WebglRenderer._updateModel and BufferLine.loadCell RectangleRenderer not done yet --- addons/xterm-addon-webgl/src/WebglRenderer.ts | 42 ++++++++++++------- src/common/buffer/BufferLine.ts | 13 ++++-- 2 files changed, 37 insertions(+), 18 deletions(-) diff --git a/addons/xterm-addon-webgl/src/WebglRenderer.ts b/addons/xterm-addon-webgl/src/WebglRenderer.ts index ea10b36833..81817a80b2 100644 --- a/addons/xterm-addon-webgl/src/WebglRenderer.ts +++ b/addons/xterm-addon-webgl/src/WebglRenderer.ts @@ -22,7 +22,7 @@ import { EventEmitter } from 'common/EventEmitter'; import { CellData } from 'common/buffer/CellData'; import { addDisposableDomListener } from 'browser/Lifecycle'; import { ICharacterJoinerService, ICoreBrowserService } from 'browser/services/Services'; -import { CharData, ICellData } from 'common/Types'; +import { CharData, IBufferLine, ICellData } from 'common/Types'; import { AttributeData } from 'common/buffer/AttributeData'; import { ICoreService, IDecorationService } from 'common/services/Services'; @@ -314,14 +314,28 @@ export class WebglRenderer extends Disposable implements IRenderer { private _updateModel(start: number, end: number): void { const terminal = this._core; let cell: ICellData = this._workCell; - let lastBg: number = 0; - for (let y = start; y <= end; y++) { - const row = y + terminal.buffer.ydisp; - const line = terminal.buffer.lines.get(row)!; + // Declare variable ahead of time to avoid garbage collection + let lastBg: number; + let y: number; + let row: number; + let line: IBufferLine; + let joinedRanges: [number, number][]; + let isJoined: boolean; + let lastCharX: number; + let range: [number, number]; + let chars: string; + let code: number; + let i: number; + let x: number; + let j: number; + + for (y = start; y <= end; y++) { + row = y + terminal.buffer.ydisp; + line = terminal.buffer.lines.get(row)!; this._model.lineLengths[y] = 0; - const joinedRanges = this._characterJoinerService.getJoinedCharacters(row); - for (let x = 0; x < terminal.cols; x++) { + joinedRanges = this._characterJoinerService.getJoinedCharacters(row); + for (x = 0; x < terminal.cols; x++) { lastBg = this._workColors.bg; line.loadCell(x, cell); @@ -330,15 +344,15 @@ export class WebglRenderer extends Disposable implements IRenderer { } // If true, indicates that the current character(s) to draw were joined. - let isJoined = false; - let lastCharX = x; + isJoined = false; + lastCharX = x; // Process any joined character ranges as needed. Because of how the // ranges are produced, we know that they are valid for the characters // and attributes of our input. if (joinedRanges.length > 0 && x === joinedRanges[0][0]) { isJoined = true; - const range = joinedRanges.shift()!; + range = joinedRanges.shift()!; // We already know the exact start and end column of the joined range, // so we get the string and width representing it directly. @@ -352,9 +366,9 @@ export class WebglRenderer extends Disposable implements IRenderer { lastCharX = range[1] - 1; } - const chars = cell.getChars(); - let code = cell.getCode(); - const i = ((y * terminal.cols) + x) * RENDER_MODEL_INDICIES_PER_CELL; + chars = cell.getChars(); + code = cell.getCode(); + i = ((y * terminal.cols) + x) * RENDER_MODEL_INDICIES_PER_CELL; // Load colors/resolve overrides into work colors this._loadColorsForCell(x, row); @@ -390,7 +404,7 @@ export class WebglRenderer extends Disposable implements IRenderer { // Null out non-first cells for (x++; x < lastCharX; x++) { - const j = ((y * terminal.cols) + x) * RENDER_MODEL_INDICIES_PER_CELL; + j = ((y * terminal.cols) + x) * RENDER_MODEL_INDICIES_PER_CELL; this._glyphRenderer.updateCell(x, y, NULL_CELL_CODE, 0, 0, 0, NULL_CELL_CHAR, 0); this._model.cells[j] = NULL_CELL_CODE; this._model.cells[j + RENDER_MODEL_BG_OFFSET] = this._workColors.bg; diff --git a/src/common/buffer/BufferLine.ts b/src/common/buffer/BufferLine.ts index 6d2a442f6e..eed87e6b07 100644 --- a/src/common/buffer/BufferLine.ts +++ b/src/common/buffer/BufferLine.ts @@ -37,6 +37,11 @@ const enum Cell { export const DEFAULT_ATTR_DATA = Object.freeze(new AttributeData()); +/** Work variables to avoid garbage collection. */ +const w: { startIndex: number } = { + startIndex: 0 +}; + /** * Typed array based bufferline implementation. * @@ -168,10 +173,10 @@ export class BufferLine implements IBufferLine { * to GC as it significantly reduced the amount of new objects/references needed. */ public loadCell(index: number, cell: ICellData): ICellData { - const startIndex = index * CELL_SIZE; - cell.content = this._data[startIndex + Cell.CONTENT]; - cell.fg = this._data[startIndex + Cell.FG]; - cell.bg = this._data[startIndex + Cell.BG]; + w.startIndex = index * CELL_SIZE; + cell.content = this._data[w.startIndex + Cell.CONTENT]; + cell.fg = this._data[w.startIndex + Cell.FG]; + cell.bg = this._data[w.startIndex + Cell.BG]; if (cell.content & Content.IS_COMBINED_MASK) { cell.combinedData = this._combined[index]; } From 3b4ecd0709baeac86404a5609b3ff7c9f4ff0963 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 28 Aug 2022 09:20:24 -0700 Subject: [PATCH 27/28] Optimize gc in rectangle renderer --- .../src/RectangleRenderer.ts | 83 ++++++++++++------- 1 file changed, 53 insertions(+), 30 deletions(-) diff --git a/addons/xterm-addon-webgl/src/RectangleRenderer.ts b/addons/xterm-addon-webgl/src/RectangleRenderer.ts index f16fdec041..acaea56a7d 100644 --- a/addons/xterm-addon-webgl/src/RectangleRenderer.ts +++ b/addons/xterm-addon-webgl/src/RectangleRenderer.ts @@ -59,6 +59,18 @@ const BYTES_PER_RECTANGLE = INDICES_PER_RECTANGLE * Float32Array.BYTES_PER_ELEME const INITIAL_BUFFER_RECTANGLE_CAPACITY = 20 * INDICES_PER_RECTANGLE; +/** Work variables to avoid garbage collection. */ +const w: { rgba: number, isDefault: boolean, x1: number, y1: number, r: number, g: number, b: number, a: number } = { + rgba: 0, + isDefault: false, + x1: 0, + y1: 0, + r: 0, + g: 0, + b: 0, + a: 0 +}; + export class RectangleRenderer extends Disposable { private _program: WebGLProgram; @@ -174,22 +186,34 @@ export class RectangleRenderer extends Disposable { const terminal = this._terminal; const vertices = this._vertices; + // Declare variable ahead of time to avoid garbage collection let rectangleCount = 1; - - for (let y = 0; y < terminal.rows; y++) { - let currentStartX = -1; - let currentBg = 0; - let currentFg = 0; - let currentInverse = false; - for (let x = 0; x < terminal.cols; x++) { - const modelIndex = ((y * terminal.cols) + x) * RENDER_MODEL_INDICIES_PER_CELL; - const bg = model.cells[modelIndex + RENDER_MODEL_BG_OFFSET]; - const fg = model.cells[modelIndex + RENDER_MODEL_FG_OFFSET]; - const inverse = !!(fg & FgFlags.INVERSE); + let y: number; + let x: number; + let currentStartX: number; + let currentBg: number; + let currentFg: number; + let currentInverse: boolean; + let modelIndex: number; + let bg: number; + let fg: number; + let inverse: boolean; + let offset: number; + + for (y = 0; y < terminal.rows; y++) { + currentStartX = -1; + currentBg = 0; + currentFg = 0; + currentInverse = false; + for (x = 0; x < terminal.cols; x++) { + modelIndex = ((y * terminal.cols) + x) * RENDER_MODEL_INDICIES_PER_CELL; + bg = model.cells[modelIndex + RENDER_MODEL_BG_OFFSET]; + fg = model.cells[modelIndex + RENDER_MODEL_FG_OFFSET]; + inverse = !!(fg & FgFlags.INVERSE); if (bg !== currentBg || (fg !== currentFg && (currentInverse || inverse))) { // A rectangle needs to be drawn if going from non-default to another color if (currentBg !== 0 || (currentInverse && currentFg !== 0)) { - const offset = rectangleCount++ * INDICES_PER_RECTANGLE; + offset = rectangleCount++ * INDICES_PER_RECTANGLE; this._updateRectangle(vertices, offset, currentFg, currentBg, currentStartX, x, y); } currentStartX = x; @@ -200,7 +224,7 @@ export class RectangleRenderer extends Disposable { } // Finish rectangle if it's still going if (currentBg !== 0 || (currentInverse && currentFg !== 0)) { - const offset = rectangleCount++ * INDICES_PER_RECTANGLE; + offset = rectangleCount++ * INDICES_PER_RECTANGLE; this._updateRectangle(vertices, offset, currentFg, currentBg, currentStartX, terminal.cols, y); } } @@ -208,48 +232,47 @@ export class RectangleRenderer extends Disposable { } private _updateRectangle(vertices: IVertices, offset: number, fg: number, bg: number, startX: number, endX: number, y: number): void { - let rgba: number | undefined; - let isDefault = false; + w.isDefault = false; if (fg & FgFlags.INVERSE) { switch (fg & Attributes.CM_MASK) { case Attributes.CM_P16: case Attributes.CM_P256: - rgba = this._colors.ansi[fg & Attributes.PCOLOR_MASK].rgba; + w.rgba = this._colors.ansi[fg & Attributes.PCOLOR_MASK].rgba; break; case Attributes.CM_RGB: - rgba = (fg & Attributes.RGB_MASK) << 8; + w.rgba = (fg & Attributes.RGB_MASK) << 8; break; case Attributes.CM_DEFAULT: default: - rgba = this._colors.foreground.rgba; + w.rgba = this._colors.foreground.rgba; } } else { switch (bg & Attributes.CM_MASK) { case Attributes.CM_P16: case Attributes.CM_P256: - rgba = this._colors.ansi[bg & Attributes.PCOLOR_MASK].rgba; + w.rgba = this._colors.ansi[bg & Attributes.PCOLOR_MASK].rgba; break; case Attributes.CM_RGB: - rgba = (bg & Attributes.RGB_MASK) << 8; + w.rgba = (bg & Attributes.RGB_MASK) << 8; break; case Attributes.CM_DEFAULT: default: - rgba = this._colors.background.rgba; - isDefault = true; + w.rgba = this._colors.background.rgba; + w.isDefault = true; } } if (vertices.attributes.length < offset + 4) { vertices.attributes = expandFloat32Array(vertices.attributes, this._terminal.rows * this._terminal.cols * INDICES_PER_RECTANGLE); } - const x1 = startX * this._dimensions.scaledCellWidth; - const y1 = y * this._dimensions.scaledCellHeight; - const r = ((rgba >> 24) & 0xFF) / 255; - const g = ((rgba >> 16) & 0xFF) / 255; - const b = ((rgba >> 8 ) & 0xFF) / 255; - const a = (!isDefault && bg & BgFlags.DIM) ? DIM_OPACITY : 1; - - this._addRectangle(vertices.attributes, offset, x1, y1, (endX - startX) * this._dimensions.scaledCellWidth, this._dimensions.scaledCellHeight, r, g, b, a); + w.x1 = startX * this._dimensions.scaledCellWidth; + w.y1 = y * this._dimensions.scaledCellHeight; + w.r = ((w.rgba >> 24) & 0xFF) / 255; + w.g = ((w.rgba >> 16) & 0xFF) / 255; + w.b = ((w.rgba >> 8 ) & 0xFF) / 255; + w.a = (!w.isDefault && bg & BgFlags.DIM) ? DIM_OPACITY : 1; + + this._addRectangle(vertices.attributes, offset, w.x1, w.y1, (endX - startX) * this._dimensions.scaledCellWidth, this._dimensions.scaledCellHeight, w.r, w.g, w.b, w.a); } private _addRectangle(array: Float32Array, offset: number, x1: number, y1: number, width: number, height: number, r: number, g: number, b: number, a: number): void { From f7e0f69598dee372ec94a6a09e343afc5c4f0385 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 28 Aug 2022 13:22:29 -0700 Subject: [PATCH 28/28] Fix rounding error gap in webgl renderer Fixes #4054 Fixes #4076 --- addons/xterm-addon-webgl/src/GlyphRenderer.ts | 2 +- .../src/RectangleRenderer.ts | 28 +++++++++---------- addons/xterm-addon-webgl/src/WebglRenderer.ts | 7 +++-- 3 files changed, 19 insertions(+), 18 deletions(-) diff --git a/addons/xterm-addon-webgl/src/GlyphRenderer.ts b/addons/xterm-addon-webgl/src/GlyphRenderer.ts index 6c8df65c68..5d3fb9ac73 100644 --- a/addons/xterm-addon-webgl/src/GlyphRenderer.ts +++ b/addons/xterm-addon-webgl/src/GlyphRenderer.ts @@ -130,7 +130,7 @@ export class GlyphRenderer extends Disposable { gl.vertexAttribPointer(VertexAttribLocations.UNIT_QUAD, 2, this._gl.FLOAT, false, 0, 0); // Setup the unit quad element array buffer, this points to indices in - // unitQuadVertuces to allow is to draw 2 triangles from the vertices + // unitQuadVertices to allow is to draw 2 triangles from the vertices const unitQuadElementIndices = new Uint8Array([0, 1, 3, 0, 2, 3]); const elementIndicesBuffer = gl.createBuffer(); this.register(toDisposable(() => gl.deleteBuffer(elementIndicesBuffer))); diff --git a/addons/xterm-addon-webgl/src/RectangleRenderer.ts b/addons/xterm-addon-webgl/src/RectangleRenderer.ts index acaea56a7d..ef08fb7c2c 100644 --- a/addons/xterm-addon-webgl/src/RectangleRenderer.ts +++ b/addons/xterm-addon-webgl/src/RectangleRenderer.ts @@ -28,12 +28,11 @@ layout (location = ${VertexAttribLocations.COLOR}) in vec4 a_color; layout (location = ${VertexAttribLocations.UNIT_QUAD}) in vec2 a_unitquad; uniform mat4 u_projection; -uniform vec2 u_resolution; out vec4 v_color; void main() { - vec2 zeroToOne = (a_position + (a_unitquad * a_size)) / u_resolution; + vec2 zeroToOne = a_position + (a_unitquad * a_size); gl_Position = u_projection * vec4(zeroToOne, 0.0, 1.0); v_color = a_color; }`; @@ -75,7 +74,6 @@ export class RectangleRenderer extends Disposable { private _program: WebGLProgram; private _vertexArrayObject: IWebGLVertexArrayObject; - private _resolutionLocation: WebGLUniformLocation; private _attributesBuffer: WebGLBuffer; private _projectionLocation: WebGLUniformLocation; private _bgFloat!: Float32Array; @@ -99,7 +97,6 @@ export class RectangleRenderer extends Disposable { this.register(toDisposable(() => gl.deleteProgram(this._program))); // Uniform locations - this._resolutionLocation = throwIfFalsy(gl.getUniformLocation(this._program, 'u_resolution')); this._projectionLocation = throwIfFalsy(gl.getUniformLocation(this._program, 'u_projection')); // Create and set the vertex array object @@ -116,7 +113,7 @@ export class RectangleRenderer extends Disposable { gl.vertexAttribPointer(VertexAttribLocations.UNIT_QUAD, 2, this._gl.FLOAT, false, 0, 0); // Setup the unit quad element array buffer, this points to indices in - // unitQuadVertuces to allow is to draw 2 triangles from the vertices + // unitQuadVertices to allow is to draw 2 triangles from the vertices const unitQuadElementIndices = new Uint8Array([0, 1, 3, 0, 2, 3]); const elementIndicesBuffer = gl.createBuffer(); this.register(toDisposable(() => gl.deleteBuffer(elementIndicesBuffer))); @@ -148,7 +145,6 @@ export class RectangleRenderer extends Disposable { gl.bindVertexArray(this._vertexArrayObject); gl.uniformMatrix4fv(this._projectionLocation, false, PROJECTION_MATRIX); - gl.uniform2f(this._resolutionLocation, gl.canvas.width, gl.canvas.height); // Bind attributes buffer and draw gl.bindBuffer(gl.ARRAY_BUFFER, this._attributesBuffer); @@ -165,6 +161,10 @@ export class RectangleRenderer extends Disposable { this._updateViewportRectangle(); } + public setDimensions(dimensions: IRenderDimensions): void { + this._dimensions = dimensions; + } + private _updateCachedColors(): void { this._bgFloat = this._colorToFloat32Array(this._colors.background); } @@ -276,10 +276,10 @@ export class RectangleRenderer extends Disposable { } private _addRectangle(array: Float32Array, offset: number, x1: number, y1: number, width: number, height: number, r: number, g: number, b: number, a: number): void { - array[offset ] = x1; - array[offset + 1] = y1; - array[offset + 2] = width; - array[offset + 3] = height; + array[offset ] = x1 / this._dimensions.scaledCanvasWidth; + array[offset + 1] = y1 / this._dimensions.scaledCanvasHeight; + array[offset + 2] = width / this._dimensions.scaledCanvasWidth; + array[offset + 3] = height / this._dimensions.scaledCanvasHeight; array[offset + 4] = r; array[offset + 5] = g; array[offset + 6] = b; @@ -287,10 +287,10 @@ export class RectangleRenderer extends Disposable { } private _addRectangleFloat(array: Float32Array, offset: number, x1: number, y1: number, width: number, height: number, color: Float32Array): void { - array[offset ] = x1; - array[offset + 1] = y1; - array[offset + 2] = width; - array[offset + 3] = height; + array[offset ] = x1 / this._dimensions.scaledCanvasWidth; + array[offset + 1] = y1 / this._dimensions.scaledCanvasHeight; + array[offset + 2] = width / this._dimensions.scaledCanvasWidth; + array[offset + 3] = height / this._dimensions.scaledCanvasHeight; array[offset + 4] = color[0]; array[offset + 5] = color[1]; array[offset + 6] = color[2]; diff --git a/addons/xterm-addon-webgl/src/WebglRenderer.ts b/addons/xterm-addon-webgl/src/WebglRenderer.ts index 81817a80b2..43de1c98b2 100644 --- a/addons/xterm-addon-webgl/src/WebglRenderer.ts +++ b/addons/xterm-addon-webgl/src/WebglRenderer.ts @@ -182,6 +182,7 @@ export class WebglRenderer extends Disposable implements IRenderer { this._core.screenElement!.style.width = `${this.dimensions.canvasWidth}px`; this._core.screenElement!.style.height = `${this.dimensions.canvasHeight}px`; + this._rectangleRenderer.setDimensions(this.dimensions); this._rectangleRenderer.onResize(); this._glyphRenderer.setDimensions(this.dimensions); this._glyphRenderer.onResize(); @@ -622,11 +623,11 @@ export class WebglRenderer extends Disposable implements IRenderer { } private _setCanvasDevicePixelDimensions(width: number, height: number): void { - if (this.dimensions.scaledCanvasWidth === width && this.dimensions.scaledCanvasHeight === height) { + if (this._canvas.width === width && this._canvas.height === height) { return; } - this.dimensions.scaledCanvasWidth = width; - this.dimensions.scaledCanvasHeight = height; + // While the actual canvas size has changed, keep scaledCanvasWidth/Height as the value before + // the change as it's an exact multiple of the cell sizes. this._canvas.width = width; this._canvas.height = height; this._requestRedrawViewport();