From 8b2b0f6abc44a53f7434620bba1103df69e37d27 Mon Sep 17 00:00:00 2001 From: Jakob Schrettenbrunner Date: Tue, 12 Jan 2021 22:32:20 +0000 Subject: [PATCH 01/16] add richer ScrollEvent that includes the source --- src/browser/Terminal.ts | 12 ++++++------ src/common/CoreTerminal.ts | 32 +++++++++++++++++++++++--------- src/common/Types.d.ts | 10 ++++++++++ 3 files changed, 39 insertions(+), 15 deletions(-) diff --git a/src/browser/Terminal.ts b/src/browser/Terminal.ts index f8b8b3e4..b507a6b1 100644 --- a/src/browser/Terminal.ts +++ b/src/browser/Terminal.ts @@ -39,7 +39,7 @@ import { MouseZoneManager } from 'browser/MouseZoneManager'; import { AccessibilityManager } from './AccessibilityManager'; import { ITheme, IMarker, IDisposable, ISelectionPosition, ILinkProvider } from 'xterm'; import { DomRenderer } from 'browser/renderer/dom/DomRenderer'; -import { IKeyboardEvent, KeyboardResultType, CoreMouseEventType, CoreMouseButton, CoreMouseAction, ITerminalOptions } from 'common/Types'; +import { IKeyboardEvent, KeyboardResultType, CoreMouseEventType, CoreMouseButton, CoreMouseAction, ITerminalOptions, ScrollSource } from 'common/Types'; import { evaluateKeyboardEvent } from 'common/input/Keyboard'; import { EventEmitter, IEvent, forwardEvent } from 'common/EventEmitter'; import { DEFAULT_ATTR_DATA } from 'common/buffer/BufferLine'; @@ -448,7 +448,7 @@ export class Terminal extends CoreTerminal implements ITerminal { this._instantiationService.setService(IMouseService, this._mouseService); this.viewport = this._instantiationService.createInstance(Viewport, - (amount: number, suppressEvent: boolean) => this.scrollLines(amount, suppressEvent), + (amount: number, suppressEvent: boolean) => this.scrollLines(amount, suppressEvent, ScrollSource.VIEWPORT), this._viewportElement, this._viewportScrollArea ); @@ -481,7 +481,7 @@ export class Terminal extends CoreTerminal implements ITerminal { this.textarea!.select(); })); this.register(this.onScroll(() => { - this.viewport!.syncScrollArea(); + this.viewport!.syncScrollArea(); this._selectionService!.refresh(); })); this.register(addDisposableDomListener(this._viewportElement, 'scroll', () => this._selectionService!.refresh())); @@ -836,8 +836,8 @@ export class Terminal extends CoreTerminal implements ITerminal { } } - public scrollLines(disp: number, suppressScrollEvent?: boolean): void { - super.scrollLines(disp, suppressScrollEvent); + public scrollLines(disp: number, suppressScrollEvent?: boolean, source = ScrollSource.TERMINAL): void { + super.scrollLines(disp, suppressScrollEvent, source); this.refresh(0, this.rows - 1); } @@ -1168,7 +1168,7 @@ export class Terminal extends CoreTerminal implements ITerminal { this.buffer.lines.push(this.buffer.getBlankLine(DEFAULT_ATTR_DATA)); } this.refresh(0, this.rows - 1); - this._onScroll.fire(this.buffer.ydisp); + this._onScroll.fire({position: this.buffer.ydisp, source: ScrollSource.TERMINAL }); } /** diff --git a/src/common/CoreTerminal.ts b/src/common/CoreTerminal.ts index 2f636349..86492ab3 100644 --- a/src/common/CoreTerminal.ts +++ b/src/common/CoreTerminal.ts @@ -27,7 +27,7 @@ import { InstantiationService } from 'common/services/InstantiationService'; import { LogService } from 'common/services/LogService'; import { BufferService, MINIMUM_COLS, MINIMUM_ROWS } from 'common/services/BufferService'; import { OptionsService } from 'common/services/OptionsService'; -import { ITerminalOptions, IDisposable, IBufferLine, IAttributeData, ICoreTerminal } from 'common/Types'; +import { ITerminalOptions, IDisposable, IBufferLine, IAttributeData, ICoreTerminal, IKeyboardEvent, IScrollEvent, ScrollSource } from 'common/Types'; import { CoreService } from 'common/services/CoreService'; import { EventEmitter, IEvent, forwardEvent } from 'common/EventEmitter'; import { CoreMouseService } from 'common/services/CoreMouseService'; @@ -66,8 +66,12 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal { public get onLineFeed(): IEvent { return this._onLineFeed.event; } private _onResize = new EventEmitter<{ cols: number, rows: number }>(); public get onResize(): IEvent<{ cols: number, rows: number }> { return this._onResize.event; } - protected _onScroll = new EventEmitter(); - public get onScroll(): IEvent { return this._onScroll.event; } + protected _onScroll = new EventEmitter(); + /** + * An emitter for legacy on scroll events that just included the position, and not the source. + * Used to maintain API consistency for the onScroll method. + */ + protected _legacyOnScroll?: EventEmitter; public get cols(): number { return this._bufferService.cols; } public get rows(): number { return this._bufferService.rows; } @@ -204,17 +208,17 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal { // Flag rows that need updating this._dirtyRowService.markRangeDirty(buffer.scrollTop, buffer.scrollBottom); - this._onScroll.fire(buffer.ydisp); + this._onScroll.fire({position: buffer.ydisp, source: ScrollSource.TERMINAL}); } /** * Scroll the display of the terminal * @param disp The number of lines to scroll down (negative scroll up). - * @param suppressScrollEvent Don't emit the scroll event as scrollLines. This is used - * to avoid unwanted events being handled by the viewport when the event was triggered from the - * viewport originally. + * @param suppressScrollEvent Don't emit an onScroll event. + * @param source The source of the scroll action. Emitted as part of the onScroll event + * to avoid cyclic invocations if the event originated from the Viewport. */ - public scrollLines(disp: number, suppressScrollEvent?: boolean): void { + public scrollLines(disp: number, suppressScrollEvent = false, source = ScrollSource.TERMINAL): void { const buffer = this._bufferService.buffer; if (disp < 0) { if (buffer.ydisp === 0) { @@ -234,7 +238,7 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal { } if (!suppressScrollEvent) { - this._onScroll.fire(buffer.ydisp); + this._onScroll.fire({position: buffer.ydisp, source}); } } @@ -267,6 +271,16 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal { } } + public get onScroll(): IEvent { + if (!this._legacyOnScroll) { + this._legacyOnScroll = new EventEmitter(); + this.register(this._onScroll.event(ev => { + this._legacyOnScroll?.fire(ev.position); + })); + } + return this._legacyOnScroll.event; + } + /** Add handler for ESC escape sequence. See xterm.d.ts for details. */ public addEscHandler(id: IFunctionIdentifier, callback: () => boolean): IDisposable { return this._inputHandler.addEscHandler(id, callback); diff --git a/src/common/Types.d.ts b/src/common/Types.d.ts index bd0d11c6..4fff64f5 100644 --- a/src/common/Types.d.ts +++ b/src/common/Types.d.ts @@ -42,6 +42,16 @@ export interface IKeyboardEvent { type: string; } +export interface IScrollEvent { + position: number; + source: ScrollSource; +} + +export const enum ScrollSource { + TERMINAL, + VIEWPORT, +} + export interface ICircularList { length: number; maxLength: number; From 82a9ee62d77d04cb26db3282900b3c7a83ea1d28 Mon Sep 17 00:00:00 2001 From: Jakob Schrettenbrunner Date: Tue, 12 Jan 2021 22:35:47 +0000 Subject: [PATCH 02/16] emit onScroll event when user is scrolling --- src/browser/Terminal.ts | 6 ++++-- src/browser/Viewport.ts | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/browser/Terminal.ts b/src/browser/Terminal.ts index b507a6b1..2a43405f 100644 --- a/src/browser/Terminal.ts +++ b/src/browser/Terminal.ts @@ -448,7 +448,7 @@ export class Terminal extends CoreTerminal implements ITerminal { this._instantiationService.setService(IMouseService, this._mouseService); this.viewport = this._instantiationService.createInstance(Viewport, - (amount: number, suppressEvent: boolean) => this.scrollLines(amount, suppressEvent, ScrollSource.VIEWPORT), + (amount: number) => this.scrollLines(amount, false, ScrollSource.VIEWPORT), this._viewportElement, this._viewportScrollArea ); @@ -480,8 +480,10 @@ export class Terminal extends CoreTerminal implements ITerminal { this.textarea!.focus(); this.textarea!.select(); })); - this.register(this.onScroll(() => { + this.register(this._onScroll.event(ev => { + if (ev.source !== ScrollSource.VIEWPORT) { this.viewport!.syncScrollArea(); + } this._selectionService!.refresh(); })); this.register(addDisposableDomListener(this._viewportElement, 'scroll', () => this._selectionService!.refresh())); diff --git a/src/browser/Viewport.ts b/src/browser/Viewport.ts index 29edce6f..02f74ce8 100644 --- a/src/browser/Viewport.ts +++ b/src/browser/Viewport.ts @@ -33,7 +33,7 @@ export class Viewport extends Disposable implements IViewport { private _ignoreNextScrollEvent: boolean = false; constructor( - private readonly _scrollLines: (amount: number, suppressEvent: boolean) => void, + private readonly _scrollLines: (amount: number) => void, private readonly _viewportElement: HTMLElement, private readonly _scrollArea: HTMLElement, @IBufferService private readonly _bufferService: IBufferService, @@ -156,7 +156,7 @@ export class Viewport extends Disposable implements IViewport { const newRow = Math.round(this._lastScrollTop / this._currentRowHeight); const diff = newRow - this._bufferService.buffer.ydisp; - this._scrollLines(diff, true); + this._scrollLines(diff); } /** From de334ea116efbfe8badd940a4ab808619835e3f1 Mon Sep 17 00:00:00 2001 From: kena0ki Date: Wed, 31 Mar 2021 23:32:53 +0900 Subject: [PATCH 03/16] Use RenderService.dimensions instead of CharSizeService for textarea position --- src/browser/Terminal.ts | 32 ++++++++++++--------- src/browser/input/CompositionHelper.test.ts | 4 +-- src/browser/input/CompositionHelper.ts | 17 +++++++---- 3 files changed, 31 insertions(+), 22 deletions(-) diff --git a/src/browser/Terminal.ts b/src/browser/Terminal.ts index 2eba4027..95d74254 100644 --- a/src/browser/Terminal.ts +++ b/src/browser/Terminal.ts @@ -297,19 +297,23 @@ export class Terminal extends CoreTerminal implements ITerminal { } private _syncTextArea(): void { - if (!this.textarea || !this.buffer.isCursorInViewport || this._compositionHelper!.isComposing) { + if (!this.textarea || !this.buffer.isCursorInViewport || this._compositionHelper!.isComposing || !this._renderService) { return; } - - const cellHeight = Math.ceil(this._charSizeService!.height * this.optionsService.options.lineHeight); - const cursorTop = this._bufferService.buffer.y * cellHeight; - const cursorLeft = this._bufferService.buffer.x * this._charSizeService!.width; + const cursorY = this.buffer.ybase + this.buffer.y; + const viewportRelativeCursorY = cursorY - this.buffer.ydisp; + const cursorX = Math.min(this.buffer.x, this.cols - 1); + const cellHeight = this._renderService.dimensions.actualCellHeight; + const width = this.buffer.lines.get(cursorY)!.getWidth(cursorX); + const cellWidth = this._renderService.dimensions.actualCellWidth * width; + const cursorTop = viewportRelativeCursorY * this._renderService.dimensions.actualCellHeight; + const cursorLeft = cursorX * this._renderService.dimensions.actualCellWidth; // Sync the textarea to the exact position of the composition view so the IME knows where the // text is. this.textarea.style.left = cursorLeft + 'px'; this.textarea.style.top = cursorTop + 'px'; - this.textarea.style.width = this._charSizeService!.width + 'px'; + this.textarea.style.width = cellWidth +'px'; this.textarea.style.height = cellHeight + 'px'; this.textarea.style.lineHeight = cellHeight + 'px'; this.textarea.style.zIndex = '-5'; @@ -438,14 +442,6 @@ export class Terminal extends CoreTerminal implements ITerminal { this._charSizeService = this._instantiationService.createInstance(CharSizeService, this._document, this._helperContainer); this._instantiationService.setService(ICharSizeService, this._charSizeService); - this._compositionView = document.createElement('div'); - this._compositionView.classList.add('composition-view'); - this._compositionHelper = this._instantiationService.createInstance(CompositionHelper, this.textarea, this._compositionView); - this._helperContainer.appendChild(this._compositionView); - - // Performance: Add viewport and helper elements from the fragment - this.element.appendChild(fragment); - this._theme = this.options.theme || this._theme; this._colorManager = new ColorManager(document, this.options.allowTransparency); this.register(this.optionsService.onOptionChange(e => this._colorManager!.onOptionsChange(e))); @@ -457,6 +453,14 @@ export class Terminal extends CoreTerminal implements ITerminal { this.register(this._renderService.onRenderedBufferChange(e => this._onRender.fire(e))); this.onResize(e => this._renderService!.resize(e.cols, e.rows)); + this._compositionView = document.createElement('div'); + this._compositionView.classList.add('composition-view'); + this._compositionHelper = this._instantiationService.createInstance(CompositionHelper, this.textarea, this._compositionView); + this._helperContainer.appendChild(this._compositionView); + + // Performance: Add viewport and helper elements from the fragment + this.element.appendChild(fragment); + this._soundService = this._instantiationService.createInstance(SoundService); this._instantiationService.setService(ISoundService, this._soundService); this._mouseService = this._instantiationService.createInstance(MouseService); diff --git a/src/browser/input/CompositionHelper.test.ts b/src/browser/input/CompositionHelper.test.ts index b9a4f668..080c9c47 100644 --- a/src/browser/input/CompositionHelper.test.ts +++ b/src/browser/input/CompositionHelper.test.ts @@ -5,7 +5,7 @@ import { assert } from 'chai'; import { CompositionHelper } from 'browser/input/CompositionHelper'; -import { MockCharSizeService } from 'browser/TestUtils.test'; +import { MockCharSizeService, MockRenderService } from 'browser/TestUtils.test'; import { MockCoreService, MockBufferService, MockOptionsService } from 'common/TestUtils.test'; describe('CompositionHelper', () => { @@ -42,7 +42,7 @@ describe('CompositionHelper', () => { }; handledText = ''; const bufferService = new MockBufferService(10, 5); - compositionHelper = new CompositionHelper(textarea, compositionView, bufferService, new MockOptionsService(), new MockCharSizeService(10, 10), coreService); + compositionHelper = new CompositionHelper(textarea, compositionView, bufferService, new MockOptionsService(), new MockCharSizeService(10, 10), coreService, new MockRenderService()); }); describe('Input', () => { diff --git a/src/browser/input/CompositionHelper.ts b/src/browser/input/CompositionHelper.ts index 85cfc3b6..e5389af4 100644 --- a/src/browser/input/CompositionHelper.ts +++ b/src/browser/input/CompositionHelper.ts @@ -3,7 +3,7 @@ * @license MIT */ -import { ICharSizeService } from 'browser/services/Services'; +import { ICharSizeService, IRenderService } from 'browser/services/Services'; import { IBufferService, ICoreService, IOptionsService } from 'common/services/Services'; interface IPosition { @@ -46,7 +46,8 @@ export class CompositionHelper { @IBufferService private readonly _bufferService: IBufferService, @IOptionsService private readonly _optionsService: IOptionsService, @ICharSizeService private readonly _charSizeService: ICharSizeService, - @ICoreService private readonly _coreService: ICoreService + @ICoreService private readonly _coreService: ICoreService, + @IRenderService private readonly _renderService: IRenderService ) { this._isComposing = false; this._isSendingComposition = false; @@ -202,14 +203,18 @@ export class CompositionHelper { * necessary as the IME events across browsers are not consistently triggered. */ public updateCompositionElements(dontRecurse?: boolean): void { - if (!this._isComposing) { + if (!this._isComposing || !this._renderService) { return; } if (this._bufferService.buffer.isCursorInViewport) { - const cellHeight = Math.ceil(this._charSizeService.height * this._optionsService.options.lineHeight); - const cursorTop = this._bufferService.buffer.y * cellHeight; - const cursorLeft = this._bufferService.buffer.x * this._charSizeService.width; + const cursorY = this._bufferService.buffer.ybase + this._bufferService.buffer.y; + const viewportRelativeCursorY = cursorY - this._bufferService.buffer.ydisp; + const cursorX = Math.min(this._bufferService.buffer.x, this._bufferService.cols - 1); + + const cellHeight = this._renderService.dimensions.actualCellHeight; + const cursorTop = viewportRelativeCursorY * this._renderService.dimensions.actualCellHeight; + const cursorLeft = cursorX * this._renderService.dimensions.actualCellWidth; this._compositionView.style.left = cursorLeft + 'px'; this._compositionView.style.top = cursorTop + 'px'; From a15f61bb8540cd7817170b735bee9a454eaf12a5 Mon Sep 17 00:00:00 2001 From: kena0ki Date: Thu, 1 Apr 2021 00:11:30 +0900 Subject: [PATCH 04/16] Remove non-null assertion --- src/browser/Terminal.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/browser/Terminal.ts b/src/browser/Terminal.ts index 95d74254..371f5549 100644 --- a/src/browser/Terminal.ts +++ b/src/browser/Terminal.ts @@ -304,7 +304,9 @@ export class Terminal extends CoreTerminal implements ITerminal { const viewportRelativeCursorY = cursorY - this.buffer.ydisp; const cursorX = Math.min(this.buffer.x, this.cols - 1); const cellHeight = this._renderService.dimensions.actualCellHeight; - const width = this.buffer.lines.get(cursorY)!.getWidth(cursorX); + const bufferLine = this.buffer.lines.get(cursorY); + if (!bufferLine) return; + const width = bufferLine.getWidth(cursorX); const cellWidth = this._renderService.dimensions.actualCellWidth * width; const cursorTop = viewportRelativeCursorY * this._renderService.dimensions.actualCellHeight; const cursorLeft = cursorX * this._renderService.dimensions.actualCellWidth; From e9e716964e1a24758132ac92e22276147ced54ed Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Thu, 1 Apr 2021 08:19:12 -0700 Subject: [PATCH 05/16] Remove now unneeded CharSizeService --- src/browser/input/CompositionHelper.test.ts | 4 ++-- src/browser/input/CompositionHelper.ts | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/browser/input/CompositionHelper.test.ts b/src/browser/input/CompositionHelper.test.ts index 080c9c47..c722570b 100644 --- a/src/browser/input/CompositionHelper.test.ts +++ b/src/browser/input/CompositionHelper.test.ts @@ -5,7 +5,7 @@ import { assert } from 'chai'; import { CompositionHelper } from 'browser/input/CompositionHelper'; -import { MockCharSizeService, MockRenderService } from 'browser/TestUtils.test'; +import { MockRenderService } from 'browser/TestUtils.test'; import { MockCoreService, MockBufferService, MockOptionsService } from 'common/TestUtils.test'; describe('CompositionHelper', () => { @@ -42,7 +42,7 @@ describe('CompositionHelper', () => { }; handledText = ''; const bufferService = new MockBufferService(10, 5); - compositionHelper = new CompositionHelper(textarea, compositionView, bufferService, new MockOptionsService(), new MockCharSizeService(10, 10), coreService, new MockRenderService()); + compositionHelper = new CompositionHelper(textarea, compositionView, bufferService, new MockOptionsService(), coreService, new MockRenderService()); }); describe('Input', () => { diff --git a/src/browser/input/CompositionHelper.ts b/src/browser/input/CompositionHelper.ts index e5389af4..d2626e2c 100644 --- a/src/browser/input/CompositionHelper.ts +++ b/src/browser/input/CompositionHelper.ts @@ -3,7 +3,7 @@ * @license MIT */ -import { ICharSizeService, IRenderService } from 'browser/services/Services'; +import { IRenderService } from 'browser/services/Services'; import { IBufferService, ICoreService, IOptionsService } from 'common/services/Services'; interface IPosition { @@ -45,7 +45,6 @@ export class CompositionHelper { private readonly _compositionView: HTMLElement, @IBufferService private readonly _bufferService: IBufferService, @IOptionsService private readonly _optionsService: IOptionsService, - @ICharSizeService private readonly _charSizeService: ICharSizeService, @ICoreService private readonly _coreService: ICoreService, @IRenderService private readonly _renderService: IRenderService ) { From b62a64519f567081f327c34c01a056eb9edcdcd4 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Thu, 1 Apr 2021 08:23:18 -0700 Subject: [PATCH 06/16] Use buffer y for relative cursor pos --- src/browser/input/CompositionHelper.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/browser/input/CompositionHelper.ts b/src/browser/input/CompositionHelper.ts index d2626e2c..65a5d672 100644 --- a/src/browser/input/CompositionHelper.ts +++ b/src/browser/input/CompositionHelper.ts @@ -207,12 +207,10 @@ export class CompositionHelper { } if (this._bufferService.buffer.isCursorInViewport) { - const cursorY = this._bufferService.buffer.ybase + this._bufferService.buffer.y; - const viewportRelativeCursorY = cursorY - this._bufferService.buffer.ydisp; const cursorX = Math.min(this._bufferService.buffer.x, this._bufferService.cols - 1); const cellHeight = this._renderService.dimensions.actualCellHeight; - const cursorTop = viewportRelativeCursorY * this._renderService.dimensions.actualCellHeight; + const cursorTop = this._bufferService.buffer.y * this._renderService.dimensions.actualCellHeight; const cursorLeft = cursorX * this._renderService.dimensions.actualCellWidth; this._compositionView.style.left = cursorLeft + 'px'; From 05516750b5ac832147c7153f5db1f79d7f941508 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Thu, 1 Apr 2021 08:23:35 -0700 Subject: [PATCH 07/16] Remove RenderService check, it must be passed to ctor --- src/browser/input/CompositionHelper.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/browser/input/CompositionHelper.ts b/src/browser/input/CompositionHelper.ts index 65a5d672..8a204831 100644 --- a/src/browser/input/CompositionHelper.ts +++ b/src/browser/input/CompositionHelper.ts @@ -202,7 +202,7 @@ export class CompositionHelper { * necessary as the IME events across browsers are not consistently triggered. */ public updateCompositionElements(dontRecurse?: boolean): void { - if (!this._isComposing || !this._renderService) { + if (!this._isComposing) { return; } From 7b920adbe9ace642e4188a7c60412c9cd7d620d5 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Thu, 1 Apr 2021 08:26:35 -0700 Subject: [PATCH 08/16] Tidy up syncTextArea --- src/browser/Terminal.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/browser/Terminal.ts b/src/browser/Terminal.ts index 371f5549..f95013cd 100644 --- a/src/browser/Terminal.ts +++ b/src/browser/Terminal.ts @@ -301,21 +301,22 @@ export class Terminal extends CoreTerminal implements ITerminal { return; } const cursorY = this.buffer.ybase + this.buffer.y; - const viewportRelativeCursorY = cursorY - this.buffer.ydisp; + const bufferLine = this.buffer.lines.get(cursorY); + if (!bufferLine) { + return; + } const cursorX = Math.min(this.buffer.x, this.cols - 1); const cellHeight = this._renderService.dimensions.actualCellHeight; - const bufferLine = this.buffer.lines.get(cursorY); - if (!bufferLine) return; const width = bufferLine.getWidth(cursorX); const cellWidth = this._renderService.dimensions.actualCellWidth * width; - const cursorTop = viewportRelativeCursorY * this._renderService.dimensions.actualCellHeight; + const cursorTop = this.buffer.y * this._renderService.dimensions.actualCellHeight; const cursorLeft = cursorX * this._renderService.dimensions.actualCellWidth; // Sync the textarea to the exact position of the composition view so the IME knows where the // text is. this.textarea.style.left = cursorLeft + 'px'; this.textarea.style.top = cursorTop + 'px'; - this.textarea.style.width = cellWidth +'px'; + this.textarea.style.width = cellWidth + 'px'; this.textarea.style.height = cellHeight + 'px'; this.textarea.style.lineHeight = cellHeight + 'px'; this.textarea.style.zIndex = '-5'; From 1925f1f442132919e109fdce21f6d30f8053c471 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Thu, 1 Apr 2021 08:41:29 -0700 Subject: [PATCH 09/16] Improve whitespace --- src/browser/Terminal.ts | 2 +- src/common/CoreTerminal.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/browser/Terminal.ts b/src/browser/Terminal.ts index 44ea9eb0..1ab207c7 100644 --- a/src/browser/Terminal.ts +++ b/src/browser/Terminal.ts @@ -1194,7 +1194,7 @@ export class Terminal extends CoreTerminal implements ITerminal { this.buffer.lines.push(this.buffer.getBlankLine(DEFAULT_ATTR_DATA)); } this.refresh(0, this.rows - 1); - this._onScroll.fire({position: this.buffer.ydisp, source: ScrollSource.TERMINAL }); + this._onScroll.fire({ position: this.buffer.ydisp, source: ScrollSource.TERMINAL }); } /** diff --git a/src/common/CoreTerminal.ts b/src/common/CoreTerminal.ts index 253cd4c1..50a43bf1 100644 --- a/src/common/CoreTerminal.ts +++ b/src/common/CoreTerminal.ts @@ -224,7 +224,7 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal { // Flag rows that need updating this._dirtyRowService.markRangeDirty(buffer.scrollTop, buffer.scrollBottom); - this._onScroll.fire({position: buffer.ydisp, source: ScrollSource.TERMINAL}); + this._onScroll.fire({ position: buffer.ydisp, source: ScrollSource.TERMINAL }); } /** @@ -254,7 +254,7 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal { } if (!suppressScrollEvent) { - this._onScroll.fire({position: buffer.ydisp, source}); + this._onScroll.fire({ position: buffer.ydisp, source }); } } From f4861aa74a4e98eb06cc139dd6c8c1f10744cd89 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Thu, 1 Apr 2021 08:54:29 -0700 Subject: [PATCH 10/16] Move onScroll next to _onScroll --- src/common/CoreTerminal.ts | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/src/common/CoreTerminal.ts b/src/common/CoreTerminal.ts index 50a43bf1..936aa0c8 100644 --- a/src/common/CoreTerminal.ts +++ b/src/common/CoreTerminal.ts @@ -71,10 +71,19 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal { public get onResize(): IEvent<{ cols: number, rows: number }> { return this._onResize.event; } protected _onScroll = new EventEmitter(); /** - * An emitter for legacy on scroll events that just included the position, and not the source. - * Used to maintain API consistency for the onScroll method. + * Internally we track the source of the scroll but this is meaningless outside the library so + * it's filtered out. */ - protected _legacyOnScroll?: EventEmitter; + protected _onScrollApi?: EventEmitter; + public get onScroll(): IEvent { + if (!this._onScrollApi) { + this._onScrollApi = new EventEmitter(); + this.register(this._onScroll.event(ev => { + this._onScrollApi?.fire(ev.position); + })); + } + return this._onScrollApi.event; + } public get cols(): number { return this._bufferService.cols; } public get rows(): number { return this._bufferService.rows; } @@ -287,16 +296,6 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal { } } - public get onScroll(): IEvent { - if (!this._legacyOnScroll) { - this._legacyOnScroll = new EventEmitter(); - this.register(this._onScroll.event(ev => { - this._legacyOnScroll?.fire(ev.position); - })); - } - return this._legacyOnScroll.event; - } - /** Add handler for ESC escape sequence. See xterm.d.ts for details. */ public registerEscHandler(id: IFunctionIdentifier, callback: () => boolean | Promise): IDisposable { return this._inputHandler.registerEscHandler(id, callback); From d4a93fe7e44c4d6931d4adf0d1b99011b7ca8d44 Mon Sep 17 00:00:00 2001 From: kena0ki Date: Wed, 31 Mar 2021 23:32:53 +0900 Subject: [PATCH 11/16] Use RenderService.dimensions instead of CharSizeService for textarea position --- src/browser/Terminal.ts | 32 ++++++++++++--------- src/browser/input/CompositionHelper.test.ts | 4 +-- src/browser/input/CompositionHelper.ts | 17 +++++++---- 3 files changed, 31 insertions(+), 22 deletions(-) diff --git a/src/browser/Terminal.ts b/src/browser/Terminal.ts index 2eba4027..95d74254 100644 --- a/src/browser/Terminal.ts +++ b/src/browser/Terminal.ts @@ -297,19 +297,23 @@ export class Terminal extends CoreTerminal implements ITerminal { } private _syncTextArea(): void { - if (!this.textarea || !this.buffer.isCursorInViewport || this._compositionHelper!.isComposing) { + if (!this.textarea || !this.buffer.isCursorInViewport || this._compositionHelper!.isComposing || !this._renderService) { return; } - - const cellHeight = Math.ceil(this._charSizeService!.height * this.optionsService.options.lineHeight); - const cursorTop = this._bufferService.buffer.y * cellHeight; - const cursorLeft = this._bufferService.buffer.x * this._charSizeService!.width; + const cursorY = this.buffer.ybase + this.buffer.y; + const viewportRelativeCursorY = cursorY - this.buffer.ydisp; + const cursorX = Math.min(this.buffer.x, this.cols - 1); + const cellHeight = this._renderService.dimensions.actualCellHeight; + const width = this.buffer.lines.get(cursorY)!.getWidth(cursorX); + const cellWidth = this._renderService.dimensions.actualCellWidth * width; + const cursorTop = viewportRelativeCursorY * this._renderService.dimensions.actualCellHeight; + const cursorLeft = cursorX * this._renderService.dimensions.actualCellWidth; // Sync the textarea to the exact position of the composition view so the IME knows where the // text is. this.textarea.style.left = cursorLeft + 'px'; this.textarea.style.top = cursorTop + 'px'; - this.textarea.style.width = this._charSizeService!.width + 'px'; + this.textarea.style.width = cellWidth +'px'; this.textarea.style.height = cellHeight + 'px'; this.textarea.style.lineHeight = cellHeight + 'px'; this.textarea.style.zIndex = '-5'; @@ -438,14 +442,6 @@ export class Terminal extends CoreTerminal implements ITerminal { this._charSizeService = this._instantiationService.createInstance(CharSizeService, this._document, this._helperContainer); this._instantiationService.setService(ICharSizeService, this._charSizeService); - this._compositionView = document.createElement('div'); - this._compositionView.classList.add('composition-view'); - this._compositionHelper = this._instantiationService.createInstance(CompositionHelper, this.textarea, this._compositionView); - this._helperContainer.appendChild(this._compositionView); - - // Performance: Add viewport and helper elements from the fragment - this.element.appendChild(fragment); - this._theme = this.options.theme || this._theme; this._colorManager = new ColorManager(document, this.options.allowTransparency); this.register(this.optionsService.onOptionChange(e => this._colorManager!.onOptionsChange(e))); @@ -457,6 +453,14 @@ export class Terminal extends CoreTerminal implements ITerminal { this.register(this._renderService.onRenderedBufferChange(e => this._onRender.fire(e))); this.onResize(e => this._renderService!.resize(e.cols, e.rows)); + this._compositionView = document.createElement('div'); + this._compositionView.classList.add('composition-view'); + this._compositionHelper = this._instantiationService.createInstance(CompositionHelper, this.textarea, this._compositionView); + this._helperContainer.appendChild(this._compositionView); + + // Performance: Add viewport and helper elements from the fragment + this.element.appendChild(fragment); + this._soundService = this._instantiationService.createInstance(SoundService); this._instantiationService.setService(ISoundService, this._soundService); this._mouseService = this._instantiationService.createInstance(MouseService); diff --git a/src/browser/input/CompositionHelper.test.ts b/src/browser/input/CompositionHelper.test.ts index b9a4f668..080c9c47 100644 --- a/src/browser/input/CompositionHelper.test.ts +++ b/src/browser/input/CompositionHelper.test.ts @@ -5,7 +5,7 @@ import { assert } from 'chai'; import { CompositionHelper } from 'browser/input/CompositionHelper'; -import { MockCharSizeService } from 'browser/TestUtils.test'; +import { MockCharSizeService, MockRenderService } from 'browser/TestUtils.test'; import { MockCoreService, MockBufferService, MockOptionsService } from 'common/TestUtils.test'; describe('CompositionHelper', () => { @@ -42,7 +42,7 @@ describe('CompositionHelper', () => { }; handledText = ''; const bufferService = new MockBufferService(10, 5); - compositionHelper = new CompositionHelper(textarea, compositionView, bufferService, new MockOptionsService(), new MockCharSizeService(10, 10), coreService); + compositionHelper = new CompositionHelper(textarea, compositionView, bufferService, new MockOptionsService(), new MockCharSizeService(10, 10), coreService, new MockRenderService()); }); describe('Input', () => { diff --git a/src/browser/input/CompositionHelper.ts b/src/browser/input/CompositionHelper.ts index 85cfc3b6..e5389af4 100644 --- a/src/browser/input/CompositionHelper.ts +++ b/src/browser/input/CompositionHelper.ts @@ -3,7 +3,7 @@ * @license MIT */ -import { ICharSizeService } from 'browser/services/Services'; +import { ICharSizeService, IRenderService } from 'browser/services/Services'; import { IBufferService, ICoreService, IOptionsService } from 'common/services/Services'; interface IPosition { @@ -46,7 +46,8 @@ export class CompositionHelper { @IBufferService private readonly _bufferService: IBufferService, @IOptionsService private readonly _optionsService: IOptionsService, @ICharSizeService private readonly _charSizeService: ICharSizeService, - @ICoreService private readonly _coreService: ICoreService + @ICoreService private readonly _coreService: ICoreService, + @IRenderService private readonly _renderService: IRenderService ) { this._isComposing = false; this._isSendingComposition = false; @@ -202,14 +203,18 @@ export class CompositionHelper { * necessary as the IME events across browsers are not consistently triggered. */ public updateCompositionElements(dontRecurse?: boolean): void { - if (!this._isComposing) { + if (!this._isComposing || !this._renderService) { return; } if (this._bufferService.buffer.isCursorInViewport) { - const cellHeight = Math.ceil(this._charSizeService.height * this._optionsService.options.lineHeight); - const cursorTop = this._bufferService.buffer.y * cellHeight; - const cursorLeft = this._bufferService.buffer.x * this._charSizeService.width; + const cursorY = this._bufferService.buffer.ybase + this._bufferService.buffer.y; + const viewportRelativeCursorY = cursorY - this._bufferService.buffer.ydisp; + const cursorX = Math.min(this._bufferService.buffer.x, this._bufferService.cols - 1); + + const cellHeight = this._renderService.dimensions.actualCellHeight; + const cursorTop = viewportRelativeCursorY * this._renderService.dimensions.actualCellHeight; + const cursorLeft = cursorX * this._renderService.dimensions.actualCellWidth; this._compositionView.style.left = cursorLeft + 'px'; this._compositionView.style.top = cursorTop + 'px'; From 49cb6f74bc1d106e1b68374a3b5d040b6a1b281d Mon Sep 17 00:00:00 2001 From: kena0ki Date: Thu, 1 Apr 2021 00:11:30 +0900 Subject: [PATCH 12/16] Remove non-null assertion --- src/browser/Terminal.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/browser/Terminal.ts b/src/browser/Terminal.ts index 95d74254..371f5549 100644 --- a/src/browser/Terminal.ts +++ b/src/browser/Terminal.ts @@ -304,7 +304,9 @@ export class Terminal extends CoreTerminal implements ITerminal { const viewportRelativeCursorY = cursorY - this.buffer.ydisp; const cursorX = Math.min(this.buffer.x, this.cols - 1); const cellHeight = this._renderService.dimensions.actualCellHeight; - const width = this.buffer.lines.get(cursorY)!.getWidth(cursorX); + const bufferLine = this.buffer.lines.get(cursorY); + if (!bufferLine) return; + const width = bufferLine.getWidth(cursorX); const cellWidth = this._renderService.dimensions.actualCellWidth * width; const cursorTop = viewportRelativeCursorY * this._renderService.dimensions.actualCellHeight; const cursorLeft = cursorX * this._renderService.dimensions.actualCellWidth; From 085a2b545f465508bf823549bf49081a2826e62b Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Thu, 1 Apr 2021 08:19:12 -0700 Subject: [PATCH 13/16] Remove now unneeded CharSizeService --- src/browser/input/CompositionHelper.test.ts | 4 ++-- src/browser/input/CompositionHelper.ts | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/browser/input/CompositionHelper.test.ts b/src/browser/input/CompositionHelper.test.ts index 080c9c47..c722570b 100644 --- a/src/browser/input/CompositionHelper.test.ts +++ b/src/browser/input/CompositionHelper.test.ts @@ -5,7 +5,7 @@ import { assert } from 'chai'; import { CompositionHelper } from 'browser/input/CompositionHelper'; -import { MockCharSizeService, MockRenderService } from 'browser/TestUtils.test'; +import { MockRenderService } from 'browser/TestUtils.test'; import { MockCoreService, MockBufferService, MockOptionsService } from 'common/TestUtils.test'; describe('CompositionHelper', () => { @@ -42,7 +42,7 @@ describe('CompositionHelper', () => { }; handledText = ''; const bufferService = new MockBufferService(10, 5); - compositionHelper = new CompositionHelper(textarea, compositionView, bufferService, new MockOptionsService(), new MockCharSizeService(10, 10), coreService, new MockRenderService()); + compositionHelper = new CompositionHelper(textarea, compositionView, bufferService, new MockOptionsService(), coreService, new MockRenderService()); }); describe('Input', () => { diff --git a/src/browser/input/CompositionHelper.ts b/src/browser/input/CompositionHelper.ts index e5389af4..d2626e2c 100644 --- a/src/browser/input/CompositionHelper.ts +++ b/src/browser/input/CompositionHelper.ts @@ -3,7 +3,7 @@ * @license MIT */ -import { ICharSizeService, IRenderService } from 'browser/services/Services'; +import { IRenderService } from 'browser/services/Services'; import { IBufferService, ICoreService, IOptionsService } from 'common/services/Services'; interface IPosition { @@ -45,7 +45,6 @@ export class CompositionHelper { private readonly _compositionView: HTMLElement, @IBufferService private readonly _bufferService: IBufferService, @IOptionsService private readonly _optionsService: IOptionsService, - @ICharSizeService private readonly _charSizeService: ICharSizeService, @ICoreService private readonly _coreService: ICoreService, @IRenderService private readonly _renderService: IRenderService ) { From 3ae61fc1dd783957ea12fc22b9e8b422f505cb38 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Thu, 1 Apr 2021 08:23:18 -0700 Subject: [PATCH 14/16] Use buffer y for relative cursor pos --- src/browser/input/CompositionHelper.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/browser/input/CompositionHelper.ts b/src/browser/input/CompositionHelper.ts index d2626e2c..65a5d672 100644 --- a/src/browser/input/CompositionHelper.ts +++ b/src/browser/input/CompositionHelper.ts @@ -207,12 +207,10 @@ export class CompositionHelper { } if (this._bufferService.buffer.isCursorInViewport) { - const cursorY = this._bufferService.buffer.ybase + this._bufferService.buffer.y; - const viewportRelativeCursorY = cursorY - this._bufferService.buffer.ydisp; const cursorX = Math.min(this._bufferService.buffer.x, this._bufferService.cols - 1); const cellHeight = this._renderService.dimensions.actualCellHeight; - const cursorTop = viewportRelativeCursorY * this._renderService.dimensions.actualCellHeight; + const cursorTop = this._bufferService.buffer.y * this._renderService.dimensions.actualCellHeight; const cursorLeft = cursorX * this._renderService.dimensions.actualCellWidth; this._compositionView.style.left = cursorLeft + 'px'; From 07499380d18c771ddaa70f84f6daaf63ec392afa Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Thu, 1 Apr 2021 08:23:35 -0700 Subject: [PATCH 15/16] Remove RenderService check, it must be passed to ctor --- src/browser/input/CompositionHelper.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/browser/input/CompositionHelper.ts b/src/browser/input/CompositionHelper.ts index 65a5d672..8a204831 100644 --- a/src/browser/input/CompositionHelper.ts +++ b/src/browser/input/CompositionHelper.ts @@ -202,7 +202,7 @@ export class CompositionHelper { * necessary as the IME events across browsers are not consistently triggered. */ public updateCompositionElements(dontRecurse?: boolean): void { - if (!this._isComposing || !this._renderService) { + if (!this._isComposing) { return; } From 87ed4f07cdad13123f9fb09d40b7e4651f0e4633 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Thu, 1 Apr 2021 08:26:35 -0700 Subject: [PATCH 16/16] Tidy up syncTextArea --- src/browser/Terminal.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/browser/Terminal.ts b/src/browser/Terminal.ts index 371f5549..f95013cd 100644 --- a/src/browser/Terminal.ts +++ b/src/browser/Terminal.ts @@ -301,21 +301,22 @@ export class Terminal extends CoreTerminal implements ITerminal { return; } const cursorY = this.buffer.ybase + this.buffer.y; - const viewportRelativeCursorY = cursorY - this.buffer.ydisp; + const bufferLine = this.buffer.lines.get(cursorY); + if (!bufferLine) { + return; + } const cursorX = Math.min(this.buffer.x, this.cols - 1); const cellHeight = this._renderService.dimensions.actualCellHeight; - const bufferLine = this.buffer.lines.get(cursorY); - if (!bufferLine) return; const width = bufferLine.getWidth(cursorX); const cellWidth = this._renderService.dimensions.actualCellWidth * width; - const cursorTop = viewportRelativeCursorY * this._renderService.dimensions.actualCellHeight; + const cursorTop = this.buffer.y * this._renderService.dimensions.actualCellHeight; const cursorLeft = cursorX * this._renderService.dimensions.actualCellWidth; // Sync the textarea to the exact position of the composition view so the IME knows where the // text is. this.textarea.style.left = cursorLeft + 'px'; this.textarea.style.top = cursorTop + 'px'; - this.textarea.style.width = cellWidth +'px'; + this.textarea.style.width = cellWidth + 'px'; this.textarea.style.height = cellHeight + 'px'; this.textarea.style.lineHeight = cellHeight + 'px'; this.textarea.style.zIndex = '-5';