From ae741348a635839ec04f7c65fc02594804f6d0c4 Mon Sep 17 00:00:00 2001
From: Daniel Imms <daimms@microsoft.com>
Date: Sun, 10 Sep 2017 10:31:17 -0700
Subject: [PATCH 1/3] Fix several issues with line height

Fixes #966
Fixes #967
---
 src/Interfaces.ts                     |  3 +-
 src/Terminal.ts                       |  9 +++--
 src/Viewport.ts                       | 21 +++++-----
 src/addons/fit/fit.js                 |  2 +-
 src/renderer/BackgroundRenderLayer.ts |  6 +--
 src/renderer/BaseRenderLayer.ts       | 39 +++++-------------
 src/renderer/CursorRenderLayer.ts     |  6 +--
 src/renderer/ForegroundRenderLayer.ts |  6 +--
 src/renderer/Interfaces.ts            | 19 +++++++--
 src/renderer/LinkRenderLayer.ts       |  6 +--
 src/renderer/Renderer.ts              | 57 ++++++++++++++++++++++++---
 src/renderer/SelectionRenderLayer.ts  |  6 +--
 src/utils/TestUtils.test.ts           | 13 +++++-
 13 files changed, 125 insertions(+), 68 deletions(-)

diff --git a/src/Interfaces.ts b/src/Interfaces.ts
index d022dc3e..97fa3fd8 100644
--- a/src/Interfaces.ts
+++ b/src/Interfaces.ts
@@ -5,7 +5,7 @@
 
 import { ILinkMatcherOptions } from './Interfaces';
 import { LinkMatcherHandler, LinkMatcherValidationCallback, Charset, LineData } from './Types';
-import { IColorSet } from './renderer/Interfaces';
+import { IColorSet, IRenderer } from './renderer/Interfaces';
 import { IMouseZoneManager } from './input/Interfaces';
 
 export interface IBrowser {
@@ -36,6 +36,7 @@ export interface ITerminal extends ILinkifierAccessor, IBufferAccessor, IElement
   selectionManager: ISelectionManager;
   charMeasure: ICharMeasure;
   textarea: HTMLTextAreaElement;
+  renderer: IRenderer;
   rows: number;
   cols: number;
   browser: IBrowser;
diff --git a/src/Terminal.ts b/src/Terminal.ts
index e19887cb..67026ab0 100644
--- a/src/Terminal.ts
+++ b/src/Terminal.ts
@@ -188,7 +188,7 @@ export class Terminal extends EventEmitter implements ITerminal, IInputHandlingT
 
   private inputHandler: InputHandler;
   private parser: Parser;
-  private renderer: IRenderer;
+  public renderer: IRenderer;
   public selectionManager: SelectionManager;
   public linkifier: ILinkifier;
   public buffers: BufferSet;
@@ -619,7 +619,6 @@ export class Terminal extends EventEmitter implements ITerminal, IInputHandlingT
     this.charMeasure = new CharMeasure(document, this.helperContainer);
 
     this.viewport = new Viewport(this, this.viewportElement, this.viewportScrollArea, this.charMeasure);
-    this.charMeasure.on('charsizechanged', () => this.viewport.syncScrollArea());
     this.renderer = new Renderer(this);
     this.on('cursormove', () => this.renderer.onCursorMove());
     this.on('resize', () => this.renderer.onResize(this.cols, this.rows, false));
@@ -627,6 +626,7 @@ export class Terminal extends EventEmitter implements ITerminal, IInputHandlingT
     this.on('focus', () => this.renderer.onFocus());
     window.addEventListener('resize', () => this.renderer.onWindowResize(window.devicePixelRatio));
     this.charMeasure.on('charsizechanged', () => this.renderer.onResize(this.cols, this.rows, true));
+    this.renderer.on('resize', (dimensions) => this.viewport.syncScrollArea());
 
     this.selectionManager = new SelectionManager(this, this.buffer, this.charMeasure);
     this.element.addEventListener('mousedown', (e: MouseEvent) => this.selectionManager.onMouseDown(e));
@@ -639,7 +639,10 @@ export class Terminal extends EventEmitter implements ITerminal, IInputHandlingT
       this.textarea.focus();
       this.textarea.select();
     });
