From 3ba579f680e1fd92e225a2368cf18789394e9d0b Mon Sep 17 00:00:00 2001 From: tisilent Date: Tue, 8 Aug 2023 18:36:44 +0800 Subject: [PATCH 1/5] fix double underline, upwards --- src/browser/renderer/shared/TextureAtlas.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/browser/renderer/shared/TextureAtlas.ts b/src/browser/renderer/shared/TextureAtlas.ts index d6bf1581..b7ab64ee 100644 --- a/src/browser/renderer/shared/TextureAtlas.ts +++ b/src/browser/renderer/shared/TextureAtlas.ts @@ -534,6 +534,7 @@ export class TextureAtlas implements ITextureAtlas { const yTop = Math.ceil(padding + this._config.deviceCharHeight) - yOffset; const yMid = padding + this._config.deviceCharHeight + lineWidth - yOffset; const yBot = Math.ceil(padding + this._config.deviceCharHeight + lineWidth * 2) - yOffset; + const ySpace = lineWidth * 2; for (let i = 0; i < chWidth; i++) { this._tmpCtx.save(); @@ -542,10 +543,10 @@ export class TextureAtlas implements ITextureAtlas { const xChMid = xChLeft + this._config.deviceCellWidth / 2; switch (this._workAttributeData.extended.underlineStyle) { case UnderlineStyle.DOUBLE: + this._tmpCtx.moveTo(xChLeft, yTop - ySpace); + this._tmpCtx.lineTo(xChRight, yTop - ySpace); 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 @@ -1019,13 +1020,13 @@ function clearColor(imageData: ImageData, bg: IColor, fg: IColor, enableThreshol for (let offset = 0; offset < imageData.data.length; offset += 4) { // Check exact match if (imageData.data[offset] === r && - imageData.data[offset + 1] === g && - imageData.data[offset + 2] === b) { + imageData.data[offset + 1] === g && + imageData.data[offset + 2] === b) { imageData.data[offset + 3] = 0; } else { // Check the threshold based difference if (enableThresholdCheck && - (Math.abs(imageData.data[offset] - r) + + (Math.abs(imageData.data[offset] - r) + Math.abs(imageData.data[offset + 1] - g) + Math.abs(imageData.data[offset + 2] - b)) < threshold) { imageData.data[offset + 3] = 0; From b8bf35be177abd4c739b479f1e4e63b9f82f0c51 Mon Sep 17 00:00:00 2001 From: tisilent Date: Wed, 9 Aug 2023 11:53:30 +0800 Subject: [PATCH 2/5] Add restrictToCellHeight,Restore abnormal format --- .../xterm-addon-canvas/src/BaseRenderLayer.ts | 4 +-- addons/xterm-addon-webgl/src/GlyphRenderer.ts | 4 +-- src/browser/renderer/shared/TextureAtlas.ts | 36 +++++++++++-------- src/browser/renderer/shared/Types.d.ts | 4 +-- 4 files changed, 28 insertions(+), 20 deletions(-) diff --git a/addons/xterm-addon-canvas/src/BaseRenderLayer.ts b/addons/xterm-addon-canvas/src/BaseRenderLayer.ts index e22bc8d4..ff29bbf1 100644 --- a/addons/xterm-addon-canvas/src/BaseRenderLayer.ts +++ b/addons/xterm-addon-canvas/src/BaseRenderLayer.ts @@ -375,9 +375,9 @@ export abstract class BaseRenderLayer extends Disposable implements IRenderLayer let glyph: IRasterizedGlyph; if (chars && chars.length > 1) { - glyph = this._charAtlas.getRasterizedGlyphCombinedChar(chars, this._cellColorResolver.result.bg, this._cellColorResolver.result.fg, this._cellColorResolver.result.ext); + glyph = this._charAtlas.getRasterizedGlyphCombinedChar(chars, this._cellColorResolver.result.bg, this._cellColorResolver.result.fg, this._cellColorResolver.result.ext, true); } else { - glyph = this._charAtlas.getRasterizedGlyph(cell.getCode() || WHITESPACE_CELL_CODE, this._cellColorResolver.result.bg, this._cellColorResolver.result.fg, this._cellColorResolver.result.ext); + glyph = this._charAtlas.getRasterizedGlyph(cell.getCode() || WHITESPACE_CELL_CODE, this._cellColorResolver.result.bg, this._cellColorResolver.result.fg, this._cellColorResolver.result.ext, true); } if (!glyph.size.x || !glyph.size.y) { return; diff --git a/addons/xterm-addon-webgl/src/GlyphRenderer.ts b/addons/xterm-addon-webgl/src/GlyphRenderer.ts index 80a631e0..1c75b3b9 100644 --- a/addons/xterm-addon-webgl/src/GlyphRenderer.ts +++ b/addons/xterm-addon-webgl/src/GlyphRenderer.ts @@ -236,9 +236,9 @@ export class GlyphRenderer extends Disposable { // Get the glyph if (chars && chars.length > 1) { - $glyph = this._atlas.getRasterizedGlyphCombinedChar(chars, bg, fg, ext); + $glyph = this._atlas.getRasterizedGlyphCombinedChar(chars, bg, fg, ext, false); } else { - $glyph = this._atlas.getRasterizedGlyph(code, bg, fg, ext); + $glyph = this._atlas.getRasterizedGlyph(code, bg, fg, ext, false); } $leftCellPadding = Math.floor((this._dimensions.device.cell.width - this._dimensions.device.char.width) / 2); diff --git a/src/browser/renderer/shared/TextureAtlas.ts b/src/browser/renderer/shared/TextureAtlas.ts index b7ab64ee..e77e595d 100644 --- a/src/browser/renderer/shared/TextureAtlas.ts +++ b/src/browser/renderer/shared/TextureAtlas.ts @@ -242,12 +242,12 @@ export class TextureAtlas implements ITextureAtlas { } } - public getRasterizedGlyphCombinedChar(chars: string, bg: number, fg: number, ext: number): IRasterizedGlyph { - return this._getFromCacheMap(this._cacheMapCombined, chars, bg, fg, ext); + public getRasterizedGlyphCombinedChar(chars: string, bg: number, fg: number, ext: number, restrictToCellHeight: boolean): IRasterizedGlyph { + return this._getFromCacheMap(this._cacheMapCombined, chars, bg, fg, ext, restrictToCellHeight); } - public getRasterizedGlyph(code: number, bg: number, fg: number, ext: number): IRasterizedGlyph { - return this._getFromCacheMap(this._cacheMap, code, bg, fg, ext); + public getRasterizedGlyph(code: number, bg: number, fg: number, ext: number, restrictToCellHeight: boolean): IRasterizedGlyph { + return this._getFromCacheMap(this._cacheMap, code, bg, fg, ext, restrictToCellHeight); } /** @@ -258,11 +258,12 @@ export class TextureAtlas implements ITextureAtlas { key: string | number, bg: number, fg: number, - ext: number + ext: number, + restrictToCellHeight: boolean = false ): IRasterizedGlyph { $glyph = cacheMap.get(key, bg, fg, ext); if (!$glyph) { - $glyph = this._drawToCache(key, bg, fg, ext); + $glyph = this._drawToCache(key, bg, fg, ext, restrictToCellHeight); cacheMap.set(key, bg, fg, ext, $glyph); } return $glyph; @@ -414,7 +415,7 @@ export class TextureAtlas implements ITextureAtlas { return color; } - private _drawToCache(codeOrChars: number | string, bg: number, fg: number, ext: number): IRasterizedGlyph { + private _drawToCache(codeOrChars: number | string, bg: number, fg: number, ext: number, restrictToCellHeight: boolean = false): IRasterizedGlyph { const chars = typeof codeOrChars === 'number' ? String.fromCharCode(codeOrChars) : codeOrChars; // Uncomment for debugging @@ -543,10 +544,17 @@ export class TextureAtlas implements ITextureAtlas { const xChMid = xChLeft + this._config.deviceCellWidth / 2; switch (this._workAttributeData.extended.underlineStyle) { case UnderlineStyle.DOUBLE: - this._tmpCtx.moveTo(xChLeft, yTop - ySpace); - this._tmpCtx.lineTo(xChRight, yTop - ySpace); - this._tmpCtx.moveTo(xChLeft, yTop); - this._tmpCtx.lineTo(xChRight, yTop); + if (restrictToCellHeight) { + this._tmpCtx.moveTo(xChLeft, yTop - ySpace); + this._tmpCtx.lineTo(xChRight, yTop - ySpace); + this._tmpCtx.moveTo(xChLeft, yTop); + this._tmpCtx.lineTo(xChRight, yTop); + } else { + 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 @@ -1020,13 +1028,13 @@ function clearColor(imageData: ImageData, bg: IColor, fg: IColor, enableThreshol for (let offset = 0; offset < imageData.data.length; offset += 4) { // Check exact match if (imageData.data[offset] === r && - imageData.data[offset + 1] === g && - imageData.data[offset + 2] === b) { + imageData.data[offset + 1] === g && + imageData.data[offset + 2] === b) { imageData.data[offset + 3] = 0; } else { // Check the threshold based difference if (enableThresholdCheck && - (Math.abs(imageData.data[offset] - r) + + (Math.abs(imageData.data[offset] - r) + Math.abs(imageData.data[offset + 1] - g) + Math.abs(imageData.data[offset + 2] - b)) < threshold) { imageData.data[offset + 3] = 0; diff --git a/src/browser/renderer/shared/Types.d.ts b/src/browser/renderer/shared/Types.d.ts index cf4513c2..a7e55e72 100644 --- a/src/browser/renderer/shared/Types.d.ts +++ b/src/browser/renderer/shared/Types.d.ts @@ -107,8 +107,8 @@ export interface ITextureAtlas extends IDisposable { * Clear all glyphs from the texture atlas. */ clearTexture(): void; - getRasterizedGlyph(code: number, bg: number, fg: number, ext: number): IRasterizedGlyph; - getRasterizedGlyphCombinedChar(chars: string, bg: number, fg: number, ext: number): IRasterizedGlyph; + getRasterizedGlyph(code: number, bg: number, fg: number, ext: number, restrictToCellHeight: boolean): IRasterizedGlyph; + getRasterizedGlyphCombinedChar(chars: string, bg: number, fg: number, ext: number, restrictToCellHeight: boolean): IRasterizedGlyph; } /** From a84a95055d6bb297723dadfb029b4895567a77e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Wed, 9 Aug 2023 14:20:08 +0200 Subject: [PATCH 3/5] exclude decorated cells from merging --- src/browser/renderer/dom/DomRendererRowFactory.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/browser/renderer/dom/DomRendererRowFactory.ts b/src/browser/renderer/dom/DomRendererRowFactory.ts index 3f9765f0..186a1eb3 100644 --- a/src/browser/renderer/dom/DomRendererRowFactory.ts +++ b/src/browser/renderer/dom/DomRendererRowFactory.ts @@ -131,6 +131,11 @@ export class DomRendererRowFactory { const isCursorCell = isCursorRow && x === cursorX; const isLinkHover = hasHover && x >= linkStart && x <= linkEnd; + let isDecorated = false; + this._decorationService.forEachDecorationAtCell(x, row, undefined, d => { + isDecorated = true; + }); + // get chars to render for this cell let chars = cell.getChars() || WHITESPACE_CELL_CHAR; if (chars === ' ' && (cell.isUnderline() || cell.isOverline())) { @@ -160,6 +165,7 @@ export class DomRendererRowFactory { && spacing === oldSpacing && !isCursorCell && !isJoined + && !isDecorated ) { // no span alterations, thus only account chars skipping all code below text += chars; @@ -386,7 +392,7 @@ export class DomRendererRowFactory { } // exclude conditions for cell merging - never merge these - if (!isCursorCell && !isInSelection && !isJoined) { + if (!isCursorCell && !isInSelection && !isJoined && !isDecorated) { cellAmount++; } else { charElement.textContent = text; From 2b79b7b29f91ce98bd03bf692242ec79d8c3aff5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Thu, 10 Aug 2023 00:12:00 +0200 Subject: [PATCH 4/5] faster SortedList._search --- src/common/SortedList.ts | 45 +++++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/src/common/SortedList.ts b/src/common/SortedList.ts index 737b9acb..c5e7bc36 100644 --- a/src/common/SortedList.ts +++ b/src/common/SortedList.ts @@ -28,7 +28,7 @@ export class SortedList { this._array.push(value); return; } - i = this._search(this._getKey(value), 0, this._array.length - 1); + i = this._search(this._getKey(value)); this._array.splice(i, 0, value); } @@ -40,7 +40,7 @@ export class SortedList { if (key === undefined) { return false; } - i = this._search(key, 0, this._array.length - 1); + i = this._search(key); if (i === -1) { return false; } @@ -60,7 +60,7 @@ export class SortedList { if (this._array.length === 0) { return; } - i = this._search(key, 0, this._array.length - 1); + i = this._search(key); if (i < 0 || i >= this._array.length) { return; } @@ -76,7 +76,7 @@ export class SortedList { if (this._array.length === 0) { return; } - i = this._search(key, 0, this._array.length - 1); + i = this._search(key); if (i < 0 || i >= this._array.length) { return; } @@ -92,23 +92,26 @@ export class SortedList { return this._array.values(); } - private _search(key: number, min: number, max: number): number { - if (max < min) { - return min; + private _search(key: number): number { + let min = 0; + let max = this._array.length - 1; + while (max >= min) { + let mid = (min + max) >> 1; + const midKey = this._getKey(this._array[mid]); + if (midKey > key) { + max = mid - 1; + } else if (midKey < key) { + min = mid + 1; + } else { + // key in list, walk to lowest duplicate + while (mid > 0 && this._getKey(this._array[mid - 1]) === key) { + mid--; + } + return mid; + } } - let mid = Math.floor((min + max) / 2); - const midKey = this._getKey(this._array[mid]); - if (midKey > key) { - return this._search(key, min, mid - 1); - } - if (midKey < key) { - return this._search(key, mid + 1, max); - } - // Value found! Since keys can be duplicates, move the result index back to the lowest index - // that matches the key. - while (mid > 0 && this._getKey(this._array[mid - 1]) === key) { - mid--; - } - return mid; + // key not in list + // still return closest min (also used as insert position) + return min; } } From 62055522fb0590298da31c16d3eb0df6919d8bf8 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Wed, 9 Aug 2023 15:55:31 -0700 Subject: [PATCH 5/5] Fix API facade memory leaks We need to register/dispose any event created in the facades, otherwise embedders need to dispose of the listeners manually. Fixes #4645 --- src/browser/public/Terminal.ts | 18 ++++++++++-------- src/common/public/BufferNamespaceApi.ts | 6 ++++-- src/headless/public/Terminal.ts | 14 ++++++++------ 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/src/browser/public/Terminal.ts b/src/browser/public/Terminal.ts index 3c9b6d3a..7a94356b 100644 --- a/src/browser/public/Terminal.ts +++ b/src/browser/public/Terminal.ts @@ -3,7 +3,7 @@ * @license MIT */ -import { Terminal as ITerminalApi, IMarker, IDisposable, ILocalizableStrings, ITerminalAddon, IBufferNamespace as IBufferNamespaceApi, IParser, ILinkProvider, IUnicodeHandling, IModes, IDecorationOptions, IDecoration, IBufferElementProvider } from 'xterm'; +import { Terminal as ITerminalApi, IMarker, IDisposable, ILocalizableStrings, ITerminalAddon, IBufferNamespace as IBufferNamespaceApi, IParser, ILinkProvider, IUnicodeHandling, IModes, IDecorationOptions, IDecoration, IBufferElementProvider, ITerminalInitOnlyOptions } from 'xterm'; import { IBufferRange, ITerminal } from 'browser/Types'; import { Terminal as TerminalCore } from 'browser/Terminal'; import * as Strings from 'browser/LocalizableStrings'; @@ -13,22 +13,25 @@ import { UnicodeApi } from 'common/public/UnicodeApi'; import { AddonManager } from 'common/public/AddonManager'; import { BufferNamespaceApi } from 'common/public/BufferNamespaceApi'; import { ITerminalOptions } from 'common/Types'; +import { Disposable } from 'common/Lifecycle'; /** * The set of options that only have an effect when set in the Terminal constructor. */ const CONSTRUCTOR_ONLY_OPTIONS = ['cols', 'rows']; -export class Terminal implements ITerminalApi { +export class Terminal extends Disposable implements ITerminalApi { private _core: ITerminal; private _addonManager: AddonManager; private _parser: IParser | undefined; private _buffer: BufferNamespaceApi | undefined; private _publicOptions: Required; - constructor(options?: ITerminalOptions) { - this._core = new TerminalCore(options); - this._addonManager = new AddonManager(); + constructor(options?: ITerminalOptions & ITerminalInitOnlyOptions) { + super(); + + this._core = this.register(new TerminalCore(options)); + this._addonManager = this.register(new AddonManager()); this._publicOptions = { ... this._core.options }; const getter = (propName: string): any => { @@ -92,7 +95,7 @@ export class Terminal implements ITerminalApi { public get cols(): number { return this._core.cols; } public get buffer(): IBufferNamespaceApi { if (!this._buffer) { - this._buffer = new BufferNamespaceApi(this._core); + this._buffer = this.register(new BufferNamespaceApi(this._core)); } return this._buffer; } @@ -189,8 +192,7 @@ export class Terminal implements ITerminalApi { this._core.selectLines(start, end); } public dispose(): void { - this._addonManager.dispose(); - this._core.dispose(); + super.dispose(); } public scrollLines(amount: number): void { this._verifyIntegers(amount); diff --git a/src/common/public/BufferNamespaceApi.ts b/src/common/public/BufferNamespaceApi.ts index 9e49ce2b..aeaa4ac8 100644 --- a/src/common/public/BufferNamespaceApi.ts +++ b/src/common/public/BufferNamespaceApi.ts @@ -7,15 +7,17 @@ import { IBuffer as IBufferApi, IBufferNamespace as IBufferNamespaceApi } from ' import { BufferApiView } from 'common/public/BufferApiView'; import { EventEmitter } from 'common/EventEmitter'; import { ICoreTerminal } from 'common/Types'; +import { Disposable } from 'common/Lifecycle'; -export class BufferNamespaceApi implements IBufferNamespaceApi { +export class BufferNamespaceApi extends Disposable implements IBufferNamespaceApi { private _normal: BufferApiView; private _alternate: BufferApiView; - private readonly _onBufferChange = new EventEmitter(); + private readonly _onBufferChange = this.register(new EventEmitter()); public readonly onBufferChange = this._onBufferChange.event; constructor(private _core: ICoreTerminal) { + super(); this._normal = new BufferApiView(this._core.buffers.normal, 'normal'); this._alternate = new BufferApiView(this._core.buffers.alt, 'alternate'); this._core.buffers.onBufferActivate(() => this._onBufferChange.fire(this.active)); diff --git a/src/headless/public/Terminal.ts b/src/headless/public/Terminal.ts index d4360c7b..5eeeb361 100644 --- a/src/headless/public/Terminal.ts +++ b/src/headless/public/Terminal.ts @@ -11,12 +11,13 @@ import { IBufferNamespace as IBufferNamespaceApi, IMarker, IModes, IParser, ITer import { Terminal as TerminalCore } from 'headless/Terminal'; import { AddonManager } from 'common/public/AddonManager'; import { ITerminalOptions } from 'common/Types'; +import { Disposable } from 'common/Lifecycle'; /** * The set of options that only have an effect when set in the Terminal constructor. */ const CONSTRUCTOR_ONLY_OPTIONS = ['cols', 'rows']; -export class Terminal implements ITerminalApi { +export class Terminal extends Disposable implements ITerminalApi { private _core: TerminalCore; private _addonManager: AddonManager; private _parser: IParser | undefined; @@ -24,8 +25,10 @@ export class Terminal implements ITerminalApi { private _publicOptions: Required; constructor(options?: ITerminalOptions & ITerminalInitOnlyOptions) { - this._core = new TerminalCore(options); - this._addonManager = new AddonManager(); + super(); + + this._core = this.register(new TerminalCore(options)); + this._addonManager = this.register(new AddonManager()); this._publicOptions = { ... this._core.options }; const getter = (propName: string): any => { @@ -94,7 +97,7 @@ export class Terminal implements ITerminalApi { public get buffer(): IBufferNamespaceApi { this._checkProposedApi(); if (!this._buffer) { - this._buffer = new BufferNamespaceApi(this._core); + this._buffer = this.register(new BufferNamespaceApi(this._core)); } return this._buffer; } @@ -144,8 +147,7 @@ export class Terminal implements ITerminalApi { return this.registerMarker(cursorYOffset); } public dispose(): void { - this._addonManager.dispose(); - this._core.dispose(); + super.dispose(); } public scrollLines(amount: number): void { this._verifyIntegers(amount);