From 38f2e7a81dd90f755dedd98fc299a1b54002c64f Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 4 Jul 2019 08:28:21 -0700 Subject: [PATCH 1/2] Simplify webgl atlas by removing BaseCharAtlas --- .../src/atlas/BaseCharAtlas.ts | 56 --------- .../src/atlas/CharAtlasCache.ts | 5 +- .../src/atlas/WebglCharAtlas.ts | 26 ++-- .../src/renderLayer/BaseRenderLayer.ts | 113 +----------------- 4 files changed, 16 insertions(+), 184 deletions(-) delete mode 100644 addons/xterm-addon-webgl/src/atlas/BaseCharAtlas.ts diff --git a/addons/xterm-addon-webgl/src/atlas/BaseCharAtlas.ts b/addons/xterm-addon-webgl/src/atlas/BaseCharAtlas.ts deleted file mode 100644 index 470736f1..00000000 --- a/addons/xterm-addon-webgl/src/atlas/BaseCharAtlas.ts +++ /dev/null @@ -1,56 +0,0 @@ -/** - * Copyright (c) 2017 The xterm.js authors. All rights reserved. - * @license MIT - */ - -import { IGlyphIdentifier } from './Types'; -import { IDisposable } from 'xterm'; - -export abstract class BaseCharAtlas implements IDisposable { - private _didWarmUp: boolean = false; - - public dispose(): void { } - - /** - * Perform any work needed to warm the cache before it can be used. May be called multiple times. - * Implement _doWarmUp instead if you only want to get called once. - */ - public warmUp(): void { - if (!this._didWarmUp) { - this._doWarmUp(); - this._didWarmUp = true; - } - } - - /** - * Perform any work needed to warm the cache before it can be used. Used by the default - * implementation of warmUp(), and will only be called once. - */ - protected _doWarmUp(): void { } - - /** - * Called when we start drawing a new frame. - * - * TODO: We rely on this getting called by TextRenderLayer. This should really be called by - * Renderer instead, but we need to make Renderer the source-of-truth for the char atlas, instead - * of BaseRenderLayer. - */ - public beginFrame(): void { } - - /** - * May be called before warmUp finishes, however it is okay for the implementation to - * do nothing and return false in that case. - * - * @param ctx Where to draw the character onto. - * @param glyph Information about what to draw - * @param x The position on the context to start drawing at - * @param y The position on the context to start drawing at - * @returns The success state. True if we drew the character. - */ - public abstract draw( - ctx: CanvasRenderingContext2D, - glyph: IGlyphIdentifier, - x: number, - y: number - ): boolean; -} diff --git a/addons/xterm-addon-webgl/src/atlas/CharAtlasCache.ts b/addons/xterm-addon-webgl/src/atlas/CharAtlasCache.ts index 647b96b4..5046006f 100644 --- a/addons/xterm-addon-webgl/src/atlas/CharAtlasCache.ts +++ b/addons/xterm-addon-webgl/src/atlas/CharAtlasCache.ts @@ -4,14 +4,13 @@ */ import { generateConfig, configEquals } from './CharAtlasUtils'; -import { BaseCharAtlas } from './BaseCharAtlas'; import { WebglCharAtlas } from './WebglCharAtlas'; import { ICharAtlasConfig } from './Types'; import { Terminal } from 'xterm'; import { IColorSet } from 'browser/Types'; interface ICharAtlasCacheEntry { - atlas: BaseCharAtlas; + atlas: WebglCharAtlas; config: ICharAtlasConfig; // N.B. This implementation potentially holds onto copies of the terminal forever, so // this may cause memory leaks. @@ -31,7 +30,7 @@ export function acquireCharAtlas( colors: IColorSet, scaledCharWidth: number, scaledCharHeight: number -): BaseCharAtlas { +): WebglCharAtlas { const newConfig = generateConfig(scaledCharWidth, scaledCharHeight, terminal, colors); // Check to see if the terminal already owns this config diff --git a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts index 6a941d55..07a97fd0 100644 --- a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts +++ b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts @@ -3,14 +3,14 @@ * @license MIT */ -import { IGlyphIdentifier, ICharAtlasConfig } from './Types'; +import { ICharAtlasConfig } from './Types'; import { DIM_OPACITY, INVERTED_DEFAULT_COLOR } from 'browser/renderer/atlas/Constants'; -import { BaseCharAtlas } from './BaseCharAtlas'; import { IRasterizedGlyph, IBoundingBox, IRasterizedGlyphSet } from '../Types'; import { DEFAULT_COLOR, DEFAULT_ATTR } from 'common/buffer/Constants'; import { is256Color } from './CharAtlasUtils'; import { IColor } from 'browser/Types'; import { FLAGS } from '../Constants'; +import { IDisposable } from 'xterm'; // In practice we're probably never going to exhaust a texture this large. For debugging purposes, // however, it can be useful to set this to a really tiny value, to verify that LRU eviction works. @@ -42,7 +42,9 @@ const NULL_RASTERIZED_GLYPH: IRasterizedGlyph = { const TMP_CANVAS_GLYPH_PADDING = 2; -export class WebglCharAtlas extends BaseCharAtlas { +export class WebglCharAtlas implements IDisposable { + private _didWarmUp: boolean = false; + private _cacheMap: { [code: number]: IRasterizedGlyphSet } = {}; private _cacheMapCombined: { [chars: string]: IRasterizedGlyphSet } = {}; @@ -67,8 +69,6 @@ export class WebglCharAtlas extends BaseCharAtlas { private _workBoundingBox: IBoundingBox = { top: 0, left: 0, bottom: 0, right: 0 }; constructor(document: Document, private _config: ICharAtlasConfig) { - super(); - this.cacheCanvas = document.createElement('canvas'); this.cacheCanvas.width = TEXTURE_WIDTH; this.cacheCanvas.height = TEXTURE_HEIGHT; @@ -92,6 +92,13 @@ export class WebglCharAtlas extends BaseCharAtlas { } } + public warmUp(): void { + if (!this._didWarmUp) { + this._doWarmUp(); + this._didWarmUp = true; + } + } + protected _doWarmUp(): void { // Pre-fill with ASCII 33-126 for (let i = 33; i < 126; i++) { @@ -146,15 +153,6 @@ export class WebglCharAtlas extends BaseCharAtlas { return rasterizedGlyph; } - public draw( - ctx: CanvasRenderingContext2D, - glyph: IGlyphIdentifier, - x: number, - y: number - ): boolean { - throw new Error('WebglCharAtlas is only compatible with the webgl renderer'); - } - private _getColorFromAnsiIndex(idx: number): IColor { if (idx >= this._config.colors.ansi.length) { throw new Error('No color found for idx ' + idx); diff --git a/addons/xterm-addon-webgl/src/renderLayer/BaseRenderLayer.ts b/addons/xterm-addon-webgl/src/renderLayer/BaseRenderLayer.ts index 0422440e..029ef308 100644 --- a/addons/xterm-addon-webgl/src/renderLayer/BaseRenderLayer.ts +++ b/addons/xterm-addon-webgl/src/renderLayer/BaseRenderLayer.ts @@ -8,13 +8,13 @@ import { ICellData } from 'common/Types'; import { DEFAULT_COLOR, WHITESPACE_CELL_CHAR, WHITESPACE_CELL_CODE } from 'common/buffer/Constants'; import { IGlyphIdentifier } from '../atlas/Types'; import { DIM_OPACITY, INVERTED_DEFAULT_COLOR } from 'browser/renderer/atlas/Constants'; -import { BaseCharAtlas } from '../atlas/BaseCharAtlas'; import { acquireCharAtlas } from '../atlas/CharAtlasCache'; import { Terminal } from 'xterm'; import { IColorSet } from 'browser/Types'; import { IRenderDimensions } from 'browser/renderer/Types'; import { CellData } from 'common/buffer/CellData'; import { AttributeData } from 'common/buffer/AttributeData'; +import { WebglCharAtlas } from 'atlas/WebglCharAtlas'; export abstract class BaseRenderLayer implements IRenderLayer { private _canvas: HTMLCanvasElement; @@ -26,7 +26,7 @@ export abstract class BaseRenderLayer implements IRenderLayer { private _scaledCharLeft: number = 0; private _scaledCharTop: number = 0; - protected _charAtlas: BaseCharAtlas; + protected _charAtlas: WebglCharAtlas; /** * An object that's reused when drawing glyphs in order to reduce GC. @@ -249,115 +249,6 @@ export abstract class BaseRenderLayer implements IRenderLayer { y * this._scaledCellHeight + this._scaledCharTop + this._scaledCharHeight / 2); } - /** - * Draws one or more characters at a cell. If possible this will draw using - * the character atlas to reduce draw time. - * @param terminal The terminal. - * @param chars The character or characters. - * @param code The character code. - * @param width The width of the characters. - * @param x The column to draw at. - * @param y The row to draw at. - * @param fg The foreground color, in the format stored within the attributes. - * @param bg The background color, in the format stored within the attributes. - * This is used to validate whether a cached image can be used. - * @param bold Whether the text is bold. - */ - protected _drawChars(terminal: Terminal, cell: ICellData, x: number, y: number): void { - - // skip cache right away if we draw in RGB - // Note: to avoid bad runtime JoinedCellData will be skipped - // in the cache handler itself (atlasDidDraw == false) and - // fall through to uncached later down below - if (cell.isFgRGB() || cell.isBgRGB()) { - this._drawUncachedChars(terminal, cell, x, y); - return; - } - - let fg; - let bg; - if (cell.isInverse()) { - fg = (cell.isBgDefault()) ? INVERTED_DEFAULT_COLOR : cell.getBgColor(); - bg = (cell.isFgDefault()) ? INVERTED_DEFAULT_COLOR : cell.getFgColor(); - } else { - bg = (cell.isBgDefault()) ? DEFAULT_COLOR : cell.getBgColor(); - fg = (cell.isFgDefault()) ? DEFAULT_COLOR : cell.getFgColor(); - } - - const drawInBrightColor = terminal.getOption('drawBoldTextInBrightColors') && cell.isBold() && fg < 8 && fg !== INVERTED_DEFAULT_COLOR; - - fg += drawInBrightColor ? 8 : 0; - this._currentGlyphIdentifier.chars = cell.getChars() || WHITESPACE_CELL_CHAR; - this._currentGlyphIdentifier.code = cell.getCode() || WHITESPACE_CELL_CODE; - this._currentGlyphIdentifier.bg = bg; - this._currentGlyphIdentifier.fg = fg; - this._currentGlyphIdentifier.bold = !!cell.isBold(); - this._currentGlyphIdentifier.dim = !!cell.isDim(); - this._currentGlyphIdentifier.italic = !!cell.isItalic(); - const atlasDidDraw = this._charAtlas && this._charAtlas.draw( - this._ctx, - this._currentGlyphIdentifier, - x * this._scaledCellWidth + this._scaledCharLeft, - y * this._scaledCellHeight + this._scaledCharTop - ); - - if (!atlasDidDraw) { - this._drawUncachedChars(terminal, cell, x, y); - } - } - - /** - * Draws one or more characters at one or more cells. The character(s) will be - * clipped to ensure that they fit with the cell(s), including the cell to the - * right if the last character is a wide character. - * @param terminal The terminal. - * @param chars The character. - * @param width The width of the character. - * @param fg The foreground color, in the format stored within the attributes. - * @param x The column to draw at. - * @param y The row to draw at. - */ - private _drawUncachedChars(terminal: Terminal, cell: ICellData, x: number, y: number): void { - this._ctx.save(); - this._ctx.font = this._getFont(terminal, !!cell.isBold(), !!cell.isItalic()); - this._ctx.textBaseline = 'middle'; - - if (cell.isInverse()) { - if (cell.isBgDefault()) { - this._ctx.fillStyle = this._colors.background.css; - } else if (cell.isBgRGB()) { - this._ctx.fillStyle = `rgb(${AttributeData.toColorRGB(cell.getBgColor()).join(',')})`; - } else { - this._ctx.fillStyle = this._colors.ansi[cell.getBgColor()].css; - } - } else { - if (cell.isFgDefault()) { - this._ctx.fillStyle = this._colors.foreground.css; - } else if (cell.isFgRGB()) { - this._ctx.fillStyle = `rgb(${AttributeData.toColorRGB(cell.getFgColor()).join(',')})`; - } else { - let fg = cell.getFgColor(); - if (terminal.getOption('drawBoldTextInBrightColors') && cell.isBold() && fg < 8) { - fg += 8; - } - this._ctx.fillStyle = this._colors.ansi[fg].css; - } - } - - this._clipRow(terminal, y); - - // Apply alpha to dim the character - if (cell.isDim()) { - this._ctx.globalAlpha = DIM_OPACITY; - } - // Draw the character - this._ctx.fillText( - cell.getChars(), - x * this._scaledCellWidth + this._scaledCharLeft, - y * this._scaledCellHeight + this._scaledCharTop + this._scaledCharHeight / 2); - this._ctx.restore(); - } - /** * Clips a row to ensure no pixels will be drawn outside the cells in the row. * @param terminal The terminal. From 0898de147fb538b4ab71074d114669a97effcc97 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 4 Jul 2019 08:36:44 -0700 Subject: [PATCH 2/2] Ensure GlyphRenderer._atlas exists on beginFrame This should fix an exception breaking it in vscode --- addons/xterm-addon-webgl/src/GlyphRenderer.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/addons/xterm-addon-webgl/src/GlyphRenderer.ts b/addons/xterm-addon-webgl/src/GlyphRenderer.ts index dc375348..a7bd5e9a 100644 --- a/addons/xterm-addon-webgl/src/GlyphRenderer.ts +++ b/addons/xterm-addon-webgl/src/GlyphRenderer.ts @@ -165,7 +165,7 @@ export class GlyphRenderer { } public beginFrame(): boolean { - return this._atlas.beginFrame(); + return this._atlas ? this._atlas.beginFrame() : true; } public updateCell(x: number, y: number, code: number, attr: number, bg: number, fg: number, chars: string): void {