From 2e243feebeeebebd46b2b21f7311e48c0a61193b Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 8 Jun 2019 14:54:05 -0700 Subject: [PATCH 01/10] Create ICharDimensionsService --- src/ui/services/CharDimensionsService.ts | 80 ++++++++++++++++++++++++ src/ui/services/Services.d.ts | 13 ++++ 2 files changed, 93 insertions(+) create mode 100644 src/ui/services/CharDimensionsService.ts create mode 100644 src/ui/services/Services.d.ts diff --git a/src/ui/services/CharDimensionsService.ts b/src/ui/services/CharDimensionsService.ts new file mode 100644 index 00000000..af44c730 --- /dev/null +++ b/src/ui/services/CharDimensionsService.ts @@ -0,0 +1,80 @@ +/** + * Copyright (c) 2016 The xterm.js authors. All rights reserved. + * @license MIT + */ + +import { IOptionsService } from 'common/options/Types'; +import { IEvent, EventEmitter2 } from 'common/EventEmitter2'; +import { ICharDimensionsService } from 'ui/services/Services'; + +export class CharDimensionsService implements ICharDimensionsService { + public width: number = 0; + public height: number = 0; + private _charDimensionsStrategy: ICharDimensionsStrategy; + + private _onCharDimensionsChange = new EventEmitter2(); + public get onCharDimensionsChange(): IEvent { return this._onCharDimensionsChange.event; } + + constructor( + document: Document, + parentElement: HTMLElement, + private _optionsService: IOptionsService + ) { + this._charDimensionsStrategy = new DomCharDimensionsStrategy(document, parentElement, this._optionsService); + } + + public measure(): void { + const result = this._charDimensionsStrategy.measure(); + this.width = result.width; + this.height = result.height; + } +} + +interface ICharDimensionsStrategy { + measure(): IReadonlyMeasureResult; +} + +interface IReadonlyMeasureResult { + readonly width: number; + readonly height: number; +} + +interface IMeasureResult { + width: number; + height: number; +} + +// TODO: For supporting browsers we should also provide a CanvasCharDimensionsProvider that uses ctx.measureText +class DomCharDimensionsStrategy implements ICharDimensionsStrategy { + private _result: IMeasureResult = { width: 0, height: 0 }; + private _measureElement: HTMLElement; + + constructor( + private _document: Document, + private _parentElement: HTMLElement, + private _optionsService: IOptionsService + ) { + this._measureElement = this._document.createElement('span'); + this._measureElement.classList.add('xterm-char-measure-element'); + this._measureElement.textContent = 'W'; + this._measureElement.setAttribute('aria-hidden', 'true'); + this._parentElement.appendChild(this._measureElement); + } + + public measure(): IReadonlyMeasureResult { + this._measureElement.style.fontFamily = this._optionsService.options.fontFamily; + this._measureElement.style.fontSize = `${this._optionsService.options.fontSize}px`; + + // Note that this triggers a synchronous layout + const geometry = this._measureElement.getBoundingClientRect(); + + // If values are 0 then the element is likely currently display:none, in which case we should + // retain the previous value. + if (geometry.width !== 0 && geometry.height !== 0) { + this._result.width = geometry.width; + this._result.height = Math.ceil(geometry.height); + } + + return this._result; + } +} diff --git a/src/ui/services/Services.d.ts b/src/ui/services/Services.d.ts new file mode 100644 index 00000000..f0c09cf9 --- /dev/null +++ b/src/ui/services/Services.d.ts @@ -0,0 +1,13 @@ +/** + * Copyright (c) 2019 The xterm.js authors. All rights reserved. + * @license MIT + */ + +import { IEvent } from 'common/EventEmitter2'; + +export interface ICharDimensionsService { + readonly width: number; + readonly height: number; + readonly onCharDimensionsChange: IEvent; + measure(): void; +} From 38bb122bbd535ac056c7e1278ed8efb21e79266b Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 8 Jun 2019 15:09:55 -0700 Subject: [PATCH 02/10] CharDimensions -> CharSize --- ...imensionsService.ts => CharSizeService.ts} | 20 ++++++++++--------- src/ui/services/Services.d.ts | 7 +++++-- 2 files changed, 16 insertions(+), 11 deletions(-) rename src/ui/services/{CharDimensionsService.ts => CharSizeService.ts} (76%) diff --git a/src/ui/services/CharDimensionsService.ts b/src/ui/services/CharSizeService.ts similarity index 76% rename from src/ui/services/CharDimensionsService.ts rename to src/ui/services/CharSizeService.ts index af44c730..2fddb802 100644 --- a/src/ui/services/CharDimensionsService.ts +++ b/src/ui/services/CharSizeService.ts @@ -5,32 +5,34 @@ import { IOptionsService } from 'common/options/Types'; import { IEvent, EventEmitter2 } from 'common/EventEmitter2'; -import { ICharDimensionsService } from 'ui/services/Services'; +import { ICharSizeService } from 'ui/services/Services'; -export class CharDimensionsService implements ICharDimensionsService { +export class CharSizeService implements ICharSizeService { public width: number = 0; public height: number = 0; - private _charDimensionsStrategy: ICharDimensionsStrategy; + private _measureStrategy: IMeasureStrategy; - private _onCharDimensionsChange = new EventEmitter2(); - public get onCharDimensionsChange(): IEvent { return this._onCharDimensionsChange.event; } + public get hasValidDimensions(): boolean { return this.width > 0 && this.height > 0; } + + private _onCharSizeChange = new EventEmitter2(); + public get onCharSizeChange(): IEvent { return this._onCharSizeChange.event; } constructor( document: Document, parentElement: HTMLElement, private _optionsService: IOptionsService ) { - this._charDimensionsStrategy = new DomCharDimensionsStrategy(document, parentElement, this._optionsService); + this._measureStrategy = new DomMeasureStrategy(document, parentElement, this._optionsService); } public measure(): void { - const result = this._charDimensionsStrategy.measure(); + const result = this._measureStrategy.measure(); this.width = result.width; this.height = result.height; } } -interface ICharDimensionsStrategy { +interface IMeasureStrategy { measure(): IReadonlyMeasureResult; } @@ -45,7 +47,7 @@ interface IMeasureResult { } // TODO: For supporting browsers we should also provide a CanvasCharDimensionsProvider that uses ctx.measureText -class DomCharDimensionsStrategy implements ICharDimensionsStrategy { +class DomMeasureStrategy implements IMeasureStrategy { private _result: IMeasureResult = { width: 0, height: 0 }; private _measureElement: HTMLElement; diff --git a/src/ui/services/Services.d.ts b/src/ui/services/Services.d.ts index f0c09cf9..51dd043a 100644 --- a/src/ui/services/Services.d.ts +++ b/src/ui/services/Services.d.ts @@ -5,9 +5,12 @@ import { IEvent } from 'common/EventEmitter2'; -export interface ICharDimensionsService { +export interface ICharSizeService { readonly width: number; readonly height: number; - readonly onCharDimensionsChange: IEvent; + readonly hasValidDimensions: boolean; + + readonly onCharSizeChange: IEvent; + measure(): void; } From 6246bb5026c073589c86b030c8fb0382462916de Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 8 Jun 2019 15:20:03 -0700 Subject: [PATCH 03/10] Adopt CharSizeService in MouseHelper --- src/MouseHelper.test.ts | 31 +++++++++--------------------- src/MouseHelper.ts | 16 ++++++++------- src/MouseZoneManager.ts | 2 +- src/SelectionManager.ts | 2 +- src/Terminal.ts | 14 ++++++++++---- src/TestUtils.test.ts | 17 ++++++++-------- src/Types.ts | 4 ++-- src/handlers/AltClickHandler.ts | 1 - src/ui/services/CharSizeService.ts | 2 +- src/ui/services/Services.d.ts | 2 +- 10 files changed, 42 insertions(+), 49 deletions(-) diff --git a/src/MouseHelper.test.ts b/src/MouseHelper.test.ts index 0925d49f..7ca165e8 100644 --- a/src/MouseHelper.test.ts +++ b/src/MouseHelper.test.ts @@ -3,62 +3,49 @@ * @license MIT */ -import jsdom = require('jsdom'); import { assert } from 'chai'; import { MouseHelper } from './MouseHelper'; -import { MockCharMeasure, MockRenderer } from './TestUtils.test'; +import { MockRenderer, MockCharSizeService } from './TestUtils.test'; const CHAR_WIDTH = 10; const CHAR_HEIGHT = 20; describe('MouseHelper.getCoords', () => { - let dom: jsdom.JSDOM; - let window: Window; - let document: Document; let mouseHelper: MouseHelper; - let charMeasure: MockCharMeasure; - beforeEach(() => { - dom = new jsdom.JSDOM(''); - window = dom.window; - document = window.document; - charMeasure = new MockCharMeasure(); - charMeasure.width = CHAR_WIDTH; - charMeasure.height = CHAR_HEIGHT; const renderer = new MockRenderer(); renderer.dimensions = { actualCellWidth: CHAR_WIDTH, actualCellHeight: CHAR_HEIGHT }; - mouseHelper = new MouseHelper(renderer as any); + mouseHelper = new MouseHelper(renderer as any, new MockCharSizeService(CHAR_WIDTH, CHAR_HEIGHT)); }); describe('when charMeasure is not initialized', () => { it('should return null', () => { - charMeasure = new MockCharMeasure(); - assert.equal(mouseHelper.getCoords({ clientX: 0, clientY: 0 }, document.createElement('div'), charMeasure, 10, 10), null); + assert.equal(mouseHelper.getCoords({ clientX: 0, clientY: 0 }, document.createElement('div'), 10, 10), null); }); }); it('should return the cell that was clicked', () => { let coords: [number, number]; - coords = mouseHelper.getCoords({ clientX: CHAR_WIDTH / 2, clientY: CHAR_HEIGHT / 2 }, document.createElement('div'), charMeasure, 10, 10); + coords = mouseHelper.getCoords({ clientX: CHAR_WIDTH / 2, clientY: CHAR_HEIGHT / 2 }, document.createElement('div'), 10, 10); assert.deepEqual(coords, [1, 1]); - coords = mouseHelper.getCoords({ clientX: CHAR_WIDTH, clientY: CHAR_HEIGHT }, document.createElement('div'), charMeasure, 10, 10); + coords = mouseHelper.getCoords({ clientX: CHAR_WIDTH, clientY: CHAR_HEIGHT }, document.createElement('div'), 10, 10); assert.deepEqual(coords, [1, 1]); - coords = mouseHelper.getCoords({ clientX: CHAR_WIDTH, clientY: CHAR_HEIGHT + 1 }, document.createElement('div'), charMeasure, 10, 10); + coords = mouseHelper.getCoords({ clientX: CHAR_WIDTH, clientY: CHAR_HEIGHT + 1 }, document.createElement('div'), 10, 10); assert.deepEqual(coords, [1, 2]); - coords = mouseHelper.getCoords({ clientX: CHAR_WIDTH + 1, clientY: CHAR_HEIGHT }, document.createElement('div'), charMeasure, 10, 10); + coords = mouseHelper.getCoords({ clientX: CHAR_WIDTH + 1, clientY: CHAR_HEIGHT }, document.createElement('div'), 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({ clientX: -1, clientY: -1 }, document.createElement('div'), charMeasure, 10, 10); + coords = mouseHelper.getCoords({ clientX: -1, clientY: -1 }, document.createElement('div'), 10, 10); assert.deepEqual(coords, [1, 1]); // Event are double the cols/rows - coords = mouseHelper.getCoords({ clientX: CHAR_WIDTH * 20, clientY: CHAR_HEIGHT * 20 }, document.createElement('div'), charMeasure, 10, 10); + coords = mouseHelper.getCoords({ clientX: CHAR_WIDTH * 20, clientY: CHAR_HEIGHT * 20 }, document.createElement('div'), 10, 10); assert.deepEqual(coords, [10, 10], 'coordinates should never come back as larger than the terminal'); }); }); diff --git a/src/MouseHelper.ts b/src/MouseHelper.ts index 12905222..091d00ae 100644 --- a/src/MouseHelper.ts +++ b/src/MouseHelper.ts @@ -3,12 +3,14 @@ * @license MIT */ -import { ICharMeasure, IMouseHelper } from './Types'; +import { IMouseHelper } from './Types'; import { RenderCoordinator } from './renderer/RenderCoordinator'; +import { ICharSizeService } from 'ui/services/Services'; export class MouseHelper implements IMouseHelper { constructor( - private _renderCoordinator: RenderCoordinator + private _renderCoordinator: RenderCoordinator, + private _charSizeService: ICharSizeService ) { } @@ -30,9 +32,9 @@ export class MouseHelper implements IMouseHelper { * 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: {clientX: number, clientY: number}, element: HTMLElement, charMeasure: ICharMeasure, colCount: number, rowCount: number, isSelection?: boolean): [number, number] { - // Coordinates cannot be measured if charMeasure has not been initialized - if (!charMeasure.width || !charMeasure.height) { + public getCoords(event: {clientX: number, clientY: number}, element: HTMLElement, colCount: number, rowCount: number, isSelection?: boolean): [number, number] { + // Coordinates cannot be measured if there are no valid + if (!this._charSizeService.hasValidSize) { return null; } @@ -63,8 +65,8 @@ export class MouseHelper implements IMouseHelper { * @param colCount The number of columns in the terminal. * @param rowCount The number of rows in the terminal. */ - public getRawByteCoords(event: MouseEvent, element: HTMLElement, charMeasure: ICharMeasure, colCount: number, rowCount: number): { x: number, y: number } { - const coords = this.getCoords(event, element, charMeasure, colCount, rowCount); + public getRawByteCoords(event: MouseEvent, element: HTMLElement, colCount: number, rowCount: number): { x: number, y: number } { + const coords = this.getCoords(event, element, colCount, rowCount); let x = coords[0]; let y = coords[1]; diff --git a/src/MouseZoneManager.ts b/src/MouseZoneManager.ts index 737bfc91..109bc517 100644 --- a/src/MouseZoneManager.ts +++ b/src/MouseZoneManager.ts @@ -203,7 +203,7 @@ export class MouseZoneManager extends Disposable implements IMouseZoneManager { } private _findZoneEventAt(e: MouseEvent): IMouseZone { - const coords = this._terminal.mouseHelper.getCoords(e, this._terminal.screenElement, this._terminal.charMeasure, this._terminal.cols, this._terminal.rows); + const coords = this._terminal.mouseHelper.getCoords(e, this._terminal.screenElement, this._terminal.cols, this._terminal.rows); if (!coords) { return null; } diff --git a/src/SelectionManager.ts b/src/SelectionManager.ts index 21283b8f..d0cba71d 100644 --- a/src/SelectionManager.ts +++ b/src/SelectionManager.ts @@ -354,7 +354,7 @@ export class SelectionManager implements ISelectionManager { * @param event The mouse event. */ private _getMouseBufferCoords(event: MouseEvent): [number, number] { - const coords = this._terminal.mouseHelper.getCoords(event, this._terminal.screenElement, this._charMeasure, this._terminal.cols, this._terminal.rows, true); + const coords = this._terminal.mouseHelper.getCoords(event, this._terminal.screenElement, this._terminal.cols, this._terminal.rows, true); if (!coords) { return null; } diff --git a/src/Terminal.ts b/src/Terminal.ts index 6feedcbb..f68d3105 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -55,6 +55,8 @@ import { ColorManager } from 'ui/ColorManager'; import { RenderCoordinator } from './renderer/RenderCoordinator'; import { IOptionsService } from 'common/options/Types'; import { OptionsService } from 'common/options/OptionsService'; +import { ICharSizeService } from 'ui/services/Services'; +import { CharSizeService } from 'ui/services/CharSizeService'; // Let it work inside Node.js for automated testing purposes. const document = (typeof window !== 'undefined') ? window.document : null; @@ -107,9 +109,12 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II private _customKeyEventHandler: CustomKeyEventHandler; - // services + // common services public optionsService: IOptionsService; + // browser services + private _charSizeService: ICharSizeService; + // modes public applicationKeypad: boolean; public applicationCursor: boolean; @@ -615,6 +620,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II this._helperContainer.appendChild(this._compositionView); this.charMeasure = new CharMeasure(document, this._helperContainer); + this._charSizeService = new CharSizeService(this._document, this._helperContainer, this.optionsService); // Performance: Add viewport and helper elements from the fragment this.element.appendChild(fragment); @@ -658,7 +664,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II })); this.register(addDisposableDomListener(this._viewportElement, 'scroll', () => this.selectionManager.refresh())); - this.mouseHelper = new MouseHelper(this._renderCoordinator); + this.mouseHelper = new MouseHelper(this._renderCoordinator, this._charSizeService); // apply mouse event classes set by escape codes before terminal was attached this.element.classList.toggle('enable-mouse-events', this.mouseEvents); if (this.mouseEvents) { @@ -738,7 +744,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II button = getButton(ev); // get mouse coordinates - pos = self.mouseHelper.getRawByteCoords(ev, self.screenElement, self.charMeasure, self.cols, self.rows); + pos = self.mouseHelper.getRawByteCoords(ev, self.screenElement, self.cols, self.rows); if (!pos) return; sendEvent(button, pos); @@ -764,7 +770,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II // ^[[M 3<^[[M@4<^[[M@5<^[[M@6<^[[M@7<^[[M#7< function sendMove(ev: MouseEvent): void { let button = pressed; - const pos = self.mouseHelper.getRawByteCoords(ev, self.screenElement, self.charMeasure, self.cols, self.rows); + const pos = self.mouseHelper.getRawByteCoords(ev, self.screenElement, self.cols, self.rows); if (!pos) return; // buttons marked as motions diff --git a/src/TestUtils.test.ts b/src/TestUtils.test.ts index e44489ea..32b7a65c 100644 --- a/src/TestUtils.test.ts +++ b/src/TestUtils.test.ts @@ -14,6 +14,7 @@ import { Terminal } from './Terminal'; import { AttributeData } from 'core/buffer/BufferLine'; import { IColorManager, IColorSet } from 'ui/Types'; import { IOptionsService } from 'common/options/Types'; +import { ICharSizeService } from 'ui/services/Services'; export class TestTerminal extends Terminal { writeSync(data: string): void { @@ -182,15 +183,6 @@ export class MockTerminal implements ITerminal { deregisterCharacterJoiner(joinerId: number): void { } } -export class MockCharMeasure implements ICharMeasure { - onCharSizeChanged: IEvent; - width: number; - height: number; - measure(options: ITerminalOptions): void { - throw new Error('Method not implemented.'); - } -} - export class MockInputHandlingTerminal implements IInputHandlingTerminal { element: HTMLElement; options: ITerminalOptions = {}; @@ -438,3 +430,10 @@ export class MockCompositionHelper implements ICompositionHelper { return true; } } + +export class MockCharSizeService implements ICharSizeService { + get hasValidSize(): boolean { return this.width > 0 && this.height > 0; } + onCharSizeChange: IEvent; + constructor(public width: number, public height: number) {} + measure(): void {} +} diff --git a/src/Types.ts b/src/Types.ts index 7b9c68c5..0aecdc96 100644 --- a/src/Types.ts +++ b/src/Types.ts @@ -284,8 +284,8 @@ export interface ILinkifierAccessor { } export interface IMouseHelper { - getCoords(event: { clientX: number, clientY: number }, element: HTMLElement, charMeasure: ICharMeasure, colCount: number, rowCount: number, isSelection?: boolean): [number, number]; - getRawByteCoords(event: MouseEvent, element: HTMLElement, charMeasure: ICharMeasure, colCount: number, rowCount: number): { x: number, y: number }; + getCoords(event: { clientX: number, clientY: number }, element: HTMLElement, colCount: number, rowCount: number, isSelection?: boolean): [number, number]; + getRawByteCoords(event: MouseEvent, element: HTMLElement, colCount: number, rowCount: number): { x: number, y: number }; } export interface ICharMeasure { diff --git a/src/handlers/AltClickHandler.ts b/src/handlers/AltClickHandler.ts index 43c26498..eebabd7d 100644 --- a/src/handlers/AltClickHandler.ts +++ b/src/handlers/AltClickHandler.ts @@ -33,7 +33,6 @@ export class AltClickHandler { const coordinates = this._terminal.mouseHelper.getCoords( this._mouseEvent, this._terminal.element, - this._terminal.charMeasure, this._terminal.cols, this._terminal.rows, false diff --git a/src/ui/services/CharSizeService.ts b/src/ui/services/CharSizeService.ts index 2fddb802..7d9d3a2c 100644 --- a/src/ui/services/CharSizeService.ts +++ b/src/ui/services/CharSizeService.ts @@ -12,7 +12,7 @@ export class CharSizeService implements ICharSizeService { public height: number = 0; private _measureStrategy: IMeasureStrategy; - public get hasValidDimensions(): boolean { return this.width > 0 && this.height > 0; } + public get hasValidSize(): boolean { return this.width > 0 && this.height > 0; } private _onCharSizeChange = new EventEmitter2(); public get onCharSizeChange(): IEvent { return this._onCharSizeChange.event; } diff --git a/src/ui/services/Services.d.ts b/src/ui/services/Services.d.ts index 51dd043a..17cc4f3b 100644 --- a/src/ui/services/Services.d.ts +++ b/src/ui/services/Services.d.ts @@ -8,7 +8,7 @@ import { IEvent } from 'common/EventEmitter2'; export interface ICharSizeService { readonly width: number; readonly height: number; - readonly hasValidDimensions: boolean; + readonly hasValidSize: boolean; readonly onCharSizeChange: IEvent; From bfcd2b52c2fdf4dce6cc2969ee5aeeab9a0fde8e Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 8 Jun 2019 15:22:17 -0700 Subject: [PATCH 04/10] Adopt CharSizeService in CompositionHelper --- src/CompositionHelper.test.ts | 7 ++----- src/CompositionHelper.ts | 8 +++++--- src/MouseHelper.test.ts | 6 ------ src/MouseHelper.ts | 2 -- src/Terminal.ts | 10 +++++----- 5 files changed, 12 insertions(+), 21 deletions(-) diff --git a/src/CompositionHelper.test.ts b/src/CompositionHelper.test.ts index 053a0c29..156f5a44 100644 --- a/src/CompositionHelper.test.ts +++ b/src/CompositionHelper.test.ts @@ -6,6 +6,7 @@ import { assert } from 'chai'; import { CompositionHelper } from './CompositionHelper'; import { ITerminal } from './Types'; +import { MockCharSizeService } from 'TestUtils.test'; describe('CompositionHelper', () => { let terminal: ITerminal; @@ -48,16 +49,12 @@ describe('CompositionHelper', () => { buffer: { isCursorInViewport: true }, - charMeasure: { - height: 10, - width: 10 - }, options: { lineHeight: 1 } } as any; handledText = ''; - compositionHelper = new CompositionHelper(textarea, compositionView, terminal); + compositionHelper = new CompositionHelper(textarea, compositionView, terminal, new MockCharSizeService(10, 10)); }); describe('Input', () => { diff --git a/src/CompositionHelper.ts b/src/CompositionHelper.ts index 840bef55..e1179fed 100644 --- a/src/CompositionHelper.ts +++ b/src/CompositionHelper.ts @@ -4,6 +4,7 @@ */ import { ITerminal } from './Types'; +import { ICharSizeService } from 'ui/services/Services'; interface IPosition { start: number; @@ -42,7 +43,8 @@ export class CompositionHelper { constructor( private _textarea: HTMLTextAreaElement, private _compositionView: HTMLElement, - private _terminal: ITerminal + private _terminal: ITerminal, + private _charSizeService: ICharSizeService ) { this._isComposing = false; this._isSendingComposition = false; @@ -195,9 +197,9 @@ export class CompositionHelper { } if (this._terminal.buffer.isCursorInViewport) { - const cellHeight = Math.ceil(this._terminal.charMeasure.height * this._terminal.options.lineHeight); + const cellHeight = Math.ceil(this._charSizeService.height * this._terminal.options.lineHeight); const cursorTop = this._terminal.buffer.y * cellHeight; - const cursorLeft = this._terminal.buffer.x * this._terminal.charMeasure.width; + const cursorLeft = this._terminal.buffer.x * this._charSizeService.width; this._compositionView.style.left = cursorLeft + 'px'; this._compositionView.style.top = cursorTop + 'px'; diff --git a/src/MouseHelper.test.ts b/src/MouseHelper.test.ts index 7ca165e8..946b3cb8 100644 --- a/src/MouseHelper.test.ts +++ b/src/MouseHelper.test.ts @@ -22,12 +22,6 @@ describe('MouseHelper.getCoords', () => { mouseHelper = new MouseHelper(renderer as any, new MockCharSizeService(CHAR_WIDTH, CHAR_HEIGHT)); }); - describe('when charMeasure is not initialized', () => { - it('should return null', () => { - assert.equal(mouseHelper.getCoords({ clientX: 0, clientY: 0 }, document.createElement('div'), 10, 10), null); - }); - }); - it('should return the cell that was clicked', () => { let coords: [number, number]; coords = mouseHelper.getCoords({ clientX: CHAR_WIDTH / 2, clientY: CHAR_HEIGHT / 2 }, document.createElement('div'), 10, 10); diff --git a/src/MouseHelper.ts b/src/MouseHelper.ts index 091d00ae..d5662ab0 100644 --- a/src/MouseHelper.ts +++ b/src/MouseHelper.ts @@ -25,7 +25,6 @@ export class MouseHelper implements IMouseHelper { * little faster and this function is used in some low level code. * @param event The mouse event. * @param element The terminal's container element. - * @param charMeasure The char measure object used to determine character sizes. * @param colCount The number of columns in the terminal. * @param rowCount The number of rows n the terminal. * @param isSelection Whether the request is for the selection or not. This will @@ -61,7 +60,6 @@ export class MouseHelper implements IMouseHelper { * as expected by xterm. * @param event The mouse event. * @param element The terminal's container element. - * @param charMeasure The char measure object used to determine character sizes. * @param colCount The number of columns in the terminal. * @param rowCount The number of rows in the terminal. */ diff --git a/src/Terminal.ts b/src/Terminal.ts index f68d3105..1f010382 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -614,14 +614,14 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II this.register(addDisposableDomListener(this.textarea, 'blur', () => this._onTextAreaBlur())); this._helperContainer.appendChild(this.textarea); - this._compositionView = document.createElement('div'); - this._compositionView.classList.add('composition-view'); - this._compositionHelper = new CompositionHelper(this.textarea, this._compositionView, this); - this._helperContainer.appendChild(this._compositionView); - this.charMeasure = new CharMeasure(document, this._helperContainer); this._charSizeService = new CharSizeService(this._document, this._helperContainer, this.optionsService); + this._compositionView = document.createElement('div'); + this._compositionView.classList.add('composition-view'); + this._compositionHelper = new CompositionHelper(this.textarea, this._compositionView, this, this._charSizeService); + this._helperContainer.appendChild(this._compositionView); + // Performance: Add viewport and helper elements from the fragment this.element.appendChild(fragment); From 760128e0f62b9a1b01c1bfabe5a454185619a5d1 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 8 Jun 2019 15:24:30 -0700 Subject: [PATCH 05/10] Adopt in renderers --- src/Terminal.ts | 4 ++-- src/renderer/Renderer.ts | 18 +++++++++--------- src/renderer/dom/DomRenderer.ts | 8 +++++--- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/Terminal.ts b/src/Terminal.ts index 1f010382..414e7dac 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -697,8 +697,8 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II private _createRenderer(): IRenderer { switch (this.options.rendererType) { - case 'canvas': return new Renderer(this, this._colorManager.colors); break; - case 'dom': return new DomRenderer(this, this._colorManager.colors); break; + case 'canvas': return new Renderer(this, this._colorManager.colors, this._charSizeService); break; + case 'dom': return new DomRenderer(this, this._colorManager.colors, this._charSizeService); break; default: throw new Error(`Unrecognized rendererType "${this.options.rendererType}"`); } } diff --git a/src/renderer/Renderer.ts b/src/renderer/Renderer.ts index 2fa09824..19adddbf 100644 --- a/src/renderer/Renderer.ts +++ b/src/renderer/Renderer.ts @@ -12,6 +12,7 @@ import { LinkRenderLayer } from './LinkRenderLayer'; import { CharacterJoinerRegistry } from '../renderer/CharacterJoinerRegistry'; import { Disposable } from 'common/Lifecycle'; import { IColorSet } from 'ui/Types'; +import { ICharSizeService } from 'ui/services/Services'; export class Renderer extends Disposable implements IRenderer { private _renderLayers: IRenderLayer[]; @@ -22,7 +23,8 @@ export class Renderer extends Disposable implements IRenderer { constructor( private _terminal: ITerminal, - private _colors: IColorSet + private _colors: IColorSet, + private _charSizeService: ICharSizeService ) { super(); const allowTransparency = this._terminal.options.allowTransparency; @@ -133,8 +135,7 @@ export class Renderer extends Disposable implements IRenderer { * Recalculates the character and canvas dimensions. */ private _updateDimensions(): void { - // Perform a new measure if the CharMeasure dimensions are not yet available - if (!this._terminal.charMeasure.width || !this._terminal.charMeasure.height) { + if (!this._charSizeService.hasValidSize) { return; } @@ -142,12 +143,12 @@ export class Renderer extends Disposable implements IRenderer { // drawn to an integer grid in order for the CharAtlas "stamps" to not be // blurry. When text is drawn to the grid not using the CharAtlas, it is // clipped to ensure there is no overlap with the next cell. - this.dimensions.scaledCharWidth = Math.floor(this._terminal.charMeasure.width * window.devicePixelRatio); + this.dimensions.scaledCharWidth = Math.floor(this._charSizeService.width * window.devicePixelRatio); // Calculate the scaled character height. Height is ceiled in case // devicePixelRatio is a floating point number in order to ensure there is // enough space to draw the character to the cell. - this.dimensions.scaledCharHeight = Math.ceil(this._terminal.charMeasure.height * window.devicePixelRatio); + this.dimensions.scaledCharHeight = Math.ceil(this._charSizeService.height * window.devicePixelRatio); // Calculate the scaled cell height, if lineHeight is not 1 then the value // will be floored because since lineHeight can never be lower then 1, there @@ -181,10 +182,9 @@ export class Renderer extends Disposable implements IRenderer { // Get the _actual_ dimensions of an individual cell. This needs to be // derived from the canvasWidth/Height calculated above which takes into - // account window.devicePixelRatio. CharMeasure.width/height by itself is - // insufficient when the page is not at 100% zoom level as CharMeasure is - // measured in CSS pixels, but the actual char size on the canvas can - // differ. + // account window.devicePixelRatio. ICharSizeService.width/height by itself + // is insufficient when the page is not at 100% zoom level as it's measured + // in CSS pixels, but the actual char size on the canvas can differ. this.dimensions.actualCellHeight = this.dimensions.canvasHeight / this._terminal.rows; this.dimensions.actualCellWidth = this.dimensions.canvasWidth / this._terminal.cols; } diff --git a/src/renderer/dom/DomRenderer.ts b/src/renderer/dom/DomRenderer.ts index a1922541..77cfcba8 100644 --- a/src/renderer/dom/DomRenderer.ts +++ b/src/renderer/dom/DomRenderer.ts @@ -9,6 +9,7 @@ import { BOLD_CLASS, ITALIC_CLASS, CURSOR_CLASS, CURSOR_STYLE_BLOCK_CLASS, CURSO import { INVERTED_DEFAULT_COLOR } from '../atlas/Types'; import { Disposable } from 'common/Lifecycle'; import { IColorSet } from 'ui/Types'; +import { ICharSizeService } from 'ui/services/Services'; const TERMINAL_CLASS_PREFIX = 'xterm-dom-renderer-owner-'; const ROW_CONTAINER_CLASS = 'xterm-rows'; @@ -41,7 +42,8 @@ export class DomRenderer extends Disposable implements IRenderer { constructor( private _terminal: ITerminal, - private _colors: IColorSet + private _colors: IColorSet, + private _charSizeService: ICharSizeService ) { super(); @@ -91,8 +93,8 @@ export class DomRenderer extends Disposable implements IRenderer { } private _updateDimensions(): void { - this.dimensions.scaledCharWidth = this._terminal.charMeasure.width * window.devicePixelRatio; - this.dimensions.scaledCharHeight = Math.ceil(this._terminal.charMeasure.height * window.devicePixelRatio); + this.dimensions.scaledCharWidth = this._charSizeService.width * window.devicePixelRatio; + this.dimensions.scaledCharHeight = Math.ceil(this._charSizeService.height * window.devicePixelRatio); this.dimensions.scaledCellWidth = this.dimensions.scaledCharWidth + Math.round(this._terminal.options.letterSpacing); this.dimensions.scaledCellHeight = Math.floor(this.dimensions.scaledCharHeight * this._terminal.options.lineHeight); this.dimensions.scaledCharLeft = 0; From 53c3151de6f2e8d6d286098aac3c1eadf28caba3 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 8 Jun 2019 15:25:34 -0700 Subject: [PATCH 06/10] Adopt in Viewport --- src/Terminal.ts | 2 +- src/Viewport.ts | 17 +++++------------ 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/src/Terminal.ts b/src/Terminal.ts index 414e7dac..a80bbf52 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -635,7 +635,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II this._renderCoordinator.onRender(e => this._onRender.fire(e)); this.onResize(e => this._renderCoordinator.resize(e.cols, e.rows)); - this.viewport = new Viewport(this, this._viewportElement, this._viewportScrollArea, this.charMeasure, this._renderCoordinator.dimensions); + this.viewport = new Viewport(this, this._viewportElement, this._viewportScrollArea, this._renderCoordinator.dimensions, this._charSizeService); this.viewport.onThemeChange(this._colorManager.colors); this.register(this.viewport); diff --git a/src/Viewport.ts b/src/Viewport.ts index a7afdb9e..90565a34 100644 --- a/src/Viewport.ts +++ b/src/Viewport.ts @@ -4,11 +4,11 @@ */ import { ITerminal, IViewport } from './Types'; -import { CharMeasure } from './CharMeasure'; import { Disposable } from 'common/Lifecycle'; import { addDisposableDomListener } from 'ui/Lifecycle'; import { IColorSet } from 'ui/Types'; import { IRenderDimensions } from './renderer/Types'; +import { ICharSizeService } from 'ui/services/Services'; const FALLBACK_SCROLL_BAR_WIDTH = 15; @@ -33,19 +33,12 @@ export class Viewport extends Disposable implements IViewport { private _refreshAnimationFrame: number | null = null; private _ignoreNextScrollEvent: boolean = false; - /** - * Creates a new Viewport. - * @param _terminal The terminal this viewport belongs to. - * @param _viewportElement The DOM element acting as the viewport. - * @param _scrollArea The DOM element acting as the scroll area. - * @param _charMeasure A DOM element used to measure the character size of. the terminal. - */ constructor( private _terminal: ITerminal, private _viewportElement: HTMLElement, private _scrollArea: HTMLElement, - private _charMeasure: CharMeasure, - private _dimensions: IRenderDimensions + private _dimensions: IRenderDimensions, + private _charSizeService: ICharSizeService ) { super(); @@ -55,7 +48,7 @@ export class Viewport extends Disposable implements IViewport { this.scrollBarWidth = (this._viewportElement.offsetWidth - this._scrollArea.offsetWidth) || FALLBACK_SCROLL_BAR_WIDTH; this.register(addDisposableDomListener(this._viewportElement, 'scroll', this._onScroll.bind(this))); - // Perform this async to ensure the CharMeasure is ready. + // Perform this async to ensure the ICharSizeService is ready. setTimeout(() => this.syncScrollArea(), 0); } @@ -78,7 +71,7 @@ export class Viewport extends Disposable implements IViewport { } private _innerRefresh(): void { - if (this._charMeasure.height > 0) { + if (this._charSizeService.height > 0) { this._currentRowHeight = this._dimensions.scaledCellHeight / window.devicePixelRatio; this._lastRecordedViewportHeight = this._viewportElement.offsetHeight; const newBufferHeight = Math.round(this._currentRowHeight * this._lastRecordedBufferLength) + (this._lastRecordedViewportHeight - this._dimensions.canvasHeight); From 0f2d560e01f104f74545dfd1b3246c8014601fd5 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 8 Jun 2019 15:26:53 -0700 Subject: [PATCH 07/10] Adopt in SelectionManager --- src/SelectionManager.test.ts | 10 ++++------ src/SelectionManager.ts | 6 +++--- src/Terminal.ts | 2 +- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/SelectionManager.test.ts b/src/SelectionManager.test.ts index 77a1612f..538abc91 100644 --- a/src/SelectionManager.test.ts +++ b/src/SelectionManager.test.ts @@ -4,13 +4,12 @@ */ import { assert } from 'chai'; -import { CharMeasure } from './CharMeasure'; import { SelectionManager, SelectionMode } from './SelectionManager'; import { SelectionModel } from './SelectionModel'; import { BufferSet } from './BufferSet'; import { ITerminal, IBuffer } from './Types'; import { IBufferLine } from 'core/Types'; -import { MockTerminal } from './TestUtils.test'; +import { MockTerminal, MockCharSizeService } from './TestUtils.test'; import { BufferLine, CellData } from 'core/buffer/BufferLine'; class TestMockTerminal extends MockTerminal { @@ -19,10 +18,9 @@ class TestMockTerminal extends MockTerminal { class TestSelectionManager extends SelectionManager { constructor( - terminal: ITerminal, - charMeasure: CharMeasure + terminal: ITerminal ) { - super(terminal, charMeasure); + super(terminal, new MockCharSizeService(10, 10)); } public get model(): SelectionModel { return this._model; } @@ -52,7 +50,7 @@ describe('SelectionManager', () => { terminal.buffers = new BufferSet(terminal); terminal.buffer = terminal.buffers.active; buffer = terminal.buffer; - selectionManager = new TestSelectionManager(terminal, null); + selectionManager = new TestSelectionManager(terminal); }); function stringToRow(text: string): IBufferLine { diff --git a/src/SelectionManager.ts b/src/SelectionManager.ts index d0cba71d..0b939562 100644 --- a/src/SelectionManager.ts +++ b/src/SelectionManager.ts @@ -7,12 +7,12 @@ import { ITerminal, ISelectionManager, IBuffer, ISelectionRedrawRequestEvent } f import { IBufferLine } from 'core/Types'; import { MouseHelper } from './MouseHelper'; import * as Browser from 'common/Platform'; -import { CharMeasure } from './CharMeasure'; import { SelectionModel } from './SelectionModel'; import { AltClickHandler } from './handlers/AltClickHandler'; import { CellData } from 'core/buffer/BufferLine'; import { IDisposable } from 'xterm'; import { EventEmitter2, IEvent } from 'common/EventEmitter2'; +import { ICharSizeService } from 'ui/services/Services'; /** * The number of pixels the mouse needs to be above or below the viewport in @@ -117,7 +117,7 @@ export class SelectionManager implements ISelectionManager { constructor( private _terminal: ITerminal, - private _charMeasure: CharMeasure + private _charSizeService: ICharSizeService ) { this._initListeners(); this.enable(); @@ -375,7 +375,7 @@ export class SelectionManager implements ISelectionManager { */ private _getMouseEventScrollAmount(event: MouseEvent): number { let offset = MouseHelper.getCoordsRelativeToElement(event, this._terminal.screenElement)[1]; - const terminalHeight = this._terminal.rows * Math.ceil(this._charMeasure.height * this._terminal.options.lineHeight); + const terminalHeight = this._terminal.rows * Math.ceil(this._charSizeService.height * this._terminal.options.lineHeight); if (offset >= 0 && offset <= terminalHeight) { return 0; } diff --git a/src/Terminal.ts b/src/Terminal.ts index a80bbf52..85594fb7 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -646,7 +646,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II this.register(this.charMeasure.onCharSizeChanged(() => this._renderCoordinator.onCharSizeChanged())); this.register(this._renderCoordinator.onDimensionsChange(() => this.viewport.syncScrollArea())); - this.selectionManager = new SelectionManager(this, this.charMeasure); + this.selectionManager = new SelectionManager(this, this._charSizeService); this.register(this.selectionManager.onSelectionChange(() => this._onSelectionChange.fire())); this.register(addDisposableDomListener(this.element, 'mousedown', (e: MouseEvent) => this.selectionManager.onMouseDown(e))); this.register(this.selectionManager.onRedrawRequest(e => this._renderCoordinator.onSelectionChanged(e.start, e.end, e.columnSelectMode))); From 47fab14efc541b9f855ab97b9930313e46d1ca75 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 8 Jun 2019 15:29:16 -0700 Subject: [PATCH 08/10] Adopt in Terminal --- src/CharMeasure.test.ts | 54 -------------------------------------- src/CharMeasure.ts | 58 ----------------------------------------- src/Terminal.ts | 20 +++++++------- src/TestUtils.test.ts | 3 +-- src/Types.ts | 10 ------- 5 files changed, 11 insertions(+), 134 deletions(-) delete mode 100644 src/CharMeasure.test.ts delete mode 100644 src/CharMeasure.ts diff --git a/src/CharMeasure.test.ts b/src/CharMeasure.test.ts deleted file mode 100644 index 5fd17eb2..00000000 --- a/src/CharMeasure.test.ts +++ /dev/null @@ -1,54 +0,0 @@ -/** - * Copyright (c) 2016 The xterm.js authors. All rights reserved. - * @license MIT - */ - -import jsdom = require('jsdom'); -import { ICharMeasure } from './Types'; -import { assert } from 'chai'; -import { CharMeasure } from './CharMeasure'; - -describe('CharMeasure', () => { - let dom: jsdom.JSDOM; - let window: Window; - let document: Document; - let container: HTMLElement; - let charMeasure: ICharMeasure; - - beforeEach(() => { - dom = new jsdom.JSDOM(''); - window = dom.window; - document = window.document; - container = document.createElement('div'); - document.body.appendChild(container); - charMeasure = new CharMeasure(document, container); - }); - - describe('measure', () => { - it('should have _measureElement', () => { - assert.isDefined((charMeasure)._measureElement, 'new CharMeasure() should have created _measureElement'); - }); - - it('should be performed sync', () => { - // Mock getBoundingClientRect since jsdom doesn't have a layout engine - (charMeasure)._measureElement.getBoundingClientRect = () => { - return { width: 1, height: 1 }; - }; - charMeasure.measure({}); - assert.equal(charMeasure.height, 1); - assert.equal(charMeasure.width, 1); - }); - - it('should NOT do a measure when the parent is hidden', done => { - charMeasure.measure({}); - setTimeout(() => { - const firstWidth = charMeasure.width; - container.style.display = 'none'; - container.style.fontSize = '2em'; - charMeasure.measure({}); - assert.equal(charMeasure.width, firstWidth); - done(); - }, 0); - }); - }); -}); diff --git a/src/CharMeasure.ts b/src/CharMeasure.ts deleted file mode 100644 index 9ef22b63..00000000 --- a/src/CharMeasure.ts +++ /dev/null @@ -1,58 +0,0 @@ -/** - * Copyright (c) 2016 The xterm.js authors. All rights reserved. - * @license MIT - */ - -import { ICharMeasure, ITerminalOptions } from './Types'; -import { EventEmitter2, IEvent } from 'common/EventEmitter2'; - -/** - * Utility class that measures the size of a character. Measurements are done in - * the DOM rather than with a canvas context because support for extracting the - * height of characters is patchy across browsers. - */ -export class CharMeasure implements ICharMeasure { - private _document: Document; - private _parentElement: HTMLElement; - private _measureElement: HTMLElement; - private _width: number; - private _height: number; - - private _onCharSizeChanged = new EventEmitter2(); - public get onCharSizeChanged(): IEvent { return this._onCharSizeChanged.event; } - - constructor(document: Document, parentElement: HTMLElement) { - this._document = document; - this._parentElement = parentElement; - this._measureElement = this._document.createElement('span'); - this._measureElement.classList.add('xterm-char-measure-element'); - this._measureElement.textContent = 'W'; - this._measureElement.setAttribute('aria-hidden', 'true'); - this._parentElement.appendChild(this._measureElement); - } - - public get width(): number { - return this._width; - } - - public get height(): number { - return this._height; - } - - public measure(options: ITerminalOptions): void { - this._measureElement.style.fontFamily = options.fontFamily; - this._measureElement.style.fontSize = `${options.fontSize}px`; - const geometry = this._measureElement.getBoundingClientRect(); - // The element is likely currently display:none, we should retain the - // previous value. - if (geometry.width === 0 || geometry.height === 0) { - return; - } - const adjustedHeight = Math.ceil(geometry.height); - if (this._width !== geometry.width || this._height !== adjustedHeight) { - this._width = geometry.width; - this._height = adjustedHeight; - this._onCharSizeChanged.fire(); - } - } -} diff --git a/src/Terminal.ts b/src/Terminal.ts index 85594fb7..1a02fe2a 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -34,7 +34,6 @@ import { InputHandler } from './InputHandler'; import { Renderer } from './renderer/Renderer'; import { Linkifier } from './Linkifier'; import { SelectionManager } from './SelectionManager'; -import { CharMeasure } from './CharMeasure'; import * as Browser from 'common/Platform'; import { addDisposableDomListener } from 'ui/Lifecycle'; import * as Strings from './Strings'; @@ -180,7 +179,6 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II public buffers: BufferSet; public viewport: IViewport; private _compositionHelper: ICompositionHelper; - public charMeasure: CharMeasure; private _mouseZoneManager: IMouseZoneManager; public mouseHelper: MouseHelper; private _accessibilityManager: AccessibilityManager; @@ -365,7 +363,9 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II // When the font changes the size of the cells may change which requires a renderer clear if (this._renderCoordinator) { this._renderCoordinator.clear(); - this.charMeasure.measure(this.options); + } + if (this._charSizeService) { + this._charSizeService.measure(); } break; case 'drawBoldTextInBrightColors': @@ -614,7 +614,6 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II this.register(addDisposableDomListener(this.textarea, 'blur', () => this._onTextAreaBlur())); this._helperContainer.appendChild(this.textarea); - this.charMeasure = new CharMeasure(document, this._helperContainer); this._charSizeService = new CharSizeService(this._document, this._helperContainer, this.optionsService); this._compositionView = document.createElement('div'); @@ -643,7 +642,8 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II this.register(this.onResize(() => this._renderCoordinator.onResize(this.cols, this.rows))); this.register(this.addDisposableListener('blur', () => this._renderCoordinator.onBlur())); this.register(this.addDisposableListener('focus', () => this._renderCoordinator.onFocus())); - this.register(this.charMeasure.onCharSizeChanged(() => this._renderCoordinator.onCharSizeChanged())); + // TODO: Move to RenderCoordinator + this.register(this._charSizeService.onCharSizeChange(() => this._renderCoordinator.onCharSizeChanged())); this.register(this._renderCoordinator.onDimensionsChange(() => this.viewport.syncScrollArea())); this.selectionManager = new SelectionManager(this, this._charSizeService); @@ -681,7 +681,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II } // Measure the character size - this.charMeasure.measure(this.options); + this._charSizeService.measure(); // Setup loop that draws to screen this.refresh(0, this.rows - 1); @@ -1737,8 +1737,8 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II if (x === this.cols && y === this.rows) { // Check if we still need to measure the char size (fixes #785). - if (this.charMeasure && (!this.charMeasure.width || !this.charMeasure.height)) { - this.charMeasure.measure(this.options); + if (this._charSizeService && !this._charSizeService.hasValidSize) { + this._charSizeService.measure(); } return; } @@ -1752,8 +1752,8 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II this.rows = y; this.buffers.setupTabStops(this.cols); - if (this.charMeasure) { - this.charMeasure.measure(this.options); + if (this._charSizeService) { + this._charSizeService.measure(); } this.refresh(0, this.rows - 1); diff --git a/src/TestUtils.test.ts b/src/TestUtils.test.ts index 32b7a65c..490a1707 100644 --- a/src/TestUtils.test.ts +++ b/src/TestUtils.test.ts @@ -4,7 +4,7 @@ */ import { IRenderer, IRenderDimensions } from './renderer/Types'; -import { IInputHandlingTerminal, IViewport, ICompositionHelper, ITerminal, IBuffer, IBufferSet, IBrowser, ICharMeasure, ISelectionManager, ITerminalOptions, ILinkifier, IMouseHelper, ILinkMatcherOptions, CharacterJoinerHandler, IBufferStringIterator } from './Types'; +import { IInputHandlingTerminal, IViewport, ICompositionHelper, ITerminal, IBuffer, IBufferSet, IBrowser, ISelectionManager, ITerminalOptions, ILinkifier, IMouseHelper, ILinkMatcherOptions, CharacterJoinerHandler, IBufferStringIterator } from './Types'; import { IBufferLine, ICellData, IAttributeData } from 'core/Types'; import { ICircularList, XtermListener } from 'common/Types'; import { Buffer } from './Buffer'; @@ -129,7 +129,6 @@ export class MockTerminal implements ITerminal { rowContainer: HTMLElement; selectionContainer: HTMLElement; selectionManager: ISelectionManager; - charMeasure: ICharMeasure; textarea: HTMLTextAreaElement; rows: number; cols: number; diff --git a/src/Types.ts b/src/Types.ts index 0aecdc96..2225ef3f 100644 --- a/src/Types.ts +++ b/src/Types.ts @@ -199,7 +199,6 @@ export interface ILinkifierEvent { export interface ITerminal extends IPublicTerminal, IElementAccessor, IBufferAccessor, ILinkifierAccessor { screenElement: HTMLElement; selectionManager: ISelectionManager; - charMeasure: ICharMeasure; browser: IBrowser; writeBuffer: string[]; cursorHidden: boolean; @@ -288,15 +287,6 @@ export interface IMouseHelper { getRawByteCoords(event: MouseEvent, element: HTMLElement, colCount: number, rowCount: number): { x: number, y: number }; } -export interface ICharMeasure { - width: number; - height: number; - - onCharSizeChanged: IEvent; - - measure(options: ITerminalOptions): void; -} - // TODO: The options that are not in the public API should be reviewed export interface ITerminalOptions extends IPublicTerminalOptions { [key: string]: any; From 6375d9b88e5b66031ea8fbafd35be2b1b9d3811b Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 8 Jun 2019 15:45:07 -0700 Subject: [PATCH 09/10] Fix event firing --- src/TestUtils.test.ts | 2 +- src/ui/services/CharSizeService.ts | 13 ++++++++----- src/ui/services/Services.d.ts | 2 +- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/TestUtils.test.ts b/src/TestUtils.test.ts index 490a1707..06da0ac1 100644 --- a/src/TestUtils.test.ts +++ b/src/TestUtils.test.ts @@ -432,7 +432,7 @@ export class MockCompositionHelper implements ICompositionHelper { export class MockCharSizeService implements ICharSizeService { get hasValidSize(): boolean { return this.width > 0 && this.height > 0; } - onCharSizeChange: IEvent; + onCharSizeChange: IEvent; constructor(public width: number, public height: number) {} measure(): void {} } diff --git a/src/ui/services/CharSizeService.ts b/src/ui/services/CharSizeService.ts index 7d9d3a2c..4a297fe7 100644 --- a/src/ui/services/CharSizeService.ts +++ b/src/ui/services/CharSizeService.ts @@ -14,8 +14,8 @@ export class CharSizeService implements ICharSizeService { public get hasValidSize(): boolean { return this.width > 0 && this.height > 0; } - private _onCharSizeChange = new EventEmitter2(); - public get onCharSizeChange(): IEvent { return this._onCharSizeChange.event; } + private _onCharSizeChange = new EventEmitter2(); + public get onCharSizeChange(): IEvent { return this._onCharSizeChange.event; } constructor( document: Document, @@ -27,8 +27,11 @@ export class CharSizeService implements ICharSizeService { public measure(): void { const result = this._measureStrategy.measure(); - this.width = result.width; - this.height = result.height; + if (result.width !== this.width || result.height !== this.height) { + this.width = result.width; + this.height = result.height; + this._onCharSizeChange.fire(); + } } } @@ -69,7 +72,7 @@ class DomMeasureStrategy implements IMeasureStrategy { // Note that this triggers a synchronous layout const geometry = this._measureElement.getBoundingClientRect(); - +console.log('measure', geometry); // If values are 0 then the element is likely currently display:none, in which case we should // retain the previous value. if (geometry.width !== 0 && geometry.height !== 0) { diff --git a/src/ui/services/Services.d.ts b/src/ui/services/Services.d.ts index 17cc4f3b..b7a94ea1 100644 --- a/src/ui/services/Services.d.ts +++ b/src/ui/services/Services.d.ts @@ -10,7 +10,7 @@ export interface ICharSizeService { readonly height: number; readonly hasValidSize: boolean; - readonly onCharSizeChange: IEvent; + readonly onCharSizeChange: IEvent; measure(): void; } From 9df3a3671da998ec0672c11762f65cf3352b62bc Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 8 Jun 2019 15:46:12 -0700 Subject: [PATCH 10/10] Adopt CharSizeService in RenderCoordinator --- src/Terminal.ts | 4 +--- src/renderer/RenderCoordinator.ts | 5 ++++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Terminal.ts b/src/Terminal.ts index 1a02fe2a..f6e38594 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -630,7 +630,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II this._colorManager.setTheme(this._theme); const renderer = this._createRenderer(); - this._renderCoordinator = new RenderCoordinator(renderer, this.rows, this.screenElement, this.optionsService); + this._renderCoordinator = new RenderCoordinator(renderer, this.rows, this.screenElement, this.optionsService, this._charSizeService); this._renderCoordinator.onRender(e => this._onRender.fire(e)); this.onResize(e => this._renderCoordinator.resize(e.cols, e.rows)); @@ -642,8 +642,6 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II this.register(this.onResize(() => this._renderCoordinator.onResize(this.cols, this.rows))); this.register(this.addDisposableListener('blur', () => this._renderCoordinator.onBlur())); this.register(this.addDisposableListener('focus', () => this._renderCoordinator.onFocus())); - // TODO: Move to RenderCoordinator - this.register(this._charSizeService.onCharSizeChange(() => this._renderCoordinator.onCharSizeChanged())); this.register(this._renderCoordinator.onDimensionsChange(() => this.viewport.syncScrollArea())); this.selectionManager = new SelectionManager(this, this._charSizeService); diff --git a/src/renderer/RenderCoordinator.ts b/src/renderer/RenderCoordinator.ts index 98c9ae86..9136b063 100644 --- a/src/renderer/RenderCoordinator.ts +++ b/src/renderer/RenderCoordinator.ts @@ -12,6 +12,7 @@ import { addDisposableDomListener } from 'ui/Lifecycle'; import { IColorSet } from 'ui/Types'; import { CharacterJoinerHandler } from '../Types'; import { IOptionsService } from 'common/options/Types'; +import { ICharSizeService } from 'ui/services/Services'; export class RenderCoordinator extends Disposable { private _renderDebouncer: RenderDebouncer; @@ -35,7 +36,8 @@ export class RenderCoordinator extends Disposable { private _renderer: IRenderer, private _rowCount: number, screenElement: HTMLElement, - optionsService: IOptionsService + optionsService: IOptionsService, + charSizeService: ICharSizeService ) { super(); this._renderDebouncer = new RenderDebouncer((start, end) => this._renderRows(start, end)); @@ -46,6 +48,7 @@ export class RenderCoordinator extends Disposable { this.register(this._screenDprMonitor); this.register(optionsService.onOptionChange(() => this._renderer.onOptionsChanged())); + this.register(charSizeService.onCharSizeChange(() => this.onCharSizeChanged())); // dprchange should handle this case, we need this as well for browsers that don't support the // matchMedia query.