From 3d2035e1a068ca54e5a4c0b87a308ba53f94cfd1 Mon Sep 17 00:00:00 2001 From: gou4shi1 Date: Thu, 15 Nov 2018 11:43:38 +0800 Subject: [PATCH 1/7] getCoordsRelativeToElement: use getBoundingClientRect and clientX/Y. --- src/utils/MouseHelper.ts | 27 ++++----------------------- 1 file changed, 4 insertions(+), 23 deletions(-) diff --git a/src/utils/MouseHelper.ts b/src/utils/MouseHelper.ts index ca1bb27e..c3f78e84 100644 --- a/src/utils/MouseHelper.ts +++ b/src/utils/MouseHelper.ts @@ -13,29 +13,10 @@ export class MouseHelper { this._renderer = renderer; } - public static getCoordsRelativeToElement(event: {pageX: number, pageY: number}, element: HTMLElement): [number, number] { - // Ignore browsers that don't support MouseEvent.pageX - if (event.pageX === null || event.pageX === undefined) { - return null; - } - - const originalElement = element; - let x = event.pageX; - let y = event.pageY; - - // Converts the coordinates from being relative to the document to being - // relative to the terminal. - while (element) { - x -= element.offsetLeft; - y -= element.offsetTop; - element = element.offsetParent; - } - element = originalElement; - while (element && element !== element.ownerDocument.body) { - x += element.scrollLeft; - y += element.scrollTop; - element = element.parentElement; - } + public static getCoordsRelativeToElement(event: {target: HTMLElement, clientX: number, clientY: number}, element: HTMLElement): [number, number] { + let rect = event.target.getBoundingClientRect(); + let x = event.clientX - rect.left; + let y = event.clientY - rect.top; return [x, y]; } From 3ca0598279d5526a4e50607f2b0b6720d507030a Mon Sep 17 00:00:00 2001 From: gou4shi1 Date: Thu, 15 Nov 2018 11:56:41 +0800 Subject: [PATCH 2/7] fix EventTarget type --- src/utils/MouseHelper.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/MouseHelper.ts b/src/utils/MouseHelper.ts index c3f78e84..11efd647 100644 --- a/src/utils/MouseHelper.ts +++ b/src/utils/MouseHelper.ts @@ -13,7 +13,7 @@ export class MouseHelper { this._renderer = renderer; } - public static getCoordsRelativeToElement(event: {target: HTMLElement, clientX: number, clientY: number}, element: HTMLElement): [number, number] { + public static getCoordsRelativeToElement(event: {target: EventTarget, clientX: number, clientY: number}, element: HTMLElement): [number, number] { let rect = event.target.getBoundingClientRect(); let x = event.clientX - rect.left; let y = event.clientY - rect.top; From 7f181999854930c8fa123bcdf4dca4982d0e7c41 Mon Sep 17 00:00:00 2001 From: gou4shi1 Date: Thu, 6 Dec 2018 14:37:27 +0800 Subject: [PATCH 3/7] Fix build fail --- src/utils/MouseHelper.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/MouseHelper.ts b/src/utils/MouseHelper.ts index 11efd647..7870f477 100644 --- a/src/utils/MouseHelper.ts +++ b/src/utils/MouseHelper.ts @@ -33,7 +33,7 @@ export class MouseHelper { * apply an offset to the x value such that the left half of the cell will * select that cell and the right half will select the next cell. */ - public getCoords(event: {pageX: number, pageY: number}, element: HTMLElement, charMeasure: ICharMeasure, lineHeight: number, colCount: number, rowCount: number, isSelection?: boolean): [number, number] { + public getCoords(event: {target: EventTarget, pageX: number, pageY: number}, element: HTMLElement, charMeasure: ICharMeasure, lineHeight: number, colCount: number, rowCount: number, isSelection?: boolean): [number, number] { // Coordinates cannot be measured if charMeasure has not been initialized if (!charMeasure.width || !charMeasure.height) { return null; From b5b2bcd0c911f4261abecd106a3b0f660ace621f Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sun, 9 Dec 2018 09:08:11 -0800 Subject: [PATCH 4/7] Allow decimal line heights in demo --- demo/client.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/demo/client.ts b/demo/client.ts index d5196d37..6c69899e 100644 --- a/demo/client.ts +++ b/demo/client.ts @@ -235,7 +235,7 @@ function initOptions(term: TerminalType): void { }); html += '
'; numberOptions.forEach(o => { - html += `
`; + html += `
`; }); html += '
'; Object.keys(stringOptions).forEach(o => { @@ -265,7 +265,7 @@ function initOptions(term: TerminalType): void { if (o === 'cols' || o === 'rows') { updateTerminalSize(); } else { - term.setOption(o, parseInt(input.value, 10)); + term.setOption(o, o === 'lineHeight' ? parseFloat(input.value) : parseInt(input.value, 10)); } }); }); From 693e17a5cbbea22e6bdcfeb98c71bb300dc9681d Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sun, 9 Dec 2018 09:13:30 -0800 Subject: [PATCH 5/7] Add missing break in options code Atlas is reset as part of onResize, this should be safe --- src/Terminal.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Terminal.ts b/src/Terminal.ts index 2cfc1ca8..779d9bf5 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -464,6 +464,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II this.renderer.onResize(this.cols, this.rows); this.refresh(0, this.rows - 1); } + break; case 'rendererType': if (this.renderer) { this.unregister(this.renderer); From fe9eb7d31ebb0a5935eaf311d97dfbdeb5102477 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sun, 9 Dec 2018 09:19:56 -0800 Subject: [PATCH 6/7] Call dispose when atlas is no longer used in cache Only runtime change is that this clears the bitmap commit timeout in DynamicCharAtlas --- src/renderer/atlas/CharAtlasCache.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/renderer/atlas/CharAtlasCache.ts b/src/renderer/atlas/CharAtlasCache.ts index 3cb0e1cd..388a181b 100644 --- a/src/renderer/atlas/CharAtlasCache.ts +++ b/src/renderer/atlas/CharAtlasCache.ts @@ -42,8 +42,6 @@ export function acquireCharAtlas( ): BaseCharAtlas { const newConfig = generateConfig(scaledCharWidth, scaledCharHeight, terminal, colors); - // TODO: Currently if a terminal changes configs it will not free the entry reference (until it's disposed) - // Check to see if the terminal already owns this config for (let i = 0; i < charAtlasCache.length; i++) { const entry = charAtlasCache[i]; @@ -54,6 +52,7 @@ export function acquireCharAtlas( } // The configs differ, release the terminal from the entry if (entry.ownedBy.length === 1) { + entry.atlas.dispose(); charAtlasCache.splice(i, 1); } else { entry.ownedBy.splice(ownedByIndex, 1); @@ -94,6 +93,7 @@ export function removeTerminalFromCache(terminal: ITerminal): void { if (index !== -1) { if (charAtlasCache[i].ownedBy.length === 1) { // Remove the cache entry if it's the only terminal + charAtlasCache[i].atlas.dispose(); charAtlasCache.splice(i, 1); } else { // Remove the reference from the cache entry From 6b9fb48898eb9d3741660d88deee1af747db5929 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sun, 9 Dec 2018 09:59:33 -0800 Subject: [PATCH 7/7] Fix tests/lint, use element instead of event.target --- src/Types.ts | 2 +- src/utils/MouseHelper.test.ts | 20 +++++++------------- src/utils/MouseHelper.ts | 14 ++++++-------- 3 files changed, 14 insertions(+), 22 deletions(-) diff --git a/src/Types.ts b/src/Types.ts index e8578426..d0830fb1 100644 --- a/src/Types.ts +++ b/src/Types.ts @@ -246,7 +246,7 @@ export interface ILinkifierAccessor { } export interface IMouseHelper { - getCoords(event: { pageX: number, pageY: number }, element: HTMLElement, charMeasure: ICharMeasure, lineHeight: number, colCount: number, rowCount: number, isSelection?: boolean): [number, number]; + getCoords(event: { clientX: number, clientY: number }, element: HTMLElement, charMeasure: ICharMeasure, lineHeight: number, colCount: number, rowCount: number, isSelection?: boolean): [number, number]; getRawByteCoords(event: MouseEvent, element: HTMLElement, charMeasure: ICharMeasure, lineHeight: number, colCount: number, rowCount: number): { x: number, y: number }; } diff --git a/src/utils/MouseHelper.test.ts b/src/utils/MouseHelper.test.ts index ac3137f4..23fa7a67 100644 --- a/src/utils/MouseHelper.test.ts +++ b/src/utils/MouseHelper.test.ts @@ -37,34 +37,28 @@ describe('MouseHelper.getCoords', () => { describe('when charMeasure is not initialized', () => { it('should return null', () => { charMeasure = new MockCharMeasure(); - assert.equal(mouseHelper.getCoords({ pageX: 0, pageY: 0 }, document.createElement('div'), charMeasure, 1, 10, 10), null); - }); - }); - - describe('when pageX/pageY are not supported', () => { - it('should return null', () => { - assert.equal(mouseHelper.getCoords({ pageX: undefined, pageY: undefined }, document.createElement('div'), charMeasure, 1, 10, 10), null); + assert.equal(mouseHelper.getCoords({ clientX: 0, clientY: 0 }, document.createElement('div'), charMeasure, 1, 10, 10), null); }); }); it('should return the cell that was clicked', () => { let coords: [number, number]; - coords = mouseHelper.getCoords({ pageX: CHAR_WIDTH / 2, pageY: CHAR_HEIGHT / 2 }, document.createElement('div'), charMeasure, 1, 10, 10); + coords = mouseHelper.getCoords({ clientX: CHAR_WIDTH / 2, clientY: CHAR_HEIGHT / 2 }, document.createElement('div'), charMeasure, 1, 10, 10); assert.deepEqual(coords, [1, 1]); - coords = mouseHelper.getCoords({ pageX: CHAR_WIDTH, pageY: CHAR_HEIGHT }, document.createElement('div'), charMeasure, 1, 10, 10); + coords = mouseHelper.getCoords({ clientX: CHAR_WIDTH, clientY: CHAR_HEIGHT }, document.createElement('div'), charMeasure, 1, 10, 10); assert.deepEqual(coords, [1, 1]); - coords = mouseHelper.getCoords({ pageX: CHAR_WIDTH, pageY: CHAR_HEIGHT + 1 }, document.createElement('div'), charMeasure, 1, 10, 10); + coords = mouseHelper.getCoords({ clientX: CHAR_WIDTH, clientY: CHAR_HEIGHT + 1 }, document.createElement('div'), charMeasure, 1, 10, 10); assert.deepEqual(coords, [1, 2]); - coords = mouseHelper.getCoords({ pageX: CHAR_WIDTH + 1, pageY: CHAR_HEIGHT }, document.createElement('div'), charMeasure, 1, 10, 10); + coords = mouseHelper.getCoords({ clientX: CHAR_WIDTH + 1, clientY: CHAR_HEIGHT }, document.createElement('div'), charMeasure, 1, 10, 10); assert.deepEqual(coords, [2, 1]); }); it('should ensure the coordinates are returned within the terminal bounds', () => { let coords: [number, number]; - coords = mouseHelper.getCoords({ pageX: -1, pageY: -1 }, document.createElement('div'), charMeasure, 1, 10, 10); + coords = mouseHelper.getCoords({ clientX: -1, clientY: -1 }, document.createElement('div'), charMeasure, 1, 10, 10); assert.deepEqual(coords, [1, 1]); // Event are double the cols/rows - coords = mouseHelper.getCoords({ pageX: CHAR_WIDTH * 20, pageY: CHAR_HEIGHT * 20 }, document.createElement('div'), charMeasure, 1, 10, 10); + coords = mouseHelper.getCoords({ clientX: CHAR_WIDTH * 20, clientY: CHAR_HEIGHT * 20 }, document.createElement('div'), charMeasure, 1, 10, 10); assert.deepEqual(coords, [10, 10], 'coordinates should never come back as larger than the terminal'); }); }); diff --git a/src/utils/MouseHelper.ts b/src/utils/MouseHelper.ts index 7870f477..967218b6 100644 --- a/src/utils/MouseHelper.ts +++ b/src/utils/MouseHelper.ts @@ -3,21 +3,19 @@ * @license MIT */ -import { ICharMeasure } from '../Types'; +import { ICharMeasure, IMouseHelper } from '../Types'; import { IRenderer } from '../renderer/Types'; -export class MouseHelper { +export class MouseHelper implements IMouseHelper { constructor(private _renderer: IRenderer) {} public setRenderer(renderer: IRenderer): void { this._renderer = renderer; } - public static getCoordsRelativeToElement(event: {target: EventTarget, clientX: number, clientY: number}, element: HTMLElement): [number, number] { - let rect = event.target.getBoundingClientRect(); - let x = event.clientX - rect.left; - let y = event.clientY - rect.top; - return [x, y]; + public static getCoordsRelativeToElement(event: {clientX: number, clientY: number}, element: HTMLElement): [number, number] { + const rect = element.getBoundingClientRect(); + return [event.clientX - rect.left, event.clientY - rect.top]; } /** @@ -33,7 +31,7 @@ export class MouseHelper { * apply an offset to the x value such that the left half of the cell will * select that cell and the right half will select the next cell. */ - public getCoords(event: {target: EventTarget, pageX: number, pageY: number}, element: HTMLElement, charMeasure: ICharMeasure, lineHeight: number, colCount: number, rowCount: number, isSelection?: boolean): [number, number] { + public getCoords(event: {clientX: number, clientY: number}, element: HTMLElement, charMeasure: ICharMeasure, lineHeight: number, colCount: number, rowCount: number, isSelection?: boolean): [number, number] { // Coordinates cannot be measured if charMeasure has not been initialized if (!charMeasure.width || !charMeasure.height) { return null;