From 04f2d52f26342dc9a88ba65d28e889a43f38ab39 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 14 Sep 2018 09:58:49 -0700 Subject: [PATCH] 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; -}