Merge branch 'master' into common_layering_strict

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)
 

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:

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;

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();
+    });
+  });
 });

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 {
+  public iterator(trimRight: boolean, startIndex?: number, endIndex?: number, startOverscan?: number, endOverscan?: number): IBufferStringIterator {
-    return new BufferStringIterator(this, trimRight, startIndex, endIndex);
+    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

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)
+    // Iterate over the range of unwrapped content strings within start..end
-    // _doLinkifyRow gets full unwrapped lines with the start row as buffer offset for every matcher
+    // (excluding).
-    // for wrapped content over several rows the iterator might return rows outside the viewport
+    // _doLinkifyRow gets full unwrapped lines with the start row as buffer offset
-    // we skip those later in _doLinkifyRow
+    // for every matcher.
-    const iterator = buffer.iterator(false, absoluteRowIndexStart, absoluteRowIndexEnd);
+    // 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;

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 {

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
     );

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.

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 {
+function getGlyphCacheKey(glyph: IGlyphIdentifier): number {
-  const styleFlags = (glyph.bold ? 0 : 4) + (glyph.dim ? 0 : 2) + (glyph.italic ? 0 : 1);
+  // Note that this only returns a valid key when code < 256
-  return `${glyph.bg}_${glyph.fg}_${styleFlags}${glyph.chars}`;
+  // 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] {
+  private _toCoordinateX(index: number): number {
-    return [
+    return (index % this._width) * this._config.scaledCharWidth;
-      (index % this._width) * this._config.scaledCharWidth,
+  }
-      Math.floor(index / this._width) * this._config.scaledCharHeight
+
-    ];
+  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;
   }
 }

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(1, 'valuea');
-    map.set('keyb', 'valueb');
+    map.set(2, 'valueb');
-    map.set('keyc', 'valuec');
+    map.set(3, 'valuec');
-    assert.strictEqual(map.get('keya'), 'valuea');
+    assert.strictEqual(map.get(1), 'valuea');
-    assert.strictEqual(map.get('keyb'), 'valueb');
+    assert.strictEqual(map.get(2), 'valueb');
-    assert.strictEqual(map.get('keyc'), 'valuec');
+    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(1, 'value');
-    map.set('b', 'value');
+    map.set(2, 'value');
-    map.set('c', 'value');
+    map.set(3, 'value');
-    map.set('d', 'value');
+    map.set(4, 'value');
-    map.set('e', 'value');
+    map.set(5, 'value');
-    assert.isNull(map.get('a'));
+    assert.isNull(map.get(1));
-    assert.isNotNull(map.get('b'));
+    assert.isNotNull(map.get(2));
-    assert.isNotNull(map.get('c'));
+    assert.isNotNull(map.get(3));
-    assert.isNotNull(map.get('d'));
+    assert.isNotNull(map.get(4));
-    assert.isNotNull(map.get('e'));
+    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(1, 'value');
-    map.set('b', 'value');
+    map.set(2, 'value');
-    map.get('a');
+    map.get(1);
     // a would normally get deleted here, except that we called get()
-    map.set('c', 'value');
+    map.set(3, 'value');
-    assert.isNotNull(map.get('a'));
+    assert.isNotNull(map.get(1));
     // b got deleted instead of a
-    assert.isNull(map.get('b'));
+    assert.isNull(map.get(2));
-    assert.isNotNull(map.get('c'));
+    assert.isNotNull(map.get(3));
   });
 
   it('supports mutation', () => {
     const map = new LRUMap(10);
-    map.set('keya', 'oldvalue');
+    map.set(1, 'oldvalue');
-    map.set('keya', 'newvalue');
+    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');
   });
 });

src/renderer/atlas/LRUMap.ts

@@ -6,12 +6,12 @@
 interface ILinkedListNode<T> {
   prev: ILinkedListNode<T>;
   next: ILinkedListNode<T>;
-  key: string;
+  key: number;
   value: T;
 }
 
 export default class LRUMap<T> {
-  private _map: { [key: string]: ILinkedListNode<T> } = {};
+  private _map: { [key: number]: ILinkedListNode<T> } = {};
   private _head: ILinkedListNode<T> = null;
   private _tail: ILinkedListNode<T> = null;
   private _nodePool: ILinkedListNode<T>[] = [];
@@ -68,7 +68,7 @@ export default class LRUMap<T> {
     }
   }
 
-  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<T> {
     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) {