diff --git a/README.md b/README.md index f2f16c98..6b9b877c 100644 --- a/README.md +++ b/README.md @@ -170,6 +170,7 @@ computational environment for Jupyter, supporting interactive data science and s - [**Nutanix**](https://github.com/nutanix): Nutanix Enterprise Cloud uses xterm in the webssh functionality within Nutanix Calm, and is also looking to move our old noserial (termjs) functionality to xterm.js - [**SSH Web Client**](https://github.com/roke22/PHP-SSH2-Web-Client): SSH Web Client with PHP. - [**Shellvault**](https://www.shellvault.io): The cloud-based SSH terminal you can access from anywhere. +- [**Juno**](http://junolab.org/): A flexible Julia IDE, based on Atom. [And much more...](https://github.com/xtermjs/xterm.js/network/dependents) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 4ab162c8..a26bd3d8 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -25,9 +25,6 @@ jobs: - script: | yarn lint displayName: 'Lint' - - script: | - yarn test-coverage - displayName: 'Generate and publish coverage' - job: macOS pool: @@ -46,6 +43,11 @@ jobs: - script: | yarn lint displayName: 'Lint' + - script: | + yarn test-coverage + export COVERALLS_GIT_BRANCH=$BUILD_SOURCEBRANCH + yarn coveralls + displayName: 'Generate and publish coverage' - job: Windows pool: diff --git a/demo/client.ts b/demo/client.ts index 91cebf07..4de78999 100644 --- a/demo/client.ts +++ b/demo/client.ts @@ -149,7 +149,6 @@ function runRealTerminal(): void { term._initialized = true; } -// TODO: Maybe fake terminal should be removed? Not sure it's useful anymore function runFakeTerminal(): void { if (term._initialized) { return; diff --git a/src/Buffer.test.ts b/src/Buffer.test.ts index 32492ee0..0dcdd02d 100644 --- a/src/Buffer.test.ts +++ b/src/Buffer.test.ts @@ -3,7 +3,7 @@ * @license MIT */ -import { assert } from 'chai'; +import { assert, expect } from 'chai'; import { ITerminal } from './Types'; import { Buffer, DEFAULT_ATTR, CHAR_DATA_CHAR_INDEX } from './Buffer'; import { CircularList } from './common/CircularList'; @@ -514,4 +514,35 @@ describe('Buffer', () => { } }); }); + describe('BufferStringIterator', function(): void { + it('iterator does not ovrflow buffer limits', function(): void { + const terminal = new TestTerminal({rows: 5, cols: 10, scrollback: 5}); + const data = [ + 'aaaaaaaaaa', + 'aaaaaaaaa\n', + 'aaaaaaaaaa', + 'aaaaaaaaa\n', + 'aaaaaaaaaa', + 'aaaaaaaaaa', + 'aaaaaaaaaa', + 'aaaaaaaaa\n', + 'aaaaaaaaaa', + 'aaaaaaaaaa' + ]; + terminal.writeSync(data.join('')); + // brute force test with insane values + expect(() => { + for (let overscan = 0; overscan < 20; ++overscan) { + for (let start = -10; start < 20; ++start) { + for (let end = -10; end < 20; ++end) { + const it = terminal.buffer.iterator(false, start, end, overscan, overscan); + while (it.hasNext()) { + it.next(); + } + } + } + } + }).to.not.throw(); + }); + }); }); diff --git a/src/Buffer.ts b/src/Buffer.ts index 29b034ac..be5a47f1 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -371,8 +371,8 @@ export class Buffer implements IBuffer { this.markers.splice(this.markers.indexOf(marker), 1); } - public iterator(trimRight: boolean, startIndex?: number, endIndex?: number): IBufferStringIterator { - return new BufferStringIterator(this, trimRight, startIndex, endIndex); + public iterator(trimRight: boolean, startIndex?: number, endIndex?: number, startOverscan?: number, endOverscan?: number): IBufferStringIterator { + return new BufferStringIterator(this, trimRight, startIndex, endIndex, startOverscan, endOverscan); } } @@ -401,6 +401,18 @@ export class Marker extends EventEmitter implements IMarker { } } +/** + * Iterator to get unwrapped content strings from the buffer. + * The iterator returns at least the string data between the borders + * `startIndex` and `endIndex` (exclusive) and will expand the lines + * by `startOverscan` to the top and by `endOverscan` to the bottom, + * if no new line was found in between. + * It will never read/return string data beyond `startIndex - startOverscan` + * or `endIndex + endOverscan`. Therefore the first and last line might be truncated. + * It is possible to always get the full string for the first and last line as well + * by setting the overscan values to the actual buffer length. This not recommended + * since it might return the whole buffer within a single string in a worst case scenario. + */ export class BufferStringIterator implements IBufferStringIterator { private _current: number; @@ -408,8 +420,16 @@ export class BufferStringIterator implements IBufferStringIterator { private _buffer: IBuffer, private _trimRight: boolean, private _startIndex: number = 0, - private _endIndex: number = _buffer.lines.length + private _endIndex: number = _buffer.lines.length, + private _startOverscan: number = 0, + private _endOverscan: number = 0 ) { + if (this._startIndex < 0) { + this._startIndex = 0; + } + if (this._endIndex > this._buffer.lines.length) { + this._endIndex = this._buffer.lines.length; + } this._current = this._startIndex; } @@ -419,6 +439,16 @@ export class BufferStringIterator implements IBufferStringIterator { public next(): IBufferStringIteratorResult { const range = this._buffer.getWrappedRangeForLine(this._current); + // limit search window to overscan value at both borders + if (range.first < this._startIndex - this._startOverscan) { + range.first = this._startIndex - this._startOverscan; + } + if (range.last > this._endIndex + this._endOverscan) { + range.last = this._endIndex + this._endOverscan; + } + // limit to current buffer length + range.first = Math.max(range.first, 0); + range.last = Math.min(range.last, this._buffer.lines.length); let result = ''; for (let i = range.first; i <= range.last; ++i) { // TODO: always apply trimRight after fixing #1685 diff --git a/src/Linkifier.ts b/src/Linkifier.ts index 0dd33a85..2ec3ed40 100644 --- a/src/Linkifier.ts +++ b/src/Linkifier.ts @@ -21,6 +21,13 @@ export class Linkifier extends EventEmitter implements ILinkifier { */ protected static readonly TIME_BEFORE_LINKIFY = 200; + /** + * Limit of the unwrapping line expansion (overscan) at the top and bottom + * of the actual viewport in ASCII characters. + * A limit of 2000 should match most sane urls. + */ + protected static readonly OVERSCAN_CHAR_LIMIT = 2000; + protected _linkMatchers: ILinkMatcher[] = []; private _mouseZoneManager: IMouseZoneManager; @@ -92,11 +99,19 @@ export class Linkifier extends EventEmitter implements ILinkifier { // Invalidate bad end row values (if a resize happened) const absoluteRowIndexEnd = buffer.ydisp + Math.min(this._rowsToLinkify.end, this._terminal.rows) + 1; - // iterate over the range of unwrapped content strings within start..end (excluding) - // _doLinkifyRow gets full unwrapped lines with the start row as buffer offset for every matcher - // for wrapped content over several rows the iterator might return rows outside the viewport - // we skip those later in _doLinkifyRow - const iterator = buffer.iterator(false, absoluteRowIndexStart, absoluteRowIndexEnd); + // Iterate over the range of unwrapped content strings within start..end + // (excluding). + // _doLinkifyRow gets full unwrapped lines with the start row as buffer offset + // for every matcher. + // The unwrapping is needed to also match content that got wrapped across + // several buffer lines. To avoid a worst case scenario where the whole buffer + // contains just a single unwrapped string we limit this line expansion beyond + // the viewport to +OVERSCAN_CHAR_LIMIT chars (overscan) at top and bottom. + // This comes with the tradeoff that matches longer than OVERSCAN_CHAR_LIMIT + // chars will not match anymore at the viewport borders. + const overscanLineLimit = Math.ceil(Linkifier.OVERSCAN_CHAR_LIMIT / this._terminal.cols); + const iterator = this._terminal.buffer.iterator( + false, absoluteRowIndexStart, absoluteRowIndexEnd, overscanLineLimit, overscanLineLimit); while (iterator.hasNext()) { const lineData: IBufferStringIteratorResult = iterator.next(); for (let i = 0; i < this._linkMatchers.length; i++) { @@ -208,14 +223,6 @@ export class Linkifier extends EventEmitter implements ILinkifier { // get the buffer index as [absolute row, col] for the match const bufferIndex = this._terminal.buffer.stringIndexToBufferIndex(rowIndex, stringIndex); - // skip rows outside of the viewport - if (bufferIndex[0] - this._terminal.buffer.ydisp < 0) { - continue; - } - if (bufferIndex[0] - this._terminal.buffer.ydisp > this._terminal.rows) { - break; - } - const line = this._terminal.buffer.lines.get(bufferIndex[0]); const char = line.get(bufferIndex[1]); let fg: number | undefined; diff --git a/src/Types.ts b/src/Types.ts index 69c40151..40b3bd02 100644 --- a/src/Types.ts +++ b/src/Types.ts @@ -296,7 +296,7 @@ export interface IBuffer { nextStop(x?: number): number; prevStop(x?: number): number; stringIndexToBufferIndex(lineIndex: number, stringIndex: number): number[]; - iterator(trimRight: boolean, startIndex?: number, endIndex?: number): IBufferStringIterator; + iterator(trimRight: boolean, startIndex?: number, endIndex?: number, startOverscan?: number, endOverscan?: number): IBufferStringIterator; } export interface IBufferSet extends IEventEmitter { diff --git a/src/renderer/BaseRenderLayer.ts b/src/renderer/BaseRenderLayer.ts index 1df9c3ea..1590d6e2 100644 --- a/src/renderer/BaseRenderLayer.ts +++ b/src/renderer/BaseRenderLayer.ts @@ -5,7 +5,7 @@ import { IRenderLayer, IColorSet, IRenderDimensions } from './Types'; import { CharData, ITerminal } from '../Types'; -import { DIM_OPACITY, INVERTED_DEFAULT_COLOR } from './atlas/Types'; +import { DIM_OPACITY, INVERTED_DEFAULT_COLOR, IGlyphIdentifier } from './atlas/Types'; import BaseCharAtlas from './atlas/BaseCharAtlas'; import { acquireCharAtlas } from './atlas/CharAtlasCache'; import { CHAR_DATA_CHAR_INDEX } from '../Buffer'; @@ -22,6 +22,19 @@ export abstract class BaseRenderLayer implements IRenderLayer { protected _charAtlas: BaseCharAtlas; + /** + * An object that's reused when drawing glyphs in order to reduce GC. + */ + private _currentGlyphIdentifier: IGlyphIdentifier = { + chars: '', + code: 0, + bg: 0, + fg: 0, + bold: false, + dim: false, + italic: false + }; + constructor( private _container: HTMLElement, id: string, @@ -38,6 +51,7 @@ export abstract class BaseRenderLayer implements IRenderLayer { public dispose(): void { this._container.removeChild(this._canvas); + this._charAtlas.dispose(); } private _initCanvas(): void { @@ -245,9 +259,16 @@ export abstract class BaseRenderLayer implements IRenderLayer { const drawInBrightColor = terminal.options.drawBoldTextInBrightColors && bold && fg < 8 && fg !== INVERTED_DEFAULT_COLOR; fg += drawInBrightColor ? 8 : 0; + this._currentGlyphIdentifier.chars = chars; + this._currentGlyphIdentifier.code = code; + this._currentGlyphIdentifier.bg = bg; + this._currentGlyphIdentifier.fg = fg; + this._currentGlyphIdentifier.bold = bold && terminal.options.enableBold; + this._currentGlyphIdentifier.dim = dim; + this._currentGlyphIdentifier.italic = italic; const atlasDidDraw = this._charAtlas && this._charAtlas.draw( this._ctx, - {chars, code, bg, fg, bold: bold && terminal.options.enableBold, dim, italic}, + this._currentGlyphIdentifier, x * this._scaledCellWidth + this._scaledCharLeft, y * this._scaledCellHeight + this._scaledCharTop ); diff --git a/src/renderer/atlas/BaseCharAtlas.ts b/src/renderer/atlas/BaseCharAtlas.ts index 50d35faa..ee69b381 100644 --- a/src/renderer/atlas/BaseCharAtlas.ts +++ b/src/renderer/atlas/BaseCharAtlas.ts @@ -4,10 +4,13 @@ */ import { IGlyphIdentifier } from './Types'; +import { IDisposable } from 'xterm'; -export default abstract class BaseCharAtlas { +export default 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. diff --git a/src/renderer/atlas/DynamicCharAtlas.ts b/src/renderer/atlas/DynamicCharAtlas.ts index f6df365e..e67fe4f0 100644 --- a/src/renderer/atlas/DynamicCharAtlas.ts +++ b/src/renderer/atlas/DynamicCharAtlas.ts @@ -10,6 +10,7 @@ import BaseCharAtlas from './BaseCharAtlas'; import { DEFAULT_ANSI_COLORS } from '../ColorManager'; import { clearColor } from '../../shared/atlas/CharAtlasGenerator'; import LRUMap from './LRUMap'; +import { isFirefox, isSafari } from '../../shared/utils/Browser'; // 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. @@ -29,14 +30,29 @@ const TRANSPARENT_COLOR = { // cache. const FRAME_CACHE_DRAW_LIMIT = 100; +/** + * The number of milliseconds to wait before generating the ImageBitmap, this is to debounce/batch + * the operation as window.createImageBitmap is asynchronous. + */ +const GLYPH_BITMAP_COMMIT_DELAY = 100; + interface IGlyphCacheValue { index: number; isEmpty: boolean; + inBitmap: boolean; } -function getGlyphCacheKey(glyph: IGlyphIdentifier): string { - const styleFlags = (glyph.bold ? 0 : 4) + (glyph.dim ? 0 : 2) + (glyph.italic ? 0 : 1); - return `${glyph.bg}_${glyph.fg}_${styleFlags}${glyph.chars}`; +function getGlyphCacheKey(glyph: IGlyphIdentifier): number { + // Note that this only returns a valid key when code < 256 + // Layout: + // 0b00000000000000000000000000000001: italic (1) + // 0b00000000000000000000000000000010: dim (1) + // 0b00000000000000000000000000000100: bold (1) + // 0b00000000000000000000111111111000: fg (9) + // 0b00000000000111111111000000000000: bg (9) + // 0b00011111111000000000000000000000: code (8) + // 0b11100000000000000000000000000000: unused (3) + return glyph.code << 21 | glyph.bg << 12 | glyph.fg << 3 | (glyph.bold ? 0 : 4) + (glyph.dim ? 0 : 2) + (glyph.italic ? 0 : 1); } export default class DynamicCharAtlas extends BaseCharAtlas { @@ -57,6 +73,15 @@ export default class DynamicCharAtlas extends BaseCharAtlas { private _drawToCacheCount: number = 0; + // An array of glyph keys that are waiting on the bitmap to be generated. + private _glyphsWaitingOnBitmap: IGlyphCacheValue[] = []; + + // The timeout that is used to batch bitmap generation so it's not requested for every new glyph. + private _bitmapCommitTimeout: number | null = null; + + // The bitmap to draw from, this is much faster on other browsers than others. + private _bitmap: ImageBitmap | null = null; + constructor(document: Document, private _config: ICharAtlasConfig) { super(); this._cacheCanvas = document.createElement('canvas'); @@ -82,6 +107,13 @@ export default class DynamicCharAtlas extends BaseCharAtlas { // document.body.appendChild(this._cacheCanvas); } + public dispose(): void { + if (this._bitmapCommitTimeout !== null) { + window.clearTimeout(this._bitmapCommitTimeout); + this._bitmapCommitTimeout = null; + } + } + public beginFrame(): void { this._drawToCacheCount = 0; } @@ -92,6 +124,11 @@ export default class DynamicCharAtlas extends BaseCharAtlas { x: number, y: number ): boolean { + // Space is always an empty cell, special case this as it's so common + if (glyph.code === 32) { + return true; + } + const glyphKey = getGlyphCacheKey(glyph); const cacheValue = this._cacheMap.get(glyphKey); if (cacheValue !== null && cacheValue !== undefined) { @@ -124,11 +161,12 @@ export default class DynamicCharAtlas extends BaseCharAtlas { return glyph.code < 256; } - private _toCoordinates(index: number): [number, number] { - return [ - (index % this._width) * this._config.scaledCharWidth, - Math.floor(index / this._width) * this._config.scaledCharHeight - ]; + private _toCoordinateX(index: number): number { + return (index % this._width) * this._config.scaledCharWidth; + } + + private _toCoordinateY(index: number): number { + return Math.floor(index / this._width) * this._config.scaledCharHeight; } private _drawFromCache( @@ -141,9 +179,10 @@ export default class DynamicCharAtlas extends BaseCharAtlas { if (cacheValue.isEmpty) { return; } - const [cacheX, cacheY] = this._toCoordinates(cacheValue.index); + const cacheX = this._toCoordinateX(cacheValue.index); + const cacheY = this._toCoordinateY(cacheValue.index); ctx.drawImage( - this._cacheCanvas, + cacheValue.inBitmap ? this._bitmap : this._cacheCanvas, cacheX, cacheY, this._config.scaledCharWidth, @@ -230,13 +269,58 @@ export default class DynamicCharAtlas extends BaseCharAtlas { } // copy the data from imageData to _cacheCanvas - const [x, y] = this._toCoordinates(index); + const x = this._toCoordinateX(index); + const y = this._toCoordinateY(index); // putImageData doesn't do any blending, so it will overwrite any existing cache entry for us this._cacheCtx.putImageData(imageData, x, y); - return { + // Add the glyph and queue it to the bitmap (if the browser supports it) + const cacheValue = { index, - isEmpty + isEmpty, + inBitmap: false }; + this._addGlyphToBitmap(cacheValue); + + return cacheValue; + } + + private _addGlyphToBitmap(cacheValue: IGlyphCacheValue): void { + // Support is patchy for createImageBitmap at the moment, pass a canvas back + // if support is lacking as drawImage works there too. Firefox is also + // included here as ImageBitmap appears both buggy and has horrible + // performance (tested on v55). + if (!('createImageBitmap' in window) || isFirefox || isSafari) { + return; + } + + // Add the glyph to the queue + this._glyphsWaitingOnBitmap.push(cacheValue); + + // Check if bitmap generation timeout already exists + if (this._bitmapCommitTimeout !== null) { + return; + } + + this._bitmapCommitTimeout = window.setTimeout(() => this._generateBitmap(), GLYPH_BITMAP_COMMIT_DELAY); + } + + private _generateBitmap(): void { + const glyphsMovingToBitmap = this._glyphsWaitingOnBitmap; + this._glyphsWaitingOnBitmap = []; + window.createImageBitmap(this._cacheCanvas).then(bitmap => { + // Set bitmap + this._bitmap = bitmap; + + // Mark all new glyphs as in bitmap, excluding glyphs that came in after + // the bitmap was requested + for (let i = 0; i < glyphsMovingToBitmap.length; i++) { + const value = glyphsMovingToBitmap[i]; + // It doesn't matter if the value was already evicted, it will be + // released from memory after this block if so. + value.inBitmap = true; + } + }); + this._bitmapCommitTimeout = null; } } diff --git a/src/renderer/atlas/LRUMap.test.ts b/src/renderer/atlas/LRUMap.test.ts index ba01e410..197d1159 100644 --- a/src/renderer/atlas/LRUMap.test.ts +++ b/src/renderer/atlas/LRUMap.test.ts @@ -9,57 +9,57 @@ import LRUMap from './LRUMap'; describe('LRUMap', () => { it('can be used to store and retrieve values', () => { const map = new LRUMap(10); - map.set('keya', 'valuea'); - map.set('keyb', 'valueb'); - map.set('keyc', 'valuec'); - assert.strictEqual(map.get('keya'), 'valuea'); - assert.strictEqual(map.get('keyb'), 'valueb'); - assert.strictEqual(map.get('keyc'), 'valuec'); + map.set(1, 'valuea'); + map.set(2, 'valueb'); + map.set(3, 'valuec'); + assert.strictEqual(map.get(1), 'valuea'); + assert.strictEqual(map.get(2), 'valueb'); + assert.strictEqual(map.get(3), 'valuec'); }); it('maintains a size from insertions', () => { const map = new LRUMap(10); assert.strictEqual(map.size, 0); - map.set('a', 'value'); + map.set(1, 'value'); assert.strictEqual(map.size, 1); - map.set('b', 'value'); + map.set(2, 'value'); assert.strictEqual(map.size, 2); }); it('deletes the oldest entry when the capacity is exceeded', () => { const map = new LRUMap(4); - map.set('a', 'value'); - map.set('b', 'value'); - map.set('c', 'value'); - map.set('d', 'value'); - map.set('e', 'value'); - assert.isNull(map.get('a')); - assert.isNotNull(map.get('b')); - assert.isNotNull(map.get('c')); - assert.isNotNull(map.get('d')); - assert.isNotNull(map.get('e')); + map.set(1, 'value'); + map.set(2, 'value'); + map.set(3, 'value'); + map.set(4, 'value'); + map.set(5, 'value'); + assert.isNull(map.get(1)); + assert.isNotNull(map.get(2)); + assert.isNotNull(map.get(3)); + assert.isNotNull(map.get(4)); + assert.isNotNull(map.get(5)); assert.strictEqual(map.size, 4); }); it('prevents a recently accessed entry from getting deleted', () => { const map = new LRUMap(2); - map.set('a', 'value'); - map.set('b', 'value'); - map.get('a'); + map.set(1, 'value'); + map.set(2, 'value'); + map.get(1); // a would normally get deleted here, except that we called get() - map.set('c', 'value'); - assert.isNotNull(map.get('a')); + map.set(3, 'value'); + assert.isNotNull(map.get(1)); // b got deleted instead of a - assert.isNull(map.get('b')); - assert.isNotNull(map.get('c')); + assert.isNull(map.get(2)); + assert.isNotNull(map.get(3)); }); it('supports mutation', () => { const map = new LRUMap(10); - map.set('keya', 'oldvalue'); - map.set('keya', 'newvalue'); + map.set(1, 'oldvalue'); + map.set(1, 'newvalue'); // mutation doesn't change the size assert.strictEqual(map.size, 1); - assert.strictEqual(map.get('keya'), 'newvalue'); + assert.strictEqual(map.get(1), 'newvalue'); }); }); diff --git a/src/renderer/atlas/LRUMap.ts b/src/renderer/atlas/LRUMap.ts index eccfbfea..d7e01ec6 100644 --- a/src/renderer/atlas/LRUMap.ts +++ b/src/renderer/atlas/LRUMap.ts @@ -6,12 +6,12 @@ interface ILinkedListNode { prev: ILinkedListNode; next: ILinkedListNode; - key: string; + key: number; value: T; } export default class LRUMap { - private _map: { [key: string]: ILinkedListNode } = {}; + private _map: { [key: number]: ILinkedListNode } = {}; private _head: ILinkedListNode = null; private _tail: ILinkedListNode = null; private _nodePool: ILinkedListNode[] = []; @@ -68,7 +68,7 @@ export default class LRUMap { } } - public get(key: string): T | null { + public get(key: number): T | null { // This is unsafe: We're assuming our keyspace doesn't overlap with Object.prototype. However, // it's faster than calling hasOwnProperty, and in our case, it would never overlap. const node = this._map[key]; @@ -80,12 +80,23 @@ export default class LRUMap { return null; } + /** + * Gets a value from a key without marking it as the most recently used item. + */ + public peekValue(key: number): T | null { + const node = this._map[key]; + if (node !== undefined) { + return node.value; + } + return null; + } + public peek(): T | null { const head = this._head; return head === null ? null : head.value; } - public set(key: string, value: T): void { + public set(key: number, value: T): void { // This is unsafe: See note above. let node = this._map[key]; if (node !== undefined) {