Revert "Remove IGlyphIdentifier in-between object"

This reverts commit 04f2d52f26.

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,
+      {chars, code, bg, fg, bold: bold && terminal.options.enableBold, dim, italic},
-      code,
-      bg,
-      fg,
-      bold,
-      dim,
-      italic,
       x * this._scaledCellWidth + this._scaledCharLeft,
       y * this._scaledCellHeight + this._scaledCharTop
     );

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
+   * @param glyph Information about what to draw
-   * 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,
-    chars: string,
+    glyph: IGlyphIdentifier,
-    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, 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,
+    glyph: IGlyphIdentifier,
-    code: number,
-    bg: number,
-    fg: number,
-    bold: boolean,
-    dim: boolean,
-    italic: boolean,
     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) {
+    } else if (glyph.bg < 256) {
-      return this._getColorFromAnsiIndex(bg);
+      return this._getColorFromAnsiIndex(glyph.bg);
     }
     return this._config.colors.background;
   }
 
-  private _getForegroundColor(fg: number): IColor {
+  private _getForegroundColor(glyph: IGlyphIdentifier): IColor {
-    if (fg === INVERTED_DEFAULT_COLOR) {
+    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(
+  private _drawToCache(glyph: IGlyphIdentifier, index: number): IGlyphCacheValue {
-    chars: string,
-    code: number,
-    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(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 fontWeight = glyph.bold ? this._config.fontWeightBold : this._config.fontWeight;
-    const fontStyle = italic ? 'italic' : '';
+    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(
+  private _addGlyphToBitmap(glyph: IGlyphIdentifier): void {
-    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
@@ -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) {

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,
+    glyph: IGlyphIdentifier,
-    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 } 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 {
+  private _isCached(glyph: IGlyphIdentifier, colorIndex: number): boolean {
-    const isAscii = code < 256;
+    const isAscii = glyph.code < 256;
     // A color is basic if it is one of the 4 bit ANSI colors.
-    const isBasicColor = fg < 16;
+    const isBasicColor = glyph.fg < 16;
-    const isDefaultColor = fg >= 256;
+    const isDefaultColor = glyph.fg >= 256;
-    const isDefaultBackground = bg >= 256;
+    const isDefaultBackground = glyph.bg >= 256;
-    return isAscii && (isBasicColor || isDefaultColor) && isDefaultBackground && !italic;
+    return isAscii && (isBasicColor || isDefaultColor) && isDefaultBackground && !glyph.italic;
   }
 
   public draw(
     ctx: CanvasRenderingContext2D,
-    chars: string,
+    glyph: IGlyphIdentifier,
-    code: number,
-    bg: number,
-    fg: number,
-    bold: boolean,
-    dim: boolean,
-    italic: boolean,
     x: number,
     y: number
   ): boolean {
@@ -64,15 +58,15 @@ export default class StaticCharAtlas extends BaseCharAtlas {
     }
 
     let colorIndex = 0;
-    if (fg < 256) {
+    if (glyph.fg < 256) {
-      colorIndex = 2 + fg + (bold ? 16 : 0);
+      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,

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