Remove IGlyphIdentifier in-between object

This was being constructed for every character draw

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

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;

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

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 {

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,

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