Skip to content

Commit

Permalink
Merge pull request #4733 from Tyriar/mutable
Browse files Browse the repository at this point in the history 
Adopt MutableDisposable for most usages of pattern `<var>?.dispose(); <var> = undefined`
  • Loading branch information
@Tyriar
Tyriar committed Aug 27, 2023
2 parents a0d0c92 + 38c6af9 commit abae7b6
Show file tree
Hide file tree
Showing 10 changed files with 114 additions and 144 deletions.
20 changes: 9 additions & 11 deletions addons/xterm-addon-canvas/src/BaseRenderLayer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,24 @@
* @license MIT
*/

import { ReadonlyColorSet } from 'browser/Types';
import { CellColorResolver } from 'browser/renderer/shared/CellColorResolver';
import { acquireTextureAtlas } from 'browser/renderer/shared/CharAtlasCache';
import { TEXT_BASELINE } from 'browser/renderer/shared/Constants';
import { tryDrawCustomChar } from 'browser/renderer/shared/CustomGlyphs';
import { throwIfFalsy } from 'browser/renderer/shared/RendererUtils';
import { IRasterizedGlyph, IRenderDimensions, ISelectionRenderModel, ITextureAtlas } from 'browser/renderer/shared/Types';
import { createSelectionRenderModel } from 'browser/renderer/shared/SelectionRenderModel';
import { IRasterizedGlyph, IRenderDimensions, ISelectionRenderModel, ITextureAtlas } from 'browser/renderer/shared/Types';
import { ICoreBrowserService, IThemeService } from 'browser/services/Services';
import { ReadonlyColorSet } from 'browser/Types';
import { EventEmitter, forwardEvent } from 'common/EventEmitter';
import { Disposable, MutableDisposable, toDisposable } from 'common/Lifecycle';
import { isSafari } from 'common/Platform';
import { ICellData } from 'common/Types';
import { CellData } from 'common/buffer/CellData';
import { WHITESPACE_CELL_CODE } from 'common/buffer/Constants';
import { IBufferService, IDecorationService, IOptionsService } from 'common/services/Services';
import { ICellData, IDisposable } from 'common/Types';
import { Terminal } from 'xterm';
import { IRenderLayer } from './Types';
import { CellColorResolver } from 'browser/renderer/shared/CellColorResolver';
import { Disposable, toDisposable } from 'common/Lifecycle';
import { isSafari } from 'common/Platform';
import { EventEmitter, forwardEvent } from 'common/EventEmitter';

export abstract class BaseRenderLayer extends Disposable implements IRenderLayer {
private _canvas: HTMLCanvasElement;
Expand All @@ -37,7 +37,7 @@ export abstract class BaseRenderLayer extends Disposable implements IRenderLayer
private _bitmapGenerator: (BitmapGenerator | undefined)[] = [];

protected _charAtlas!: ITextureAtlas;
private _charAtlasDisposable?: IDisposable;
protected _charAtlasDisposable = this.register(new MutableDisposable());

public get canvas(): HTMLCanvasElement { return this._canvas; }
public get cacheCanvas(): HTMLCanvasElement { return this._charAtlas?.pages[0].canvas!; }
Expand Down Expand Up @@ -74,7 +74,6 @@ export abstract class BaseRenderLayer extends Disposable implements IRenderLayer

this.register(toDisposable(() => {
this._canvas.remove();
this._charAtlas?.dispose();
}));
}

