From f983b3cac19aca040568144f3d439db0615f4e5a Mon Sep 17 00:00:00 2001 From: Simon Lamon Date: Sun, 9 Oct 2022 10:15:13 +0000 Subject: [PATCH 01/16] Ubuntu 18.04 deprecation --- azure-pipelines.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 1710922e..7b015064 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -9,7 +9,7 @@ trigger: jobs: - job: Linux pool: - vmImage: 'ubuntu-18.04' + vmImage: 'ubuntu-20.04' steps: - task: NodeTool@0 inputs: From bd661fdb071db5adb132722f1f946c32c0320ffe Mon Sep 17 00:00:00 2001 From: Simon Lamon Date: Sun, 9 Oct 2022 11:29:33 +0000 Subject: [PATCH 02/16] Fix demo again --- addons/xterm-addon-canvas/src/CanvasAddon.ts | 4 +++- addons/xterm-addon-webgl/src/WebglAddon.ts | 4 +++- demo/client.ts | 15 ++++++++++----- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/addons/xterm-addon-canvas/src/CanvasAddon.ts b/addons/xterm-addon-canvas/src/CanvasAddon.ts index 1dc607e5..dcd35704 100644 --- a/addons/xterm-addon-canvas/src/CanvasAddon.ts +++ b/addons/xterm-addon-canvas/src/CanvasAddon.ts @@ -25,7 +25,9 @@ export class CanvasAddon extends Disposable implements ITerminalAddon { public activate(terminal: Terminal): void { const core = (terminal as any)._core; if (!terminal.element) { - this.register(core.onWillOpen(() => this.activate(terminal))); + this.register(toDisposable(() => { + core.onWillOpen(() => this.activate(terminal)); + })); return; } diff --git a/addons/xterm-addon-webgl/src/WebglAddon.ts b/addons/xterm-addon-webgl/src/WebglAddon.ts index 71487315..538713c2 100644 --- a/addons/xterm-addon-webgl/src/WebglAddon.ts +++ b/addons/xterm-addon-webgl/src/WebglAddon.ts @@ -33,7 +33,9 @@ export class WebglAddon extends Disposable implements ITerminalAddon { } const core = (terminal as any)._core; if (!terminal.element) { - this.register(core.onWillOpen(() => this.activate(terminal))); + this.register(toDisposable(() => { + core.onWillOpen(() => this.activate(terminal)); + })); return; } this._terminal = terminal; diff --git a/demo/client.ts b/demo/client.ts index ea55d6d9..3228bd7f 100644 --- a/demo/client.ts +++ b/demo/client.ts @@ -271,11 +271,16 @@ function createTerminal(): void { socketURL = protocol + location.hostname + ((location.port) ? (':' + location.port) : '') + '/terminals/'; addons.fit.instance!.fit(); - typedTerm.loadAddon(addons.webgl.instance); - setTimeout(() => { - addTextureAtlas(addons.webgl.instance.textureAtlas); - addons.webgl.instance.onChangeTextureAtlas(e => addTextureAtlas(e)); - }, 0); + try { // try-catch to allow the demo to load if webgl is not supported + typedTerm.loadAddon(addons.webgl.instance); + setTimeout(() => { + addTextureAtlas(addons.webgl.instance.textureAtlas); + addons.webgl.instance.onChangeTextureAtlas(e => addTextureAtlas(e)); + }, 0); + } + catch { + addons.webgl.instance = undefined; + } term.open(terminalContainer); term.focus(); From a87b6c3f2e056c66969eb185b1c21465a1abb8ea Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 9 Oct 2022 07:09:35 -0700 Subject: [PATCH 03/16] Implement IOptionsService.onSpecificOptionChange Part of #4190 --- src/common/CoreTerminal.ts | 2 +- src/common/TestUtils.test.ts | 12 ++++++++++-- src/common/services/OptionsService.ts | 13 +++++++++++-- src/common/services/Services.ts | 18 +++++++++++++++++- 4 files changed, 39 insertions(+), 6 deletions(-) diff --git a/src/common/CoreTerminal.ts b/src/common/CoreTerminal.ts index ea2528db..5f42d8ea 100644 --- a/src/common/CoreTerminal.ts +++ b/src/common/CoreTerminal.ts @@ -261,7 +261,7 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal { this.coreMouseService.reset(); } - protected _updateOptions(key: string): void { + protected _updateOptions(key: keyof ITerminalOptions): void { // TODO: These listeners should be owned by individual components switch (key) { case 'scrollback': diff --git a/src/common/TestUtils.test.ts b/src/common/TestUtils.test.ts index e302d3e4..050b1cec 100644 --- a/src/common/TestUtils.test.ts +++ b/src/common/TestUtils.test.ts @@ -9,7 +9,7 @@ import { clone } from 'common/Clone'; import { DEFAULT_OPTIONS } from 'common/services/OptionsService'; import { IBufferSet, IBuffer } from 'common/buffer/Types'; import { BufferSet } from 'common/buffer/BufferSet'; -import { IDecPrivateModes, ICoreMouseEvent, CoreMouseEventType, ICharset, IModes, IAttributeData, IOscLinkData } from 'common/Types'; +import { IDecPrivateModes, ICoreMouseEvent, CoreMouseEventType, ICharset, IModes, IAttributeData, IOscLinkData, IDisposable } from 'common/Types'; import { UnicodeV6 } from 'common/input/UnicodeV6'; import { IDecorationOptions, IDecoration } from 'xterm'; @@ -113,7 +113,7 @@ export class MockOptionsService implements IOptionsService { public serviceBrand: any; public readonly rawOptions: Required = clone(DEFAULT_OPTIONS); public options: Required = this.rawOptions; - public onOptionChange: IEvent = new EventEmitter().event; + public onOptionChange: IEvent = new EventEmitter().event; constructor(testOptions?: Partial) { if (testOptions) { for (const key of Object.keys(testOptions)) { @@ -121,6 +121,14 @@ export class MockOptionsService implements IOptionsService { } } } + // eslint-disable-next-line @typescript-eslint/naming-convention + public onSpecificOptionChange(key: T, listener: (arg1: ITerminalOptions[T]) => any): IDisposable { + return this.onOptionChange(eventKey => { + if (eventKey === key) { + listener(this.rawOptions[key]); + } + }); + } public setOptions(options: ITerminalOptions): void { for (const key of Object.keys(options)) { this.options[key] = options[key]; diff --git a/src/common/services/OptionsService.ts b/src/common/services/OptionsService.ts index 336591e5..439d6a77 100644 --- a/src/common/services/OptionsService.ts +++ b/src/common/services/OptionsService.ts @@ -6,7 +6,7 @@ import { IOptionsService, ITerminalOptions, FontWeight } from 'common/services/Services'; import { EventEmitter, IEvent } from 'common/EventEmitter'; import { isMac } from 'common/Platform'; -import { CursorStyle } from 'common/Types'; +import { CursorStyle, IDisposable } from 'common/Types'; import { Disposable } from 'common/Lifecycle'; export const DEFAULT_OPTIONS: Readonly> = { @@ -58,7 +58,7 @@ export class OptionsService extends Disposable implements IOptionsService { public readonly rawOptions: Required; public options: Required; - private readonly _onOptionChange = this.register(new EventEmitter()); + private readonly _onOptionChange = this.register(new EventEmitter()); public readonly onOptionChange = this._onOptionChange.event; constructor(options: Partial) { @@ -82,6 +82,15 @@ export class OptionsService extends Disposable implements IOptionsService { this._setupOptions(); } + // eslint-disable-next-line @typescript-eslint/naming-convention + public onSpecificOptionChange(key: T, listener: (value: ITerminalOptions[T]) => any): IDisposable { + return this.onOptionChange(eventKey => { + if (eventKey === key) { + listener(this.rawOptions[key]); + } + }); + } + private _setupOptions(): void { const getter = (propName: string): any => { if (!(propName in DEFAULT_OPTIONS)) { diff --git a/src/common/services/Services.ts b/src/common/services/Services.ts index e2b517cd..9b591962 100644 --- a/src/common/services/Services.ts +++ b/src/common/services/Services.ts @@ -182,9 +182,25 @@ export interface IOptionsService { * internally. */ readonly rawOptions: Required; + + /** + * Options as exposed through the public API, this property uses getters and setters with + * validation which makes it safer but slower. {@link rawOptions} should be used for pretty much + * all internal usage for performance reasons. + */ readonly options: Required; - readonly onOptionChange: IEvent; + /** + * Adds an event listener for when any option changes. + */ + readonly onOptionChange: IEvent; + + /** + * Adds an event listener for when a specific option changes, this is a convenience method that is + * preferred over {@link onOptionChange} when only a single option is being listened to. + */ + // eslint-disable-next-line @typescript-eslint/naming-convention + onSpecificOptionChange(key: T, listener: (arg1: ITerminalOptions[T]) => any): IDisposable; } export type FontWeight = 'normal' | 'bold' | '100' | '200' | '300' | '400' | '500' | '600' | '700' | '800' | '900' | number; From cad9c4164cac2433b7e9b99f34c6693264e4ee31 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 9 Oct 2022 07:16:56 -0700 Subject: [PATCH 04/16] onOptionChange/onSpecificOptionChange tests --- src/common/services/OptionsService.test.ts | 41 ++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/src/common/services/OptionsService.test.ts b/src/common/services/OptionsService.test.ts index 3ff22ac0..a0351516 100644 --- a/src/common/services/OptionsService.test.ts +++ b/src/common/services/OptionsService.test.ts @@ -5,6 +5,7 @@ import { assert } from 'chai'; import { OptionsService, DEFAULT_OPTIONS } from 'common/services/OptionsService'; +import { IDisposable } from 'common/Types'; describe('OptionsService', () => { describe('constructor', () => { @@ -71,4 +72,44 @@ describe('OptionsService', () => { assert.equal(service.options.fontWeight, DEFAULT_OPTIONS.fontWeight, 'Wrong string literals should be reset to default'); }); }); + describe('onOptionChange', () => { + let service: OptionsService; + beforeEach(() => { + service = new OptionsService({}); + }); + it('should fire on any option change', async () => { + let disposable: IDisposable; + await new Promise(r => { + disposable = service.onOptionChange(e => { + assert.strictEqual(e, 'cursorWidth'); + r(); + }); + service.options.cursorWidth = 10; + }); + disposable!.dispose(); + await new Promise(r => { + service.onOptionChange(e => { + assert.strictEqual(e, 'scrollback'); + r(); + }); + service.options.scrollback = 20; + }); + }); + }); + describe('onSpecificOptionChange', () => { + let service: OptionsService; + beforeEach(() => { + service = new OptionsService({}); + }); + it('should fire only on a specific option change', async () => { + await new Promise(r => { + service.onSpecificOptionChange('scrollback', e => { + assert.strictEqual(e, 20); + r(); + }); + service.options.cursorWidth = 10; + service.options.scrollback = 20; + }); + }); + }); }); From 627410a53743a7fe420f99b7db7b48eef0b654eb Mon Sep 17 00:00:00 2001 From: Simon Lamon Date: Sun, 9 Oct 2022 14:26:08 +0000 Subject: [PATCH 05/16] Add broken demo to investigate --- addons/xterm-addon-canvas/src/CanvasAddon.ts | 4 +- addons/xterm-addon-webgl/src/WebglAddon.ts | 4 +- addons/xterm-addon-webgl/src/WebglRenderer.ts | 3 ++ demo/client.ts | 48 ++++++++++--------- 4 files changed, 30 insertions(+), 29 deletions(-) diff --git a/addons/xterm-addon-canvas/src/CanvasAddon.ts b/addons/xterm-addon-canvas/src/CanvasAddon.ts index dcd35704..1dc607e5 100644 --- a/addons/xterm-addon-canvas/src/CanvasAddon.ts +++ b/addons/xterm-addon-canvas/src/CanvasAddon.ts @@ -25,9 +25,7 @@ export class CanvasAddon extends Disposable implements ITerminalAddon { public activate(terminal: Terminal): void { const core = (terminal as any)._core; if (!terminal.element) { - this.register(toDisposable(() => { - core.onWillOpen(() => this.activate(terminal)); - })); + this.register(core.onWillOpen(() => this.activate(terminal))); return; } diff --git a/addons/xterm-addon-webgl/src/WebglAddon.ts b/addons/xterm-addon-webgl/src/WebglAddon.ts index 538713c2..71487315 100644 --- a/addons/xterm-addon-webgl/src/WebglAddon.ts +++ b/addons/xterm-addon-webgl/src/WebglAddon.ts @@ -33,9 +33,7 @@ export class WebglAddon extends Disposable implements ITerminalAddon { } const core = (terminal as any)._core; if (!terminal.element) { - this.register(toDisposable(() => { - core.onWillOpen(() => this.activate(terminal)); - })); + this.register(core.onWillOpen(() => this.activate(terminal))); return; } this._terminal = terminal; diff --git a/addons/xterm-addon-webgl/src/WebglRenderer.ts b/addons/xterm-addon-webgl/src/WebglRenderer.ts index ed99a225..8f747c03 100644 --- a/addons/xterm-addon-webgl/src/WebglRenderer.ts +++ b/addons/xterm-addon-webgl/src/WebglRenderer.ts @@ -103,6 +103,9 @@ export class WebglRenderer extends Disposable implements IRenderer { throw new Error('WebGL2 not supported ' + this._gl); } + // TODO: Remove me. + throw new Error('WebGL2 not supported ' + this._gl); + this.register(addDisposableDomListener(this._canvas, 'webglcontextlost', (e) => { console.log('webglcontextlost event received'); // Prevent the default behavior in order to enable WebGL context restoration. diff --git a/demo/client.ts b/demo/client.ts index 3228bd7f..6d48b302 100644 --- a/demo/client.ts +++ b/demo/client.ts @@ -61,26 +61,26 @@ interface IDemoAddon { canChange: boolean; ctor: ( T extends 'attach' ? typeof AttachAddon : - T extends 'canvas' ? typeof CanvasAddon : - T extends 'fit' ? typeof FitAddon : - T extends 'search' ? typeof SearchAddon : - T extends 'serialize' ? typeof SerializeAddon : - T extends 'web-links' ? typeof WebLinksAddon : - T extends 'unicode11' ? typeof Unicode11Addon : - T extends 'ligatures' ? typeof LigaturesAddon : - typeof WebglAddon + T extends 'canvas' ? typeof CanvasAddon : + T extends 'fit' ? typeof FitAddon : + T extends 'search' ? typeof SearchAddon : + T extends 'serialize' ? typeof SerializeAddon : + T extends 'web-links' ? typeof WebLinksAddon : + T extends 'unicode11' ? typeof Unicode11Addon : + T extends 'ligatures' ? typeof LigaturesAddon : + typeof WebglAddon ); instance?: ( T extends 'attach' ? AttachAddon : - T extends 'canvas' ? CanvasAddon : - T extends 'fit' ? FitAddon : - T extends 'search' ? SearchAddon : - T extends 'serialize' ? SerializeAddon : - T extends 'web-links' ? WebLinksAddon : - T extends 'webgl' ? WebglAddon : - T extends 'unicode11' ? typeof Unicode11Addon : - T extends 'ligatures' ? typeof LigaturesAddon : - never + T extends 'canvas' ? CanvasAddon : + T extends 'fit' ? FitAddon : + T extends 'search' ? SearchAddon : + T extends 'serialize' ? SerializeAddon : + T extends 'web-links' ? WebLinksAddon : + T extends 'webgl' ? WebglAddon : + T extends 'unicode11' ? typeof Unicode11Addon : + T extends 'ligatures' ? typeof LigaturesAddon : + never ); } @@ -271,18 +271,20 @@ function createTerminal(): void { socketURL = protocol + location.hostname + ((location.port) ? (':' + location.port) : '') + '/terminals/'; addons.fit.instance!.fit(); - try { // try-catch to allow the demo to load if webgl is not supported - typedTerm.loadAddon(addons.webgl.instance); - setTimeout(() => { + typedTerm.loadAddon(addons.webgl.instance); + setTimeout(() => { + if (addons.webgl.instance !== undefined) { addTextureAtlas(addons.webgl.instance.textureAtlas); addons.webgl.instance.onChangeTextureAtlas(e => addTextureAtlas(e)); - }, 0); + } + }, 0); + + try { // try-catch to allow the demo to load if webgl is not supported + term.open(terminalContainer); } catch { addons.webgl.instance = undefined; } - - term.open(terminalContainer); term.focus(); addDomListener(paddingElement, 'change', setPadding); From 415100e8b843d748e2c117ceca4c8662eafc0041 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 9 Oct 2022 07:46:10 -0700 Subject: [PATCH 06/16] Adopt onSpecificOptionChange --- src/browser/Terminal.ts | 7 ++---- .../decorations/OverviewRulerRenderer.ts | 10 ++------- src/browser/services/ThemeService.ts | 12 ++-------- src/common/CoreTerminal.ts | 22 ++++++------------- src/common/buffer/BufferSet.ts | 3 +++ src/common/services/BufferService.ts | 2 +- src/common/services/LogService.ts | 6 +---- src/headless/Terminal.ts | 9 -------- 8 files changed, 18 insertions(+), 53 deletions(-) diff --git a/src/browser/Terminal.ts b/src/browser/Terminal.ts index e0ba8a94..abb5dfdf 100644 --- a/src/browser/Terminal.ts +++ b/src/browser/Terminal.ts @@ -265,8 +265,6 @@ export class Terminal extends CoreTerminal implements ITerminal { } protected _updateOptions(key: string): void { - super._updateOptions(key); - // TODO: These listeners should be owned by individual components switch (key) { case 'fontFamily': @@ -307,7 +305,6 @@ export class Terminal extends CoreTerminal implements ITerminal { this._accessibilityManager = undefined; } break; - case 'tabStopWidth': this.buffers.setupTabStops(); break; } } @@ -584,8 +581,8 @@ export class Terminal extends CoreTerminal implements ITerminal { if (this.options.overviewRulerWidth) { this._overviewRulerRenderer = this.register(this._instantiationService.createInstance(OverviewRulerRenderer, this._viewportElement, this.screenElement)); } - this.optionsService.onOptionChange(() => { - if (!this._overviewRulerRenderer && this.options.overviewRulerWidth && this._viewportElement && this.screenElement) { + this.optionsService.onSpecificOptionChange('overviewRulerWidth', value => { + if (!this._overviewRulerRenderer && value && this._viewportElement && this.screenElement) { this._overviewRulerRenderer = this.register(this._instantiationService.createInstance(OverviewRulerRenderer, this._viewportElement, this.screenElement)); } }); diff --git a/src/browser/decorations/OverviewRulerRenderer.ts b/src/browser/decorations/OverviewRulerRenderer.ts index 9251d2e3..90166960 100644 --- a/src/browser/decorations/OverviewRulerRenderer.ts +++ b/src/browser/decorations/OverviewRulerRenderer.ts @@ -110,15 +110,9 @@ export class OverviewRulerRenderer extends Disposable { } })); // overview ruler width changed - this.register(this._optionsService.onOptionChange(o => { - if (o === 'overviewRulerWidth') { - this._queueRefresh(true); - } - })); + this.register(this._optionsService.onSpecificOptionChange('overviewRulerWidth', () => this._queueRefresh(true))); // device pixel ratio changed - this.register(addDisposableDomListener(this._coreBrowseService.window, 'resize', () => { - this._queueRefresh(true); - })); + this.register(addDisposableDomListener(this._coreBrowseService.window, 'resize', () => this._queueRefresh(true))); // set the canvas dimensions this._queueRefresh(true); } diff --git a/src/browser/services/ThemeService.ts b/src/browser/services/ThemeService.ts index e584a8d1..ac0104d5 100644 --- a/src/browser/services/ThemeService.ts +++ b/src/browser/services/ThemeService.ts @@ -111,16 +111,8 @@ export class ThemeService extends Disposable implements IThemeService { this._updateRestoreColors(); this._setTheme(this._optionsService.rawOptions.theme); - this.register(this._optionsService.onOptionChange(key => { - switch (key) { - case 'minimumContrastRatio': - this._contrastCache.clear(); - break; - case 'theme': - this._setTheme(this._optionsService.rawOptions.theme); - break; - } - })); + this.register(this._optionsService.onSpecificOptionChange('minimumContrastRatio', () => this._contrastCache.clear())); + this.register(this._optionsService.onSpecificOptionChange('theme', () => this._setTheme(this._optionsService.rawOptions.theme))); } /** diff --git a/src/common/CoreTerminal.ts b/src/common/CoreTerminal.ts index 5f42d8ea..8f1c2a0d 100644 --- a/src/common/CoreTerminal.ts +++ b/src/common/CoreTerminal.ts @@ -130,7 +130,7 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal { this.register(forwardEvent(this.coreService.onData, this._onData)); this.register(forwardEvent(this.coreService.onBinary, this._onBinary)); this.register(this.coreService.onUserInput(() => this._writeBuffer.handleUserInput())); - this.register(this.optionsService.onOptionChange(key => this._updateOptions(key))); + this.register(this.optionsService.onSpecificOptionChange('windowsMode', e => this._handleWindowsModeOptionChange(e))); this.register(this._bufferService.onScroll(event => { this._onScroll.fire({ position: this._bufferService.buffer.ydisp, source: ScrollSource.TERMINAL }); this._inputHandler.markRangeDirty(this._bufferService.buffer.scrollTop, this._bufferService.buffer.scrollBottom); @@ -261,20 +261,12 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal { this.coreMouseService.reset(); } - protected _updateOptions(key: keyof ITerminalOptions): void { - // TODO: These listeners should be owned by individual components - switch (key) { - case 'scrollback': - this.buffers.resize(this.cols, this.rows); - break; - case 'windowsMode': - if (this.optionsService.rawOptions.windowsMode) { - this._enableWindowsMode(); - } else { - this._windowsMode?.dispose(); - this._windowsMode = undefined; - } - break; + private _handleWindowsModeOptionChange(value: boolean | undefined): void { + if (value) { + this._enableWindowsMode(); + } else { + this._windowsMode?.dispose(); + this._windowsMode = undefined; } } diff --git a/src/common/buffer/BufferSet.ts b/src/common/buffer/BufferSet.ts index 46fcb097..bc7aa58e 100644 --- a/src/common/buffer/BufferSet.ts +++ b/src/common/buffer/BufferSet.ts @@ -32,6 +32,8 @@ export class BufferSet extends Disposable implements IBufferSet { ) { super(); this.reset(); + this.register(this._optionsService.onSpecificOptionChange('scrollback', () => this.resize(this._bufferService.cols, this._bufferService.rows))); + this.register(this._optionsService.onSpecificOptionChange('tabStopWidth', () => this.setupTabStops())); } public reset(): void { @@ -119,6 +121,7 @@ export class BufferSet extends Disposable implements IBufferSet { public resize(newCols: number, newRows: number): void { this._normal.resize(newCols, newRows); this._alt.resize(newCols, newRows); + this.setupTabStops(newCols); } /** diff --git a/src/common/services/BufferService.ts b/src/common/services/BufferService.ts index f238206c..3f15f242 100644 --- a/src/common/services/BufferService.ts +++ b/src/common/services/BufferService.ts @@ -43,7 +43,7 @@ export class BufferService extends Disposable implements IBufferService { this.cols = cols; this.rows = rows; this.buffers.resize(cols, rows); - this.buffers.setupTabStops(this.cols); + // TODO: This doesn't fire when scrollback changes - add a resize event to BufferSet and forward event this._onResize.fire({ cols, rows }); } diff --git a/src/common/services/LogService.ts b/src/common/services/LogService.ts index 854f85de..4b56a097 100644 --- a/src/common/services/LogService.ts +++ b/src/common/services/LogService.ts @@ -40,11 +40,7 @@ export class LogService extends Disposable implements ILogService { ) { super(); this._updateLogLevel(); - this.register(this._optionsService.onOptionChange(key => { - if (key === 'logLevel') { - this._updateLogLevel(); - } - })); + this.register(this._optionsService.onSpecificOptionChange('logLevel', () => this._updateLogLevel())); } private _updateLogLevel(): void { diff --git a/src/headless/Terminal.ts b/src/headless/Terminal.ts index f021c42b..2c244f21 100644 --- a/src/headless/Terminal.ts +++ b/src/headless/Terminal.ts @@ -78,15 +78,6 @@ export class Terminal extends CoreTerminal { return this.buffers.active; } - protected _updateOptions(key: string): void { - super._updateOptions(key); - - // TODO: These listeners should be owned by individual components - switch (key) { - case 'tabStopWidth': this.buffers.setupTabStops(); break; - } - } - // TODO: Support paste here? public get markers(): IMarker[] { From 1c345bef48977ce409f29313f1c442168c75f2aa Mon Sep 17 00:00:00 2001 From: Simon Lamon Date: Sun, 9 Oct 2022 15:01:15 +0000 Subject: [PATCH 07/16] Handle onWillFire if it fails, so it loads dom renderer --- addons/xterm-addon-webgl/src/WebglRenderer.ts | 3 --- src/browser/Terminal.ts | 5 ++++- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/addons/xterm-addon-webgl/src/WebglRenderer.ts b/addons/xterm-addon-webgl/src/WebglRenderer.ts index 8f747c03..ed99a225 100644 --- a/addons/xterm-addon-webgl/src/WebglRenderer.ts +++ b/addons/xterm-addon-webgl/src/WebglRenderer.ts @@ -103,9 +103,6 @@ export class WebglRenderer extends Disposable implements IRenderer { throw new Error('WebGL2 not supported ' + this._gl); } - // TODO: Remove me. - throw new Error('WebGL2 not supported ' + this._gl); - this.register(addDisposableDomListener(this._canvas, 'webglcontextlost', (e) => { console.log('webglcontextlost event received'); // Prevent the default behavior in order to enable WebGL context restoration. diff --git a/src/browser/Terminal.ts b/src/browser/Terminal.ts index e0ba8a94..9b17c55c 100644 --- a/src/browser/Terminal.ts +++ b/src/browser/Terminal.ts @@ -515,7 +515,10 @@ export class Terminal extends CoreTerminal implements ITerminal { // Performance: Add viewport and helper elements from the fragment this.element.appendChild(fragment); - this._onWillOpen.fire(this.element); + try { + this._onWillOpen.fire(this.element); + } + catch { /* fails to load addon for some reason */ } if (!this._renderService.hasRenderer()) { this._renderService.setRenderer(this._createRenderer()); } From a4c62792df5b732e34bd25257fa2cfdf05d7c0a8 Mon Sep 17 00:00:00 2001 From: Simon Lamon Date: Sun, 9 Oct 2022 15:03:54 +0000 Subject: [PATCH 08/16] revert formatting changes --- demo/client.ts | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/demo/client.ts b/demo/client.ts index 6d48b302..44a96eb3 100644 --- a/demo/client.ts +++ b/demo/client.ts @@ -61,26 +61,26 @@ interface IDemoAddon { canChange: boolean; ctor: ( T extends 'attach' ? typeof AttachAddon : - T extends 'canvas' ? typeof CanvasAddon : - T extends 'fit' ? typeof FitAddon : - T extends 'search' ? typeof SearchAddon : - T extends 'serialize' ? typeof SerializeAddon : - T extends 'web-links' ? typeof WebLinksAddon : - T extends 'unicode11' ? typeof Unicode11Addon : - T extends 'ligatures' ? typeof LigaturesAddon : - typeof WebglAddon + T extends 'canvas' ? typeof CanvasAddon : + T extends 'fit' ? typeof FitAddon : + T extends 'search' ? typeof SearchAddon : + T extends 'serialize' ? typeof SerializeAddon : + T extends 'web-links' ? typeof WebLinksAddon : + T extends 'unicode11' ? typeof Unicode11Addon : + T extends 'ligatures' ? typeof LigaturesAddon : + typeof WebglAddon ); instance?: ( T extends 'attach' ? AttachAddon : - T extends 'canvas' ? CanvasAddon : - T extends 'fit' ? FitAddon : - T extends 'search' ? SearchAddon : - T extends 'serialize' ? SerializeAddon : - T extends 'web-links' ? WebLinksAddon : - T extends 'webgl' ? WebglAddon : - T extends 'unicode11' ? typeof Unicode11Addon : - T extends 'ligatures' ? typeof LigaturesAddon : - never + T extends 'canvas' ? CanvasAddon : + T extends 'fit' ? FitAddon : + T extends 'search' ? SearchAddon : + T extends 'serialize' ? SerializeAddon : + T extends 'web-links' ? WebLinksAddon : + T extends 'webgl' ? WebglAddon : + T extends 'unicode11' ? typeof Unicode11Addon : + T extends 'ligatures' ? typeof LigaturesAddon : + never ); } From 49a0904857c9a44decc7fbed8f7d6633d1091456 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 9 Oct 2022 08:07:43 -0700 Subject: [PATCH 09/16] Move onSpecificOptionChange adoption --- src/browser/Terminal.ts | 26 +++++++++++++------------- src/browser/Viewport.ts | 1 + src/common/CoreTerminal.ts | 2 +- src/common/services/Services.ts | 2 +- 4 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/browser/Terminal.ts b/src/browser/Terminal.ts index abb5dfdf..37883ded 100644 --- a/src/browser/Terminal.ts +++ b/src/browser/Terminal.ts @@ -264,6 +264,17 @@ export class Terminal extends CoreTerminal implements ITerminal { } } + private _handleScreenReaderModeOptionChange(value: boolean): void { + if (value) { + if (!this._accessibilityManager && this._renderService) { + this._accessibilityManager = new AccessibilityManager(this, this._renderService); + } + } else { + this._accessibilityManager?.dispose(); + this._accessibilityManager = undefined; + } + } + protected _updateOptions(key: string): void { // TODO: These listeners should be owned by individual components switch (key) { @@ -285,6 +296,7 @@ export class Terminal extends CoreTerminal implements ITerminal { case 'fontWeight': case 'fontWeightBold': case 'minimumContrastRatio': + // TODO: move to render service // When the font changes the size of the cells may change which requires a renderer clear if (this._renderService) { this._renderService.clear(); @@ -292,19 +304,6 @@ export class Terminal extends CoreTerminal implements ITerminal { this.refresh(0, this.rows - 1); } break; - case 'scrollback': - this.viewport?.syncScrollArea(); - break; - case 'screenReaderMode': - if (this.optionsService.rawOptions.screenReaderMode) { - if (!this._accessibilityManager && this._renderService) { - this._accessibilityManager = new AccessibilityManager(this, this._renderService); - } - } else { - this._accessibilityManager?.dispose(); - this._accessibilityManager = undefined; - } - break; } } @@ -577,6 +576,7 @@ export class Terminal extends CoreTerminal implements ITerminal { // ensure the correct order of the dprchange event this._accessibilityManager = new AccessibilityManager(this, this._renderService); } + this.register(this.optionsService.onSpecificOptionChange('screenReaderMode', e => this._handleScreenReaderModeOptionChange(e))); if (this.options.overviewRulerWidth) { this._overviewRulerRenderer = this.register(this._instantiationService.createInstance(OverviewRulerRenderer, this._viewportElement, this.screenElement)); diff --git a/src/browser/Viewport.ts b/src/browser/Viewport.ts index 8f88c559..700c9e22 100644 --- a/src/browser/Viewport.ts +++ b/src/browser/Viewport.ts @@ -75,6 +75,7 @@ export class Viewport extends Disposable implements IViewport { this._handleThemeChange(themeService.colors); this.register(themeService.onChangeColors(e => this._handleThemeChange(e))); + this.register(this._optionsService.onSpecificOptionChange('scrollback', () => this.syncScrollArea())); // Perform this async to ensure the ICharSizeService is ready. setTimeout(() => this.syncScrollArea(), 0); diff --git a/src/common/CoreTerminal.ts b/src/common/CoreTerminal.ts index 8f1c2a0d..f4a7d301 100644 --- a/src/common/CoreTerminal.ts +++ b/src/common/CoreTerminal.ts @@ -261,7 +261,7 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal { this.coreMouseService.reset(); } - private _handleWindowsModeOptionChange(value: boolean | undefined): void { + private _handleWindowsModeOptionChange(value: boolean): void { if (value) { this._enableWindowsMode(); } else { diff --git a/src/common/services/Services.ts b/src/common/services/Services.ts index 9b591962..5e6751e9 100644 --- a/src/common/services/Services.ts +++ b/src/common/services/Services.ts @@ -200,7 +200,7 @@ export interface IOptionsService { * preferred over {@link onOptionChange} when only a single option is being listened to. */ // eslint-disable-next-line @typescript-eslint/naming-convention - onSpecificOptionChange(key: T, listener: (arg1: ITerminalOptions[T]) => any): IDisposable; + onSpecificOptionChange(key: T, listener: (arg1: Required[T]) => any): IDisposable; } export type FontWeight = 'normal' | 'bold' | '100' | '200' | '300' | '400' | '500' | '600' | '700' | '800' | '900' | number; From 7eead4404848878e5bd3cadd6fb88c0138ae3db5 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 9 Oct 2022 08:14:56 -0700 Subject: [PATCH 10/16] Add onMultipleOptionChange --- src/browser/services/CharSizeService.ts | 2 ++ src/common/TestUtils.test.ts | 8 +++++ src/common/services/OptionsService.test.ts | 35 ++++++++++++++++++++++ src/common/services/OptionsService.ts | 9 ++++++ src/common/services/Services.ts | 8 +++++ 5 files changed, 62 insertions(+) diff --git a/src/browser/services/CharSizeService.ts b/src/browser/services/CharSizeService.ts index 267a361b..c75b8de0 100644 --- a/src/browser/services/CharSizeService.ts +++ b/src/browser/services/CharSizeService.ts @@ -27,6 +27,8 @@ export class CharSizeService extends Disposable implements ICharSizeService { ) { super(); this._measureStrategy = new DomMeasureStrategy(document, parentElement, this._optionsService); + // TODO: ... + // this.register(this._optionsService.onSpecificOptionChange( } public measure(): void { diff --git a/src/common/TestUtils.test.ts b/src/common/TestUtils.test.ts index 050b1cec..3aa0f694 100644 --- a/src/common/TestUtils.test.ts +++ b/src/common/TestUtils.test.ts @@ -129,6 +129,14 @@ export class MockOptionsService implements IOptionsService { } }); } + // eslint-disable-next-line @typescript-eslint/naming-convention + public onMultipleOptionChange(keys: (keyof ITerminalOptions)[], listener: () => any): IDisposable { + return this.onOptionChange(eventKey => { + if (keys.indexOf(eventKey) !== -1) { + listener(); + } + }); + } public setOptions(options: ITerminalOptions): void { for (const key of Object.keys(options)) { this.options[key] = options[key]; diff --git a/src/common/services/OptionsService.test.ts b/src/common/services/OptionsService.test.ts index a0351516..004f7316 100644 --- a/src/common/services/OptionsService.test.ts +++ b/src/common/services/OptionsService.test.ts @@ -112,4 +112,39 @@ describe('OptionsService', () => { }); }); }); + describe('onSpecificOptionChange', () => { + let service: OptionsService; + beforeEach(() => { + service = new OptionsService({}); + }); + it('should fire only on a specific option change', async () => { + await new Promise(r => { + service.onSpecificOptionChange('scrollback', e => { + assert.strictEqual(e, 20); + r(); + }); + service.options.cursorWidth = 10; + service.options.scrollback = 20; + }); + }); + }); + describe('onMultipleOptionChange', () => { + let service: OptionsService; + beforeEach(() => { + service = new OptionsService({}); + }); + it('should fire only for specific options', async () => { + await new Promise(r => { + let called = false; + service.onMultipleOptionChange(['scrollback'], () => { + called = true; + }); + service.options.cursorWidth = 10; + assert.notOk(called); + service.options.scrollback = 20; + assert.ok(called); + r(); + }); + }); + }); }); diff --git a/src/common/services/OptionsService.ts b/src/common/services/OptionsService.ts index 439d6a77..976cdf8d 100644 --- a/src/common/services/OptionsService.ts +++ b/src/common/services/OptionsService.ts @@ -91,6 +91,15 @@ export class OptionsService extends Disposable implements IOptionsService { }); } + // eslint-disable-next-line @typescript-eslint/naming-convention + public onMultipleOptionChange(keys: (keyof ITerminalOptions)[], listener: () => any): IDisposable { + return this.onOptionChange(eventKey => { + if (keys.indexOf(eventKey) !== -1) { + listener(); + } + }); + } + private _setupOptions(): void { const getter = (propName: string): any => { if (!(propName in DEFAULT_OPTIONS)) { diff --git a/src/common/services/Services.ts b/src/common/services/Services.ts index 5e6751e9..cc388063 100644 --- a/src/common/services/Services.ts +++ b/src/common/services/Services.ts @@ -201,6 +201,14 @@ export interface IOptionsService { */ // eslint-disable-next-line @typescript-eslint/naming-convention onSpecificOptionChange(key: T, listener: (arg1: Required[T]) => any): IDisposable; + + /** + * Adds an event listener for when a set of specific options change, this is a convenience method + * that is preferred over {@link onOptionChange} when multiple options are being listened to and + * handled the same way. + */ + // eslint-disable-next-line @typescript-eslint/naming-convention + onMultipleOptionChange(keys: (keyof ITerminalOptions)[], listener: () => any): IDisposable; } export type FontWeight = 'normal' | 'bold' | '100' | '200' | '300' | '400' | '500' | '600' | '700' | '800' | '900' | number; From 8ac88bc6bf23795cd67a96c28ffe9617f9dd211c Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 9 Oct 2022 09:07:01 -0700 Subject: [PATCH 11/16] Move Terminal option updates into owning components --- src/browser/Terminal.ts | 32 ------------------------- src/browser/services/CharSizeService.ts | 4 ++-- src/browser/services/RenderService.ts | 24 +++++++++++++++++-- 3 files changed, 24 insertions(+), 36 deletions(-) diff --git a/src/browser/Terminal.ts b/src/browser/Terminal.ts index 37883ded..c8769e0b 100644 --- a/src/browser/Terminal.ts +++ b/src/browser/Terminal.ts @@ -275,38 +275,6 @@ export class Terminal extends CoreTerminal implements ITerminal { } } - protected _updateOptions(key: string): void { - // TODO: These listeners should be owned by individual components - switch (key) { - case 'fontFamily': - case 'fontSize': - // When the font changes the size of the cells may change which requires a renderer clear - this._renderService?.clear(); - this._charSizeService?.measure(); - break; - case 'cursorBlink': - case 'cursorStyle': - // The DOM renderer needs a row refresh to update the cursor styles - this.refresh(this.buffer.y, this.buffer.y); - break; - case 'customGlyphs': - case 'drawBoldTextInBrightColors': - case 'letterSpacing': - case 'lineHeight': - case 'fontWeight': - case 'fontWeightBold': - case 'minimumContrastRatio': - // TODO: move to render service - // When the font changes the size of the cells may change which requires a renderer clear - if (this._renderService) { - this._renderService.clear(); - this._renderService.handleResize(this.cols, this.rows); - this.refresh(0, this.rows - 1); - } - break; - } - } - /** * Binds the desired focus behavior on a given terminal object. */ diff --git a/src/browser/services/CharSizeService.ts b/src/browser/services/CharSizeService.ts index c75b8de0..45bbe840 100644 --- a/src/browser/services/CharSizeService.ts +++ b/src/browser/services/CharSizeService.ts @@ -7,6 +7,7 @@ import { IOptionsService } from 'common/services/Services'; import { EventEmitter } from 'common/EventEmitter'; import { ICharSizeService } from 'browser/services/Services'; import { Disposable } from 'common/Lifecycle'; +import { ITerminalOptions } from 'common/Types'; export class CharSizeService extends Disposable implements ICharSizeService { public serviceBrand: undefined; @@ -27,8 +28,7 @@ export class CharSizeService extends Disposable implements ICharSizeService { ) { super(); this._measureStrategy = new DomMeasureStrategy(document, parentElement, this._optionsService); - // TODO: ... - // this.register(this._optionsService.onSpecificOptionChange( + this.register(this._optionsService.onMultipleOptionChange(['fontFamily', 'fontSize'], () => this.measure())); } public measure(): void { diff --git a/src/browser/services/RenderService.ts b/src/browser/services/RenderService.ts index dd0c6b50..1e1943df 100644 --- a/src/browser/services/RenderService.ts +++ b/src/browser/services/RenderService.ts @@ -84,8 +84,28 @@ export class RenderService extends Disposable implements IRenderService { this.register(decorationService.onDecorationRegistered(() => this._fullRefresh())); this.register(decorationService.onDecorationRemoved(() => this._fullRefresh())); - // No need to register this as renderer is explicitly disposed in RenderService.dispose - // this._renderer.onRequestRedraw(e => this.refreshRows(e.start, e.end, true)); + // Clear the renderer when the a change that could affect glyphs occurs + this.register(optionsService.onMultipleOptionChange([ + 'customGlyphs', + 'drawBoldTextInBrightColors', + 'letterSpacing', + 'lineHeight', + 'fontFamily', + 'fontSize', + 'fontWeight', + 'fontWeightBold', + 'minimumContrastRatio' + ], () => { + this.clear(); + this.handleResize(bufferService.cols, bufferService.rows); + this._fullRefresh(); + })); + + // Refresh the cursor line when the cursor changes + this.register(optionsService.onMultipleOptionChange([ + 'cursorBlink', + 'cursorStyle' + ], () => this.refreshRows(bufferService.buffer.y, bufferService.buffer.y, true))); // dprchange should handle this case, we need this as well for browsers that don't support the // matchMedia query. From e47296c1b9f72596699f2cf1b2d82cc217915e99 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 9 Oct 2022 09:28:18 -0700 Subject: [PATCH 12/16] Use options service directly in renderers --- addons/xterm-addon-canvas/src/CanvasRenderer.ts | 1 + addons/xterm-addon-webgl/src/WebglAddon.ts | 12 ++++++------ addons/xterm-addon-webgl/src/WebglRenderer.ts | 4 +++- src/browser/renderer/dom/DomRenderer.ts | 1 + src/browser/renderer/shared/Types.d.ts | 1 - src/browser/services/RenderService.ts | 1 - 6 files changed, 11 insertions(+), 9 deletions(-) diff --git a/addons/xterm-addon-canvas/src/CanvasRenderer.ts b/addons/xterm-addon-canvas/src/CanvasRenderer.ts index ff92e94b..d6f2f481 100644 --- a/addons/xterm-addon-canvas/src/CanvasRenderer.ts +++ b/addons/xterm-addon-canvas/src/CanvasRenderer.ts @@ -70,6 +70,7 @@ export class CanvasRenderer extends Disposable implements IRenderer { this.register(observeDevicePixelDimensions(this._renderLayers[0].canvas, this._coreBrowserService.window, (w, h) => this._setCanvasDevicePixelDimensions(w, h))); this.handleOptionsChanged(); + this.register(this._optionsService.onOptionChange(() => this.handleOptionsChanged())); this.register(toDisposable(() => { for (const l of this._renderLayers) { diff --git a/addons/xterm-addon-webgl/src/WebglAddon.ts b/addons/xterm-addon-webgl/src/WebglAddon.ts index 71487315..295db320 100644 --- a/addons/xterm-addon-webgl/src/WebglAddon.ts +++ b/addons/xterm-addon-webgl/src/WebglAddon.ts @@ -3,14 +3,13 @@ * @license MIT */ -import { Terminal, ITerminalAddon, IEvent } from 'xterm'; -import { WebglRenderer } from './WebglRenderer'; import { ICharacterJoinerService, ICoreBrowserService, IRenderService, IThemeService } from 'browser/services/Services'; -import { IColorSet } from 'browser/Types'; import { EventEmitter, forwardEvent } from 'common/EventEmitter'; -import { isSafari } from 'common/Platform'; -import { ICoreService, IDecorationService } from 'common/services/Services'; import { Disposable, toDisposable } from 'common/Lifecycle'; +import { isSafari } from 'common/Platform'; +import { ICoreService, IDecorationService, IOptionsService } from 'common/services/Services'; +import { ITerminalAddon, Terminal } from 'xterm'; +import { WebglRenderer } from './WebglRenderer'; export class WebglAddon extends Disposable implements ITerminalAddon { private _terminal?: Terminal; @@ -43,7 +42,8 @@ export class WebglAddon extends Disposable implements ITerminalAddon { const coreService: ICoreService = core.coreService; const decorationService: IDecorationService = core._decorationService; const themeService: IThemeService = core._themeService; - this._renderer = this.register(new WebglRenderer(terminal, themeService, characterJoinerService, coreBrowserService, coreService, decorationService, this._preserveDrawingBuffer)); + const optionsService: IOptionsService = core.optionsService; + this._renderer = this.register(new WebglRenderer(terminal, themeService, characterJoinerService, coreBrowserService, optionsService, coreService, decorationService, this._preserveDrawingBuffer)); this.register(forwardEvent(this._renderer.onContextLoss, this._onContextLoss)); this.register(forwardEvent(this._renderer.onChangeTextureAtlas, this._onChangeTextureAtlas)); renderService.setRenderer(this._renderer); diff --git a/addons/xterm-addon-webgl/src/WebglRenderer.ts b/addons/xterm-addon-webgl/src/WebglRenderer.ts index ed99a225..41cfa7e4 100644 --- a/addons/xterm-addon-webgl/src/WebglRenderer.ts +++ b/addons/xterm-addon-webgl/src/WebglRenderer.ts @@ -15,7 +15,7 @@ import { CellData } from 'common/buffer/CellData'; import { Content, NULL_CELL_CHAR, NULL_CELL_CODE } from 'common/buffer/Constants'; import { EventEmitter } from 'common/EventEmitter'; import { Disposable, toDisposable } from 'common/Lifecycle'; -import { ICoreService, IDecorationService } from 'common/services/Services'; +import { ICoreService, IDecorationService, IOptionsService } from 'common/services/Services'; import { CharData, IBufferLine, ICellData } from 'common/Types'; import { Terminal } from 'xterm'; import { GlyphRenderer } from './GlyphRenderer'; @@ -58,6 +58,7 @@ export class WebglRenderer extends Disposable implements IRenderer { private readonly _themeService: IThemeService, private readonly _characterJoinerService: ICharacterJoinerService, private readonly _coreBrowserService: ICoreBrowserService, + optionsService: IOptionsService, coreService: ICoreService, private readonly _decorationService: IDecorationService, preserveDrawingBuffer?: boolean @@ -90,6 +91,7 @@ export class WebglRenderer extends Disposable implements IRenderer { }; this._devicePixelRatio = this._coreBrowserService.dpr; this._updateDimensions(); + this.register(optionsService.onOptionChange(() => this.handleOptionsChanged())); this._canvas = document.createElement('canvas'); diff --git a/src/browser/renderer/dom/DomRenderer.ts b/src/browser/renderer/dom/DomRenderer.ts index 5d2729c0..a5ef7c78 100644 --- a/src/browser/renderer/dom/DomRenderer.ts +++ b/src/browser/renderer/dom/DomRenderer.ts @@ -79,6 +79,7 @@ export class DomRenderer extends Disposable implements IRenderer { actualCellHeight: 0 }; this._updateDimensions(); + this.register(this._optionsService.onOptionChange(() => this.handleOptionsChanged())); this.register(themeService.onChangeColors(e => this._injectCss(e))); this._injectCss(themeService.colors); diff --git a/src/browser/renderer/shared/Types.d.ts b/src/browser/renderer/shared/Types.d.ts index 1def0f77..61a90890 100644 --- a/src/browser/renderer/shared/Types.d.ts +++ b/src/browser/renderer/shared/Types.d.ts @@ -68,7 +68,6 @@ export interface IRenderer extends IDisposable { handleFocus(): void; handleSelectionChanged(start: [number, number] | undefined, end: [number, number] | undefined, columnSelectMode: boolean): void; handleCursorMove(): void; - handleOptionsChanged(): void; clear(): void; renderRows(start: number, end: number): void; clearTextureAtlas?(): void; diff --git a/src/browser/services/RenderService.ts b/src/browser/services/RenderService.ts index 1e1943df..190967db 100644 --- a/src/browser/services/RenderService.ts +++ b/src/browser/services/RenderService.ts @@ -177,7 +177,6 @@ export class RenderService extends Disposable implements IRenderService { if (!this._renderer) { return; } - this._renderer.handleOptionsChanged(); this.refreshRows(0, this._rowCount - 1); this._fireOnCanvasResize(); } From 8d7137417bc8200cab8bfb6eef9038dcddce7700 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 9 Oct 2022 10:12:19 -0700 Subject: [PATCH 13/16] Split renderer safe vs unsafe api on activate --- addons/xterm-addon-canvas/src/CanvasAddon.ts | 26 +++++++++++--------- addons/xterm-addon-webgl/src/WebglAddon.ts | 21 ++++++++++------ 2 files changed, 28 insertions(+), 19 deletions(-) diff --git a/addons/xterm-addon-canvas/src/CanvasAddon.ts b/addons/xterm-addon-canvas/src/CanvasAddon.ts index 1dc607e5..74248f1c 100644 --- a/addons/xterm-addon-canvas/src/CanvasAddon.ts +++ b/addons/xterm-addon-canvas/src/CanvasAddon.ts @@ -4,7 +4,7 @@ */ import { ICharacterJoinerService, ICharSizeService, ICoreBrowserService, IRenderService, IThemeService } from 'browser/services/Services'; -import { IColorSet } from 'browser/Types'; +import { IColorSet, ITerminal } from 'browser/Types'; import { CanvasRenderer } from './CanvasRenderer'; import { IBufferService, ICoreService, IDecorationService, IOptionsService } from 'common/services/Services'; import { ITerminalAddon, Terminal } from 'xterm'; @@ -23,25 +23,27 @@ export class CanvasAddon extends Disposable implements ITerminalAddon { } public activate(terminal: Terminal): void { - const core = (terminal as any)._core; + const core = (terminal as any)._core as ITerminal; + const unsafeCore = core as any; if (!terminal.element) { this.register(core.onWillOpen(() => this.activate(terminal))); return; } this._terminal = terminal; - const bufferService: IBufferService = core._bufferService; - const renderService: IRenderService = core._renderService; - const characterJoinerService: ICharacterJoinerService = core._characterJoinerService; - const charSizeService: ICharSizeService = core._charSizeService; - const coreService: ICoreService = core.coreService; - const coreBrowserService: ICoreBrowserService = core._coreBrowserService; - const decorationService: IDecorationService = core._decorationService; - const optionsService: IOptionsService = core.optionsService; - const themeService: IThemeService = core._themeService; - const screenElement: HTMLElement = core.screenElement; + const coreService = core.coreService; + const optionsService = core.optionsService; + const screenElement = core.screenElement!; const linkifier = core.linkifier2; + const bufferService: IBufferService = unsafeCore._bufferService; + const renderService: IRenderService = unsafeCore._renderService; + const characterJoinerService: ICharacterJoinerService = unsafeCore._characterJoinerService; + const charSizeService: ICharSizeService = unsafeCore._charSizeService; + const coreBrowserService: ICoreBrowserService = unsafeCore._coreBrowserService; + const decorationService: IDecorationService = unsafeCore._decorationService; + const themeService: IThemeService = unsafeCore._themeService; + this._renderer = new CanvasRenderer(terminal, screenElement, linkifier, bufferService, charSizeService, optionsService, characterJoinerService, coreService, coreBrowserService, decorationService, themeService); this.register(forwardEvent(this._renderer.onChangeTextureAtlas, this._onChangeTextureAtlas)); renderService.setRenderer(this._renderer); diff --git a/addons/xterm-addon-webgl/src/WebglAddon.ts b/addons/xterm-addon-webgl/src/WebglAddon.ts index 295db320..149a93d7 100644 --- a/addons/xterm-addon-webgl/src/WebglAddon.ts +++ b/addons/xterm-addon-webgl/src/WebglAddon.ts @@ -4,10 +4,12 @@ */ import { ICharacterJoinerService, ICoreBrowserService, IRenderService, IThemeService } from 'browser/services/Services'; +import { ITerminal } from 'browser/Types'; import { EventEmitter, forwardEvent } from 'common/EventEmitter'; import { Disposable, toDisposable } from 'common/Lifecycle'; import { isSafari } from 'common/Platform'; import { ICoreService, IDecorationService, IOptionsService } from 'common/services/Services'; +import { ICoreTerminal } from 'common/Types'; import { ITerminalAddon, Terminal } from 'xterm'; import { WebglRenderer } from './WebglRenderer'; @@ -30,19 +32,24 @@ export class WebglAddon extends Disposable implements ITerminalAddon { if (isSafari) { throw new Error('Webgl is not currently supported on Safari'); } - const core = (terminal as any)._core; + + const core = (terminal as any)._core as ITerminal; + const unsafeCore = core as any; if (!terminal.element) { - this.register(core.onWillOpen(() => this.activate(terminal))); + this.register(unsafeCore.onWillOpen(() => this.activate(terminal))); return; } + this._terminal = terminal; - const renderService: IRenderService = core._renderService; - const characterJoinerService: ICharacterJoinerService = core._characterJoinerService; - const coreBrowserService: ICoreBrowserService = core._coreBrowserService; const coreService: ICoreService = core.coreService; - const decorationService: IDecorationService = core._decorationService; - const themeService: IThemeService = core._themeService; const optionsService: IOptionsService = core.optionsService; + + const renderService: IRenderService = unsafeCore._renderService; + const characterJoinerService: ICharacterJoinerService = unsafeCore._characterJoinerService; + const coreBrowserService: ICoreBrowserService = unsafeCore._coreBrowserService; + const decorationService: IDecorationService = unsafeCore._decorationService; + const themeService: IThemeService = unsafeCore._themeService; + this._renderer = this.register(new WebglRenderer(terminal, themeService, characterJoinerService, coreBrowserService, optionsService, coreService, decorationService, this._preserveDrawingBuffer)); this.register(forwardEvent(this._renderer.onContextLoss, this._onContextLoss)); this.register(forwardEvent(this._renderer.onChangeTextureAtlas, this._onChangeTextureAtlas)); From a37ea14d10140c952f2749840e719d4511cd8c9e Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 9 Oct 2022 10:57:49 -0700 Subject: [PATCH 14/16] Use options service in renderers --- addons/xterm-addon-canvas/src/BaseRenderLayer.ts | 1 - addons/xterm-addon-canvas/src/CanvasAddon.ts | 2 +- addons/xterm-addon-canvas/src/CanvasRenderer.ts | 8 -------- addons/xterm-addon-canvas/src/CursorRenderLayer.ts | 5 +++-- addons/xterm-addon-canvas/src/TextRenderLayer.ts | 5 +---- addons/xterm-addon-canvas/src/Types.d.ts | 5 ----- addons/xterm-addon-webgl/src/WebglAddon.ts | 4 ++-- addons/xterm-addon-webgl/src/WebglRenderer.ts | 9 +++------ .../src/renderLayer/BaseRenderLayer.ts | 1 - .../src/renderLayer/CursorRenderLayer.ts | 12 +++++++----- .../src/renderLayer/LinkRenderLayer.ts | 2 +- addons/xterm-addon-webgl/src/renderLayer/Types.ts | 6 ------ src/browser/renderer/dom/DomRenderer.ts | 4 ++-- 13 files changed, 20 insertions(+), 44 deletions(-) diff --git a/addons/xterm-addon-canvas/src/BaseRenderLayer.ts b/addons/xterm-addon-canvas/src/BaseRenderLayer.ts index 54fea5f2..8f203400 100644 --- a/addons/xterm-addon-canvas/src/BaseRenderLayer.ts +++ b/addons/xterm-addon-canvas/src/BaseRenderLayer.ts @@ -79,7 +79,6 @@ export abstract class BaseRenderLayer extends Disposable implements IRenderLayer } } - public handleOptionsChanged(): void {} public handleBlur(): void {} public handleFocus(): void {} public handleCursorMove(): void {} diff --git a/addons/xterm-addon-canvas/src/CanvasAddon.ts b/addons/xterm-addon-canvas/src/CanvasAddon.ts index 74248f1c..e39b56f1 100644 --- a/addons/xterm-addon-canvas/src/CanvasAddon.ts +++ b/addons/xterm-addon-canvas/src/CanvasAddon.ts @@ -24,7 +24,6 @@ export class CanvasAddon extends Disposable implements ITerminalAddon { public activate(terminal: Terminal): void { const core = (terminal as any)._core as ITerminal; - const unsafeCore = core as any; if (!terminal.element) { this.register(core.onWillOpen(() => this.activate(terminal))); return; @@ -36,6 +35,7 @@ export class CanvasAddon extends Disposable implements ITerminalAddon { const screenElement = core.screenElement!; const linkifier = core.linkifier2; + const unsafeCore = core as any; const bufferService: IBufferService = unsafeCore._bufferService; const renderService: IRenderService = unsafeCore._renderService; const characterJoinerService: ICharacterJoinerService = unsafeCore._characterJoinerService; diff --git a/addons/xterm-addon-canvas/src/CanvasRenderer.ts b/addons/xterm-addon-canvas/src/CanvasRenderer.ts index d6f2f481..b657270f 100644 --- a/addons/xterm-addon-canvas/src/CanvasRenderer.ts +++ b/addons/xterm-addon-canvas/src/CanvasRenderer.ts @@ -68,10 +68,6 @@ export class CanvasRenderer extends Disposable implements IRenderer { this._updateDimensions(); this.register(observeDevicePixelDimensions(this._renderLayers[0].canvas, this._coreBrowserService.window, (w, h) => this._setCanvasDevicePixelDimensions(w, h))); - - this.handleOptionsChanged(); - this.register(this._optionsService.onOptionChange(() => this.handleOptionsChanged())); - this.register(toDisposable(() => { for (const l of this._renderLayers) { l.dispose(); @@ -131,10 +127,6 @@ export class CanvasRenderer extends Disposable implements IRenderer { this._runOperation(l => l.handleCursorMove()); } - public handleOptionsChanged(): void { - this._runOperation(l => l.handleOptionsChanged()); - } - public clear(): void { this._runOperation(l => l.reset()); } diff --git a/addons/xterm-addon-canvas/src/CursorRenderLayer.ts b/addons/xterm-addon-canvas/src/CursorRenderLayer.ts index ab8b1e66..83806697 100644 --- a/addons/xterm-addon-canvas/src/CursorRenderLayer.ts +++ b/addons/xterm-addon-canvas/src/CursorRenderLayer.ts @@ -58,6 +58,7 @@ export class CursorRenderLayer extends BaseRenderLayer { 'block': this._renderBlockCursor.bind(this), 'underline': this._renderUnderlineCursor.bind(this) }; + this.register(optionsService.onOptionChange(() => this._handleOptionsChanged())); this.register(toDisposable(() => { this._cursorBlinkStateManager?.dispose(); this._cursorBlinkStateManager = undefined; @@ -79,7 +80,7 @@ export class CursorRenderLayer extends BaseRenderLayer { public reset(): void { this._clearCursor(); this._cursorBlinkStateManager?.restartBlinkAnimation(); - this.handleOptionsChanged(); + this._handleOptionsChanged(); } public handleBlur(): void { @@ -92,7 +93,7 @@ export class CursorRenderLayer extends BaseRenderLayer { this._onRequestRedraw.fire({ start: this._bufferService.buffer.y, end: this._bufferService.buffer.y }); } - public handleOptionsChanged(): void { + private _handleOptionsChanged(): void { if (this._optionsService.rawOptions.cursorBlink) { if (!this._cursorBlinkStateManager) { this._cursorBlinkStateManager = new CursorBlinkStateManager(this._coreBrowserService.isFocused, () => { diff --git a/addons/xterm-addon-canvas/src/TextRenderLayer.ts b/addons/xterm-addon-canvas/src/TextRenderLayer.ts index ca9eae56..e2a35751 100644 --- a/addons/xterm-addon-canvas/src/TextRenderLayer.ts +++ b/addons/xterm-addon-canvas/src/TextRenderLayer.ts @@ -45,6 +45,7 @@ export class TextRenderLayer extends BaseRenderLayer { ) { super(terminal, container, 'text', zIndex, alpha, themeService, bufferService, optionsService, decorationService, coreBrowserService); this._state = new GridCache(); + this.register(optionsService.onSpecificOptionChange('allowTransparency', value => this._setTransparency(value))); } public resize(dim: IRenderDimensions): void { @@ -251,10 +252,6 @@ export class TextRenderLayer extends BaseRenderLayer { this._drawForeground(firstRow, lastRow); } - public handleOptionsChanged(): void { - this._setTransparency(this._optionsService.rawOptions.allowTransparency); - } - /** * Whether a character is overlapping to the next cell. */ diff --git a/addons/xterm-addon-canvas/src/Types.d.ts b/addons/xterm-addon-canvas/src/Types.d.ts index 1840284f..753bd127 100644 --- a/addons/xterm-addon-canvas/src/Types.d.ts +++ b/addons/xterm-addon-canvas/src/Types.d.ts @@ -73,11 +73,6 @@ export interface IRenderLayer extends IDisposable { */ handleCursorMove(): void; - /** - * Called when options change. - */ - handleOptionsChanged(): void; - /** * Called when the data in the grid has changed (or needs to be rendered * again). diff --git a/addons/xterm-addon-webgl/src/WebglAddon.ts b/addons/xterm-addon-webgl/src/WebglAddon.ts index 149a93d7..1f8e2616 100644 --- a/addons/xterm-addon-webgl/src/WebglAddon.ts +++ b/addons/xterm-addon-webgl/src/WebglAddon.ts @@ -34,9 +34,8 @@ export class WebglAddon extends Disposable implements ITerminalAddon { } const core = (terminal as any)._core as ITerminal; - const unsafeCore = core as any; if (!terminal.element) { - this.register(unsafeCore.onWillOpen(() => this.activate(terminal))); + this.register(core.onWillOpen(() => this.activate(terminal))); return; } @@ -44,6 +43,7 @@ export class WebglAddon extends Disposable implements ITerminalAddon { const coreService: ICoreService = core.coreService; const optionsService: IOptionsService = core.optionsService; + const unsafeCore = core as any; const renderService: IRenderService = unsafeCore._renderService; const characterJoinerService: ICharacterJoinerService = unsafeCore._characterJoinerService; const coreBrowserService: ICoreBrowserService = unsafeCore._coreBrowserService; diff --git a/addons/xterm-addon-webgl/src/WebglRenderer.ts b/addons/xterm-addon-webgl/src/WebglRenderer.ts index 41cfa7e4..fddeb99c 100644 --- a/addons/xterm-addon-webgl/src/WebglRenderer.ts +++ b/addons/xterm-addon-webgl/src/WebglRenderer.ts @@ -73,7 +73,7 @@ export class WebglRenderer extends Disposable implements IRenderer { this._renderLayers = [ new LinkRenderLayer(this._core.screenElement!, 2, this._terminal, this._core.linkifier2, this._coreBrowserService, this._themeService), - new CursorRenderLayer(_terminal, this._core.screenElement!, 3, this._onRequestRedraw, this._coreBrowserService, coreService, this._themeService) + new CursorRenderLayer(_terminal, this._core.screenElement!, 3, this._onRequestRedraw, this._coreBrowserService, coreService, this._themeService, optionsService) ]; this.dimensions = { scaledCharWidth: 0, @@ -91,7 +91,7 @@ export class WebglRenderer extends Disposable implements IRenderer { }; this._devicePixelRatio = this._coreBrowserService.dpr; this._updateDimensions(); - this.register(optionsService.onOptionChange(() => this.handleOptionsChanged())); + this.register(optionsService.onOptionChange(() => this._handleOptionsChanged())); this._canvas = document.createElement('canvas'); @@ -232,10 +232,7 @@ export class WebglRenderer extends Disposable implements IRenderer { } } - public handleOptionsChanged(): void { - for (const l of this._renderLayers) { - l.handleOptionsChanged(this._terminal); - } + private _handleOptionsChanged(): void { this._updateDimensions(); this._refreshCharAtlas(); } diff --git a/addons/xterm-addon-webgl/src/renderLayer/BaseRenderLayer.ts b/addons/xterm-addon-webgl/src/renderLayer/BaseRenderLayer.ts index e99a1464..8ed12d86 100644 --- a/addons/xterm-addon-webgl/src/renderLayer/BaseRenderLayer.ts +++ b/addons/xterm-addon-webgl/src/renderLayer/BaseRenderLayer.ts @@ -59,7 +59,6 @@ export abstract class BaseRenderLayer extends Disposable implements IRenderLayer } } - public handleOptionsChanged(terminal: Terminal): void {} public handleBlur(terminal: Terminal): void {} public handleFocus(terminal: Terminal): void {} public handleCursorMove(terminal: Terminal): void {} diff --git a/addons/xterm-addon-webgl/src/renderLayer/CursorRenderLayer.ts b/addons/xterm-addon-webgl/src/renderLayer/CursorRenderLayer.ts index 74801f4e..a6325dcb 100644 --- a/addons/xterm-addon-webgl/src/renderLayer/CursorRenderLayer.ts +++ b/addons/xterm-addon-webgl/src/renderLayer/CursorRenderLayer.ts @@ -11,7 +11,7 @@ import { IColorSet, ReadonlyColorSet } from 'browser/Types'; import { IRenderDimensions, IRequestRedrawEvent } from 'browser/renderer/shared/Types'; import { IEventEmitter } from 'common/EventEmitter'; import { ICoreBrowserService, IThemeService } from 'browser/services/Services'; -import { ICoreService } from 'common/services/Services'; +import { ICoreService, IOptionsService } from 'common/services/Services'; import { toDisposable } from 'common/Lifecycle'; interface ICursorState { @@ -40,7 +40,8 @@ export class CursorRenderLayer extends BaseRenderLayer { private _onRequestRefreshRowsEvent: IEventEmitter, coreBrowserService: ICoreBrowserService, private readonly _coreService: ICoreService, - themeService: IThemeService + themeService: IThemeService, + optionsService: IOptionsService ) { super(terminal, container, 'cursor', zIndex, true, coreBrowserService, themeService); this._state = { @@ -55,7 +56,8 @@ export class CursorRenderLayer extends BaseRenderLayer { 'block': this._renderBlockCursor.bind(this), 'underline': this._renderUnderlineCursor.bind(this) }; - this.handleOptionsChanged(terminal); + this._handleOptionsChanged(terminal); + this.register(optionsService.onOptionChange(() => this._handleOptionsChanged(terminal))); this.register(toDisposable(() => { this._cursorBlinkStateManager?.dispose(); this._cursorBlinkStateManager = undefined; @@ -77,7 +79,7 @@ export class CursorRenderLayer extends BaseRenderLayer { public reset(terminal: Terminal): void { this._clearCursor(); this._cursorBlinkStateManager?.restartBlinkAnimation(terminal); - this.handleOptionsChanged(terminal); + this._handleOptionsChanged(terminal); } public handleBlur(terminal: Terminal): void { @@ -90,7 +92,7 @@ export class CursorRenderLayer extends BaseRenderLayer { this._onRequestRefreshRowsEvent.fire({ start: terminal.buffer.active.cursorY, end: terminal.buffer.active.cursorY }); } - public handleOptionsChanged(terminal: Terminal): void { + private _handleOptionsChanged(terminal: Terminal): void { if (terminal.options.cursorBlink) { if (!this._cursorBlinkStateManager) { this._cursorBlinkStateManager = new CursorBlinkStateManager(() => { diff --git a/addons/xterm-addon-webgl/src/renderLayer/LinkRenderLayer.ts b/addons/xterm-addon-webgl/src/renderLayer/LinkRenderLayer.ts index 2d2af192..77b02420 100644 --- a/addons/xterm-addon-webgl/src/renderLayer/LinkRenderLayer.ts +++ b/addons/xterm-addon-webgl/src/renderLayer/LinkRenderLayer.ts @@ -7,7 +7,7 @@ import { is256Color } from 'browser/renderer/shared/CharAtlasUtils'; import { INVERTED_DEFAULT_COLOR } from 'browser/renderer/shared/Constants'; import { IRenderDimensions } from 'browser/renderer/shared/Types'; import { ICoreBrowserService, IThemeService } from 'browser/services/Services'; -import { ILinkifier2, ILinkifierEvent, ITerminal } from 'browser/Types'; +import { ILinkifier2, ILinkifierEvent } from 'browser/Types'; import { Terminal } from 'xterm'; import { BaseRenderLayer } from './BaseRenderLayer'; diff --git a/addons/xterm-addon-webgl/src/renderLayer/Types.ts b/addons/xterm-addon-webgl/src/renderLayer/Types.ts index 089680ca..bad56091 100644 --- a/addons/xterm-addon-webgl/src/renderLayer/Types.ts +++ b/addons/xterm-addon-webgl/src/renderLayer/Types.ts @@ -4,7 +4,6 @@ */ import { IDisposable, Terminal } from 'xterm'; -import { IColorSet, ReadonlyColorSet } from 'browser/Types'; import { IRenderDimensions } from 'browser/renderer/shared/Types'; export interface IRenderLayer extends IDisposable { @@ -23,11 +22,6 @@ export interface IRenderLayer extends IDisposable { */ handleCursorMove(terminal: Terminal): void; - /** - * Called when options change. - */ - handleOptionsChanged(terminal: Terminal): void; - /** * Called when the data in the grid has changed (or needs to be rendered * again). diff --git a/src/browser/renderer/dom/DomRenderer.ts b/src/browser/renderer/dom/DomRenderer.ts index a5ef7c78..02738b5b 100644 --- a/src/browser/renderer/dom/DomRenderer.ts +++ b/src/browser/renderer/dom/DomRenderer.ts @@ -79,7 +79,7 @@ export class DomRenderer extends Disposable implements IRenderer { actualCellHeight: 0 }; this._updateDimensions(); - this.register(this._optionsService.onOptionChange(() => this.handleOptionsChanged())); + this.register(this._optionsService.onOptionChange(() => this._handleOptionsChanged())); this.register(themeService.onChangeColors(e => this._injectCss(e))); this._injectCss(themeService.colors); @@ -343,7 +343,7 @@ export class DomRenderer extends Disposable implements IRenderer { // No-op, the cursor is drawn when rows are drawn } - public handleOptionsChanged(): void { + private _handleOptionsChanged(): void { // Force a refresh this._updateDimensions(); } From 28fcffe46c50bf0ed6089ea1c04b0cfcd829837c Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Wed, 12 Oct 2022 10:16:26 -0700 Subject: [PATCH 15/16] Optimize _clipImageData This makes clip image data around twice as fast, it's a little tricky to measure it though. Changes: - Work on Uint32Array instead of Uint8 so it's 1 assignment per pixel instead of 1 per channel. - Perform the clipping in-place using the original image data to avoid allocation a new buffer. Fixes #4197 --- src/browser/renderer/shared/TextureAtlas.ts | 24 ++++++++++++--------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/browser/renderer/shared/TextureAtlas.ts b/src/browser/renderer/shared/TextureAtlas.ts index d450cfdc..ed2b25f0 100644 --- a/src/browser/renderer/shared/TextureAtlas.ts +++ b/src/browser/renderer/shared/TextureAtlas.ts @@ -770,20 +770,24 @@ export class TextureAtlas implements ITextureAtlas { } private _clipImageData(imageData: ImageData, boundingBox: IBoundingBox): ImageData { + // Operate on pixels instead of channels to reduce the amount of work + const originalData = new Uint32Array(imageData.data.buffer); + + // Create a new view on the same buffer for the clipped data. The clipping operation is done in + // place to avoid allocating another buffer const width = boundingBox.right - boundingBox.left + 1; const height = boundingBox.bottom - boundingBox.top + 1; - const clippedData = new Uint8ClampedArray(width * height * 4); - for (let y = boundingBox.top; y <= boundingBox.bottom; y++) { - for (let x = boundingBox.left; x <= boundingBox.right; x++) { - const oldOffset = y * this._tmpCanvas.width * 4 + x * 4; - const newOffset = (y - boundingBox.top) * width * 4 + (x - boundingBox.left) * 4; - clippedData[newOffset] = imageData.data[oldOffset]; - clippedData[newOffset + 1] = imageData.data[oldOffset + 1]; - clippedData[newOffset + 2] = imageData.data[oldOffset + 2]; - clippedData[newOffset + 3] = imageData.data[oldOffset + 3]; + const clippedData = new Uint32Array(imageData.data.buffer, 0, width * height); + + // Perform clipping and return the result + let x = 0; + let y = 0; + for (y = boundingBox.top; y <= boundingBox.bottom; y++) { + for (x = boundingBox.left; x <= boundingBox.right; x++) { + clippedData[(y - boundingBox.top) * width + (x - boundingBox.left)] = originalData[y * imageData.width + x]; } } - return new ImageData(clippedData, width, height); + return new ImageData(new Uint8ClampedArray(clippedData.buffer, clippedData.byteOffset, clippedData.byteLength), width, height); } } From 50d1834871bb4b4d336bd8d4650f8e5779c64841 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Thu, 13 Oct 2022 06:25:13 -0700 Subject: [PATCH 16/16] Remove clipImageData completely putImageData works a little differently to drawImage which confused me for a bit, you need to offset the destination as putImageData draws the 'dirty' parts of the texture using the same source image dimensions Fixes #4197 --- src/browser/renderer/shared/TextureAtlas.ts | 28 +++++++-------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/src/browser/renderer/shared/TextureAtlas.ts b/src/browser/renderer/shared/TextureAtlas.ts index d450cfdc..162c7bdf 100644 --- a/src/browser/renderer/shared/TextureAtlas.ts +++ b/src/browser/renderer/shared/TextureAtlas.ts @@ -612,7 +612,6 @@ export class TextureAtlas implements ITextureAtlas { } const rasterizedGlyph = this._findGlyphBoundingBox(imageData, this._workBoundingBox, allowedWidth, restrictedPowerlineGlyph, customGlyph, padding); - const clippedImageData = this._clipImageData(imageData, this._workBoundingBox); // Find the best atlas row to use let activeRow: ICharAtlasActiveRow; @@ -676,7 +675,15 @@ export class TextureAtlas implements ITextureAtlas { activeRow.x += rasterizedGlyph.size.x; // putImageData doesn't do any blending, so it will overwrite any existing cache entry for us - this._cacheCtx.putImageData(clippedImageData, rasterizedGlyph.texturePosition.x, rasterizedGlyph.texturePosition.y); + this._cacheCtx.putImageData( + imageData, + rasterizedGlyph.texturePosition.x - this._workBoundingBox.left, + rasterizedGlyph.texturePosition.y - this._workBoundingBox.top, + this._workBoundingBox.left, + this._workBoundingBox.top, + rasterizedGlyph.size.x, + rasterizedGlyph.size.y + ); return rasterizedGlyph; } @@ -768,23 +775,6 @@ export class TextureAtlas implements ITextureAtlas { } }; } - - private _clipImageData(imageData: ImageData, boundingBox: IBoundingBox): ImageData { - const width = boundingBox.right - boundingBox.left + 1; - const height = boundingBox.bottom - boundingBox.top + 1; - const clippedData = new Uint8ClampedArray(width * height * 4); - for (let y = boundingBox.top; y <= boundingBox.bottom; y++) { - for (let x = boundingBox.left; x <= boundingBox.right; x++) { - const oldOffset = y * this._tmpCanvas.width * 4 + x * 4; - const newOffset = (y - boundingBox.top) * width * 4 + (x - boundingBox.left) * 4; - clippedData[newOffset] = imageData.data[oldOffset]; - clippedData[newOffset + 1] = imageData.data[oldOffset + 1]; - clippedData[newOffset + 2] = imageData.data[oldOffset + 2]; - clippedData[newOffset + 3] = imageData.data[oldOffset + 3]; - } - } - return new ImageData(clippedData, width, height); - } } /**