Fix fit() sizing and also support letterSpacing, enhanced (#1157)

Fixes #1081

* Make sure character dimensions are available right after calling term.open()
* Do not assume var assignments are global anymore, webpack will isolate them
* Use DocumentFragment to build Viewport and helpers before attaching them to the DOM
* Shield term.focus call from crashing when used before term.open

demo/main.js

@@ -39,8 +39,8 @@ function setTerminalSize() {
   var rows = parseInt(rowsElement.value, 10);
   var viewportElement = document.querySelector('.xterm-viewport');
   var scrollBarWidth = viewportElement.offsetWidth - viewportElement.clientWidth;
-  var width = (cols * term.charMeasure.width + 20 /*room for scrollbar*/).toString() + 'px';
-  var height = (rows * term.charMeasure.height).toString() + 'px';
+  var width = (cols * term.renderer.dimensions.actualCellWidth + 20 /*room for scrollbar*/).toString() + 'px';
+  var height = (rows * term.renderer.dimensions.actualCellHeight).toString() + 'px';
 
   terminalContainer.style.width = width;
   terminalContainer.style.height = height;
@@ -120,9 +120,9 @@ function createTerminal() {
 
     fetch('/terminals?cols=' + term.cols + '&rows=' + term.rows, {method: 'POST'}).then(function (res) {
 
-      res.text().then(function (pid) {
-        window.pid = pid;
-        socketURL += pid;
+      res.text().then(function (processId) {
+        pid = processId;
+        socketURL += processId;
         socket = new WebSocket(socketURL);
         socket.onopen = runRealTerminal;
         socket.onclose = runFakeTerminal;

src/Terminal.ts

@@ -316,7 +316,9 @@ export class Terminal extends EventEmitter implements ITerminal, IInputHandlingT
    * Focus the terminal. Delegates focus handling to the terminal's DOM element.
    */
   public focus(): void {
-    this.textarea.focus();
+    if (this.textarea) {
+      this.textarea.focus();
+    }
   }
 
   public get isFocused(): boolean {
@@ -578,12 +580,15 @@ export class Terminal extends EventEmitter implements ITerminal, IInputHandlingT
     this.element = this.document.createElement('div');
     this.element.classList.add('terminal');
     this.element.classList.add('xterm');
-
     this.element.setAttribute('tabindex', '0');
+    this.parent.appendChild(this.element);
 
+    // Performance: Use a document fragment to build the terminal
+    // viewport and helper elements detached from the DOM
+    const fragment = document.createDocumentFragment();
     this.viewportElement = document.createElement('div');
     this.viewportElement.classList.add('xterm-viewport');
-    this.element.appendChild(this.viewportElement);
+    fragment.appendChild(this.viewportElement);
     this.viewportScrollArea = document.createElement('div');
     this.viewportScrollArea.classList.add('xterm-scroll-area');
     this.viewportElement.appendChild(this.viewportScrollArea);
@@ -599,8 +604,8 @@ export class Terminal extends EventEmitter implements ITerminal, IInputHandlingT
     // capturing DOM Events. Then produce the helpers.
     this.helperContainer = document.createElement('div');
     this.helperContainer.classList.add('xterm-helpers');
-    // TODO: This should probably be inserted once it's filled to prevent an additional layout
-    this.element.appendChild(this.helperContainer);
+    fragment.appendChild(this.helperContainer);
+
     this.textarea = document.createElement('textarea');
     this.textarea.classList.add('xterm-helper-textarea');
     this.textarea.setAttribute('autocorrect', 'off');
@@ -618,11 +623,11 @@ export class Terminal extends EventEmitter implements ITerminal, IInputHandlingT
 
     this.charSizeStyleElement = document.createElement('style');
     this.helperContainer.appendChild(this.charSizeStyleElement);
-
-    this.parent.appendChild(this.element);
-
     this.charMeasure = new CharMeasure(document, this.helperContainer);
 
+    // Performance: Add viewport and helper elements from the fragment
+    this.element.appendChild(fragment);
+
     this.renderer = new Renderer(this, this.options.theme);
     this.options.theme = null;
     this.viewport = new Viewport(this, this.viewportElement, this.viewportScrollArea, this.charMeasure);
@@ -667,6 +672,7 @@ export class Terminal extends EventEmitter implements ITerminal, IInputHandlingT
     // Listen for mouse events and translate
     // them into terminal mouse protocols.
     this.bindMouse();
+
   }
 
   /**

src/addons/fit/fit.ts

@@ -26,27 +26,22 @@ export function proposeGeometry(term) {
   var availableHeight = parentElementHeight - elementPaddingVer;
   var availableWidth = parentElementWidth - elementPaddingHor;
   var geometry = {
-    cols: Math.floor(availableWidth / term.charMeasure.width),
-    rows: Math.floor(availableHeight / Math.floor(term.charMeasure.height * term.getOption('lineHeight')))
+    cols: Math.floor(availableWidth / term.renderer.dimensions.actualCellWidth),
+    rows: Math.floor(availableHeight / term.renderer.dimensions.actualCellHeight)
   };
 
   return geometry;
 };
 
 export function fit(term) {
-  // Wrap fit in a setTimeout as charMeasure needs time to get initialized
-  // after calling Terminal.open
-  setTimeout(function () {
-    var geometry = exports.proposeGeometry(term);
-
-    if (geometry) {
-      // Force a full render
-      if (term.rows !== geometry.rows || term.cols !== geometry.cols) {
-        term.renderer.clear();
-        term.resize(geometry.cols, geometry.rows);
-      }
+  var geometry = exports.proposeGeometry(term);
+  if (geometry) {
+    // Force a full render
+    if (term.rows !== geometry.rows || term.cols !== geometry.cols) {
+      term.renderer.clear();
+      term.resize(geometry.cols, geometry.rows);
     }
-  }, 0);
+  }
 };
 
 export function apply(terminalConstructor) {

src/renderer/Renderer.ts

@@ -52,6 +52,7 @@ export class Renderer extends EventEmitter implements IRenderer {
       actualCellHeight: null
     };
     this._devicePixelRatio = window.devicePixelRatio;
+    this._updateDimensions();
   }
 
   public onWindowResize(devicePixelRatio: number): void {
@@ -78,59 +79,8 @@ export class Renderer extends EventEmitter implements IRenderer {
   }
 
   public onResize(cols: number, rows: number, didCharSizeChange: boolean): void {
-    if (!this._terminal.charMeasure.width || !this._terminal.charMeasure.height) {
-      return;
-    }
-
-    // Calculate the scaled character width. Width is floored as it must be
-    // drawn to an integer grid in order for the CharAtlas "stamps" to not be
-    // blurry. When text is drawn to the grid not using the CharAtlas, it is
-    // clipped to ensure there is no overlap with the next cell.
-    this.dimensions.scaledCharWidth = Math.floor(this._terminal.charMeasure.width * window.devicePixelRatio);
-
-    // Calculate the scaled character height. Height is ceiled in case
-    // devicePixelRatio is a floating point number in order to ensure there is
-    // enough space to draw the character to the cell.
-    this.dimensions.scaledCharHeight = Math.ceil(this._terminal.charMeasure.height * window.devicePixelRatio);
-
-    // Calculate the scaled cell 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.scaledCellHeight = 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.scaledCharTop = this._terminal.options.lineHeight === 1 ? 0 : Math.round((this.dimensions.scaledCellHeight - this.dimensions.scaledCharHeight) / 2);
-
-    // Calculate the scaled cell width, taking the letterSpacing into account.
-    this.dimensions.scaledCellWidth = this.dimensions.scaledCharWidth + Math.round(this._terminal.options.letterSpacing);
-
-    // Calculate the x coordinate with a cell that text should draw from in
-    // order to draw in the center of a cell.
-    this.dimensions.scaledCharLeft = Math.floor(this._terminal.options.letterSpacing / 2);
-
-    // Recalculate the canvas dimensions; scaled* define the actual number of
-    // pixel in the canvas
-    this.dimensions.scaledCanvasHeight = this._terminal.rows * this.dimensions.scaledCellHeight;
-    this.dimensions.scaledCanvasWidth = this._terminal.cols * this.dimensions.scaledCellWidth;
-
-    // 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);
-
-    // Get the _actual_ dimensions of an individual cell. This needs to be
-    // derived from the canvasWidth/Height calculated above which takes into
-    // account window.devicePixelRatio. CharMeasure.width/height by itself is
-    // insufficient when the page is not at 100% zoom level as CharMeasure is
-    // measured in CSS pixels, but the actual char size on the canvas can
-    // differ.
-    this.dimensions.actualCellHeight = this.dimensions.canvasHeight / this._terminal.rows;
-    this.dimensions.actualCellWidth = this.dimensions.canvasWidth / this._terminal.cols;
+    // Update character and canvas dimensions
+    this._updateDimensions();
 
     // Resize all render layers
     this._renderLayers.forEach(l => l.resize(this._terminal, this.dimensions, didCharSizeChange));
@@ -218,4 +168,66 @@ export class Renderer extends EventEmitter implements IRenderer {
     this._renderLayers.forEach(l => l.onGridChanged(this._terminal, start, end));
     this._terminal.emit('refresh', {start, end});
   }
+
+  /**
+   * Recalculates the character and canvas dimensions.
+   */
+  private _updateDimensions(): void {
+    // Perform a new measure if the CharMeasure dimensions are not yet available
+    if (!this._terminal.charMeasure.width || !this._terminal.charMeasure.height) {
+      return;
+    }
+
+    // Calculate the scaled character width. Width is floored as it must be
+    // drawn to an integer grid in order for the CharAtlas "stamps" to not be
+    // blurry. When text is drawn to the grid not using the CharAtlas, it is
+    // clipped to ensure there is no overlap with the next cell.
+    this.dimensions.scaledCharWidth = Math.floor(this._terminal.charMeasure.width * window.devicePixelRatio);
+
+    // Calculate the scaled character height. Height is ceiled in case
+    // devicePixelRatio is a floating point number in order to ensure there is
+    // enough space to draw the character to the cell.
+    this.dimensions.scaledCharHeight = Math.ceil(this._terminal.charMeasure.height * window.devicePixelRatio);
+
+    // Calculate the scaled cell 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.scaledCellHeight = 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.scaledCharTop = this._terminal.options.lineHeight === 1 ? 0 : Math.round((this.dimensions.scaledCellHeight - this.dimensions.scaledCharHeight) / 2);
+
+    // Calculate the scaled cell width, taking the letterSpacing into account.
+    this.dimensions.scaledCellWidth = this.dimensions.scaledCharWidth + Math.round(this._terminal.options.letterSpacing);
+
+    // Calculate the x coordinate with a cell that text should draw from in
+    // order to draw in the center of a cell.
+    this.dimensions.scaledCharLeft = Math.floor(this._terminal.options.letterSpacing / 2);
+
+    // Recalculate the canvas dimensions; scaled* define the actual number of
+    // pixel in the canvas
+    this.dimensions.scaledCanvasHeight = this._terminal.rows * this.dimensions.scaledCellHeight;
+    this.dimensions.scaledCanvasWidth = this._terminal.cols * this.dimensions.scaledCellWidth;
+
+    // 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);
+
+    // Get the _actual_ dimensions of an individual cell. This needs to be
+    // derived from the canvasWidth/Height calculated above which takes into
+    // account window.devicePixelRatio. CharMeasure.width/height by itself is
+    // insufficient when the page is not at 100% zoom level as CharMeasure is
+    // measured in CSS pixels, but the actual char size on the canvas can
+    // differ.
+    this.dimensions.actualCellHeight = this.dimensions.canvasHeight / this._terminal.rows;
+    this.dimensions.actualCellWidth = this.dimensions.canvasWidth / this._terminal.cols;
+
+  }
+
 }

src/utils/CharMeasure.test.ts

@@ -25,41 +25,18 @@ describe('CharMeasure', () => {
   });
 
   describe('measure', () => {
-    it('should set _measureElement on first call', () => {
-      charMeasure.measure({});
-      assert.isDefined((<any>charMeasure)._measureElement, 'CharMeasure.measure should have created _measureElement');
+    it('should have _measureElement', () => {
+      assert.isDefined((<any>charMeasure)._measureElement, 'new CharMeasure() should have created _measureElement');
     });
 
-    it('should be performed async on first call', done => {
-      assert.equal(charMeasure.width, null);
-      charMeasure.measure({});
+    it('should be performed sync', () => {
       // Mock getBoundingClientRect since jsdom doesn't have a layout engine
       (<any>charMeasure)._measureElement.getBoundingClientRect = () => {
         return { width: 1, height: 1 };
       };
-      assert.equal(charMeasure.width, null);
-      setTimeout(() => {
-        assert.equal(charMeasure.width, 1);
-        done();
-      }, 0);
-    });
-
-    it('should be performed sync on successive calls', done => {
       charMeasure.measure({});
-      // Mock getBoundingClientRect since jsdom doesn't have a layout engine
-      (<any>charMeasure)._measureElement.getBoundingClientRect = () => {
-        return { width: 1, height: 1 };
-      };
-      setTimeout(() => {
-        const firstWidth = charMeasure.width;
-        // Mock getBoundingClientRect since jsdom doesn't have a layout engine
-        (<any>charMeasure)._measureElement.getBoundingClientRect = () => {
-          return { width: 2, height: 2 };
-        };
-        charMeasure.measure({});
-        assert.equal(charMeasure.width, firstWidth * 2);
-        done();
-      }, 0);
+      assert.equal(charMeasure.height, 1);
+      assert.equal(charMeasure.width, 1);
     });
 
     it('should NOT do a measure when the parent is hidden', done => {

src/utils/CharMeasure.ts

@@ -22,6 +22,14 @@ export class CharMeasure extends EventEmitter implements ICharMeasure {
     super();
     this._document = document;
     this._parentElement = parentElement;
+    this._measureElement = this._document.createElement('span');
+    this._measureElement.style.position = 'absolute';
+    this._measureElement.style.top = '0';
+    this._measureElement.style.left = '-9999em';
+    this._measureElement.style.lineHeight = 'normal';
+    this._measureElement.textContent = 'W';
+    this._measureElement.setAttribute('aria-hidden', 'true');
+    this._parentElement.appendChild(this._measureElement);
   }
 
   public get width(): number {
@@ -33,24 +41,6 @@ export class CharMeasure extends EventEmitter implements ICharMeasure {
   }
 
   public measure(options: ITerminalOptions): void {
-    if (!this._measureElement) {
-      this._measureElement = this._document.createElement('span');
-      this._measureElement.style.position = 'absolute';
-      this._measureElement.style.top = '0';
-      this._measureElement.style.left = '-9999em';
-      this._measureElement.style.lineHeight = 'normal';
-      this._measureElement.textContent = 'W';
-      this._measureElement.setAttribute('aria-hidden', 'true');
-      this._parentElement.appendChild(this._measureElement);
-      // Perform _doMeasure async if the element was just attached as sometimes
-      // getBoundingClientRect does not return accurate values without this.
-      setTimeout(() => this._doMeasure(options), 0);
-    } else {
-      this._doMeasure(options);
-    }
-  }
-
-  private _doMeasure(options: ITerminalOptions): void {
     this._measureElement.style.fontFamily = options.fontFamily;
     this._measureElement.style.fontSize = `${options.fontSize}px`;
     const geometry = this._measureElement.getBoundingClientRect();
@@ -65,4 +55,5 @@ export class CharMeasure extends EventEmitter implements ICharMeasure {
       this.emit('charsizechanged');
     }
   }
+
 }