Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dynamic atlas: Reduce unnecessary objects generated and draw from ImageBitmap #1692

Merged
merged 18 commits into from Sep 23, 2018
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 23 additions & 2 deletions src/renderer/BaseRenderLayer.ts
Expand Up @@ -5,7 +5,7 @@

import { IRenderLayer, IColorSet, IRenderDimensions } from './Types';
import { CharData, ITerminal } from '../Types';
import { DIM_OPACITY, INVERTED_DEFAULT_COLOR } from './atlas/Types';
import { DIM_OPACITY, INVERTED_DEFAULT_COLOR, IGlyphIdentifier } from './atlas/Types';
import BaseCharAtlas from './atlas/BaseCharAtlas';
import { acquireCharAtlas } from './atlas/CharAtlasCache';
import { CHAR_DATA_CHAR_INDEX } from '../Buffer';
Expand All @@ -22,6 +22,19 @@ export abstract class BaseRenderLayer implements IRenderLayer {

protected _charAtlas: BaseCharAtlas;

/**
* An object that's reused when drawing glyphs in order to reduce GC.
*/
private _currentGlyphIdentifier: IGlyphIdentifier = {
Tyriar marked this conversation as resolved.
Show resolved Hide resolved
chars: '',
code: 0,
bg: 0,
fg: 0,
bold: false,
dim: false,
italic: false
};

constructor(
private _container: HTMLElement,
id: string,
Expand All @@ -38,6 +51,7 @@ export abstract class BaseRenderLayer implements IRenderLayer {

public dispose(): void {
this._container.removeChild(this._canvas);
this._charAtlas.dispose();
}

private _initCanvas(): void {
Expand Down Expand Up @@ -245,9 +259,16 @@ export abstract class BaseRenderLayer implements IRenderLayer {
const drawInBrightColor = terminal.options.drawBoldTextInBrightColors && bold && fg < 8 && fg !== INVERTED_DEFAULT_COLOR;

fg += drawInBrightColor ? 8 : 0;
this._currentGlyphIdentifier.chars = chars;
this._currentGlyphIdentifier.code = code;
this._currentGlyphIdentifier.bg = bg;
this._currentGlyphIdentifier.fg = fg;
this._currentGlyphIdentifier.bold = bold && terminal.options.enableBold;
this._currentGlyphIdentifier.dim = dim;
this._currentGlyphIdentifier.italic = italic;
const atlasDidDraw = this._charAtlas && this._charAtlas.draw(
this._ctx,
{chars, code, bg, fg, bold: bold && terminal.options.enableBold, dim, italic},
this._currentGlyphIdentifier,
x * this._scaledCellWidth + this._scaledCharLeft,
y * this._scaledCellHeight + this._scaledCharTop
);
Expand Down
5 changes: 4 additions & 1 deletion src/renderer/atlas/BaseCharAtlas.ts
Expand Up @@ -4,10 +4,13 @@
*/

import { IGlyphIdentifier } from './Types';
import { IDisposable } from 'xterm';

export default abstract class BaseCharAtlas {
export default abstract class BaseCharAtlas implements IDisposable {
private _didWarmUp: boolean = false;

public dispose(): void { }

/**
* Perform any work needed to warm the cache before it can be used. May be called multiple times.
* Implement _doWarmUp instead if you only want to get called once.
Expand Down
114 changes: 101 additions & 13 deletions src/renderer/atlas/DynamicCharAtlas.ts
Expand Up @@ -10,6 +10,7 @@ import BaseCharAtlas from './BaseCharAtlas';
import { DEFAULT_ANSI_COLORS } from '../ColorManager';
import { clearColor } from '../../shared/atlas/CharAtlasGenerator';
import LRUMap from './LRUMap';
import { isFirefox, isSafari } from '../../shared/utils/Browser';

// In practice we're probably never going to exhaust a texture this large. For debugging purposes,
// however, it can be useful to set this to a really tiny value, to verify that LRU eviction works.
Expand All @@ -29,14 +30,29 @@ const TRANSPARENT_COLOR = {
// cache.
const FRAME_CACHE_DRAW_LIMIT = 100;

/**
* The number of milliseconds to wait before generating the ImageBitmap, this is to debounce/batch
* the operation as window.createImageBitmap is asynchronous.
*/
const GLYPH_BITMAP_COMMIT_DELAY = 100;

interface IGlyphCacheValue {
index: number;
isEmpty: boolean;
inBitmap: 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(glyph: IGlyphIdentifier): number {
// Note that this only returns a valid key when code < 256
// Layout:
// 0b00000000000000000000000000000001: italic (1)
// 0b00000000000000000000000000000010: dim (1)
// 0b00000000000000000000000000000100: bold (1)
// 0b00000000000000000000111111111000: fg (9)
// 0b00000000000111111111000000000000: bg (9)
// 0b00011111111000000000000000000000: code (8)
// 0b11100000000000000000000000000000: unused (3)
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 {
Expand All @@ -57,6 +73,18 @@ export default class DynamicCharAtlas extends BaseCharAtlas {

private _drawToCacheCount: number = 0;

// An array of glyph keys that are waiting on the bitmap to be generated.
private _glyphsWaitingOnBitmapQueue: IGlyphCacheValue[] = [];

// The number of glyphs keys waiting on the bitmap to be generated.
private _glyphsWaitingOnBitmapCount: number = 0;
Tyriar marked this conversation as resolved.
Show resolved Hide resolved

// The timeout that is used to batch bitmap generation so it's not requested for every new glyph.
private _bitmapCommitTimeout: number | null = null;

// The bitmap to draw from, this is much faster on other browsers than others.
private _bitmap: ImageBitmap | null = null;

constructor(document: Document, private _config: ICharAtlasConfig) {
super();
this._cacheCanvas = document.createElement('canvas');
Expand All @@ -82,6 +110,13 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
// document.body.appendChild(this._cacheCanvas);
}

public dispose(): void {
if (this._bitmapCommitTimeout !== null) {
window.clearTimeout(this._bitmapCommitTimeout);
this._bitmapCommitTimeout = null;
}
}

public beginFrame(): void {
this._drawToCacheCount = 0;
}
Expand All @@ -92,6 +127,11 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
x: number,
y: number
): boolean {
// Space is always an empty cell, special case this as it's so common
if (glyph.code === 32) {
return true;
}

const glyphKey = getGlyphCacheKey(glyph);
const cacheValue = this._cacheMap.get(glyphKey);
if (cacheValue !== null && cacheValue !== undefined) {
Expand Down Expand Up @@ -124,11 +164,12 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
return glyph.code < 256;
}

private _toCoordinates(index: number): [number, number] {
return [
(index % this._width) * this._config.scaledCharWidth,
Math.floor(index / this._width) * this._config.scaledCharHeight
];
private _toCoordinateX(index: number): number {
return (index % this._width) * this._config.scaledCharWidth;
}

private _toCoordinateY(index: number): number {
return Math.floor(index / this._width) * this._config.scaledCharHeight;
}

private _drawFromCache(
Expand All @@ -141,9 +182,10 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
if (cacheValue.isEmpty) {
return;
}
const [cacheX, cacheY] = this._toCoordinates(cacheValue.index);
const cacheX = this._toCoordinateX(cacheValue.index);
const cacheY = this._toCoordinateY(cacheValue.index);
ctx.drawImage(
this._cacheCanvas,
cacheValue.inBitmap ? this._bitmap : this._cacheCanvas,
cacheX,
cacheY,
this._config.scaledCharWidth,
Expand Down Expand Up @@ -230,13 +272,59 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
}

// copy the data from imageData to _cacheCanvas
const [x, y] = this._toCoordinates(index);
const x = this._toCoordinateX(index);
const y = this._toCoordinateY(index);
// putImageData doesn't do any blending, so it will overwrite any existing cache entry for us
this._cacheCtx.putImageData(imageData, x, y);

return {
// Add the glyph and queue it to the bitmap (if the browser supports it)
const cacheValue = {
index,
isEmpty
isEmpty,
inBitmap: false
};
this._addGlyphToBitmap(cacheValue);

return cacheValue;
}

private _addGlyphToBitmap(cacheValue: IGlyphCacheValue): 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
// performance (tested on v55).
if (!('createImageBitmap' in window) || isFirefox || isSafari) {
return;
}

// Add the glyph to the queue
this._glyphsWaitingOnBitmapQueue.push(cacheValue);
this._glyphsWaitingOnBitmapCount++;

// Check if bitmap generation timeout already exists
if (this._bitmapCommitTimeout !== null) {
return;
}

this._bitmapCommitTimeout = window.setTimeout(() => this._generateBitmap(), GLYPH_BITMAP_COMMIT_DELAY);
Tyriar marked this conversation as resolved.
Show resolved Hide resolved
}

private _generateBitmap(): void {
let countAtGeneration = this._glyphsWaitingOnBitmapCount;
window.createImageBitmap(this._cacheCanvas).then(bitmap => {
// Set bitmap
this._bitmap = bitmap;

// Mark all new glyphs as in bitmap, excluding glyphs that came in after
// the bitmap was requested
while (countAtGeneration-- > 0) {
const value = this._glyphsWaitingOnBitmapQueue[0];
Tyriar marked this conversation as resolved.
Show resolved Hide resolved
// If the value has already been evicted, do nothing
if (value) {
Tyriar marked this conversation as resolved.
Show resolved Hide resolved
value.inBitmap = true;
}
}
Tyriar marked this conversation as resolved.
Show resolved Hide resolved
});
this._bitmapCommitTimeout = null;
}
}
56 changes: 28 additions & 28 deletions src/renderer/atlas/LRUMap.test.ts
Expand Up @@ -9,57 +9,57 @@ import LRUMap from './LRUMap';
describe('LRUMap', () => {
it('can be used to store and retrieve values', () => {
const map = new LRUMap(10);
map.set('keya', 'valuea');
map.set('keyb', 'valueb');
map.set('keyc', 'valuec');
assert.strictEqual(map.get('keya'), 'valuea');
assert.strictEqual(map.get('keyb'), 'valueb');
assert.strictEqual(map.get('keyc'), 'valuec');
map.set(1, 'valuea');
map.set(2, 'valueb');
map.set(3, 'valuec');
assert.strictEqual(map.get(1), 'valuea');
assert.strictEqual(map.get(2), 'valueb');
assert.strictEqual(map.get(3), 'valuec');
});

it('maintains a size from insertions', () => {
const map = new LRUMap(10);
assert.strictEqual(map.size, 0);
map.set('a', 'value');
map.set(1, 'value');
assert.strictEqual(map.size, 1);
map.set('b', 'value');
map.set(2, 'value');
assert.strictEqual(map.size, 2);
});

it('deletes the oldest entry when the capacity is exceeded', () => {
const map = new LRUMap(4);
map.set('a', 'value');
map.set('b', 'value');
map.set('c', 'value');
map.set('d', 'value');
map.set('e', 'value');
assert.isNull(map.get('a'));
assert.isNotNull(map.get('b'));
assert.isNotNull(map.get('c'));
assert.isNotNull(map.get('d'));
assert.isNotNull(map.get('e'));
map.set(1, 'value');
map.set(2, 'value');
map.set(3, 'value');
map.set(4, 'value');
map.set(5, 'value');
assert.isNull(map.get(1));
assert.isNotNull(map.get(2));
assert.isNotNull(map.get(3));
assert.isNotNull(map.get(4));
assert.isNotNull(map.get(5));
assert.strictEqual(map.size, 4);
});

it('prevents a recently accessed entry from getting deleted', () => {
const map = new LRUMap(2);
map.set('a', 'value');
map.set('b', 'value');
map.get('a');
map.set(1, 'value');
map.set(2, 'value');
map.get(1);
// a would normally get deleted here, except that we called get()
map.set('c', 'value');
assert.isNotNull(map.get('a'));
map.set(3, 'value');
assert.isNotNull(map.get(1));
// b got deleted instead of a
assert.isNull(map.get('b'));
assert.isNotNull(map.get('c'));
assert.isNull(map.get(2));
assert.isNotNull(map.get(3));
});

it('supports mutation', () => {
const map = new LRUMap(10);
map.set('keya', 'oldvalue');
map.set('keya', 'newvalue');
map.set(1, 'oldvalue');
map.set(1, 'newvalue');
// mutation doesn't change the size
assert.strictEqual(map.size, 1);
assert.strictEqual(map.get('keya'), 'newvalue');
assert.strictEqual(map.get(1), 'newvalue');
});
});
19 changes: 15 additions & 4 deletions src/renderer/atlas/LRUMap.ts
Expand Up @@ -6,12 +6,12 @@
interface ILinkedListNode<T> {
prev: ILinkedListNode<T>;
next: ILinkedListNode<T>;
key: string;
key: number;
Tyriar marked this conversation as resolved.
Show resolved Hide resolved
value: T;
}

export default class LRUMap<T> {
private _map: { [key: string]: ILinkedListNode<T> } = {};
private _map: { [key: number]: ILinkedListNode<T> } = {};
Tyriar marked this conversation as resolved.
Show resolved Hide resolved
private _head: ILinkedListNode<T> = null;
private _tail: ILinkedListNode<T> = null;
private _nodePool: ILinkedListNode<T>[] = [];
Expand Down Expand Up @@ -68,7 +68,7 @@ export default class LRUMap<T> {
}
}

public get(key: string): T | null {
public get(key: number): T | null {
// This is unsafe: We're assuming our keyspace doesn't overlap with Object.prototype. However,
// it's faster than calling hasOwnProperty, and in our case, it would never overlap.
const node = this._map[key];
Expand All @@ -80,12 +80,23 @@ export default class LRUMap<T> {
return null;
}

/**
* Gets a value from a key without marking it as the most recently used item.
*/
public peekValue(key: number): T | null {
const node = this._map[key];
if (node !== undefined) {
return node.value;
}
return null;
}

public peek(): T | null {
const head = this._head;
return head === null ? null : head.value;
}

public set(key: string, value: T): void {
public set(key: number, value: T): void {
// This is unsafe: See note above.
let node = this._map[key];
if (node !== undefined) {
Expand Down