Expand Down Expand Up @@ -122,9 +121,8 @@ export abstract class BaseRenderLayer extends Disposable implements IRenderLayer
if (this._deviceCharWidth <= 0 && this._deviceCharHeight <= 0) {
return;
}
this._charAtlasDisposable?.dispose();
this._charAtlas = acquireTextureAtlas(this._terminal, this._optionsService.rawOptions, colorSet, this._deviceCellWidth, this._deviceCellHeight, this._deviceCharWidth, this._deviceCharHeight, this._coreBrowserService.dpr);
this._charAtlasDisposable = forwardEvent(this._charAtlas.onAddTextureAtlasCanvas, this._onAddTextureAtlasCanvas);
this._charAtlasDisposable.value = forwardEvent(this._charAtlas.onAddTextureAtlasCanvas, this._onAddTextureAtlasCanvas);
this._charAtlas.warmUp();
for (let i = 0; i < this._charAtlas.pages.length; i++) {
this._bitmapGenerator[i] = new BitmapGenerator(this._charAtlas.pages[i].canvas);
Expand Down
29 changes: 12 additions & 17 deletions addons/xterm-addon-canvas/src/CursorRenderLayer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { CursorBlinkStateManager } from 'browser/renderer/shared/CursorBlinkStat
import { IRenderDimensions, IRequestRedrawEvent } from 'browser/renderer/shared/Types';
import { ICoreBrowserService, IThemeService } from 'browser/services/Services';
import { IEventEmitter } from 'common/EventEmitter';
import { toDisposable } from 'common/Lifecycle';
import { MutableDisposable } from 'common/Lifecycle';
import { isFirefox } from 'common/Platform';
import { ICellData } from 'common/Types';
import { CellData } from 'common/buffer/CellData';
Expand All @@ -26,7 +26,7 @@ interface ICursorState {
export class CursorRenderLayer extends BaseRenderLayer {
private _state: ICursorState;
private _cursorRenderers: {[key: string]: (x: number, y: number, cell: ICellData) => void};
private _cursorBlinkStateManager: CursorBlinkStateManager | undefined;
private _cursorBlinkStateManager: MutableDisposable<CursorBlinkStateManager> = this.register(new MutableDisposable());
private _cell: ICellData = new CellData();

constructor(
Expand Down Expand Up @@ -57,10 +57,6 @@ export class CursorRenderLayer extends BaseRenderLayer {
};
this.register(optionsService.onOptionChange(() => this._handleOptionsChanged()));
this._handleOptionsChanged();
this.register(toDisposable(() => {
this._cursorBlinkStateManager?.dispose();
this._cursorBlinkStateManager = undefined;
}));
}

public resize(dim: IRenderDimensions): void {
Expand All @@ -77,43 +73,42 @@ export class CursorRenderLayer extends BaseRenderLayer {

public reset(): void {
this._clearCursor();
this._cursorBlinkStateManager?.restartBlinkAnimation();
this._cursorBlinkStateManager.value?.restartBlinkAnimation();
this._handleOptionsChanged();
}

public handleBlur(): void {
this._cursorBlinkStateManager?.pause();
this._cursorBlinkStateManager.value?.pause();
this._onRequestRedraw.fire({ start: this._bufferService.buffer.y, end: this._bufferService.buffer.y });
}

public handleFocus(): void {
this._cursorBlinkStateManager?.resume();
this._cursorBlinkStateManager.value?.resume();
this._onRequestRedraw.fire({ start: this._bufferService.buffer.y, end: this._bufferService.buffer.y });
}

private _handleOptionsChanged(): void {
if (this._optionsService.rawOptions.cursorBlink) {
if (!this._cursorBlinkStateManager) {
this._cursorBlinkStateManager = new CursorBlinkStateManager(() => this._render(true), this._coreBrowserService);
if (!this._cursorBlinkStateManager.value) {
this._cursorBlinkStateManager.value = new CursorBlinkStateManager(() => this._render(true), this._coreBrowserService);
}
} else {
this._cursorBlinkStateManager?.dispose();
this._cursorBlinkStateManager = undefined;
this._cursorBlinkStateManager.clear();
}
// Request a refresh from the terminal as management of rendering is being
// moved back to the terminal
this._onRequestRedraw.fire({ start: this._bufferService.buffer.y, end: this._bufferService.buffer.y });
}

public handleCursorMove(): void {
this._cursorBlinkStateManager?.restartBlinkAnimation();
this._cursorBlinkStateManager.value?.restartBlinkAnimation();
}

public handleGridChanged(startRow: number, endRow: number): void {
if (!this._cursorBlinkStateManager || this._cursorBlinkStateManager.isPaused) {
if (!this._cursorBlinkStateManager.value || this._cursorBlinkStateManager.value.isPaused) {
this._render(false);
} else {
this._cursorBlinkStateManager.restartBlinkAnimation();
this._cursorBlinkStateManager.value.restartBlinkAnimation();
}
}

Expand Down Expand Up @@ -159,7 +154,7 @@ export class CursorRenderLayer extends BaseRenderLayer {
}

// Don't draw the cursor if it's blinking
if (this._cursorBlinkStateManager && !this._cursorBlinkStateManager.isCursorVisible) {
if (this._cursorBlinkStateManager.value && !this._cursorBlinkStateManager.value.isCursorVisible) {
this._clearCursor();
return;
}
Expand Down
45 changes: 22 additions & 23 deletions addons/xterm-addon-image/src/ImageRenderer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import { toRGBA8888 } from 'sixel/lib/Colors';
import { IDisposable } from 'xterm';
import { ICellSize, ITerminalExt, IImageSpec, IRenderDimensions, IRenderService } from './Types';
import { Disposable, MutableDisposable, toDisposable } from 'common/Lifecycle';


const PLACEHOLDER_LENGTH = 4096;
Expand All @@ -17,12 +18,12 @@ const PLACEHOLDER_HEIGHT = 24;
* - add canvas layer to DOM (browser only for now)
* - draw image tiles onRender
*/
export class ImageRenderer implements IDisposable {
export class ImageRenderer extends Disposable implements IDisposable {
public canvas: HTMLCanvasElement | undefined;
private _ctx: CanvasRenderingContext2D | null | undefined;
private _placeholder: HTMLCanvasElement | undefined;
private _placeholderBitmap: ImageBitmap | undefined;
private _optionsRefresh: IDisposable | undefined;
private _optionsRefresh = this.register(new MutableDisposable());
private _oldOpen: ((parent: HTMLElement) => void) | undefined;
private _renderService: IRenderService | undefined;
private _oldSetRenderer: ((renderer: any) => void) | undefined;
Expand Down Expand Up @@ -68,6 +69,7 @@ export class ImageRenderer implements IDisposable {


constructor(private _terminal: ITerminalExt) {
super();
this._oldOpen = this._terminal._core.open;
this._terminal._core.open = (parent: HTMLElement): void => {
this._oldOpen?.call(this._terminal._core, parent);
Expand All @@ -77,32 +79,29 @@ export class ImageRenderer implements IDisposable {
this._open();
}
// hack to spot fontSize changes
this._optionsRefresh = this._terminal._core.optionsService.onOptionChange(option => {
this._optionsRefresh.value = this._terminal._core.optionsService.onOptionChange(option => {
if (option === 'fontSize') {
this.rescaleCanvas();
this._renderService?.refreshRows(0, this._terminal.rows);
}
});
}


public dispose(): void {
this._optionsRefresh?.dispose();
this.removeLayerFromDom();
if (this._terminal._core && this._oldOpen) {
this._terminal._core.open = this._oldOpen;
this._oldOpen = undefined;
}
if (this._renderService && this._oldSetRenderer) {
this._renderService.setRenderer = this._oldSetRenderer;
this._oldSetRenderer = undefined;
}
this._renderService = undefined;
this.canvas = undefined;
this._ctx = undefined;
this._placeholderBitmap?.close();
this._placeholderBitmap = undefined;
this._placeholder = undefined;
this.register(toDisposable(() => {
this.removeLayerFromDom();
if (this._terminal._core && this._oldOpen) {
this._terminal._core.open = this._oldOpen;
this._oldOpen = undefined;
}
if (this._renderService && this._oldSetRenderer) {
this._renderService.setRenderer = this._oldSetRenderer;
this._oldSetRenderer = undefined;
}
this._renderService = undefined;
this.canvas = undefined;
this._ctx = undefined;
this._placeholderBitmap?.close();
this._placeholderBitmap = undefined;
this._placeholder = undefined;
}));
}

/**
Expand Down
19 changes: 7 additions & 12 deletions addons/xterm-addon-search/src/SearchAddon.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -66,7 +66,7 @@ export class SearchAddon extends Disposable implements ITerminalAddon {
private _cachedSearchTerm: string | undefined;
private _highlightedLines: Set<number> = new Set();
private _highlightDecorations: IHighlight[] = [];
private _selectedDecoration: IHighlight | undefined;
private _selectedDecoration: MutableDisposable<IHighlight> = this.register(new MutableDisposable());
private _highlightLimit: number;
private _lastSearchOptions: ISearchOptions | undefined;
private _highlightTimeout: number | undefined;
Expand Down Expand Up @@ -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();
Expand All @@ -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.
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand All @@ -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(); } };
}
}
}
Expand Down
Loading

0 comments on commit abae7b6

Please sign in to comment.