-    this.on('scroll', () => this.selectionManager.refresh());
+    this.on('scroll', () => {
+      this.viewport.syncScrollArea();
+      this.selectionManager.refresh();
+    });
     this.viewportElement.addEventListener('scroll', () => this.selectionManager.refresh());
 
     // Measure the character size
diff --git a/src/Viewport.ts b/src/Viewport.ts
index 1f693cfe..b7622ba7 100644
--- a/src/Viewport.ts
+++ b/src/Viewport.ts
@@ -34,8 +34,6 @@ export class Viewport implements IViewport {
     this.lastRecordedBufferLength = 0;
     this.lastRecordedViewportHeight = 0;
 
-    this.terminal.on('scroll', this.syncScrollArea.bind(this));
-    this.terminal.on('resize', this.syncScrollArea.bind(this));
     this.viewportElement.addEventListener('scroll', this.onScroll.bind(this));
 
     // Perform this async to ensure the CharMeasure is ready.
@@ -52,18 +50,20 @@ export class Viewport implements IViewport {
    */
   private refresh(): void {
     if (this.charMeasure.height > 0) {
-      const lineHeight = Math.ceil(this.charMeasure.height * this.terminal.options.lineHeight);
+      const lineHeight = (<any>this.terminal).renderer.dimensions.scaledLineHeight / window.devicePixelRatio;
       const rowHeightChanged = lineHeight !== this.currentRowHeight;
+      // TODO: Do we need lineHeight anymore??
       if (rowHeightChanged) {
         this.currentRowHeight = lineHeight;
         this.viewportElement.style.lineHeight = lineHeight + 'px';
       }
-      const viewportHeightChanged = this.lastRecordedViewportHeight !== this.terminal.rows;
-      if (rowHeightChanged || viewportHeightChanged) {
-        this.lastRecordedViewportHeight = this.terminal.rows;
-        this.viewportElement.style.height = lineHeight * this.terminal.rows + 'px';
+      // const viewportHeightChanged = this.lastRecordedViewportHeight !== this.terminal.rows;
+      const viewportHeightChanged = this.lastRecordedViewportHeight !== this.terminal.renderer.dimensions.canvasHeight;
+      if (viewportHeightChanged) {
+        this.lastRecordedViewportHeight = this.terminal.renderer.dimensions.canvasHeight;
+        this.viewportElement.style.height = this.lastRecordedViewportHeight + 'px';
       }
-      this.scrollArea.style.height = (lineHeight * this.lastRecordedBufferLength) + 'px';
+      this.scrollArea.style.height = Math.round(lineHeight * this.lastRecordedBufferLength) + 'px';
     }
   }
 
@@ -75,12 +75,13 @@ export class Viewport implements IViewport {
       // If buffer height changed
       this.lastRecordedBufferLength = this.terminal.buffer.lines.length;
       this.refresh();
-    } else if (this.lastRecordedViewportHeight !== this.terminal.rows) {
+    } else if (this.lastRecordedViewportHeight !== (<any>this.terminal).renderer.dimensions.canvasHeight) {
       // If viewport height changed
       this.refresh();
     } else {
       // If size has changed, refresh viewport
-      if (Math.ceil(this.charMeasure.height * this.terminal.options.lineHeight) !== this.currentRowHeight) {
+      console.log(this.terminal.renderer.dimensions.scaledLineHeight / window.devicePixelRatio);
+      if (this.terminal.renderer.dimensions.scaledLineHeight / window.devicePixelRatio !== this.currentRowHeight) {
         this.refresh();
       }
     }
diff --git a/src/addons/fit/fit.js b/src/addons/fit/fit.js
index 59f0fd52..3e61a7ef 100644
--- a/src/addons/fit/fit.js
+++ b/src/addons/fit/fit.js
@@ -47,7 +47,7 @@
     var availableWidth = parentElementWidth - elementPaddingHor;
     var geometry = {
       cols: Math.floor(availableWidth / term.charMeasure.width),
-      rows: Math.floor(availableHeight / Math.ceil(term.charMeasure.height * term.getOption('lineHeight')))
+      rows: Math.floor(availableHeight / Math.floor(term.charMeasure.height * term.getOption('lineHeight')))
     };
 
     return geometry;
diff --git a/src/renderer/BackgroundRenderLayer.ts b/src/renderer/BackgroundRenderLayer.ts
index 0c09727d..279aee6d 100644
--- a/src/renderer/BackgroundRenderLayer.ts
+++ b/src/renderer/BackgroundRenderLayer.ts
@@ -3,7 +3,7 @@
  * @license MIT
  */
 
-import { IColorSet } from './Interfaces';
+import { IColorSet, IRenderDimensions } from './Interfaces';
 import { IBuffer, ICharMeasure, ITerminal } from '../Interfaces';
 import { CHAR_DATA_ATTR_INDEX } from '../Buffer';
 import { GridCache } from './GridCache';
@@ -18,8 +18,8 @@ export class BackgroundRenderLayer extends BaseRenderLayer {
     this._state = new GridCache<number>();
   }
 
-  public resize(terminal: ITerminal, canvasWidth: number, canvasHeight: number, charSizeChanged: boolean): void {
-    super.resize(terminal, canvasWidth, canvasHeight, charSizeChanged);
+  public resize(terminal: ITerminal, dim: IRenderDimensions, charSizeChanged: boolean): void {
+    super.resize(terminal, dim, charSizeChanged);
     // Resizing the canvas discards the contents of the canvas so clear state
     this._state.clear();
     this._state.resize(terminal.cols, terminal.rows);
diff --git a/src/renderer/BaseRenderLayer.ts b/src/renderer/BaseRenderLayer.ts
index cc02bcea..38457e13 100644
--- a/src/renderer/BaseRenderLayer.ts
+++ b/src/renderer/BaseRenderLayer.ts
@@ -3,7 +3,7 @@
  * @license MIT
  */
 
-import { IRenderLayer, IColorSet } from './Interfaces';
+import { IRenderLayer, IColorSet, IRenderDimensions } from './Interfaces';
 import { ITerminal, ITerminalOptions } from '../Interfaces';
 import { acquireCharAtlas, CHAR_ATLAS_CELL_SPACING } from './CharAtlas';
 import { CharData } from '../Types';
@@ -61,34 +61,15 @@ export abstract class BaseRenderLayer implements IRenderLayer {
     }
   }
 
-  public resize(terminal: ITerminal, canvasWidth: number, canvasHeight: number, charSizeChanged: boolean): void {
-    // Calculate the scaled character dimensions, if devicePixelRatio is a
-    // floating point number then the value is ceiled to ensure there is enough
-    // space to draw the character to the cell
-    this.scaledCharWidth = Math.ceil(terminal.charMeasure.width * window.devicePixelRatio);
-    this.scaledCharHeight = Math.ceil(terminal.charMeasure.height * window.devicePixelRatio);
-
-    // Calculate the scaled line height, if lineHeight is not 1 then the value
-    // will be floored because since lineHeight can never be lower then 1, there
-    // is a guarentee that the scaled line height will always be larger than
-    // scaled char height.
-    this.scaledLineHeight = Math.floor(this.scaledCharHeight * terminal.options.lineHeight);
-
-    // Calculate the y coordinate within a cell that text should draw from in
-    // order to draw in the center of a cell.
-    this.scaledLineDrawY = terminal.options.lineHeight === 1 ? 0 : Math.round((this.scaledLineHeight - this.scaledCharHeight) / 2);
-
-    // Recalcualte the canvas dimensions; width/height define the actual number
-    // of pixels in the canvas, style.width/height define the size of the canvas
-    // on the page. It's very important that this rounds to nearest integer and
-    // not ceils as browsers often set window.devicePixelRatio as something like
-    // 1.100000023841858, when it's actually 1.1. Ceiling causes blurriness as
-    // the backing canvas image is 1 pixel too large for the canvas element
-    // size.
-    this._canvas.width = Math.round(canvasWidth * window.devicePixelRatio);
-    this._canvas.height = Math.round(canvasHeight * window.devicePixelRatio);
-    this._canvas.style.width = `${canvasWidth}px`;
-    this._canvas.style.height = `${canvasHeight}px`;
+  public resize(terminal: ITerminal, dim: IRenderDimensions, charSizeChanged: boolean): void {
+    this.scaledCharWidth = dim.scaledCharWidth;
+    this.scaledCharHeight = dim.scaledCharHeight;
+    this.scaledLineHeight = dim.scaledLineHeight;
+    this.scaledLineDrawY = dim.scaledLineDrawY;
+    this._canvas.width = dim.scaledCanvasWidth;
+    this._canvas.height = dim.scaledCanvasHeight;
+    this._canvas.style.width = `${dim.canvasWidth}px`;
+    this._canvas.style.height = `${dim.canvasHeight}px`;
 
     if (charSizeChanged) {
       this._refreshCharAtlas(terminal, this.colors);
diff --git a/src/renderer/CursorRenderLayer.ts b/src/renderer/CursorRenderLayer.ts
index b136a13a..8e25d7c1 100644
--- a/src/renderer/CursorRenderLayer.ts
+++ b/src/renderer/CursorRenderLayer.ts
@@ -3,7 +3,7 @@
  * @license MIT
  */
 
-import { IColorSet } from './Interfaces';
+import { IColorSet, IRenderDimensions } from './Interfaces';
 import { IBuffer, ICharMeasure, ITerminal, ITerminalOptions } from '../Interfaces';
 import { CHAR_DATA_WIDTH_INDEX, CHAR_DATA_CODE_INDEX, CHAR_DATA_CHAR_INDEX } from '../Buffer';
 import { GridCache } from './GridCache';
@@ -47,8 +47,8 @@ export class CursorRenderLayer extends BaseRenderLayer {
     // TODO: Consider initial options? Maybe onOptionsChanged should be called at the end of open?
   }
 
-  public resize(terminal: ITerminal, canvasWidth: number, canvasHeight: number, charSizeChanged: boolean): void {
-    super.resize(terminal, canvasWidth, canvasHeight, charSizeChanged);
+  public resize(terminal: ITerminal, dim: IRenderDimensions, charSizeChanged: boolean): void {
+    super.resize(terminal, dim, charSizeChanged);
     // Resizing the canvas discards the contents of the canvas so clear state
     this._state = {
       x: null,
diff --git a/src/renderer/ForegroundRenderLayer.ts b/src/renderer/ForegroundRenderLayer.ts
index 883be5d9..8e9e23fc 100644
--- a/src/renderer/ForegroundRenderLayer.ts
+++ b/src/renderer/ForegroundRenderLayer.ts
@@ -3,7 +3,7 @@
  * @license MIT
  */
 
-import { IColorSet } from './Interfaces';
+import { IColorSet, IRenderDimensions } from './Interfaces';
 import { IBuffer, ICharMeasure, ITerminal } from '../Interfaces';
 import { CHAR_DATA_ATTR_INDEX, CHAR_DATA_CODE_INDEX, CHAR_DATA_CHAR_INDEX, CHAR_DATA_WIDTH_INDEX } from '../Buffer';
 import { FLAGS } from './Types';
@@ -26,8 +26,8 @@ export class ForegroundRenderLayer extends BaseRenderLayer {
     this._state = new GridCache<CharData>();
   }
 
-  public resize(terminal: ITerminal, canvasWidth: number, canvasHeight: number, charSizeChanged: boolean): void {
-    super.resize(terminal, canvasWidth, canvasHeight, charSizeChanged);
+  public resize(terminal: ITerminal, dim: IRenderDimensions, charSizeChanged: boolean): void {
+    super.resize(terminal, dim, charSizeChanged);
     // Resizing the canvas discards the contents of the canvas so clear state
     this._state.clear();
     this._state.resize(terminal.cols, terminal.rows);
diff --git a/src/renderer/Interfaces.ts b/src/renderer/Interfaces.ts
index 05f98300..890b01f5 100644
--- a/src/renderer/Interfaces.ts
+++ b/src/renderer/Interfaces.ts
@@ -3,9 +3,11 @@
  * @license MIT
  */
 
-import { ITerminal, ITerminalOptions, ITheme } from '../Interfaces';
+import { ITerminal, ITerminalOptions, ITheme, IEventEmitter } from '../Interfaces';
+
+export interface IRenderer extends IEventEmitter {
+  dimensions: IRenderDimensions;
 
-export interface IRenderer {
   setTheme(theme: ITheme): IColorSet;
   onWindowResize(devicePixelRatio: number): void;
   onResize(cols: number, rows: number, didCharSizeChange: boolean): void;
@@ -59,7 +61,7 @@ export interface IRenderLayer {
   /**
    * Resize the render layer.
    */
-  resize(terminal: ITerminal, canvasWidth: number, canvasHeight: number, charSizeChanged: boolean): void;
+  resize(terminal: ITerminal, dim: IRenderDimensions, charSizeChanged: boolean): void;
 
   /**
    * Clear the state of the render layer.
@@ -75,3 +77,14 @@ export interface IColorSet {
   selection: string;
   ansi: string[];
 }
+
+export interface IRenderDimensions {
+  scaledCharWidth: number;
+  scaledCharHeight: number;
+  scaledLineHeight: number;
+  scaledLineDrawY: number;
+  scaledCanvasWidth: number;
+  scaledCanvasHeight: number;
+  canvasWidth: number;
+  canvasHeight: number;
+}
diff --git a/src/renderer/LinkRenderLayer.ts b/src/renderer/LinkRenderLayer.ts
index 9b2cabb3..ad178933 100644
--- a/src/renderer/LinkRenderLayer.ts
+++ b/src/renderer/LinkRenderLayer.ts
@@ -3,7 +3,7 @@
  * @license MIT
  */
 
-import { IColorSet } from './Interfaces';
+import { IColorSet, IRenderDimensions } from './Interfaces';
 import { IBuffer, ICharMeasure, ITerminal, ILinkifierAccessor } from '../Interfaces';
 import { CHAR_DATA_ATTR_INDEX } from '../Buffer';
 import { GridCache } from './GridCache';
@@ -20,8 +20,8 @@ export class LinkRenderLayer extends BaseRenderLayer {
     terminal.linkifier.on(LinkHoverEventTypes.LEAVE, (e: LinkHoverEvent) => this._onLinkLeave(e));
   }
 
-  public resize(terminal: ITerminal, canvasWidth: number, canvasHeight: number, charSizeChanged: boolean): void {
-    super.resize(terminal, canvasWidth, canvasHeight, charSizeChanged);
+  public resize(terminal: ITerminal, dim: IRenderDimensions, charSizeChanged: boolean): void {
+    super.resize(terminal, dim, charSizeChanged);
     // Resizing the canvas discards the contents of the canvas so clear state
     this._state = null;
   }
diff --git a/src/renderer/Renderer.ts b/src/renderer/Renderer.ts
index 131a8b78..b610fd69 100644
--- a/src/renderer/Renderer.ts
+++ b/src/renderer/Renderer.ts
@@ -11,10 +11,11 @@ import { SelectionRenderLayer } from './SelectionRenderLayer';
 import { CursorRenderLayer } from './CursorRenderLayer';
 import { ColorManager } from './ColorManager';
 import { BaseRenderLayer } from './BaseRenderLayer';
-import { IRenderLayer, IColorSet, IRenderer } from './Interfaces';
+import { IRenderLayer, IColorSet, IRenderer, IRenderDimensions } from './Interfaces';
 import { LinkRenderLayer } from './LinkRenderLayer';
+import { EventEmitter } from '../EventEmitter';
 
-export class Renderer implements IRenderer {
+export class Renderer extends EventEmitter implements IRenderer {
   /** A queue of the rows to be refreshed */
   private _refreshRowsQueue: {start: number, end: number}[] = [];
   private _refreshAnimationFrame = null;
@@ -23,8 +24,10 @@ export class Renderer implements IRenderer {
   private _devicePixelRatio: number;
 
   private _colorManager: ColorManager;
+  public dimensions: IRenderDimensions;
 
   constructor(private _terminal: ITerminal) {
+    super();
     this._colorManager = new ColorManager();
     this._renderLayers = [
       new BackgroundRenderLayer(this._terminal.element, 0, this._colorManager.colors),
@@ -33,6 +36,16 @@ export class Renderer implements IRenderer {
       new LinkRenderLayer(this._terminal.element, 3, this._colorManager.colors, this._terminal),
       new CursorRenderLayer(this._terminal.element, 4, this._colorManager.colors)
     ];
+    this.dimensions = {
+      scaledCharWidth: null,
+      scaledCharHeight: null,
+      scaledLineHeight: null,
+      scaledLineDrawY: null,
+      scaledCanvasWidth: null,
+      scaledCanvasHeight: null,
+      canvasWidth: null,
+      canvasHeight: null
+    };
     this._devicePixelRatio = window.devicePixelRatio;
   }
 
@@ -63,12 +76,46 @@ export class Renderer implements IRenderer {
     if (!this._terminal.charMeasure.width || !this._terminal.charMeasure.height) {
       return;
     }
-    const width = this._terminal.charMeasure.width * cols;
-    const height = Math.floor(this._terminal.charMeasure.height * this._terminal.options.lineHeight) * rows;
+
+    // Calculate the scaled character dimensions, if devicePixelRatio is a
+    // floating point number then the value is ceiled to ensure there is enough
+    // space to draw the character to the cell
+    this.dimensions.scaledCharWidth = Math.ceil(this._terminal.charMeasure.width * window.devicePixelRatio);
+    this.dimensions.scaledCharHeight = Math.ceil(this._terminal.charMeasure.height * window.devicePixelRatio);
+
+    // Calculate the scaled line height, if lineHeight is not 1 then the value
+    // will be floored because since lineHeight can never be lower then 1, there
+    // is a guarentee that the scaled line height will always be larger than
+    // scaled char height.
+    this.dimensions.scaledLineHeight = Math.floor(this.dimensions.scaledCharHeight * this._terminal.options.lineHeight);
+
+    // Calculate the y coordinate within a cell that text should draw from in
+    // order to draw in the center of a cell.
+    this.dimensions.scaledLineDrawY = this._terminal.options.lineHeight === 1 ? 0 : Math.round((this.dimensions.scaledLineHeight - this.dimensions.scaledCharHeight) / 2);
+
+    // Recalculate the canvas dimensions; scaled* define the actual number of
+    // pixel in the canvas
+    this.dimensions.scaledCanvasHeight = this._terminal.rows * this.dimensions.scaledLineHeight;
+    this.dimensions.scaledCanvasWidth = this._terminal.cols * this.dimensions.scaledCharWidth;
+
+    // The the size of the canvas on the page. It's very important that this
+    // rounds to nearest integer and not ceils as browsers often set
+    // window.devicePixelRatio as something like 1.100000023841858, when it's
+    // actually 1.1. Ceiling causes blurriness as the backing canvas image is 1
+    // pixel too large for the canvas element size.
+    this.dimensions.canvasHeight = Math.round(this.dimensions.scaledCanvasHeight / window.devicePixelRatio);
+    this.dimensions.canvasWidth = Math.round(this.dimensions.scaledCanvasWidth / window.devicePixelRatio);
+
     // Resize all render layers
-    this._renderLayers.forEach(l => l.resize(this._terminal, width, height, didCharSizeChange));
+    this._renderLayers.forEach(l => l.resize(this._terminal, this.dimensions, didCharSizeChange));
+
     // Force a refresh
     this._terminal.refresh(0, this._terminal.rows - 1);
+
+    this.emit('resize', {
+      width: this.dimensions.canvasWidth,
+      height: this.dimensions.canvasHeight
+    });
   }
 
   public onCharSizeChanged(): void {
diff --git a/src/renderer/SelectionRenderLayer.ts b/src/renderer/SelectionRenderLayer.ts
index 39569c9b..5339c7a4 100644
--- a/src/renderer/SelectionRenderLayer.ts
+++ b/src/renderer/SelectionRenderLayer.ts
@@ -3,7 +3,7 @@
  * @license MIT
  */
 
-import { IColorSet } from './Interfaces';
+import { IColorSet, IRenderDimensions } from './Interfaces';
 import { IBuffer, ICharMeasure, ITerminal } from '../Interfaces';
 import { CHAR_DATA_ATTR_INDEX } from '../Buffer';
 import { GridCache } from './GridCache';
@@ -21,8 +21,8 @@ export class SelectionRenderLayer extends BaseRenderLayer {
     };
   }
 
-  public resize(terminal: ITerminal, canvasWidth: number, canvasHeight: number, charSizeChanged: boolean): void {
-    super.resize(terminal, canvasWidth, canvasHeight, charSizeChanged);
+  public resize(terminal: ITerminal, dim: IRenderDimensions, charSizeChanged: boolean): void {
+    super.resize(terminal, dim, charSizeChanged);
     // Resizing the canvas discards the contents of the canvas so clear state
     this._state = {
       start: null,
diff --git a/src/utils/TestUtils.test.ts b/src/utils/TestUtils.test.ts
index eb2f2501..45cf169a 100644
--- a/src/utils/TestUtils.test.ts
+++ b/src/utils/TestUtils.test.ts
@@ -7,9 +7,10 @@ import { ITerminal, IBuffer, IBufferSet, IBrowser, ICharMeasure, ISelectionManag
 import { LineData } from '../Types';
 import { Buffer } from '../Buffer';
 import * as Browser from './Browser';
-import { IColorSet, IRenderer } from '../renderer/Interfaces';
+import { IColorSet, IRenderer, IRenderDimensions } from '../renderer/Interfaces';
 
 export class MockTerminal implements ITerminal {
+  renderer: IRenderer;
   linkifier: ILinkifier;
   isFocused: boolean;
   options: ITerminalOptions = {};
@@ -215,6 +216,16 @@ export class MockBuffer implements IBuffer {
 }
 
 export class MockRenderer implements IRenderer {
+  on(type: string, listener: IListenerType): void {
+    throw new Error('Method not implemented.');
+  }
+  off(type: string, listener: IListenerType): void {
+    throw new Error('Method not implemented.');
+  }
+  emit(type: string, data?: any): void {
+    throw new Error('Method not implemented.');
+  }
+  dimensions: IRenderDimensions;
   setTheme(theme: ITheme): IColorSet { return <IColorSet>{}; }
   onResize(cols: number, rows: number, didCharSizeChange: boolean): void {}
   onCharSizeChanged(): void {}

From ac23be134b1b5926aee87654e92493cb691cd375 Mon Sep 17 00:00:00 2001
From: Daniel Imms <daimms@microsoft.com>
Date: Sun, 10 Sep 2017 10:38:08 -0700
Subject: [PATCH 2/3] Simplify viewport

---
 src/Viewport.ts | 32 +++++++++++++-------------------
 1 file changed, 13 insertions(+), 19 deletions(-)

diff --git a/src/Viewport.ts b/src/Viewport.ts
index b7622ba7..064076a2 100644
--- a/src/Viewport.ts
+++ b/src/Viewport.ts
@@ -12,9 +12,10 @@ import { IColorSet } from './renderer/Interfaces';
  * Logic for the virtual scroll bar is included in this object.
  */
 export class Viewport implements IViewport {
-  private currentRowHeight: number;
-  private lastRecordedBufferLength: number;
-  private lastRecordedViewportHeight: number;
+  private currentRowHeight: number = 0;
+  private lastRecordedBufferLength: number = 0;
+  private lastRecordedViewportHeight: number = 0;
+  private lastRecordedBufferHeight: number = 0;
   private lastTouchY: number;
 
   /**
@@ -30,10 +31,6 @@ export class Viewport implements IViewport {
     private scrollArea: HTMLElement,
     private charMeasure: CharMeasure
   ) {
-    this.currentRowHeight = 0;
-    this.lastRecordedBufferLength = 0;
-    this.lastRecordedViewportHeight = 0;
-
     this.viewportElement.addEventListener('scroll', this.onScroll.bind(this));
 
     // Perform this async to ensure the CharMeasure is ready.
@@ -50,20 +47,18 @@ export class Viewport implements IViewport {
    */
   private refresh(): void {
     if (this.charMeasure.height > 0) {
-      const lineHeight = (<any>this.terminal).renderer.dimensions.scaledLineHeight / window.devicePixelRatio;
-      const rowHeightChanged = lineHeight !== this.currentRowHeight;
-      // TODO: Do we need lineHeight anymore??
-      if (rowHeightChanged) {
-        this.currentRowHeight = lineHeight;
-        this.viewportElement.style.lineHeight = lineHeight + 'px';
-      }
-      // const viewportHeightChanged = this.lastRecordedViewportHeight !== this.terminal.rows;
-      const viewportHeightChanged = this.lastRecordedViewportHeight !== this.terminal.renderer.dimensions.canvasHeight;
-      if (viewportHeightChanged) {
+      this.currentRowHeight = this.terminal.renderer.dimensions.scaledLineHeight / window.devicePixelRatio;
+
+      if (this.lastRecordedViewportHeight !== this.terminal.renderer.dimensions.canvasHeight) {
         this.lastRecordedViewportHeight = this.terminal.renderer.dimensions.canvasHeight;
         this.viewportElement.style.height = this.lastRecordedViewportHeight + 'px';
       }
-      this.scrollArea.style.height = Math.round(lineHeight * this.lastRecordedBufferLength) + 'px';
+
+      const newBufferHeight = Math.round(this.currentRowHeight * this.lastRecordedBufferLength);
+      if (this.lastRecordedBufferHeight !== newBufferHeight) {
+        this.lastRecordedBufferHeight = newBufferHeight;
+        this.scrollArea.style.height = this.lastRecordedBufferHeight + 'px';
+      }
     }
   }
 
@@ -80,7 +75,6 @@ export class Viewport implements IViewport {
       this.refresh();
     } else {
       // If size has changed, refresh viewport
-      console.log(this.terminal.renderer.dimensions.scaledLineHeight / window.devicePixelRatio);
       if (this.terminal.renderer.dimensions.scaledLineHeight / window.devicePixelRatio !== this.currentRowHeight) {
         this.refresh();
       }

From 903ed1917e38c52c372d6b340d6a65ac0dcbef78 Mon Sep 17 00:00:00 2001
From: Daniel Imms <daimms@microsoft.com>
Date: Sun, 10 Sep 2017 10:43:07 -0700
Subject: [PATCH 3/3] Remove viewport test

It's not tightly linked to renderer and depends on devicePixelRatio.
---
 src/Viewport.test.ts | 90 --------------------------------------------
 1 file changed, 90 deletions(-)
 delete mode 100644 src/Viewport.test.ts

diff --git a/src/Viewport.test.ts b/src/Viewport.test.ts
deleted file mode 100644
index 6ce798d3..00000000
--- a/src/Viewport.test.ts
+++ /dev/null
@@ -1,90 +0,0 @@
-/**
- * Copyright (c) 2016 The xterm.js authors. All rights reserved.
- * @license MIT
- */
-
-import { assert } from 'chai';
-import { Viewport } from './Viewport';
-import { BufferSet } from './BufferSet';
-
-describe('Viewport', () => {
-  let terminal;
-  let viewportElement;
-  let charMeasure;
-  let viewport;
-  let scrollAreaElement;
-
-  const CHARACTER_HEIGHT = 10;
-
-  beforeEach(() => {
-    terminal = {
-      rows: 0,
-      ydisp: 0,
-      on: () => {},
-      rowContainer: {
-        style: {
-          lineHeight: 0
-        }
-      },
-      selectionContainer: {
-        style: {
-          height: 0
-        }
-      },
-      options: {
-        scrollback: 10,
-        lineHeight: 1
-      }
-    };
-    terminal.buffers = new BufferSet(terminal);
-    terminal.buffer = terminal.buffers.active;
-    viewportElement = {
-      addEventListener: () => {},
-      style: {
-        height: 0,
-        lineHeight: 0
-      }
-    };
-    scrollAreaElement = {
-      style: {
-        height: 0
-      }
-    };
-    charMeasure = {
-      height: CHARACTER_HEIGHT
-    };
-    viewport = new Viewport(terminal, viewportElement, scrollAreaElement, charMeasure);
-  });
-
-  describe('refresh', () => {
-    it('should set the height of the viewport when the line-height changed', () => {
-      terminal.buffer.lines.push('');
-      terminal.buffer.lines.push('');
-      terminal.rows = 1;
-      viewport.refresh();
-      assert.equal(viewportElement.style.height, 1 * CHARACTER_HEIGHT + 'px');
-      charMeasure.height = 2 * CHARACTER_HEIGHT;
-      viewport.refresh();
-      assert.equal(viewportElement.style.height, 2 * CHARACTER_HEIGHT + 'px');
-    });
-  });
-
-  describe('syncScrollArea', () => {
-    it('should sync the scroll area', done => {
-      // Allow CharMeasure to be initialized
-      setTimeout(() => {
-        terminal.buffer.lines.push('');
-        terminal.rows = 1;
-        assert.equal(scrollAreaElement.style.height, 0 * CHARACTER_HEIGHT + 'px');
-        viewport.syncScrollArea();
-        assert.equal(viewportElement.style.height, 1 * CHARACTER_HEIGHT + 'px');
-        assert.equal(scrollAreaElement.style.height, 1 * CHARACTER_HEIGHT + 'px');
-        terminal.buffer.lines.push('');
-        viewport.syncScrollArea();
-        assert.equal(viewportElement.style.height, 1 * CHARACTER_HEIGHT + 'px');
-        assert.equal(scrollAreaElement.style.height, 2 * CHARACTER_HEIGHT + 'px');
-        done();
-      }, 0);
-    });
-  });
-});