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 1 commit
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
16 changes: 12 additions & 4 deletions src/renderer/atlas/DynamicCharAtlas.ts
Expand Up @@ -34,9 +34,17 @@ interface IGlyphCacheValue {
isEmpty: boolean;
}

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}`;
function getGlyphCacheKey(code: number, fg: number, bg: number, bold: boolean, dim: boolean, italic: boolean): 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 code << 21 | bg << 12 | fg << 3 | (bold ? 0 : 4) + (dim ? 0 : 2) + (italic ? 0 : 1);
Tyriar marked this conversation as resolved.
Show resolved Hide resolved
}

export default class DynamicCharAtlas extends BaseCharAtlas {
Expand Down Expand Up @@ -98,7 +106,7 @@ export default class DynamicCharAtlas extends BaseCharAtlas {
x: number,
y: number
): boolean {
const glyphKey = getGlyphCacheKey(chars, fg, bg, bold, dim, italic);
const glyphKey = getGlyphCacheKey(code, fg, bg, bold, dim, italic);
const cacheValue = this._cacheMap.get(glyphKey);
if (cacheValue !== null && cacheValue !== undefined) {
this._drawFromCache(ctx, cacheValue, x, y);
Expand Down
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');
});
});
8 changes: 4 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 @@ -85,7 +85,7 @@ export default class LRUMap<T> {
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