From 57db72d287f76632f0b4464b7668e7fe317fa6d2 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 27 Aug 2023 06:55:16 -0700 Subject: [PATCH] More adoption of MutableDisposable --- addons/xterm-addon-search/src/SearchAddon.ts | 19 +++----- addons/xterm-addon-webgl/src/WebglRenderer.ts | 45 +++++++++---------- .../src/renderLayer/BaseRenderLayer.ts | 1 - src/browser/Terminal.ts | 13 +++--- 4 files changed, 34 insertions(+), 44 deletions(-) diff --git a/addons/xterm-addon-search/src/SearchAddon.ts b/addons/xterm-addon-search/src/SearchAddon.ts index cbde1a1b..10fc1ecc 100644 --- a/addons/xterm-addon-search/src/SearchAddon.ts +++ b/addons/xterm-addon-search/src/SearchAddon.ts @@ -5,7 +5,7 @@ import { Terminal, IDisposable, ITerminalAddon, IDecoration } from 'xterm'; import { EventEmitter } from 'common/EventEmitter'; -import { Disposable, toDisposable, disposeArray } from 'common/Lifecycle'; +import { Disposable, toDisposable, disposeArray, MutableDisposable } from 'common/Lifecycle'; export interface ISearchOptions { regex?: boolean; @@ -66,7 +66,7 @@ export class SearchAddon extends Disposable implements ITerminalAddon { private _cachedSearchTerm: string | undefined; private _highlightedLines: Set = new Set(); private _highlightDecorations: IHighlight[] = []; - private _selectedDecoration: IHighlight | undefined; + private _selectedDecoration: MutableDisposable = this.register(new MutableDisposable()); private _highlightLimit: number; private _lastSearchOptions: ISearchOptions | undefined; private _highlightTimeout: number | undefined; @@ -110,7 +110,7 @@ export class SearchAddon extends Disposable implements ITerminalAddon { } public clearDecorations(retainCachedSearchTerm?: boolean): void { - this.clearActiveDecoration(); + this._selectedDecoration.clear(); disposeArray(this._highlightDecorations); this._highlightDecorations = []; this._highlightedLines.clear(); @@ -119,11 +119,6 @@ export class SearchAddon extends Disposable implements ITerminalAddon { } } - public clearActiveDecoration(): void { - this._selectedDecoration?.dispose(); - this._selectedDecoration = undefined; - } - /** * Find the next instance of the term, then scroll to and select it. If it * doesn't exist, do nothing. @@ -320,8 +315,8 @@ export class SearchAddon extends Disposable implements ITerminalAddon { private _fireResults(searchOptions?: ISearchOptions): void { if (searchOptions?.decorations) { let resultIndex = -1; - if (this._selectedDecoration) { - const selectedMatch = this._selectedDecoration.match; + if (this._selectedDecoration.value) { + const selectedMatch = this._selectedDecoration.value.match; for (let i = 0; i < this._highlightDecorations.length; i++) { const match = this._highlightDecorations[i].match; if (match.row === selectedMatch.row && match.col === selectedMatch.col && match.size === selectedMatch.size) { @@ -642,7 +637,7 @@ export class SearchAddon extends Disposable implements ITerminalAddon { */ private _selectResult(result: ISearchResult | undefined, options?: ISearchDecorationOptions, noScroll?: boolean): boolean { const terminal = this._terminal!; - this.clearActiveDecoration(); + this._selectedDecoration.clear(); if (!result) { terminal.clearSelection(); return false; @@ -666,7 +661,7 @@ export class SearchAddon extends Disposable implements ITerminalAddon { disposables.push(marker); disposables.push(decoration.onRender((e) => this._applyStyles(e, options.activeMatchBorder, true))); disposables.push(decoration.onDispose(() => disposeArray(disposables))); - this._selectedDecoration = { decoration, match: result, dispose() { decoration.dispose(); } }; + this._selectedDecoration.value = { decoration, match: result, dispose() { decoration.dispose(); } }; } } } diff --git a/addons/xterm-addon-webgl/src/WebglRenderer.ts b/addons/xterm-addon-webgl/src/WebglRenderer.ts index 20db5faf..c57565a3 100644 --- a/addons/xterm-addon-webgl/src/WebglRenderer.ts +++ b/addons/xterm-addon-webgl/src/WebglRenderer.ts @@ -41,8 +41,8 @@ export class WebglRenderer extends Disposable implements IRenderer { private _canvas: HTMLCanvasElement; private _gl: IWebGL2RenderingContext; - private _rectangleRenderer?: RectangleRenderer; - private _glyphRenderer?: GlyphRenderer; + private _rectangleRenderer: MutableDisposable = this.register(new MutableDisposable()); + private _glyphRenderer: MutableDisposable = this.register(new MutableDisposable()); public readonly dimensions: IRenderDimensions; @@ -128,7 +128,7 @@ export class WebglRenderer extends Disposable implements IRenderer { this._core.screenElement!.appendChild(this._canvas); - [this._rectangleRenderer, this._glyphRenderer] = this._initializeWebGLState(); + [this._rectangleRenderer.value, this._glyphRenderer.value] = this._initializeWebGLState(); this._isAttached = this._coreBrowserService.window.document.body.contains(this._core.screenElement!); @@ -182,10 +182,10 @@ export class WebglRenderer extends Disposable implements IRenderer { this._core.screenElement!.style.width = `${this.dimensions.css.canvas.width}px`; this._core.screenElement!.style.height = `${this.dimensions.css.canvas.height}px`; - this._rectangleRenderer?.setDimensions(this.dimensions); - this._rectangleRenderer?.handleResize(); - this._glyphRenderer?.setDimensions(this.dimensions); - this._glyphRenderer?.handleResize(); + this._rectangleRenderer.value?.setDimensions(this.dimensions); + this._rectangleRenderer.value?.handleResize(); + this._glyphRenderer.value?.setDimensions(this.dimensions); + this._glyphRenderer.value?.handleResize(); this._refreshCharAtlas(); @@ -241,17 +241,14 @@ export class WebglRenderer extends Disposable implements IRenderer { * Initializes members dependent on WebGL context state. */ private _initializeWebGLState(): [RectangleRenderer, GlyphRenderer] { - // Dispose any previous rectangle and glyph renderers before creating new ones. - this._rectangleRenderer?.dispose(); - this._glyphRenderer?.dispose(); - - this._rectangleRenderer = this.register(new RectangleRenderer(this._terminal, this._gl, this.dimensions, this._themeService)); - this._glyphRenderer = this.register(new GlyphRenderer(this._terminal, this._gl, this.dimensions)); + this._rectangleRenderer.value = new RectangleRenderer(this._terminal, this._gl, this.dimensions, this._themeService); + this._glyphRenderer.value = new GlyphRenderer(this._terminal, this._gl, this.dimensions); // Update dimensions and acquire char atlas this.handleCharSizeChanged(); - return [this._rectangleRenderer, this._glyphRenderer]; + return [this._rectangleRenderer.value, this._glyphRenderer.value + ]; } /** @@ -284,7 +281,7 @@ export class WebglRenderer extends Disposable implements IRenderer { } this._charAtlas = atlas; this._charAtlas.warmUp(); - this._glyphRenderer?.setAtlas(this._charAtlas); + this._glyphRenderer.value?.setAtlas(this._charAtlas); } /** @@ -340,14 +337,14 @@ export class WebglRenderer extends Disposable implements IRenderer { l.handleGridChanged(this._terminal, start, end); } - if (!this._glyphRenderer || !this._rectangleRenderer) { + if (!this._glyphRenderer.value || !this._rectangleRenderer.value) { return; } // Tell renderer the frame is beginning // upon a model clear also refresh the full viewport model // (also triggered by an atlas page merge, part of #4480) - if (this._glyphRenderer.beginFrame()) { + if (this._glyphRenderer.value.beginFrame()) { this._clearModel(true); this._updateModel(0, this._terminal.rows - 1); } else { @@ -356,10 +353,10 @@ export class WebglRenderer extends Disposable implements IRenderer { } // Render - this._rectangleRenderer?.renderBackgrounds(); - this._glyphRenderer?.render(this._model); + this._rectangleRenderer.value.renderBackgrounds(); + this._glyphRenderer.value.render(this._model); if (!this._cursorBlinkStateManager.value || this._cursorBlinkStateManager.value.isCursorVisible) { - this._rectangleRenderer?.renderCursor(); + this._rectangleRenderer.value.renderCursor(); } } @@ -502,7 +499,7 @@ export class WebglRenderer extends Disposable implements IRenderer { this._model.cells[i + RENDER_MODEL_FG_OFFSET] = this._cellColorResolver.result.fg; this._model.cells[i + RENDER_MODEL_EXT_OFFSET] = this._cellColorResolver.result.ext; - this._glyphRenderer!.updateCell(x, y, code, this._cellColorResolver.result.bg, this._cellColorResolver.result.fg, this._cellColorResolver.result.ext, chars, lastBg); + this._glyphRenderer.value!.updateCell(x, y, code, this._cellColorResolver.result.bg, this._cellColorResolver.result.fg, this._cellColorResolver.result.ext, chars, lastBg); if (isJoined) { // Restore work cell @@ -511,7 +508,7 @@ export class WebglRenderer extends Disposable implements IRenderer { // Null out non-first cells for (x++; x < lastCharX; x++) { 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._glyphRenderer.value!.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._cellColorResolver.result.bg; this._model.cells[j + RENDER_MODEL_FG_OFFSET] = this._cellColorResolver.result.fg; @@ -521,9 +518,9 @@ export class WebglRenderer extends Disposable implements IRenderer { } } if (modelUpdated) { - this._rectangleRenderer!.updateBackgrounds(this._model); + this._rectangleRenderer.value!.updateBackgrounds(this._model); } - this._rectangleRenderer!.updateCursor(this._model); + this._rectangleRenderer.value!.updateCursor(this._model); } /** diff --git a/addons/xterm-addon-webgl/src/renderLayer/BaseRenderLayer.ts b/addons/xterm-addon-webgl/src/renderLayer/BaseRenderLayer.ts index f5449c4d..3aaac435 100644 --- a/addons/xterm-addon-webgl/src/renderLayer/BaseRenderLayer.ts +++ b/addons/xterm-addon-webgl/src/renderLayer/BaseRenderLayer.ts @@ -49,7 +49,6 @@ export abstract class BaseRenderLayer extends Disposable implements IRenderLayer })); this.register(toDisposable(() => { this._canvas.remove(); - this._charAtlas?.dispose(); })); } diff --git a/src/browser/Terminal.ts b/src/browser/Terminal.ts index 24dace2f..a092e1bc 100644 --- a/src/browser/Terminal.ts +++ b/src/browser/Terminal.ts @@ -44,7 +44,7 @@ import { ThemeService } from 'browser/services/ThemeService'; import { color, rgba } from 'common/Color'; import { CoreTerminal } from 'common/CoreTerminal'; import { EventEmitter, IEvent, forwardEvent } from 'common/EventEmitter'; -import { toDisposable } from 'common/Lifecycle'; +import { MutableDisposable, toDisposable } from 'common/Lifecycle'; import * as Browser from 'common/Platform'; import { ColorRequestType, CoreMouseAction, CoreMouseButton, CoreMouseEventType, IColorEvent, ITerminalOptions, KeyboardResultType, ScrollSource, SpecialColorIndex } from 'common/Types'; import { DEFAULT_ATTR_DATA } from 'common/buffer/BufferLine'; @@ -118,7 +118,7 @@ export class Terminal extends CoreTerminal implements ITerminal { public linkifier2: ILinkifier2; public viewport: IViewport | undefined; private _compositionHelper: ICompositionHelper | undefined; - private _accessibilityManager: AccessibilityManager | undefined; + private _accessibilityManager: MutableDisposable = this.register(new MutableDisposable()); private readonly _onCursorMove = this.register(new EventEmitter()); public readonly onCursorMove = this._onCursorMove.event; @@ -252,12 +252,11 @@ export class Terminal extends CoreTerminal implements ITerminal { private _handleScreenReaderModeOptionChange(value: boolean): void { if (value) { - if (!this._accessibilityManager && this._renderService) { - this._accessibilityManager = this._instantiationService.createInstance(AccessibilityManager, this); + if (!this._accessibilityManager.value && this._renderService) { + this._accessibilityManager.value = this._instantiationService.createInstance(AccessibilityManager, this); } } else { - this._accessibilityManager?.dispose(); - this._accessibilityManager = undefined; + this._accessibilityManager.clear(); } } @@ -535,7 +534,7 @@ export class Terminal extends CoreTerminal implements ITerminal { if (this.options.screenReaderMode) { // Note that this must be done *after* the renderer is created in order to // ensure the correct order of the dprchange event - this._accessibilityManager = this._instantiationService.createInstance(AccessibilityManager, this); + this._accessibilityManager.value = this._instantiationService.createInstance(AccessibilityManager, this); } this.register(this.optionsService.onSpecificOptionChange('screenReaderMode', e => this._handleScreenReaderModeOptionChange(e)));