From 04f2d52f26342dc9a88ba65d28e889a43f38ab39 Mon Sep 17 00:00:00 2001
From: Daniel Imms <daimms@microsoft.com>
Date: Fri, 14 Sep 2018 09:58:49 -0700
Subject: [PATCH 01/26] Remove IGlyphIdentifier in-between object

This was being constructed for every character draw
---
 src/renderer/BaseRenderLayer.ts        |  8 +++-
 src/renderer/atlas/BaseCharAtlas.ts    | 19 ++++++--
 src/renderer/atlas/DynamicCharAtlas.ts | 64 ++++++++++++++++----------
 src/renderer/atlas/NoneCharAtlas.ts    |  9 +++-
 src/renderer/atlas/StaticCharAtlas.ts  | 34 ++++++++------
 src/renderer/atlas/Types.ts            | 10 ----
 6 files changed, 88 insertions(+), 56 deletions(-)

diff --git a/src/renderer/BaseRenderLayer.ts b/src/renderer/BaseRenderLayer.ts
index 1df9c3ea..d7755aca 100644
--- a/src/renderer/BaseRenderLayer.ts
+++ b/src/renderer/BaseRenderLayer.ts
@@ -247,7 +247,13 @@ export abstract class BaseRenderLayer implements IRenderLayer {
     fg += drawInBrightColor ? 8 : 0;
     const atlasDidDraw = this._charAtlas && this._charAtlas.draw(
       this._ctx,
-      {chars, code, bg, fg, bold: bold && terminal.options.enableBold, dim, italic},
+      chars,
+      code,
+      bg,
+      fg,
+      bold,
+      dim,
+      italic,
       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..325818c6 100644
--- a/src/renderer/atlas/BaseCharAtlas.ts
+++ b/src/renderer/atlas/BaseCharAtlas.ts
@@ -3,8 +3,6 @@
  * @license MIT
  */
 
-import { IGlyphIdentifier } from './Types';
-
 export default abstract class BaseCharAtlas {
   private _didWarmUp: boolean = false;
 
@@ -39,14 +37,27 @@ export default abstract class BaseCharAtlas {
    * do nothing and return false in that case.
    *
    * @param ctx Where to draw the character onto.
-   * @param glyph Information about what to draw
+   * @param chars The character(s) to draw. This is typically a single character bug can be made up
+   * of multiple when character joiners are used.
+   * @param code The character code.
+   * @param bg The background color.
+   * @param fg The foreground color.
+   * @param bold Whether the text is bold.
+   * @param dim Whether the text is dim.
+   * @param italic Whether the text is italic.
    * @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,
+    chars: string,
+    code: number,
+    bg: number,
+    fg: number,
+    bold: boolean,
+    dim: boolean,
+    italic: boolean,
     x: number,
     y: number
   ): boolean;
diff --git a/src/renderer/atlas/DynamicCharAtlas.ts b/src/renderer/atlas/DynamicCharAtlas.ts
index f6df365e..66baa822 100644
--- a/src/renderer/atlas/DynamicCharAtlas.ts
+++ b/src/renderer/atlas/DynamicCharAtlas.ts
@@ -3,7 +3,7 @@
  * @license MIT
  */
 
-import { DIM_OPACITY, IGlyphIdentifier, INVERTED_DEFAULT_COLOR } from './Types';
+import { DIM_OPACITY, INVERTED_DEFAULT_COLOR } from './Types';
 import { ICharAtlasConfig } from '../../shared/atlas/Types';
 import { IColor } from '../../shared/Types';
 import BaseCharAtlas from './BaseCharAtlas';
@@ -34,9 +34,9 @@ interface IGlyphCacheValue {
   isEmpty: 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(chars: string, fg: number, bg: number, bold: boolean, dim: boolean, italic: boolean): string {
+  const styleFlags = (bold ? 0 : 4) + (dim ? 0 : 2) + (italic ? 0 : 1);
+  return `${bg}_${fg}_${styleFlags}${chars}`;
 }
 
 export default class DynamicCharAtlas extends BaseCharAtlas {
@@ -88,16 +88,22 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
 
   public draw(
     ctx: CanvasRenderingContext2D,
-    glyph: IGlyphIdentifier,
+    chars: string,
+    code: number,
+    bg: number,
+    fg: number,
+    bold: boolean,
+    dim: boolean,
+    italic: boolean,
     x: number,
     y: number
   ): boolean {
-    const glyphKey = getGlyphCacheKey(glyph);
+    const glyphKey = getGlyphCacheKey(chars, fg, bg, bold, dim, italic);
     const cacheValue = this._cacheMap.get(glyphKey);
     if (cacheValue !== null && cacheValue !== undefined) {
       this._drawFromCache(ctx, cacheValue, x, y);
       return true;
-    } else if (this._canCache(glyph) && this._drawToCacheCount < FRAME_CACHE_DRAW_LIMIT) {
+    } else if (this._canCache(code) && this._drawToCacheCount < FRAME_CACHE_DRAW_LIMIT) {
       let index;
       if (this._cacheMap.size < this._cacheMap.capacity) {
         index = this._cacheMap.size;
@@ -105,7 +111,7 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
         // we're out of space, so our call to set will delete this item
         index = this._cacheMap.peek().index;
       }
-      const cacheValue = this._drawToCache(glyph, index);
+      const cacheValue = this._drawToCache(chars, bg, fg, bold, dim, italic, index);
       this._cacheMap.set(glyphKey, cacheValue);
       this._drawFromCache(ctx, cacheValue, x, y);
       return true;
@@ -113,7 +119,7 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
     return false;
   }
 
-  private _canCache(glyph: IGlyphIdentifier): boolean {
+  private _canCache(code: number): boolean {
     // Only cache ascii and extended characters for now, to be safe. In the future, we could do
     // something more complicated to determine the expected width of a character.
     //
@@ -121,7 +127,7 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
     // to draw overlapping glyphs from the atlas:
     // https://github.com/servo/webrender/issues/464#issuecomment-255632875
     // https://webglfundamentals.org/webgl/lessons/webgl-text-texture.html
-    return glyph.code < 256;
+    return code < 256;
   }
 
   private _toCoordinates(index: number): [number, number] {
@@ -162,39 +168,47 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
     return DEFAULT_ANSI_COLORS[idx];
   }
 
-  private _getBackgroundColor(glyph: IGlyphIdentifier): IColor {
+  private _getBackgroundColor(bg: number): IColor {
     if (this._config.allowTransparency) {
       // The background color might have some transparency, so we need to render it as fully
       // transparent in the atlas. Otherwise we'd end up drawing the transparent background twice
       // around the anti-aliased edges of the glyph, and it would look too dark.
       return TRANSPARENT_COLOR;
-    } else if (glyph.bg === INVERTED_DEFAULT_COLOR) {
+    } else if (bg === INVERTED_DEFAULT_COLOR) {
       return this._config.colors.foreground;
-    } else if (glyph.bg < 256) {
-      return this._getColorFromAnsiIndex(glyph.bg);
+    } else if (bg < 256) {
+      return this._getColorFromAnsiIndex(bg);
     }
     return this._config.colors.background;
   }
 
-  private _getForegroundColor(glyph: IGlyphIdentifier): IColor {
-    if (glyph.fg === INVERTED_DEFAULT_COLOR) {
+  private _getForegroundColor(fg: number): IColor {
+    if (fg === INVERTED_DEFAULT_COLOR) {
       return this._config.colors.background;
-    } else if (glyph.fg < 256) {
+    } else if (fg < 256) {
       // 256 color support
-      return this._getColorFromAnsiIndex(glyph.fg);
+      return this._getColorFromAnsiIndex(fg);
     }
     return this._config.colors.foreground;
   }
 
   // TODO: We do this (or something similar) in multiple places. We should split this off
   // into a shared function.
-  private _drawToCache(glyph: IGlyphIdentifier, index: number): IGlyphCacheValue {
+  private _drawToCache(
+    chars: string,
+    bg: number,
+    fg: number,
+    bold: boolean,
+    dim: boolean,
+    italic: boolean,
+    index: number
+  ): IGlyphCacheValue {
     this._drawToCacheCount++;
 
     this._tmpCtx.save();
 
     // draw the background
-    const backgroundColor = this._getBackgroundColor(glyph);
+    const backgroundColor = this._getBackgroundColor(bg);
     // Use a 'copy' composite operation to clear any existing glyph out of _tmpCtxWithAlpha, regardless of
     // transparency in backgroundColor
     this._tmpCtx.globalCompositeOperation = 'copy';
@@ -203,20 +217,20 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
     this._tmpCtx.globalCompositeOperation = 'source-over';
 
     // draw the foreground/glyph
-    const fontWeight = glyph.bold ? this._config.fontWeightBold : this._config.fontWeight;
-    const fontStyle = glyph.italic ? 'italic' : '';
+    const fontWeight = bold ? this._config.fontWeightBold : this._config.fontWeight;
+    const fontStyle = italic ? 'italic' : '';
     this._tmpCtx.font =
       `${fontStyle} ${fontWeight} ${this._config.fontSize * this._config.devicePixelRatio}px ${this._config.fontFamily}`;
     this._tmpCtx.textBaseline = 'top';
 
-    this._tmpCtx.fillStyle = this._getForegroundColor(glyph).css;
+    this._tmpCtx.fillStyle = this._getForegroundColor(fg).css;
 
     // Apply alpha to dim the character
-    if (glyph.dim) {
+    if (dim) {
       this._tmpCtx.globalAlpha = DIM_OPACITY;
     }
     // Draw the character
-    this._tmpCtx.fillText(glyph.chars, 0, 0);
+    this._tmpCtx.fillText(chars, 0, 0);
     this._tmpCtx.restore();
 
     // clear the background from the character to avoid issues with drawing over the previous
diff --git a/src/renderer/atlas/NoneCharAtlas.ts b/src/renderer/atlas/NoneCharAtlas.ts
index 1cbc9eea..163baf38 100644
--- a/src/renderer/atlas/NoneCharAtlas.ts
+++ b/src/renderer/atlas/NoneCharAtlas.ts
@@ -5,7 +5,6 @@
  * A dummy CharAtlas implementation that always fails to draw characters.
  */
 
-import { IGlyphIdentifier } from './Types';
 import { ICharAtlasConfig } from '../../shared/atlas/Types';
 import BaseCharAtlas from './BaseCharAtlas';
 
@@ -16,7 +15,13 @@ export default class NoneCharAtlas extends BaseCharAtlas {
 
   public draw(
     ctx: CanvasRenderingContext2D,
-    glyph: IGlyphIdentifier,
+    chars: string,
+    code: number,
+    bg: number,
+    fg: number,
+    bold: boolean,
+    dim: boolean,
+    italic: boolean,
     x: number,
     y: number
   ): boolean {
diff --git a/src/renderer/atlas/StaticCharAtlas.ts b/src/renderer/atlas/StaticCharAtlas.ts
index c0d8a814..0e022fa6 100644
--- a/src/renderer/atlas/StaticCharAtlas.ts
+++ b/src/renderer/atlas/StaticCharAtlas.ts
@@ -3,7 +3,7 @@
  * @license MIT
  */
 
-import { DIM_OPACITY, IGlyphIdentifier } from './Types';
+import { DIM_OPACITY } from './Types';
 import { CHAR_ATLAS_CELL_SPACING, ICharAtlasConfig } from '../../shared/atlas/Types';
 import { generateStaticCharAtlasTexture } from '../../shared/atlas/CharAtlasGenerator';
 import BaseCharAtlas from './BaseCharAtlas';
@@ -37,18 +37,24 @@ export default class StaticCharAtlas extends BaseCharAtlas {
     }
   }
 
-  private _isCached(glyph: IGlyphIdentifier, colorIndex: number): boolean {
-    const isAscii = glyph.code < 256;
+  private _isCached(code: number, fg: number, bg: number, italic: boolean): boolean {
+    const isAscii = code < 256;
     // A color is basic if it is one of the 4 bit ANSI colors.
-    const isBasicColor = glyph.fg < 16;
-    const isDefaultColor = glyph.fg >= 256;
-    const isDefaultBackground = glyph.bg >= 256;
-    return isAscii && (isBasicColor || isDefaultColor) && isDefaultBackground && !glyph.italic;
+    const isBasicColor = fg < 16;
+    const isDefaultColor = fg >= 256;
+    const isDefaultBackground = bg >= 256;
+    return isAscii && (isBasicColor || isDefaultColor) && isDefaultBackground && !italic;
   }
 
   public draw(
     ctx: CanvasRenderingContext2D,
-    glyph: IGlyphIdentifier,
+    chars: string,
+    code: number,
+    bg: number,
+    fg: number,
+    bold: boolean,
+    dim: boolean,
+    italic: boolean,
     x: number,
     y: number
   ): boolean {
@@ -58,15 +64,15 @@ export default class StaticCharAtlas extends BaseCharAtlas {
     }
 
     let colorIndex = 0;
-    if (glyph.fg < 256) {
-      colorIndex = 2 + glyph.fg + (glyph.bold ? 16 : 0);
+    if (fg < 256) {
+      colorIndex = 2 + fg + (bold ? 16 : 0);
     } else {
       // If default color and bold
-      if (glyph.bold) {
+      if (bold) {
         colorIndex = 1;
       }
     }
-    if (!this._isCached(glyph, colorIndex)) {
+    if (!this._isCached(code, fg, bg, italic)) {
       return false;
     }
 
@@ -77,13 +83,13 @@ export default class StaticCharAtlas extends BaseCharAtlas {
     const charAtlasCellHeight = this._config.scaledCharHeight + CHAR_ATLAS_CELL_SPACING;
 
     // Apply alpha to dim the character
-    if (glyph.dim) {
+    if (dim) {
       ctx.globalAlpha = DIM_OPACITY;
     }
 
     ctx.drawImage(
       this._texture,
-      glyph.code * charAtlasCellWidth,
+      code * charAtlasCellWidth,
       colorIndex * charAtlasCellHeight,
       charAtlasCellWidth,
       this._config.scaledCharHeight,
diff --git a/src/renderer/atlas/Types.ts b/src/renderer/atlas/Types.ts
index 6fb3c5d1..34f01d39 100644
--- a/src/renderer/atlas/Types.ts
+++ b/src/renderer/atlas/Types.ts
@@ -5,13 +5,3 @@
 
 export const INVERTED_DEFAULT_COLOR = -1;
 export const DIM_OPACITY = 0.5;
-
-export interface IGlyphIdentifier {
-  chars: string;
-  code: number;
-  bg: number;
-  fg: number;
-  bold: boolean;
-  dim: boolean;
-  italic: boolean;
-}

From cf9cb80cb4b82e5dbf11fce59d274175d629b0b8 Mon Sep 17 00:00:00 2001
From: Daniel Imms <daimms@microsoft.com>
Date: Fri, 14 Sep 2018 10:08:26 -0700
Subject: [PATCH 02/26] Use a number as the dynamic cache key

This prevents string creation on every draw
---
 src/renderer/atlas/DynamicCharAtlas.ts | 16 ++++++--
 src/renderer/atlas/LRUMap.test.ts      | 56 +++++++++++++-------------
 src/renderer/atlas/LRUMap.ts           |  8 ++--
 3 files changed, 44 insertions(+), 36 deletions(-)

diff --git a/src/renderer/atlas/DynamicCharAtlas.ts b/src/renderer/atlas/DynamicCharAtlas.ts
index 66baa822..c3927646 100644
--- a/src/renderer/atlas/DynamicCharAtlas.ts
+++ b/src/renderer/atlas/DynamicCharAtlas.ts
@@ -34,9 +34,17 @@ interface IGlyphCacheValue {
   isEmpty: boolean;
 }
 
-function getGlyphCacheKey(chars: string, fg: number, bg: number, bold: boolean, dim: boolean, italic: boolean): string {
-  const styleFlags = (bold ? 0 : 4) + (dim ? 0 : 2) + (italic ? 0 : 1);
-  return `${bg}_${fg}_${styleFlags}${chars}`;
+function getGlyphCacheKey(code: number, fg: number, bg: number, bold: boolean, dim: boolean, italic: boolean): 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 code << 21 | bg << 12 | fg << 3 | (bold ? 0 : 4) + (dim ? 0 : 2) + (italic ? 0 : 1);
 }
 
 export default class DynamicCharAtlas extends BaseCharAtlas {
@@ -98,7 +106,7 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
     x: number,
     y: number
   ): boolean {
-    const glyphKey = getGlyphCacheKey(chars, fg, bg, bold, dim, italic);
+    const glyphKey = getGlyphCacheKey(code, fg, bg, bold, dim, italic);
     const cacheValue = this._cacheMap.get(glyphKey);
     if (cacheValue !== null && cacheValue !== undefined) {
       this._drawFromCache(ctx, cacheValue, x, y);
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..984dbb72 100644
--- a/src/renderer/atlas/LRUMap.ts
+++ b/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];
@@ -85,7 +85,7 @@ export default class LRUMap<T> {
     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) {

From 6528ba84ffecfe7ca230d40a59c06c97092ac948 Mon Sep 17 00:00:00 2001
From: Daniel Imms <daimms@microsoft.com>
Date: Fri, 14 Sep 2018 10:12:43 -0700
Subject: [PATCH 03/26] Avoid creation of array converting index to coordinates

---
 src/renderer/atlas/DynamicCharAtlas.ts | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/src/renderer/atlas/DynamicCharAtlas.ts b/src/renderer/atlas/DynamicCharAtlas.ts
index c3927646..7476a9f4 100644
--- a/src/renderer/atlas/DynamicCharAtlas.ts
+++ b/src/renderer/atlas/DynamicCharAtlas.ts
@@ -138,11 +138,12 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
     return 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(
@@ -155,7 +156,8 @@ 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,
       cacheX,
@@ -252,7 +254,8 @@ 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);
 

From 7762ab96a5b0d51b8bfcba20b4bf736f35f5a808 Mon Sep 17 00:00:00 2001
From: Daniel Imms <daimms@microsoft.com>
Date: Fri, 14 Sep 2018 12:06:14 -0700
Subject: [PATCH 04/26] Exit draw early for the space character

---
 src/renderer/atlas/DynamicCharAtlas.ts | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/renderer/atlas/DynamicCharAtlas.ts b/src/renderer/atlas/DynamicCharAtlas.ts
index 7476a9f4..58aa9a18 100644
--- a/src/renderer/atlas/DynamicCharAtlas.ts
+++ b/src/renderer/atlas/DynamicCharAtlas.ts
@@ -106,6 +106,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 (code === 32) {
+      return true;
+    }
+
     const glyphKey = getGlyphCacheKey(code, fg, bg, bold, dim, italic);
     const cacheValue = this._cacheMap.get(glyphKey);
     if (cacheValue !== null && cacheValue !== undefined) {

From ac3595e8b41b03181ba49d3da08713e4cc074edd Mon Sep 17 00:00:00 2001
From: Daniel Imms <daimms@microsoft.com>
Date: Fri, 14 Sep 2018 12:05:16 -0700
Subject: [PATCH 05/26] Generate and use a bitmap instead of canvas

---
 src/renderer/atlas/DynamicCharAtlas.ts | 55 ++++++++++++++++++++++++--
 1 file changed, 52 insertions(+), 3 deletions(-)

diff --git a/src/renderer/atlas/DynamicCharAtlas.ts b/src/renderer/atlas/DynamicCharAtlas.ts
index 58aa9a18..a00bb472 100644
--- a/src/renderer/atlas/DynamicCharAtlas.ts
+++ b/src/renderer/atlas/DynamicCharAtlas.ts
@@ -29,9 +29,14 @@ const TRANSPARENT_COLOR = {
 // cache.
 const FRAME_CACHE_DRAW_LIMIT = 100;
 
+const GLYPH_BITMAP_COMMIT_DELAY = 100;
+
+const GLYPH_BITMAP_COMMIT_LIMIT = 100;
+
 interface IGlyphCacheValue {
   index: number;
   isEmpty: boolean;
+  inBitmap: boolean;
 }
 
 function getGlyphCacheKey(code: number, fg: number, bg: number, bold: boolean, dim: boolean, italic: boolean): number {
@@ -65,6 +70,11 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
 
   private _drawToCacheCount: number = 0;
 
+  private _glyphsWaitingOnBitmap: Uint32Array = new Uint32Array(GLYPH_BITMAP_COMMIT_LIMIT);
+  private _glyphsWaitingOnBitmapCount: number = 0;
+  private _bitmapCommitTimeout: number | null = null;
+  private _bitmap: ImageBitmap | null = null;
+
   constructor(document: Document, private _config: ICharAtlasConfig) {
     super();
     this._cacheCanvas = document.createElement('canvas');
@@ -124,7 +134,7 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
         // we're out of space, so our call to set will delete this item
         index = this._cacheMap.peek().index;
       }
-      const cacheValue = this._drawToCache(chars, bg, fg, bold, dim, italic, index);
+      const cacheValue = this._drawToCache(chars, code, bg, fg, bold, dim, italic, index);
       this._cacheMap.set(glyphKey, cacheValue);
       this._drawFromCache(ctx, cacheValue, x, y);
       return true;
@@ -164,7 +174,7 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
     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,
@@ -211,6 +221,7 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
   // into a shared function.
   private _drawToCache(
     chars: string,
+    code: number,
     bg: number,
     fg: number,
     bold: boolean,
@@ -264,9 +275,47 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
     // putImageData doesn't do any blending, so it will overwrite any existing cache entry for us
     this._cacheCtx.putImageData(imageData, x, y);
 
+    this._glyphsWaitingOnBitmap[this._glyphsWaitingOnBitmapCount++] = getGlyphCacheKey(code, fg, bg, bold, dim, italic);
+    this._queueGenerateBitmap();
+
     return {
       index,
-      isEmpty
+      isEmpty,
+      inBitmap: false
     };
   }
+
+  private _queueGenerateBitmap(): void {
+    // Check if it's already queued
+    if (this._bitmapCommitTimeout !== null) {
+      return;
+    }
+
+    this._bitmapCommitTimeout = window.setTimeout(() => this._generateBitmap(), GLYPH_BITMAP_COMMIT_DELAY);
+  }
+
+  private _generateBitmap(): void {
+    const countAtGeneration = this._glyphsWaitingOnBitmapCount;
+    // TODO: Fallback when createImageBitmap not supported
+    window.createImageBitmap(this._cacheCanvas).then(bitmap => {
+      // Set bitmap
+      this._bitmap = bitmap;
+
+      // Mark all new glyphs as in bitmap
+      for (let i = 0; i < countAtGeneration; i++) {
+        const key = this._glyphsWaitingOnBitmap[i];
+        this._cacheMap.get(key).inBitmap = true;
+        this._glyphsWaitingOnBitmap[i] = 0;
+      }
+
+      // Fix up any glyphs that were added since image bitmap was created
+      if (countAtGeneration > this._glyphsWaitingOnBitmapCount) {
+        // TODO: Verify this
+        // TODO: Use 2 arrays to speed up set?
+        this._glyphsWaitingOnBitmap.set(this._glyphsWaitingOnBitmap.subarray(countAtGeneration, this._glyphsWaitingOnBitmapCount - countAtGeneration), 0);
+      }
+      this._glyphsWaitingOnBitmapCount -= countAtGeneration;
+    });
+    this._bitmapCommitTimeout = null;
+  }
 }

From 608d78b6ac2f48fc34269e1a70b82af6cf031ef9 Mon Sep 17 00:00:00 2001
From: Daniel Imms <daimms@microsoft.com>
Date: Sat, 15 Sep 2018 09:46:19 -0700
Subject: [PATCH 06/26] Add fallback for window.createImageBitmap

---
 src/renderer/atlas/DynamicCharAtlas.ts | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/renderer/atlas/DynamicCharAtlas.ts b/src/renderer/atlas/DynamicCharAtlas.ts
index a00bb472..4516524f 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.
@@ -286,6 +287,14 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
   }
 
   private _queueGenerateBitmap(): 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 context) || isFirefox || isSafari) {
+      return;
+    }
+
     // Check if it's already queued
     if (this._bitmapCommitTimeout !== null) {
       return;
@@ -296,7 +305,6 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
 
   private _generateBitmap(): void {
     const countAtGeneration = this._glyphsWaitingOnBitmapCount;
-    // TODO: Fallback when createImageBitmap not supported
     window.createImageBitmap(this._cacheCanvas).then(bitmap => {
       // Set bitmap
       this._bitmap = bitmap;

From 00af63b271f9f29031e0494c0216bc5c6d0c5ee0 Mon Sep 17 00:00:00 2001
From: Daniel Imms <daimms@microsoft.com>
Date: Sat, 15 Sep 2018 10:10:50 -0700
Subject: [PATCH 07/26] Expand glyph when needed

---
 src/renderer/atlas/DynamicCharAtlas.ts | 58 +++++++++++++++++++++-----
 1 file changed, 47 insertions(+), 11 deletions(-)

diff --git a/src/renderer/atlas/DynamicCharAtlas.ts b/src/renderer/atlas/DynamicCharAtlas.ts
index 4516524f..e4f4dfb9 100644
--- a/src/renderer/atlas/DynamicCharAtlas.ts
+++ b/src/renderer/atlas/DynamicCharAtlas.ts
@@ -30,9 +30,21 @@ 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;
 
-const GLYPH_BITMAP_COMMIT_LIMIT = 100;
+/**
+ * The initial size of the queue used to track glyphs waiting on bitmap generation.
+ */
+const GLYPHS_WAITING_ON_BITMAP_QUEUE_INITIAL_SIZE = 100;
+
+/**
+ * When the limit of the bitmap queue is reached, the queue increases by this factor.
+ */
+const GLYPHS_WAITING_ON_BITMAP_QUEUE_INCREMENT_FACTOR = 2;
 
 interface IGlyphCacheValue {
   index: number;
@@ -71,9 +83,16 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
 
   private _drawToCacheCount: number = 0;
 
-  private _glyphsWaitingOnBitmap: Uint32Array = new Uint32Array(GLYPH_BITMAP_COMMIT_LIMIT);
+  // An array of glyph keys that are waiting on the bitmap to be generated.
+  private _glyphsWaitingOnBitmapQueue: Uint32Array = new Uint32Array(GLYPHS_WAITING_ON_BITMAP_QUEUE_INITIAL_SIZE);
+
+  // The number of glyphs keys waiting on the bitmap to be generated.
   private _glyphsWaitingOnBitmapCount: number = 0;
+
+  // 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) {
@@ -276,8 +295,8 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
     // putImageData doesn't do any blending, so it will overwrite any existing cache entry for us
     this._cacheCtx.putImageData(imageData, x, y);
 
-    this._glyphsWaitingOnBitmap[this._glyphsWaitingOnBitmapCount++] = getGlyphCacheKey(code, fg, bg, bold, dim, italic);
-    this._queueGenerateBitmap();
+    // Add the glyph and queue it to the bitmap (if the browser supports it)
+    this._addGlyphToBitmap(code, fg, bg, bold, dim, italic);
 
     return {
       index,
@@ -286,7 +305,14 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
     };
   }
 
-  private _queueGenerateBitmap(): void {
+  private _addGlyphToBitmap(
+    code: number,
+    bg: number,
+    fg: number,
+    bold: boolean,
+    dim: boolean,
+    italic: boolean
+  ): 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
@@ -295,7 +321,13 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
       return;
     }
 
-    // Check if it's already queued
+    // Add the glyph to the queue, increasing the size of it if needed
+    if (this._glyphsWaitingOnBitmapCount >= this._glyphsWaitingOnBitmapQueue.length) {
+      this._expandGlyphWaitingOnBitmapQueue();
+    }
+    this._glyphsWaitingOnBitmapQueue[this._glyphsWaitingOnBitmapCount++] = getGlyphCacheKey(code, fg, bg, bold, dim, italic);
+
+    // Check if bitmap generation timeout already exists
     if (this._bitmapCommitTimeout !== null) {
       return;
     }
@@ -303,6 +335,12 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
     this._bitmapCommitTimeout = window.setTimeout(() => this._generateBitmap(), GLYPH_BITMAP_COMMIT_DELAY);
   }
 
+  private _expandGlyphWaitingOnBitmapQueue(): void {
+    const newQueue = new Uint32Array(this._glyphsWaitingOnBitmapQueue.length * GLYPHS_WAITING_ON_BITMAP_QUEUE_INCREMENT_FACTOR);
+    newQueue.set(this._glyphsWaitingOnBitmapQueue, 0);
+    this._glyphsWaitingOnBitmapQueue = newQueue;
+  }
+
   private _generateBitmap(): void {
     const countAtGeneration = this._glyphsWaitingOnBitmapCount;
     window.createImageBitmap(this._cacheCanvas).then(bitmap => {
@@ -311,16 +349,14 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
 
       // Mark all new glyphs as in bitmap
       for (let i = 0; i < countAtGeneration; i++) {
-        const key = this._glyphsWaitingOnBitmap[i];
+        const key = this._glyphsWaitingOnBitmapQueue[i];
         this._cacheMap.get(key).inBitmap = true;
-        this._glyphsWaitingOnBitmap[i] = 0;
+        this._glyphsWaitingOnBitmapQueue[i] = 0;
       }
 
       // Fix up any glyphs that were added since image bitmap was created
       if (countAtGeneration > this._glyphsWaitingOnBitmapCount) {
-        // TODO: Verify this
-        // TODO: Use 2 arrays to speed up set?
-        this._glyphsWaitingOnBitmap.set(this._glyphsWaitingOnBitmap.subarray(countAtGeneration, this._glyphsWaitingOnBitmapCount - countAtGeneration), 0);
+        this._glyphsWaitingOnBitmapQueue.set(this._glyphsWaitingOnBitmapQueue.subarray(countAtGeneration, this._glyphsWaitingOnBitmapCount - countAtGeneration), 0);
       }
       this._glyphsWaitingOnBitmapCount -= countAtGeneration;
     });

From 8d4b5cdc55e70e6335b00f7459b03cac8db94fcd Mon Sep 17 00:00:00 2001
From: Daniel Imms <daimms@microsoft.com>
Date: Sat, 15 Sep 2018 10:38:06 -0700
Subject: [PATCH 08/26] Run yarn coveralls after generating report

---
 azure-pipelines.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/azure-pipelines.yml b/azure-pipelines.yml
index 4ab162c8..cba35b39 100644
--- a/azure-pipelines.yml
+++ b/azure-pipelines.yml
@@ -27,6 +27,7 @@ jobs:
     displayName: 'Lint'
   - script: |
       yarn test-coverage
+      yarn coveralls
     displayName: 'Generate and publish coverage'
 
 - job: macOS

From 39047767e4fa5e1453a699900dcf62c9df47cb28 Mon Sep 17 00:00:00 2001
From: Daniel Imms <daimms@microsoft.com>
Date: Sat, 15 Sep 2018 10:40:54 -0700
Subject: [PATCH 09/26] Move coverage to mac agent as it's much faster

---
 azure-pipelines.yml | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/azure-pipelines.yml b/azure-pipelines.yml
index cba35b39..342ab832 100644
--- a/azure-pipelines.yml
+++ b/azure-pipelines.yml
@@ -25,10 +25,6 @@ jobs:
   - script: |
       yarn lint
     displayName: 'Lint'
-  - script: |
-      yarn test-coverage
-      yarn coveralls
-    displayName: 'Generate and publish coverage'
 
 - job: macOS
   pool:
@@ -47,6 +43,10 @@ jobs:
   - script: |
       yarn lint
     displayName: 'Lint'
+  - script: |
+      yarn test-coverage
+      yarn coveralls
+    displayName: 'Generate and publish coverage'
 
 - job: Windows
   pool:

From 4d7ce4da460aa3201cbecd57a4c3d1894d397456 Mon Sep 17 00:00:00 2001
From: Daniel Imms <daimms@microsoft.com>
Date: Sat, 15 Sep 2018 10:59:41 -0700
Subject: [PATCH 10/26] Set coveralls branch name

---
 azure-pipelines.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/azure-pipelines.yml b/azure-pipelines.yml
index 342ab832..a26bd3d8 100644
--- a/azure-pipelines.yml
+++ b/azure-pipelines.yml
@@ -45,6 +45,7 @@ jobs:
     displayName: 'Lint'
   - script: |
       yarn test-coverage
+      export COVERALLS_GIT_BRANCH=$BUILD_SOURCEBRANCH
       yarn coveralls
     displayName: 'Generate and publish coverage'
 

From 6ea56cd85ea086946342c8b6115ee428e14844fe Mon Sep 17 00:00:00 2001
From: Daniel Imms <daimms@microsoft.com>
Date: Sun, 16 Sep 2018 11:42:09 -0700
Subject: [PATCH 11/26] Remove npm-run-all

We can just use posttest
---
 package.json |  4 +--
 yarn.lock    | 89 ++++------------------------------------------------
 2 files changed, 8 insertions(+), 85 deletions(-)

diff --git a/package.json b/package.json
index 4bdac048..f83e498c 100644
--- a/package.json
+++ b/package.json
@@ -33,7 +33,6 @@
     "merge-stream": "^1.0.1",
     "node-pty": "0.7.6",
     "nodemon": "1.10.2",
-    "npm-run-all": "^4.1.2",
     "nyc": "^11.8.0",
     "sorcery": "^0.10.0",
     "source-map-loader": "^0.2.3",
@@ -51,7 +50,8 @@
     "start": "node demo/start",
     "start-zmodem": "node demo/zmodem/app",
     "lint": "tslint 'src/**/*.ts' './demo/**/*.ts'",
-    "test": "npm-run-all mocha lint",
+    "test": "npm run mocha",
+    "posttest": "npm run lint",
     "test-debug": "node --inspect-brk node_modules/.bin/gulp test",
     "test-suite": "gulp mocha-suite --test",
     "test-coverage": "nyc -x gulpfile.js -x '**/*test*' npm run mocha",
diff --git a/yarn.lock b/yarn.lock
index f12043f7..8cebcd9c 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -338,7 +338,7 @@ ansi-styles@^2.2.1:
   version "2.2.1"
   resolved "https://registry.yarnpkg.com/ansi-styles/-/ansi-styles-2.2.1.tgz#b432dd3358b634cf75e1e4664368240533c1ddbe"
 
-ansi-styles@^3.2.0, ansi-styles@^3.2.1:
+ansi-styles@^3.2.1:
   version "3.2.1"
   resolved "https://registry.yarnpkg.com/ansi-styles/-/ansi-styles-3.2.1.tgz#41fbb20243e50b12be0f04b8dedbf07520ce841d"
   dependencies:
@@ -942,7 +942,7 @@ chalk@^1.0.0, chalk@^1.1.0, chalk@^1.1.3:
     strip-ansi "^3.0.0"
     supports-color "^2.0.0"
 
-chalk@^2.0.0, chalk@^2.1.0, chalk@^2.3.0, chalk@^2.4.1:
+chalk@^2.0.0, chalk@^2.3.0, chalk@^2.4.1:
   version "2.4.1"
   resolved "https://registry.yarnpkg.com/chalk/-/chalk-2.4.1.tgz#18c49ab16a037b6eb0152cc83e3471338215b66e"
   dependencies:
@@ -1333,7 +1333,7 @@ cross-spawn@^5.0.1:
     shebang-command "^1.2.0"
     which "^1.2.9"
 
-cross-spawn@^6.0.4, cross-spawn@^6.0.5:
+cross-spawn@^6.0.5:
   version "6.0.5"
   resolved "https://registry.yarnpkg.com/cross-spawn/-/cross-spawn-6.0.5.tgz#4a5ec7c64dfae22c3a14124dbacdee846d80cbc4"
   dependencies:
@@ -1490,13 +1490,6 @@ defaults@^1.0.0:
   dependencies:
     clone "^1.0.2"
 
-define-properties@^1.1.2:
-  version "1.1.2"
-  resolved "https://registry.yarnpkg.com/define-properties/-/define-properties-1.1.2.tgz#83a73f2fea569898fb737193c8f873caf6d45c94"
-  dependencies:
-    foreach "^2.0.5"
-    object-keys "^1.0.8"
-
 define-property@^0.2.5:
   version "0.2.5"
   resolved "https://registry.yarnpkg.com/define-property/-/define-property-0.2.5.tgz#c35b1ef918ec3c990f9a5bc57be04aacec5c8116"
@@ -1701,24 +1694,6 @@ error-ex@^1.2.0, error-ex@^1.3.1:
   dependencies:
     is-arrayish "^0.2.1"
 
-es-abstract@^1.4.3:
-  version "1.12.0"
-  resolved "https://registry.yarnpkg.com/es-abstract/-/es-abstract-1.12.0.tgz#9dbbdd27c6856f0001421ca18782d786bf8a6165"
-  dependencies:
-    es-to-primitive "^1.1.1"
-    function-bind "^1.1.1"
-    has "^1.0.1"
-    is-callable "^1.1.3"
-    is-regex "^1.0.4"
-
-es-to-primitive@^1.1.1:
-  version "1.1.1"
-  resolved "https://registry.yarnpkg.com/es-to-primitive/-/es-to-primitive-1.1.1.tgz#45355248a88979034b6792e19bb81f2b7975dd0d"
-  dependencies:
-    is-callable "^1.1.1"
-    is-date-object "^1.0.1"
-    is-symbol "^1.0.1"
-
 es5-ext@^0.10.35, es5-ext@^0.10.9, es5-ext@~0.10.14:
   version "0.10.45"
   resolved "https://registry.yarnpkg.com/es5-ext/-/es5-ext-0.10.45.tgz#0bfdf7b473da5919d5adf3bd25ceb754fccc3653"
@@ -2119,10 +2094,6 @@ for-own@^1.0.0:
   dependencies:
     for-in "^1.0.1"
 
-foreach@^2.0.5:
-  version "2.0.5"
-  resolved "https://registry.yarnpkg.com/foreach/-/foreach-2.0.5.tgz#0bee005018aeb260d0a3af3ae658dd0136ec1b99"
-
 foreground-child@^1.5.3, foreground-child@^1.5.6:
   version "1.5.6"
   resolved "https://registry.yarnpkg.com/foreground-child/-/foreground-child-1.5.6.tgz#4fd71ad2dfde96789b980a5c0a295937cb2f5ce9"
@@ -2201,7 +2172,7 @@ fsevents@^1.0.0, fsevents@^1.2.2:
     nan "^2.9.2"
     node-pre-gyp "^0.10.0"
 
-function-bind@^1.0.2, function-bind@^1.1.1:
+function-bind@^1.1.1:
   version "1.1.1"
   resolved "https://registry.yarnpkg.com/function-bind/-/function-bind-1.1.1.tgz#a56899d3ea3c9bab874bb9773b7c5ede92f4895d"
 
@@ -2638,7 +2609,7 @@ has-values@^1.0.0:
     is-number "^3.0.0"
     kind-of "^4.0.0"
 
-has@^1.0.0, has@^1.0.1:
+has@^1.0.0:
   version "1.0.3"
   resolved "https://registry.yarnpkg.com/has/-/has-1.0.3.tgz#722d7cbfc1f6aa8241f16dd814e011e1f41e8796"
   dependencies:
@@ -2897,10 +2868,6 @@ is-builtin-module@^1.0.0:
   dependencies:
     builtin-modules "^1.0.0"
 
-is-callable@^1.1.1, is-callable@^1.1.3:
-  version "1.1.4"
-  resolved "https://registry.yarnpkg.com/is-callable/-/is-callable-1.1.4.tgz#1e1adf219e1eeb684d691f9d6a05ff0d30a24d75"
-
 is-data-descriptor@^0.1.4:
   version "0.1.4"
   resolved "https://registry.yarnpkg.com/is-data-descriptor/-/is-data-descriptor-0.1.4.tgz#0b5ee648388e2c860282e793f1856fec3f301b56"
@@ -2913,10 +2880,6 @@ is-data-descriptor@^1.0.0:
   dependencies:
     kind-of "^6.0.0"
 
-is-date-object@^1.0.1:
-  version "1.0.1"
-  resolved "https://registry.yarnpkg.com/is-date-object/-/is-date-object-1.0.1.tgz#9aa20eb6aeebbff77fbd33e74ca01b33581d3a16"
-
 is-descriptor@^0.1.0:
   version "0.1.6"
   resolved "https://registry.yarnpkg.com/is-descriptor/-/is-descriptor-0.1.6.tgz#366d8240dde487ca51823b1ab9f07a10a78251ca"
@@ -3037,12 +3000,6 @@ is-redirect@^1.0.0:
   version "1.0.0"
   resolved "https://registry.yarnpkg.com/is-redirect/-/is-redirect-1.0.0.tgz#1d03dded53bd8db0f30c26e4f95d36fc7c87dc24"
 
-is-regex@^1.0.4:
-  version "1.0.4"
-  resolved "https://registry.yarnpkg.com/is-regex/-/is-regex-1.0.4.tgz#5517489b547091b0930e095654ced25ee97e9491"
-  dependencies:
-    has "^1.0.1"
-
 is-relative@^1.0.0:
   version "1.0.0"
   resolved "https://registry.yarnpkg.com/is-relative/-/is-relative-1.0.0.tgz#a1bb6935ce8c5dba1e8b9754b9b2dcc020e2260d"
@@ -3053,10 +3010,6 @@ is-stream@^1.0.0, is-stream@^1.0.1, is-stream@^1.1.0:
   version "1.1.0"
   resolved "https://registry.yarnpkg.com/is-stream/-/is-stream-1.1.0.tgz#12d4a3dd4e68e0b79ceb8dbc84173ae80d91ca44"
 
-is-symbol@^1.0.1:
-  version "1.0.1"
-  resolved "https://registry.yarnpkg.com/is-symbol/-/is-symbol-1.0.1.tgz#3cc59f00025194b6ab2e38dbae6689256b660572"
-
 is-typedarray@~1.0.0:
   version "1.0.0"
   resolved "https://registry.yarnpkg.com/is-typedarray/-/is-typedarray-1.0.0.tgz#e479c80858df0c1b11ddda6940f96011fcda4a9a"
@@ -3715,10 +3668,6 @@ memory-fs@^0.4.0, memory-fs@~0.4.1:
     errno "^0.1.3"
     readable-stream "^2.0.1"
 
-memorystream@^0.3.1:
-  version "0.3.1"
-  resolved "https://registry.yarnpkg.com/memorystream/-/memorystream-0.3.1.tgz#86d7090b30ce455d63fbae12dda51a47ddcaf9b2"
-
 merge-descriptors@1.0.1:
   version "1.0.1"
   resolved "https://registry.yarnpkg.com/merge-descriptors/-/merge-descriptors-1.0.1.tgz#b00aaa556dd8b44568150ec9d1b953f3f90cbb61"
@@ -4107,20 +4056,6 @@ npm-packlist@^1.1.6:
     ignore-walk "^3.0.1"
     npm-bundled "^1.0.1"
 
-npm-run-all@^4.1.2:
-  version "4.1.3"
-  resolved "https://registry.yarnpkg.com/npm-run-all/-/npm-run-all-4.1.3.tgz#49f15b55a66bb4101664ce270cb18e7103f8f185"
-  dependencies:
-    ansi-styles "^3.2.0"
-    chalk "^2.1.0"
-    cross-spawn "^6.0.4"
-    memorystream "^0.3.1"
-    minimatch "^3.0.4"
-    ps-tree "^1.1.0"
-    read-pkg "^3.0.0"
-    shell-quote "^1.6.1"
-    string.prototype.padend "^3.0.0"
-
 npm-run-path@^2.0.0:
   version "2.0.2"
   resolved "https://registry.yarnpkg.com/npm-run-path/-/npm-run-path-2.0.2.tgz#35a9232dfa35d7067b4cb2ddf2357b1871536c5f"
@@ -4200,10 +4135,6 @@ object-copy@^0.1.0:
     define-property "^0.2.5"
     kind-of "^3.0.3"
 
-object-keys@^1.0.8:
-  version "1.0.12"
-  resolved "https://registry.yarnpkg.com/object-keys/-/object-keys-1.0.12.tgz#09c53855377575310cca62f55bb334abff7b3ed2"
-
 object-visit@^1.0.0:
   version "1.0.1"
   resolved "https://registry.yarnpkg.com/object-visit/-/object-visit-1.0.1.tgz#f79c4493af0c5377b59fe39d395e41042dd045bb"
@@ -4645,7 +4576,7 @@ prr@~1.0.1:
   version "1.0.1"
   resolved "https://registry.yarnpkg.com/prr/-/prr-1.0.1.tgz#d3fc114ba06995a45ec6893f484ceb1d78f5f476"
 
-ps-tree@^1.0.1, ps-tree@^1.1.0:
+ps-tree@^1.0.1:
   version "1.1.0"
   resolved "https://registry.yarnpkg.com/ps-tree/-/ps-tree-1.1.0.tgz#b421b24140d6203f1ed3c76996b4427b08e8c014"
   dependencies:
@@ -5504,14 +5435,6 @@ string-width@^1.0.1:
     is-fullwidth-code-point "^2.0.0"
     strip-ansi "^4.0.0"
 
-string.prototype.padend@^3.0.0:
-  version "3.0.0"
-  resolved "https://registry.yarnpkg.com/string.prototype.padend/-/string.prototype.padend-3.0.0.tgz#f3aaef7c1719f170c5eab1c32bf780d96e21f2f0"
-  dependencies:
-    define-properties "^1.1.2"
-    es-abstract "^1.4.3"
-    function-bind "^1.0.2"
-
 string_decoder@^1.0.0, string_decoder@~1.1.1:
   version "1.1.1"
   resolved "https://registry.yarnpkg.com/string_decoder/-/string_decoder-1.1.1.tgz#9cf1611ba62685d7030ae9e4ba34149c3af03fc8"

From f6d5abf7b3f205eac66a393bd393dfcfaf377364 Mon Sep 17 00:00:00 2001
From: Daniel Imms <daimms@microsoft.com>
Date: Mon, 17 Sep 2018 06:37:28 -0700
Subject: [PATCH 12/26] Fix scroll APIs not affecting scroll bar

Fixes #1697
---
 src/Viewport.ts | 41 +++++++++++++++++++++++++++++++----------
 1 file changed, 31 insertions(+), 10 deletions(-)

diff --git a/src/Viewport.ts b/src/Viewport.ts
index e89676d9..50d04457 100644
--- a/src/Viewport.ts
+++ b/src/Viewport.ts
@@ -22,6 +22,7 @@ export class Viewport extends Disposable implements IViewport {
   private _lastRecordedViewportHeight: number = 0;
   private _lastRecordedBufferHeight: number = 0;
   private _lastTouchY: number;
+  private _lastScrollTop: number = 0;
 
   // Stores a partial line amount when scrolling, this is used to keep track of how much of a line
   // is scrolled so we can "scroll" over partial lines and feel natural on touchpads. This is a
@@ -97,18 +98,36 @@ export class Viewport extends Disposable implements IViewport {
    * Updates dimensions and synchronizes the scroll area if necessary.
    */
   public syncScrollArea(): void {
+    // If buffer height changed
     if (this._lastRecordedBufferLength !== this._terminal.buffer.lines.length) {
-      // If buffer height changed
       this._lastRecordedBufferLength = this._terminal.buffer.lines.length;
       this._refresh();
-    } else if (this._lastRecordedViewportHeight !== (<any>this._terminal).renderer.dimensions.canvasHeight) {
-      // If viewport height changed
+      return;
+    }
+
+    // If viewport height changed
+    if (this._lastRecordedViewportHeight !== (<any>this._terminal).renderer.dimensions.canvasHeight) {
       this._refresh();
-    } else {
-      // If size has changed, refresh viewport
-      if (this._terminal.renderer.dimensions.scaledCellHeight / window.devicePixelRatio !== this._currentRowHeight) {
-        this._refresh();
-      }
+      return;
+    }
+
+    // If the buffer position doesn't match last scroll top
+    const newScrollTop = this._terminal.buffer.ydisp * this._currentRowHeight;
+    if (this._lastScrollTop !== newScrollTop) {
+      this._refresh();
+      return;
+    }
+
+    // If element's scroll top changed, this can happen when hiding the element
+    if (this._lastScrollTop !== this._viewportElement.scrollTop) {
+      this._refresh();
+      return;
+    }
+
+    // If row height changed
+    if (this._terminal.renderer.dimensions.scaledCellHeight / window.devicePixelRatio !== this._currentRowHeight) {
+      this._refresh();
+      return;
     }
   }
 
@@ -118,6 +137,9 @@ export class Viewport extends Disposable implements IViewport {
    * @param ev The scroll event.
    */
   private _onScroll(ev: Event): void {
+    // Record current scroll top position
+    this._lastScrollTop = this._viewportElement.scrollTop;
+
     // Don't attempt to scroll if the element is not visible, otherwise scrollTop will be corrupt
     // which causes the terminal to scroll the buffer to the top
     if (!this._viewportElement.offsetParent) {
@@ -130,8 +152,7 @@ export class Viewport extends Disposable implements IViewport {
       return;
     }
 
-
-    const newRow = Math.round(this._viewportElement.scrollTop / this._currentRowHeight);
+    const newRow = Math.round(this._lastScrollTop / this._currentRowHeight);
     const diff = newRow - this._terminal.buffer.ydisp;
     this._terminal.scrollLines(diff, true);
   }

From 4656bd433cd2ed2c532c1e6183b3c5943bd5e33d Mon Sep 17 00:00:00 2001
From: Daniel Imms <daimms@microsoft.com>
Date: Mon, 17 Sep 2018 07:37:04 -0700
Subject: [PATCH 13/26] Revert "Remove IGlyphIdentifier in-between object"

This reverts commit 04f2d52f26342dc9a88ba65d28e889a43f38ab39.
---
 src/renderer/BaseRenderLayer.ts        |  8 +--
 src/renderer/atlas/BaseCharAtlas.ts    | 19 ++-----
 src/renderer/atlas/DynamicCharAtlas.ts | 78 +++++++++-----------------
 src/renderer/atlas/NoneCharAtlas.ts    |  9 +--
 src/renderer/atlas/StaticCharAtlas.ts  | 34 +++++------
 src/renderer/atlas/Types.ts            | 10 ++++
 6 files changed, 59 insertions(+), 99 deletions(-)

diff --git a/src/renderer/BaseRenderLayer.ts b/src/renderer/BaseRenderLayer.ts
index d7755aca..1df9c3ea 100644
--- a/src/renderer/BaseRenderLayer.ts
+++ b/src/renderer/BaseRenderLayer.ts
@@ -247,13 +247,7 @@ export abstract class BaseRenderLayer implements IRenderLayer {
     fg += drawInBrightColor ? 8 : 0;
     const atlasDidDraw = this._charAtlas && this._charAtlas.draw(
       this._ctx,
-      chars,
-      code,
-      bg,
-      fg,
-      bold,
-      dim,
-      italic,
+      {chars, code, bg, fg, bold: bold && terminal.options.enableBold, dim, italic},
       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 325818c6..50d35faa 100644
--- a/src/renderer/atlas/BaseCharAtlas.ts
+++ b/src/renderer/atlas/BaseCharAtlas.ts
@@ -3,6 +3,8 @@
  * @license MIT
  */
 
+import { IGlyphIdentifier } from './Types';
+
 export default abstract class BaseCharAtlas {
   private _didWarmUp: boolean = false;
 
@@ -37,27 +39,14 @@ export default abstract class BaseCharAtlas {
    * do nothing and return false in that case.
    *
    * @param ctx Where to draw the character onto.
-   * @param chars The character(s) to draw. This is typically a single character bug can be made up
-   * of multiple when character joiners are used.
-   * @param code The character code.
-   * @param bg The background color.
-   * @param fg The foreground color.
-   * @param bold Whether the text is bold.
-   * @param dim Whether the text is dim.
-   * @param italic Whether the text is italic.
+   * @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,
-    chars: string,
-    code: number,
-    bg: number,
-    fg: number,
-    bold: boolean,
-    dim: boolean,
-    italic: boolean,
+    glyph: IGlyphIdentifier,
     x: number,
     y: number
   ): boolean;
diff --git a/src/renderer/atlas/DynamicCharAtlas.ts b/src/renderer/atlas/DynamicCharAtlas.ts
index e4f4dfb9..2e35b8c5 100644
--- a/src/renderer/atlas/DynamicCharAtlas.ts
+++ b/src/renderer/atlas/DynamicCharAtlas.ts
@@ -3,7 +3,7 @@
  * @license MIT
  */
 
-import { DIM_OPACITY, INVERTED_DEFAULT_COLOR } from './Types';
+import { DIM_OPACITY, IGlyphIdentifier, INVERTED_DEFAULT_COLOR } from './Types';
 import { ICharAtlasConfig } from '../../shared/atlas/Types';
 import { IColor } from '../../shared/Types';
 import BaseCharAtlas from './BaseCharAtlas';
@@ -52,7 +52,7 @@ interface IGlyphCacheValue {
   inBitmap: boolean;
 }
 
-function getGlyphCacheKey(code: number, fg: number, bg: number, bold: boolean, dim: boolean, italic: boolean): number {
+function getGlyphCacheKey(glyph: IGlyphIdentifier): number {
   // Note that this only returns a valid key when code < 256
   // Layout:
   // 0b00000000000000000000000000000001: italic (1)
@@ -62,7 +62,7 @@ function getGlyphCacheKey(code: number, fg: number, bg: number, bold: boolean, d
   // 0b00000000000111111111000000000000: bg (9)
   // 0b00011111111000000000000000000000: code (8)
   // 0b11100000000000000000000000000000: unused (3)
-  return code << 21 | bg << 12 | fg << 3 | (bold ? 0 : 4) + (dim ? 0 : 2) + (italic ? 0 : 1);
+  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 {
@@ -126,27 +126,21 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
 
   public draw(
     ctx: CanvasRenderingContext2D,
-    chars: string,
-    code: number,
-    bg: number,
-    fg: number,
-    bold: boolean,
-    dim: boolean,
-    italic: boolean,
+    glyph: IGlyphIdentifier,
     x: number,
     y: number
   ): boolean {
     // Space is always an empty cell, special case this as it's so common
-    if (code === 32) {
+    if (glyph.code === 32) {
       return true;
     }
 
-    const glyphKey = getGlyphCacheKey(code, fg, bg, bold, dim, italic);
+    const glyphKey = getGlyphCacheKey(glyph);
     const cacheValue = this._cacheMap.get(glyphKey);
     if (cacheValue !== null && cacheValue !== undefined) {
       this._drawFromCache(ctx, cacheValue, x, y);
       return true;
-    } else if (this._canCache(code) && this._drawToCacheCount < FRAME_CACHE_DRAW_LIMIT) {
+    } else if (this._canCache(glyph) && this._drawToCacheCount < FRAME_CACHE_DRAW_LIMIT) {
       let index;
       if (this._cacheMap.size < this._cacheMap.capacity) {
         index = this._cacheMap.size;
@@ -154,7 +148,7 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
         // we're out of space, so our call to set will delete this item
         index = this._cacheMap.peek().index;
       }
-      const cacheValue = this._drawToCache(chars, code, bg, fg, bold, dim, italic, index);
+      const cacheValue = this._drawToCache(glyph, index);
       this._cacheMap.set(glyphKey, cacheValue);
       this._drawFromCache(ctx, cacheValue, x, y);
       return true;
@@ -162,7 +156,7 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
     return false;
   }
 
-  private _canCache(code: number): boolean {
+  private _canCache(glyph: IGlyphIdentifier): boolean {
     // Only cache ascii and extended characters for now, to be safe. In the future, we could do
     // something more complicated to determine the expected width of a character.
     //
@@ -170,7 +164,7 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
     // to draw overlapping glyphs from the atlas:
     // https://github.com/servo/webrender/issues/464#issuecomment-255632875
     // https://webglfundamentals.org/webgl/lessons/webgl-text-texture.html
-    return code < 256;
+    return glyph.code < 256;
   }
 
   private _toCoordinateX(index: number): number {
@@ -213,48 +207,39 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
     return DEFAULT_ANSI_COLORS[idx];
   }
 
-  private _getBackgroundColor(bg: number): IColor {
+  private _getBackgroundColor(glyph: IGlyphIdentifier): IColor {
     if (this._config.allowTransparency) {
       // The background color might have some transparency, so we need to render it as fully
       // transparent in the atlas. Otherwise we'd end up drawing the transparent background twice
       // around the anti-aliased edges of the glyph, and it would look too dark.
       return TRANSPARENT_COLOR;
-    } else if (bg === INVERTED_DEFAULT_COLOR) {
+    } else if (glyph.bg === INVERTED_DEFAULT_COLOR) {
       return this._config.colors.foreground;
-    } else if (bg < 256) {
-      return this._getColorFromAnsiIndex(bg);
+    } else if (glyph.bg < 256) {
+      return this._getColorFromAnsiIndex(glyph.bg);
     }
     return this._config.colors.background;
   }
 
-  private _getForegroundColor(fg: number): IColor {
-    if (fg === INVERTED_DEFAULT_COLOR) {
+  private _getForegroundColor(glyph: IGlyphIdentifier): IColor {
+    if (glyph.fg === INVERTED_DEFAULT_COLOR) {
       return this._config.colors.background;
-    } else if (fg < 256) {
+    } else if (glyph.fg < 256) {
       // 256 color support
-      return this._getColorFromAnsiIndex(fg);
+      return this._getColorFromAnsiIndex(glyph.fg);
     }
     return this._config.colors.foreground;
   }
 
   // TODO: We do this (or something similar) in multiple places. We should split this off
   // into a shared function.
-  private _drawToCache(
-    chars: string,
-    code: number,
-    bg: number,
-    fg: number,
-    bold: boolean,
-    dim: boolean,
-    italic: boolean,
-    index: number
-  ): IGlyphCacheValue {
+  private _drawToCache(glyph: IGlyphIdentifier, index: number): IGlyphCacheValue {
     this._drawToCacheCount++;
 
     this._tmpCtx.save();
 
     // draw the background
-    const backgroundColor = this._getBackgroundColor(bg);
+    const backgroundColor = this._getBackgroundColor(glyph);
     // Use a 'copy' composite operation to clear any existing glyph out of _tmpCtxWithAlpha, regardless of
     // transparency in backgroundColor
     this._tmpCtx.globalCompositeOperation = 'copy';
@@ -263,20 +248,20 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
     this._tmpCtx.globalCompositeOperation = 'source-over';
 
     // draw the foreground/glyph
-    const fontWeight = bold ? this._config.fontWeightBold : this._config.fontWeight;
-    const fontStyle = italic ? 'italic' : '';
+    const fontWeight = glyph.bold ? this._config.fontWeightBold : this._config.fontWeight;
+    const fontStyle = glyph.italic ? 'italic' : '';
     this._tmpCtx.font =
       `${fontStyle} ${fontWeight} ${this._config.fontSize * this._config.devicePixelRatio}px ${this._config.fontFamily}`;
     this._tmpCtx.textBaseline = 'top';
 
-    this._tmpCtx.fillStyle = this._getForegroundColor(fg).css;
+    this._tmpCtx.fillStyle = this._getForegroundColor(glyph).css;
 
     // Apply alpha to dim the character
-    if (dim) {
+    if (glyph.dim) {
       this._tmpCtx.globalAlpha = DIM_OPACITY;
     }
     // Draw the character
-    this._tmpCtx.fillText(chars, 0, 0);
+    this._tmpCtx.fillText(glyph.chars, 0, 0);
     this._tmpCtx.restore();
 
     // clear the background from the character to avoid issues with drawing over the previous
@@ -296,7 +281,7 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
     this._cacheCtx.putImageData(imageData, x, y);
 
     // Add the glyph and queue it to the bitmap (if the browser supports it)
-    this._addGlyphToBitmap(code, fg, bg, bold, dim, italic);
+    this._addGlyphToBitmap(glyph);
 
     return {
       index,
@@ -305,14 +290,7 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
     };
   }
 
-  private _addGlyphToBitmap(
-    code: number,
-    bg: number,
-    fg: number,
-    bold: boolean,
-    dim: boolean,
-    italic: boolean
-  ): void {
+  private _addGlyphToBitmap(glyph: IGlyphIdentifier): 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
@@ -325,7 +303,7 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
     if (this._glyphsWaitingOnBitmapCount >= this._glyphsWaitingOnBitmapQueue.length) {
       this._expandGlyphWaitingOnBitmapQueue();
     }
-    this._glyphsWaitingOnBitmapQueue[this._glyphsWaitingOnBitmapCount++] = getGlyphCacheKey(code, fg, bg, bold, dim, italic);
+    this._glyphsWaitingOnBitmapQueue[this._glyphsWaitingOnBitmapCount++] = getGlyphCacheKey(glyph);
 
     // Check if bitmap generation timeout already exists
     if (this._bitmapCommitTimeout !== null) {
diff --git a/src/renderer/atlas/NoneCharAtlas.ts b/src/renderer/atlas/NoneCharAtlas.ts
index 163baf38..1cbc9eea 100644
--- a/src/renderer/atlas/NoneCharAtlas.ts
+++ b/src/renderer/atlas/NoneCharAtlas.ts
@@ -5,6 +5,7 @@
  * A dummy CharAtlas implementation that always fails to draw characters.
  */
 
+import { IGlyphIdentifier } from './Types';
 import { ICharAtlasConfig } from '../../shared/atlas/Types';
 import BaseCharAtlas from './BaseCharAtlas';
 
@@ -15,13 +16,7 @@ export default class NoneCharAtlas extends BaseCharAtlas {
 
   public draw(
     ctx: CanvasRenderingContext2D,
-    chars: string,
-    code: number,
-    bg: number,
-    fg: number,
-    bold: boolean,
-    dim: boolean,
-    italic: boolean,
+    glyph: IGlyphIdentifier,
     x: number,
     y: number
   ): boolean {
diff --git a/src/renderer/atlas/StaticCharAtlas.ts b/src/renderer/atlas/StaticCharAtlas.ts
index 0e022fa6..c0d8a814 100644
--- a/src/renderer/atlas/StaticCharAtlas.ts
+++ b/src/renderer/atlas/StaticCharAtlas.ts
@@ -3,7 +3,7 @@
  * @license MIT
  */
 
-import { DIM_OPACITY } from './Types';
+import { DIM_OPACITY, IGlyphIdentifier } from './Types';
 import { CHAR_ATLAS_CELL_SPACING, ICharAtlasConfig } from '../../shared/atlas/Types';
 import { generateStaticCharAtlasTexture } from '../../shared/atlas/CharAtlasGenerator';
 import BaseCharAtlas from './BaseCharAtlas';
@@ -37,24 +37,18 @@ export default class StaticCharAtlas extends BaseCharAtlas {
     }
   }
 
-  private _isCached(code: number, fg: number, bg: number, italic: boolean): boolean {
-    const isAscii = code < 256;
+  private _isCached(glyph: IGlyphIdentifier, colorIndex: number): boolean {
+    const isAscii = glyph.code < 256;
     // A color is basic if it is one of the 4 bit ANSI colors.
-    const isBasicColor = fg < 16;
-    const isDefaultColor = fg >= 256;
-    const isDefaultBackground = bg >= 256;
-    return isAscii && (isBasicColor || isDefaultColor) && isDefaultBackground && !italic;
+    const isBasicColor = glyph.fg < 16;
+    const isDefaultColor = glyph.fg >= 256;
+    const isDefaultBackground = glyph.bg >= 256;
+    return isAscii && (isBasicColor || isDefaultColor) && isDefaultBackground && !glyph.italic;
   }
 
   public draw(
     ctx: CanvasRenderingContext2D,
-    chars: string,
-    code: number,
-    bg: number,
-    fg: number,
-    bold: boolean,
-    dim: boolean,
-    italic: boolean,
+    glyph: IGlyphIdentifier,
     x: number,
     y: number
   ): boolean {
@@ -64,15 +58,15 @@ export default class StaticCharAtlas extends BaseCharAtlas {
     }
 
     let colorIndex = 0;
-    if (fg < 256) {
-      colorIndex = 2 + fg + (bold ? 16 : 0);
+    if (glyph.fg < 256) {
+      colorIndex = 2 + glyph.fg + (glyph.bold ? 16 : 0);
     } else {
       // If default color and bold
-      if (bold) {
+      if (glyph.bold) {
         colorIndex = 1;
       }
     }
-    if (!this._isCached(code, fg, bg, italic)) {
+    if (!this._isCached(glyph, colorIndex)) {
       return false;
     }
 
@@ -83,13 +77,13 @@ export default class StaticCharAtlas extends BaseCharAtlas {
     const charAtlasCellHeight = this._config.scaledCharHeight + CHAR_ATLAS_CELL_SPACING;
 
     // Apply alpha to dim the character
-    if (dim) {
+    if (glyph.dim) {
       ctx.globalAlpha = DIM_OPACITY;
     }
 
     ctx.drawImage(
       this._texture,
-      code * charAtlasCellWidth,
+      glyph.code * charAtlasCellWidth,
       colorIndex * charAtlasCellHeight,
       charAtlasCellWidth,
       this._config.scaledCharHeight,
diff --git a/src/renderer/atlas/Types.ts b/src/renderer/atlas/Types.ts
index 34f01d39..6fb3c5d1 100644
--- a/src/renderer/atlas/Types.ts
+++ b/src/renderer/atlas/Types.ts
@@ -5,3 +5,13 @@
 
 export const INVERTED_DEFAULT_COLOR = -1;
 export const DIM_OPACITY = 0.5;
+
+export interface IGlyphIdentifier {
+  chars: string;
+  code: number;
+  bg: number;
+  fg: number;
+  bold: boolean;
+  dim: boolean;
+  italic: boolean;
+}

From 202734f10c2fea3b75dba03dd804f00fa9798767 Mon Sep 17 00:00:00 2001
From: Daniel Imms <daimms@microsoft.com>
Date: Mon, 17 Sep 2018 07:43:10 -0700
Subject: [PATCH 14/26] Fix typo which broke dynamic atlas

---
 src/renderer/atlas/DynamicCharAtlas.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/renderer/atlas/DynamicCharAtlas.ts b/src/renderer/atlas/DynamicCharAtlas.ts
index 2e35b8c5..e025ef34 100644
--- a/src/renderer/atlas/DynamicCharAtlas.ts
+++ b/src/renderer/atlas/DynamicCharAtlas.ts
@@ -295,7 +295,7 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
     // 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 context) || isFirefox || isSafari) {
+    if (!('createImageBitmap' in window) || isFirefox || isSafari) {
       return;
     }
 

From 94b20d40bf298576fc207ea583a01fe21ddfd7a2 Mon Sep 17 00:00:00 2001
From: Daniel Imms <daimms@microsoft.com>
Date: Mon, 17 Sep 2018 07:43:31 -0700
Subject: [PATCH 15/26] Reuse object for glyph identifier

---
 src/renderer/BaseRenderLayer.ts | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/src/renderer/BaseRenderLayer.ts b/src/renderer/BaseRenderLayer.ts
index 1df9c3ea..55cc747e 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,
@@ -245,9 +258,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
     );

From d9ec47ab7aaa37bdc773edab29f0226a14f57fa2 Mon Sep 17 00:00:00 2001
From: Daniel Imms <daimms@microsoft.com>
Date: Mon, 17 Sep 2018 07:48:05 -0700
Subject: [PATCH 16/26] Clear bitmap timeout on dispose

---
 src/renderer/BaseRenderLayer.ts        | 1 +
 src/renderer/atlas/BaseCharAtlas.ts    | 5 ++++-
 src/renderer/atlas/DynamicCharAtlas.ts | 7 +++++++
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/renderer/BaseRenderLayer.ts b/src/renderer/BaseRenderLayer.ts
index 55cc747e..1590d6e2 100644
--- a/src/renderer/BaseRenderLayer.ts
+++ b/src/renderer/BaseRenderLayer.ts
@@ -51,6 +51,7 @@ export abstract class BaseRenderLayer implements IRenderLayer {
 
   public dispose(): void {
     this._container.removeChild(this._canvas);
+    this._charAtlas.dispose();
   }
 
   private _initCanvas(): void {
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 e025ef34..31deff55 100644
--- a/src/renderer/atlas/DynamicCharAtlas.ts
+++ b/src/renderer/atlas/DynamicCharAtlas.ts
@@ -120,6 +120,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;
   }

From cbe7b21090b7744b5ab3116b20db0bd12215c8d3 Mon Sep 17 00:00:00 2001
From: Daniel Imms <daimms@microsoft.com>
Date: Mon, 17 Sep 2018 07:52:37 -0700
Subject: [PATCH 17/26] Don't unlink node when updating inBitmap and verify
 undefined

---
 src/renderer/atlas/DynamicCharAtlas.ts |  6 +++++-
 src/renderer/atlas/LRUMap.ts           | 11 +++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/src/renderer/atlas/DynamicCharAtlas.ts b/src/renderer/atlas/DynamicCharAtlas.ts
index 31deff55..04d28c87 100644
--- a/src/renderer/atlas/DynamicCharAtlas.ts
+++ b/src/renderer/atlas/DynamicCharAtlas.ts
@@ -335,7 +335,11 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
       // Mark all new glyphs as in bitmap
       for (let i = 0; i < countAtGeneration; i++) {
         const key = this._glyphsWaitingOnBitmapQueue[i];
-        this._cacheMap.get(key).inBitmap = true;
+        const value = this._cacheMap.peekValue(key);
+        // If the value has already been evicted, do nothing
+        if (value) {
+          value.inBitmap = true;
+        }
         this._glyphsWaitingOnBitmapQueue[i] = 0;
       }
 
diff --git a/src/renderer/atlas/LRUMap.ts b/src/renderer/atlas/LRUMap.ts
index 984dbb72..d7e01ec6 100644
--- a/src/renderer/atlas/LRUMap.ts
+++ b/src/renderer/atlas/LRUMap.ts
@@ -80,6 +80,17 @@ 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;

From 4463f8d8102c4ae490b5aca0e53e7048263c2ddf Mon Sep 17 00:00:00 2001
From: Daniel Imms <daimms@microsoft.com>
Date: Wed, 19 Sep 2018 19:47:16 -0700
Subject: [PATCH 18/26] Speculative fix for NPE

Fixes #1702
---
 src/Linkifier.ts | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/Linkifier.ts b/src/Linkifier.ts
index 4d84b1a7..eec1688c 100644
--- a/src/Linkifier.ts
+++ b/src/Linkifier.ts
@@ -81,18 +81,22 @@ export class Linkifier extends EventEmitter implements ILinkifier {
    */
   private _linkifyRows(): void {
     this._rowsTimeoutId = null;
+    const buffer = this._terminal.buffer;
 
-    // Ensure the row exists
-    const absoluteRowIndexStart = this._terminal.buffer.ydisp + this._rowsToLinkify.start;
-    if (absoluteRowIndexStart >= this._terminal.buffer.lines.length) {
+    // Ensure the start row exists
+    const absoluteRowIndexStart = buffer.ydisp + this._rowsToLinkify.start;
+    if (absoluteRowIndexStart >= buffer.lines.length) {
       return;
     }
 
+    // Invalidate bad end row values (if a resize happened)
+    const absoluteRowIndexEnd = Math.min(buffer.ydisp + this._rowsToLinkify.end + 1, buffer.ydisp + this._terminal.rows);
+
     // 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 = this._terminal.buffer.iterator(false, absoluteRowIndexStart, this._terminal.buffer.ydisp + this._rowsToLinkify.end + 1);
+    const iterator = buffer.iterator(false, absoluteRowIndexStart, absoluteRowIndexEnd);
     while (iterator.hasNext()) {
       const lineData: IBufferStringIteratorResult = iterator.next();
       for (let i = 0; i < this._linkMatchers.length; i++) {

From 2c62c3a8c33c8268a4fe03ffa45a10224640e3a2 Mon Sep 17 00:00:00 2001
From: Daniel Imms <daimms@microsoft.com>
Date: Wed, 19 Sep 2018 20:06:09 -0700
Subject: [PATCH 19/26] Fix tests

---
 src/Linkifier.test.ts | 2 ++
 src/Linkifier.ts      | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/Linkifier.test.ts b/src/Linkifier.test.ts
index 35610acb..1e4c0cdc 100644
--- a/src/Linkifier.test.ts
+++ b/src/Linkifier.test.ts
@@ -42,6 +42,7 @@ describe('Linkifier', () => {
   beforeEach(() => {
     terminal = new MockTerminal();
     terminal.cols = 100;
+    terminal.rows = 10;
     terminal.buffer = new MockBuffer();
     (<MockBuffer>terminal.buffer).setLines(new CircularList<IBufferLine>(20));
     terminal.buffer.ydisp = 0;
@@ -64,6 +65,7 @@ describe('Linkifier', () => {
   function assertLinkifiesRow(rowText: string, linkMatcherRegex: RegExp, links: {x: number, length: number}[], done: MochaDone): void {
     addRow(rowText);
     linkifier.registerLinkMatcher(linkMatcherRegex, () => {});
+    terminal.rows = terminal.buffer.lines.length - 1;
     linkifier.linkifyRows();
     // Allow linkify to happen
     setTimeout(() => {
diff --git a/src/Linkifier.ts b/src/Linkifier.ts
index eec1688c..0dd33a85 100644
--- a/src/Linkifier.ts
+++ b/src/Linkifier.ts
@@ -90,7 +90,7 @@ export class Linkifier extends EventEmitter implements ILinkifier {
     }
 
     // Invalidate bad end row values (if a resize happened)
-    const absoluteRowIndexEnd = Math.min(buffer.ydisp + this._rowsToLinkify.end + 1, buffer.ydisp + this._terminal.rows);
+    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

From 39a7b18065647e289419a0824147f6af1be85900 Mon Sep 17 00:00:00 2001
From: Daniel Imms <daimms@microsoft.com>
Date: Thu, 20 Sep 2018 10:09:39 -0700
Subject: [PATCH 20/26] Keep track of glyphs using cache values, not keys

---
 src/renderer/atlas/DynamicCharAtlas.ts | 52 +++++++-------------------
 1 file changed, 14 insertions(+), 38 deletions(-)

diff --git a/src/renderer/atlas/DynamicCharAtlas.ts b/src/renderer/atlas/DynamicCharAtlas.ts
index 04d28c87..d8654784 100644
--- a/src/renderer/atlas/DynamicCharAtlas.ts
+++ b/src/renderer/atlas/DynamicCharAtlas.ts
@@ -36,16 +36,6 @@ const FRAME_CACHE_DRAW_LIMIT = 100;
  */
 const GLYPH_BITMAP_COMMIT_DELAY = 100;
 
-/**
- * The initial size of the queue used to track glyphs waiting on bitmap generation.
- */
-const GLYPHS_WAITING_ON_BITMAP_QUEUE_INITIAL_SIZE = 100;
-
-/**
- * When the limit of the bitmap queue is reached, the queue increases by this factor.
- */
-const GLYPHS_WAITING_ON_BITMAP_QUEUE_INCREMENT_FACTOR = 2;
-
 interface IGlyphCacheValue {
   index: number;
   isEmpty: boolean;
@@ -84,7 +74,7 @@ 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 _glyphsWaitingOnBitmapQueue: Uint32Array = new Uint32Array(GLYPHS_WAITING_ON_BITMAP_QUEUE_INITIAL_SIZE);
+  private _glyphsWaitingOnBitmapQueue: IGlyphCacheValue[] = [];
 
   // The number of glyphs keys waiting on the bitmap to be generated.
   private _glyphsWaitingOnBitmapCount: number = 0;
@@ -288,16 +278,17 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
     this._cacheCtx.putImageData(imageData, x, y);
 
     // Add the glyph and queue it to the bitmap (if the browser supports it)
-    this._addGlyphToBitmap(glyph);
-
-    return {
+    const cacheValue = {
       index,
       isEmpty,
       inBitmap: false
     };
+    this._addGlyphToBitmap(cacheValue);
+
+    return cacheValue;
   }
 
-  private _addGlyphToBitmap(glyph: IGlyphIdentifier): void {
+  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
@@ -306,11 +297,9 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
       return;
     }
 
-    // Add the glyph to the queue, increasing the size of it if needed
-    if (this._glyphsWaitingOnBitmapCount >= this._glyphsWaitingOnBitmapQueue.length) {
-      this._expandGlyphWaitingOnBitmapQueue();
-    }
-    this._glyphsWaitingOnBitmapQueue[this._glyphsWaitingOnBitmapCount++] = getGlyphCacheKey(glyph);
+    // Add the glyph to the queue
+    this._glyphsWaitingOnBitmapQueue.push(cacheValue);
+    this._glyphsWaitingOnBitmapCount++;
 
     // Check if bitmap generation timeout already exists
     if (this._bitmapCommitTimeout !== null) {
@@ -320,34 +309,21 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
     this._bitmapCommitTimeout = window.setTimeout(() => this._generateBitmap(), GLYPH_BITMAP_COMMIT_DELAY);
   }
 
-  private _expandGlyphWaitingOnBitmapQueue(): void {
-    const newQueue = new Uint32Array(this._glyphsWaitingOnBitmapQueue.length * GLYPHS_WAITING_ON_BITMAP_QUEUE_INCREMENT_FACTOR);
-    newQueue.set(this._glyphsWaitingOnBitmapQueue, 0);
-    this._glyphsWaitingOnBitmapQueue = newQueue;
-  }
-
   private _generateBitmap(): void {
-    const countAtGeneration = this._glyphsWaitingOnBitmapCount;
+    let countAtGeneration = this._glyphsWaitingOnBitmapCount;
     window.createImageBitmap(this._cacheCanvas).then(bitmap => {
       // Set bitmap
       this._bitmap = bitmap;
 
-      // Mark all new glyphs as in bitmap
-      for (let i = 0; i < countAtGeneration; i++) {
-        const key = this._glyphsWaitingOnBitmapQueue[i];
-        const value = this._cacheMap.peekValue(key);
+      // Mark all new glyphs as in bitmap, excluding glyphs that came in after
+      // the bitmap was requested
+      while (countAtGeneration-- > 0) {
+        const value = this._glyphsWaitingOnBitmapQueue[0];
         // If the value has already been evicted, do nothing
         if (value) {
           value.inBitmap = true;
         }
-        this._glyphsWaitingOnBitmapQueue[i] = 0;
       }
-
-      // Fix up any glyphs that were added since image bitmap was created
-      if (countAtGeneration > this._glyphsWaitingOnBitmapCount) {
-        this._glyphsWaitingOnBitmapQueue.set(this._glyphsWaitingOnBitmapQueue.subarray(countAtGeneration, this._glyphsWaitingOnBitmapCount - countAtGeneration), 0);
-      }
-      this._glyphsWaitingOnBitmapCount -= countAtGeneration;
     });
     this._bitmapCommitTimeout = null;
   }

From 478d824fa788773679c58309b1a900da580c5d73 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Agusti=CC=81n=20Rodri=CC=81guez?= <agurz@icloud.com>
Date: Thu, 20 Sep 2018 01:52:37 -0300
Subject: [PATCH 21/26] fix underline don't appearing when using the fallback
 DOM renderer

---
 src/renderer/dom/DomRenderer.ts | 51 ++++++++++++++++++++++++++++++++-
 1 file changed, 50 insertions(+), 1 deletion(-)

diff --git a/src/renderer/dom/DomRenderer.ts b/src/renderer/dom/DomRenderer.ts
index 9a2ef469..399ac406 100644
--- a/src/renderer/dom/DomRenderer.ts
+++ b/src/renderer/dom/DomRenderer.ts
@@ -4,12 +4,13 @@
  */
 
 import { IRenderer, IRenderDimensions, IColorSet } from '../Types';
-import { ITerminal, CharacterJoinerHandler } from '../../Types';
+import { ILinkHoverEvent, ITerminal, CharacterJoinerHandler, LinkHoverEventTypes } from '../../Types';
 import { ITheme } from 'xterm';
 import { EventEmitter } from '../../common/EventEmitter';
 import { ColorManager } from '../ColorManager';
 import { RenderDebouncer } from '../../ui/RenderDebouncer';
 import { BOLD_CLASS, ITALIC_CLASS, CURSOR_CLASS, CURSOR_STYLE_BLOCK_CLASS, CURSOR_STYLE_BAR_CLASS, CURSOR_STYLE_UNDERLINE_CLASS, DomRendererRowFactory } from './DomRendererRowFactory';
+import { INVERTED_DEFAULT_COLOR } from '../atlas/Types';
 
 const TERMINAL_CLASS_PREFIX = 'xterm-dom-renderer-owner-';
 const ROW_CONTAINER_CLASS = 'xterm-rows';
@@ -79,6 +80,9 @@ export class DomRenderer extends EventEmitter implements IRenderer {
     this._terminal.element.classList.add(TERMINAL_CLASS_PREFIX + this._terminalClass);
     this._terminal.screenElement.appendChild(this._rowContainer);
     this._terminal.screenElement.appendChild(this._selectionContainer);
+
+    this._terminal.linkifier.on(LinkHoverEventTypes.HOVER, (e: ILinkHoverEvent) => this._onLinkHover(e));
+    this._terminal.linkifier.on(LinkHoverEventTypes.LEAVE, (e: ILinkHoverEvent) => this._onLinkLeave(e));
   }
 
   public dispose(): void {
@@ -116,6 +120,7 @@ export class DomRenderer extends EventEmitter implements IRenderer {
 
     const styles =
         `${this._terminalSelector} .${ROW_CONTAINER_CLASS} span {` +
+        ` box-sizing: border-box;` +
         ` display: inline-block;` +
         ` height: 100%;` +
         ` vertical-align: top;` +
@@ -338,4 +343,48 @@ export class DomRenderer extends EventEmitter implements IRenderer {
 
   public registerCharacterJoiner(handler: CharacterJoinerHandler): number { return -1; }
   public deregisterCharacterJoiner(joinerId: number): boolean { return false; }
+
+  private _onLinkHover(e: ILinkHoverEvent): void {
+    let color = this.colorManager.colors.foreground.css;
+
+    if (e.fg === INVERTED_DEFAULT_COLOR) {
+      color = this.colorManager.colors.background.css;
+    } else if (e.fg < 256) {
+      // 256 color support
+      color = this.colorManager.colors.ansi[e.fg].css;
+    }
+
+    this._setBorderBottomAtCells(e.x1, e.x2, e.y1, e.y2, e.cols, `1px solid ${color}`);
+  }
+
+  private _onLinkLeave(e: ILinkHoverEvent): void {
+    this._setBorderBottomAtCells(e.x1, e.x2, e.y1, e.y2, e.cols, null);
+  }
+
+  private _setBorderBottomAtCells(x1: number, x2: number, y1: number, y2: number, cols: number, value?: string) {
+    if (y1 === y2) {
+      // Single line link
+      for (let x = x1; x < x2; x++) {
+        let span = (<HTMLElement>this._rowElements[y1].children[x]);
+        span.style.borderBottom = value;
+      }
+    } else {
+      // Multi-line link
+      for (let x = x1; x < cols - x1; x++) {
+        let span = (<HTMLElement>this._rowElements[y1].children[x]);
+        span.style.borderBottom = value;
+      }
+      for (let y = y1 + 1; y < y2 - 1; y++) {
+        for (let x = 0; x < cols; x++) {
+          let span = (<HTMLElement>this._rowElements[y].children[x]);
+          span.style.borderBottom = value;
+        }
+      }
+      for (let x = 0; x < x2; x++) {
+        let span = (<HTMLElement>this._rowElements[y2].children[x]);
+        span.style.borderBottom = value;
+      }
+    }
+  }
+
 }

From d1991396e758f5d36ecce8403bf56f046b65d42f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Agusti=CC=81n=20Rodri=CC=81guez?= <agurz@icloud.com>
Date: Fri, 21 Sep 2018 01:09:20 -0300
Subject: [PATCH 22/26] generalized loops to handle both single and multi line
 links

---
 src/renderer/dom/DomRenderer.ts | 27 ++++-----------------------
 1 file changed, 4 insertions(+), 23 deletions(-)

diff --git a/src/renderer/dom/DomRenderer.ts b/src/renderer/dom/DomRenderer.ts
index 399ac406..9933276c 100644
--- a/src/renderer/dom/DomRenderer.ts
+++ b/src/renderer/dom/DomRenderer.ts
@@ -361,29 +361,10 @@ export class DomRenderer extends EventEmitter implements IRenderer {
     this._setBorderBottomAtCells(e.x1, e.x2, e.y1, e.y2, e.cols, null);
   }
 
-  private _setBorderBottomAtCells(x1: number, x2: number, y1: number, y2: number, cols: number, value?: string) {
-    if (y1 === y2) {
-      // Single line link
-      for (let x = x1; x < x2; x++) {
-        let span = (<HTMLElement>this._rowElements[y1].children[x]);
-        span.style.borderBottom = value;
-      }
-    } else {
-      // Multi-line link
-      for (let x = x1; x < cols - x1; x++) {
-        let span = (<HTMLElement>this._rowElements[y1].children[x]);
-        span.style.borderBottom = value;
-      }
-      for (let y = y1 + 1; y < y2 - 1; y++) {
-        for (let x = 0; x < cols; x++) {
-          let span = (<HTMLElement>this._rowElements[y].children[x]);
-          span.style.borderBottom = value;
-        }
-      }
-      for (let x = 0; x < x2; x++) {
-        let span = (<HTMLElement>this._rowElements[y2].children[x]);
-        span.style.borderBottom = value;
-      }
+  private _setBorderBottomAtCells(x: number, x2: number, y: number, y2: number, cols: number, value?: string) {
+    for (; x != x2 || y != y2; x = ++x % cols, y += +(x === 0)) {
+      let span = (<HTMLElement>this._rowElements[y].children[x]);
+      span.style.borderBottom = value;
     }
   }
 

From 1f2d223e649e0ecf63839806252737d254a8aac9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Agusti=CC=81n=20Rodri=CC=81guez?= <agurz@icloud.com>
Date: Fri, 21 Sep 2018 01:42:20 -0300
Subject: [PATCH 23/26] remove line in xterm.d.ts that says link underlines are
 not supported

---
 typings/xterm.d.ts | 1 -
 1 file changed, 1 deletion(-)

diff --git a/typings/xterm.d.ts b/typings/xterm.d.ts
index cc1ebcd9..49f3810f 100644
--- a/typings/xterm.d.ts
+++ b/typings/xterm.d.ts
@@ -148,7 +148,6 @@ declare module 'xterm' {
      * when canvas is too slow for the environment. The following features do
      * not work when the DOM renderer is used:
      *
-     * - Link underlines
      * - Line height
      * - Letter spacing
      * - Cursor blink

From 4941d5a19e5f81b40982d95259c00b74a0a7e552 Mon Sep 17 00:00:00 2001
From: Daniel Imms <daimms@microsoft.com>
Date: Fri, 21 Sep 2018 07:08:17 -0700
Subject: [PATCH 24/26] Simplify loop, pass lint

---
 src/renderer/dom/DomRenderer.ts | 28 ++++++++++------------------
 1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/src/renderer/dom/DomRenderer.ts b/src/renderer/dom/DomRenderer.ts
index 9933276c..8f51c4ef 100644
--- a/src/renderer/dom/DomRenderer.ts
+++ b/src/renderer/dom/DomRenderer.ts
@@ -10,7 +10,6 @@ import { EventEmitter } from '../../common/EventEmitter';
 import { ColorManager } from '../ColorManager';
 import { RenderDebouncer } from '../../ui/RenderDebouncer';
 import { BOLD_CLASS, ITALIC_CLASS, CURSOR_CLASS, CURSOR_STYLE_BLOCK_CLASS, CURSOR_STYLE_BAR_CLASS, CURSOR_STYLE_UNDERLINE_CLASS, DomRendererRowFactory } from './DomRendererRowFactory';
-import { INVERTED_DEFAULT_COLOR } from '../atlas/Types';
 
 const TERMINAL_CLASS_PREFIX = 'xterm-dom-renderer-owner-';
 const ROW_CONTAINER_CLASS = 'xterm-rows';
@@ -120,7 +119,6 @@ export class DomRenderer extends EventEmitter implements IRenderer {
 
     const styles =
         `${this._terminalSelector} .${ROW_CONTAINER_CLASS} span {` +
-        ` box-sizing: border-box;` +
         ` display: inline-block;` +
         ` height: 100%;` +
         ` vertical-align: top;` +
@@ -345,27 +343,21 @@ export class DomRenderer extends EventEmitter implements IRenderer {
   public deregisterCharacterJoiner(joinerId: number): boolean { return false; }
 
   private _onLinkHover(e: ILinkHoverEvent): void {
-    let color = this.colorManager.colors.foreground.css;
-
-    if (e.fg === INVERTED_DEFAULT_COLOR) {
-      color = this.colorManager.colors.background.css;
-    } else if (e.fg < 256) {
-      // 256 color support
-      color = this.colorManager.colors.ansi[e.fg].css;
-    }
-
-    this._setBorderBottomAtCells(e.x1, e.x2, e.y1, e.y2, e.cols, `1px solid ${color}`);
+    this._setCellUnderline(e.x1, e.x2, e.y1, e.y2, e.cols, true);
   }
 
   private _onLinkLeave(e: ILinkHoverEvent): void {
-    this._setBorderBottomAtCells(e.x1, e.x2, e.y1, e.y2, e.cols, null);
+    this._setCellUnderline(e.x1, e.x2, e.y1, e.y2, e.cols, false);
   }
 
-  private _setBorderBottomAtCells(x: number, x2: number, y: number, y2: number, cols: number, value?: string) {
-    for (; x != x2 || y != y2; x = ++x % cols, y += +(x === 0)) {
-      let span = (<HTMLElement>this._rowElements[y].children[x]);
-      span.style.borderBottom = value;
+  private _setCellUnderline(x: number, x2: number, y: number, y2: number, cols: number, enabled: boolean): void {
+    while (x !== x2 || y !== y2) {
+      const span = <HTMLElement>this._rowElements[y].children[x];
+      span.style.textDecoration = enabled ? 'underline' : 'none';
+      x = (x + 1) % cols;
+      if (x === 0) {
+        y++;
+      }
     }
   }
-
 }

From 750ec3870cf441cfe3e518f20e85983939d2882a Mon Sep 17 00:00:00 2001
From: Daniel Imms <daimms@microsoft.com>
Date: Sat, 22 Sep 2018 13:15:39 -0700
Subject: [PATCH 25/26] Resolve feedback

---
 src/renderer/atlas/DynamicCharAtlas.ts | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/src/renderer/atlas/DynamicCharAtlas.ts b/src/renderer/atlas/DynamicCharAtlas.ts
index d8654784..f8539bef 100644
--- a/src/renderer/atlas/DynamicCharAtlas.ts
+++ b/src/renderer/atlas/DynamicCharAtlas.ts
@@ -76,9 +76,6 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
   // An array of glyph keys that are waiting on the bitmap to be generated.
   private _glyphsWaitingOnBitmapQueue: IGlyphCacheValue[] = [];
 
-  // The number of glyphs keys waiting on the bitmap to be generated.
-  private _glyphsWaitingOnBitmapCount: number = 0;
-
   // The timeout that is used to batch bitmap generation so it's not requested for every new glyph.
   private _bitmapCommitTimeout: number | null = null;
 
@@ -299,7 +296,6 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
 
     // Add the glyph to the queue
     this._glyphsWaitingOnBitmapQueue.push(cacheValue);
-    this._glyphsWaitingOnBitmapCount++;
 
     // Check if bitmap generation timeout already exists
     if (this._bitmapCommitTimeout !== null) {
@@ -310,7 +306,7 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
   }
 
   private _generateBitmap(): void {
-    let countAtGeneration = this._glyphsWaitingOnBitmapCount;
+    let countAtGeneration = this._glyphsWaitingOnBitmapQueue.length;
     window.createImageBitmap(this._cacheCanvas).then(bitmap => {
       // Set bitmap
       this._bitmap = bitmap;
@@ -318,11 +314,10 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
       // Mark all new glyphs as in bitmap, excluding glyphs that came in after
       // the bitmap was requested
       while (countAtGeneration-- > 0) {
-        const value = this._glyphsWaitingOnBitmapQueue[0];
-        // If the value has already been evicted, do nothing
-        if (value) {
-          value.inBitmap = true;
-        }
+        const value = this._glyphsWaitingOnBitmapQueue.shift();
+        // 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;

From 16c0ad858ae7dc21b76f62d6e845272b10b3aeae Mon Sep 17 00:00:00 2001
From: Daniel Imms <daimms@microsoft.com>
Date: Sat, 22 Sep 2018 13:23:22 -0700
Subject: [PATCH 26/26] Make a new empty array instead of shift

---
 src/renderer/atlas/DynamicCharAtlas.ts | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/renderer/atlas/DynamicCharAtlas.ts b/src/renderer/atlas/DynamicCharAtlas.ts
index f8539bef..e67fe4f0 100644
--- a/src/renderer/atlas/DynamicCharAtlas.ts
+++ b/src/renderer/atlas/DynamicCharAtlas.ts
@@ -74,7 +74,7 @@ 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 _glyphsWaitingOnBitmapQueue: IGlyphCacheValue[] = [];
+  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;
@@ -295,7 +295,7 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
     }
 
     // Add the glyph to the queue
-    this._glyphsWaitingOnBitmapQueue.push(cacheValue);
+    this._glyphsWaitingOnBitmap.push(cacheValue);
 
     // Check if bitmap generation timeout already exists
     if (this._bitmapCommitTimeout !== null) {
@@ -306,15 +306,16 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
   }
 
   private _generateBitmap(): void {
-    let countAtGeneration = this._glyphsWaitingOnBitmapQueue.length;
+    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
-      while (countAtGeneration-- > 0) {
-        const value = this._glyphsWaitingOnBitmapQueue.shift();
+      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;