From 14b02b9ad5d73450186b07be5e3e5bfc0982888b Mon Sep 17 00:00:00 2001 From: Svante Boberg Date: Mon, 1 Aug 2022 10:17:10 +0200 Subject: [PATCH 01/55] Bump minimum target to es2015 Now that we no longer support IE we can raise the minimum ES target a bit. Changing to es2015 shaves off ~17% from the main bundle size, also it prevents generators from being transpiled into regular functions etc. so I would guess it has the potential to give some general performance improvements too. For fun I also tried to raise it to es2022, this gave a slightly bigger bundle than es2015 so I think es2015 could be an ok target unless we need something newer for some reason. --- addons/xterm-addon-attach/src/tsconfig.json | 2 +- addons/xterm-addon-attach/test/tsconfig.json | 2 +- addons/xterm-addon-canvas/src/tsconfig.json | 2 +- addons/xterm-addon-fit/src/tsconfig.json | 2 +- addons/xterm-addon-fit/test/tsconfig.json | 2 +- addons/xterm-addon-search/src/tsconfig.json | 2 +- addons/xterm-addon-search/test/tsconfig.json | 2 +- addons/xterm-addon-serialize/src/tsconfig.json | 2 +- addons/xterm-addon-serialize/test/tsconfig.json | 2 +- addons/xterm-addon-web-links/src/tsconfig.json | 2 +- addons/xterm-addon-web-links/test/tsconfig.json | 2 +- addons/xterm-addon-webgl/src/tsconfig.json | 2 +- addons/xterm-addon-webgl/test/tsconfig.json | 2 +- src/tsconfig-base.json | 4 ++-- 14 files changed, 15 insertions(+), 15 deletions(-) diff --git a/addons/xterm-addon-attach/src/tsconfig.json b/addons/xterm-addon-attach/src/tsconfig.json index 54bfc38b..f3e409d1 100644 --- a/addons/xterm-addon-attach/src/tsconfig.json +++ b/addons/xterm-addon-attach/src/tsconfig.json @@ -1,7 +1,7 @@ { "compilerOptions": { "module": "commonjs", - "target": "es5", + "target": "es2015", "lib": [ "dom", "es2015" diff --git a/addons/xterm-addon-attach/test/tsconfig.json b/addons/xterm-addon-attach/test/tsconfig.json index 1c772984..fa67b169 100644 --- a/addons/xterm-addon-attach/test/tsconfig.json +++ b/addons/xterm-addon-attach/test/tsconfig.json @@ -1,7 +1,7 @@ { "compilerOptions": { "module": "commonjs", - "target": "es5", + "target": "es2015", "lib": [ "es2015" ], diff --git a/addons/xterm-addon-canvas/src/tsconfig.json b/addons/xterm-addon-canvas/src/tsconfig.json index 206d52ae..f752dc5b 100644 --- a/addons/xterm-addon-canvas/src/tsconfig.json +++ b/addons/xterm-addon-canvas/src/tsconfig.json @@ -1,7 +1,7 @@ { "compilerOptions": { "module": "commonjs", - "target": "es5", + "target": "es2015", "lib": [ "dom", "es6" diff --git a/addons/xterm-addon-fit/src/tsconfig.json b/addons/xterm-addon-fit/src/tsconfig.json index 54bfc38b..f3e409d1 100644 --- a/addons/xterm-addon-fit/src/tsconfig.json +++ b/addons/xterm-addon-fit/src/tsconfig.json @@ -1,7 +1,7 @@ { "compilerOptions": { "module": "commonjs", - "target": "es5", + "target": "es2015", "lib": [ "dom", "es2015" diff --git a/addons/xterm-addon-fit/test/tsconfig.json b/addons/xterm-addon-fit/test/tsconfig.json index 1c772984..fa67b169 100644 --- a/addons/xterm-addon-fit/test/tsconfig.json +++ b/addons/xterm-addon-fit/test/tsconfig.json @@ -1,7 +1,7 @@ { "compilerOptions": { "module": "commonjs", - "target": "es5", + "target": "es2015", "lib": [ "es2015" ], diff --git a/addons/xterm-addon-search/src/tsconfig.json b/addons/xterm-addon-search/src/tsconfig.json index 5a5e671f..9ef27b5c 100644 --- a/addons/xterm-addon-search/src/tsconfig.json +++ b/addons/xterm-addon-search/src/tsconfig.json @@ -1,7 +1,7 @@ { "compilerOptions": { "module": "commonjs", - "target": "es5", + "target": "es2015", "lib": [ "dom", "es6", diff --git a/addons/xterm-addon-search/test/tsconfig.json b/addons/xterm-addon-search/test/tsconfig.json index df82a457..052fac43 100644 --- a/addons/xterm-addon-search/test/tsconfig.json +++ b/addons/xterm-addon-search/test/tsconfig.json @@ -1,7 +1,7 @@ { "compilerOptions": { "module": "commonjs", - "target": "es5", + "target": "es2015", "lib": [ "es6", ], diff --git a/addons/xterm-addon-serialize/src/tsconfig.json b/addons/xterm-addon-serialize/src/tsconfig.json index 38ef6a7f..ba26f22e 100644 --- a/addons/xterm-addon-serialize/src/tsconfig.json +++ b/addons/xterm-addon-serialize/src/tsconfig.json @@ -1,7 +1,7 @@ { "compilerOptions": { "module": "commonjs", - "target": "es5", + "target": "es2015", "lib": [ "dom", "es2015" diff --git a/addons/xterm-addon-serialize/test/tsconfig.json b/addons/xterm-addon-serialize/test/tsconfig.json index 7afbc858..971f3e92 100644 --- a/addons/xterm-addon-serialize/test/tsconfig.json +++ b/addons/xterm-addon-serialize/test/tsconfig.json @@ -1,7 +1,7 @@ { "compilerOptions": { "module": "commonjs", - "target": "es5", + "target": "es2015", "lib": [ "es2015" ], diff --git a/addons/xterm-addon-web-links/src/tsconfig.json b/addons/xterm-addon-web-links/src/tsconfig.json index 54bfc38b..f3e409d1 100644 --- a/addons/xterm-addon-web-links/src/tsconfig.json +++ b/addons/xterm-addon-web-links/src/tsconfig.json @@ -1,7 +1,7 @@ { "compilerOptions": { "module": "commonjs", - "target": "es5", + "target": "es2015", "lib": [ "dom", "es2015" diff --git a/addons/xterm-addon-web-links/test/tsconfig.json b/addons/xterm-addon-web-links/test/tsconfig.json index 9f4d23df..48975764 100644 --- a/addons/xterm-addon-web-links/test/tsconfig.json +++ b/addons/xterm-addon-web-links/test/tsconfig.json @@ -1,7 +1,7 @@ { "compilerOptions": { "module": "commonjs", - "target": "es5", + "target": "es2015", "lib": [ "es2015" ], diff --git a/addons/xterm-addon-webgl/src/tsconfig.json b/addons/xterm-addon-webgl/src/tsconfig.json index 206d52ae..f752dc5b 100644 --- a/addons/xterm-addon-webgl/src/tsconfig.json +++ b/addons/xterm-addon-webgl/src/tsconfig.json @@ -1,7 +1,7 @@ { "compilerOptions": { "module": "commonjs", - "target": "es5", + "target": "es2015", "lib": [ "dom", "es6" diff --git a/addons/xterm-addon-webgl/test/tsconfig.json b/addons/xterm-addon-webgl/test/tsconfig.json index 9c7f602f..1ff9217e 100644 --- a/addons/xterm-addon-webgl/test/tsconfig.json +++ b/addons/xterm-addon-webgl/test/tsconfig.json @@ -1,7 +1,7 @@ { "compilerOptions": { "module": "commonjs", - "target": "es5", + "target": "es2015", "lib": [ "es6", ], diff --git a/src/tsconfig-base.json b/src/tsconfig-base.json index 0cd951a7..cf4454a4 100644 --- a/src/tsconfig-base.json +++ b/src/tsconfig-base.json @@ -1,7 +1,7 @@ { "compilerOptions": { - "target": "es5", - "lib": [ "es5" ], + "module": "commonjs", + "target": "es2015", "rootDir": ".", "sourceMap": true, From 91d46dae509b1940404dc9973760844c027e9674 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 7 Aug 2022 05:08:13 -0700 Subject: [PATCH 02/55] OSC link progress --- .../src/atlas/WebglCharAtlas.ts | 2 +- src/browser/OscLinkProvider.ts | 72 +++++++++++++++++++ src/browser/Terminal.ts | 2 + src/common/CoreTerminal.ts | 6 +- src/common/InputHandler.ts | 71 +++++++++++++++++- src/common/Types.d.ts | 15 +++- src/common/buffer/AttributeData.ts | 48 ++++++++++--- src/common/services/OscLinkService.ts | 22 ++++++ src/common/services/Services.ts | 14 +++- 9 files changed, 237 insertions(+), 15 deletions(-) create mode 100644 src/browser/OscLinkProvider.ts create mode 100644 src/common/services/OscLinkService.ts diff --git a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts index 13dceae6..182b3b3e 100644 --- a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts +++ b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts @@ -356,7 +356,7 @@ export class WebglCharAtlas implements IDisposable { private _drawToCache(codeOrChars: number | string, bg: number, fg: number, ext: number): IRasterizedGlyph { const chars = typeof codeOrChars === 'number' ? String.fromCharCode(codeOrChars) : codeOrChars; - + console.log('_drawToCache', chars, ext); this.hasCanvasChanged = true; // Allow 1 cell width per character, with a minimum of 2 (CJK), plus some padding. This is used diff --git a/src/browser/OscLinkProvider.ts b/src/browser/OscLinkProvider.ts new file mode 100644 index 00000000..c7594479 --- /dev/null +++ b/src/browser/OscLinkProvider.ts @@ -0,0 +1,72 @@ +/** + * Copyright (c) 2022 The xterm.js authors. All rights reserved. + * @license MIT + */ + +import { ILink, ILinkProvider } from 'browser/Types'; +import { CellData } from 'common/buffer/CellData'; +import { IBufferService, IOscLinkService } from 'common/services/Services'; + +export class OscLinkProvider implements ILinkProvider { + constructor( + @IBufferService private readonly _bufferService: IBufferService, + @IOscLinkService private readonly _oscLinkService: IOscLinkService + ) { + } + + public provideLinks(y: number, callback: (links: ILink[] | undefined) => void): void { + const line = this._bufferService.buffer.lines.get(y - 1); + if (!line) { + callback(undefined); + return; + } + + const result: ILink[] = []; + const cell = new CellData(); + const lineLength = line.getTrimmedLength(); + let currentLinkId = -1; + let currentStart = -1; + let finishLink = false; + for (let x = 0; x < lineLength; x++) { + if (!line.hasContent(x)) { + continue; + } + + line.loadCell(x, cell); + if (cell.extended.urlId) { + if (currentStart === -1) { + currentStart = x; + currentLinkId = cell.extended.urlId; + continue; + } else { + finishLink = cell.extended.urlId !== currentLinkId; + } + } else { + if (currentStart !== -1) { + finishLink = true; + } + } + + if (finishLink || (currentStart !== -1 && x === lineLength - 1)) { + const text = this._oscLinkService.getLinkData(currentLinkId)?.uri; + if (text) { + // OSC links always use underline and pointer decorations + result.push({ + text, + // These ranges are 1-based + range: { + start: { x: currentStart + 1, y }, + end: { x: x + 1, y } + }, + activate(e, text) { + console.log('activate!', text); + } + // TODO: Embedder API to handle hover + }); + } + } + } + // TODO: Handle fetching and returning other link ranges to underline other links with the same id + callback(result); + } +} diff --git a/src/browser/Terminal.ts b/src/browser/Terminal.ts index 57099ac2..aacd6cf7 100644 --- a/src/browser/Terminal.ts +++ b/src/browser/Terminal.ts @@ -55,6 +55,7 @@ import { BufferDecorationRenderer } from 'browser/decorations/BufferDecorationRe import { OverviewRulerRenderer } from 'browser/decorations/OverviewRulerRenderer'; import { DecorationService } from 'common/services/DecorationService'; import { IDecorationService } from 'common/services/Services'; +import { OscLinkProvider } from 'browser/OscLinkProvider'; // Let it work inside Node.js for automated testing purposes. const document: Document = (typeof window !== 'undefined') ? window.document : null as any; @@ -163,6 +164,7 @@ export class Terminal extends CoreTerminal implements ITerminal { this._setup(); this.linkifier2 = this.register(this._instantiationService.createInstance(Linkifier2)); + this.linkifier2.registerLinkProvider(this._instantiationService.createInstance(OscLinkProvider)); this._decorationService = this._instantiationService.createInstance(DecorationService); this._instantiationService.setService(IDecorationService, this._decorationService); diff --git a/src/common/CoreTerminal.ts b/src/common/CoreTerminal.ts index af9ec3f9..4a1c99ff 100644 --- a/src/common/CoreTerminal.ts +++ b/src/common/CoreTerminal.ts @@ -22,7 +22,7 @@ */ import { Disposable } from 'common/Lifecycle'; -import { IInstantiationService, IOptionsService, IBufferService, ILogService, ICharsetService, ICoreService, ICoreMouseService, IUnicodeService, IDirtyRowService, LogLevelEnum, ITerminalOptions } from 'common/services/Services'; +import { IInstantiationService, IOptionsService, IBufferService, ILogService, ICharsetService, ICoreService, ICoreMouseService, IUnicodeService, IDirtyRowService, LogLevelEnum, ITerminalOptions, IOscLinkService } from 'common/services/Services'; import { InstantiationService } from 'common/services/InstantiationService'; import { LogService } from 'common/services/LogService'; import { BufferService, MINIMUM_COLS, MINIMUM_ROWS } from 'common/services/BufferService'; @@ -39,6 +39,7 @@ import { IFunctionIdentifier, IParams } from 'common/parser/Types'; import { IBufferSet } from 'common/buffer/Types'; import { InputHandler } from 'common/InputHandler'; import { WriteBuffer } from 'common/input/WriteBuffer'; +import { OscLinkService } from 'common/services/OscLinkService'; // Only trigger this warning a single time per session let hasWriteSyncWarnHappened = false; @@ -49,6 +50,7 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal { protected readonly _logService: ILogService; protected readonly _charsetService: ICharsetService; protected readonly _dirtyRowService: IDirtyRowService; + protected readonly _oscLinkService: IOscLinkService; public readonly coreMouseService: ICoreMouseService; public readonly coreService: ICoreService; @@ -118,6 +120,8 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal { this._instantiationService.setService(IUnicodeService, this.unicodeService); this._charsetService = this._instantiationService.createInstance(CharsetService); this._instantiationService.setService(ICharsetService, this._charsetService); + this._oscLinkService = this._instantiationService.createInstance(OscLinkService); + this._instantiationService.setService(IOscLinkService, this._oscLinkService); // Register input handler and handle/forward events this._inputHandler = new InputHandler(this._bufferService, this._charsetService, this.coreService, this._dirtyRowService, this._logService, this.optionsService, this.coreMouseService, this.unicodeService); diff --git a/src/common/InputHandler.ts b/src/common/InputHandler.ts index d5b8d948..b613a24a 100644 --- a/src/common/InputHandler.ts +++ b/src/common/InputHandler.ts @@ -4,7 +4,7 @@ * @license MIT */ -import { IInputHandler, IAttributeData, IDisposable, IWindowOptions, IColorEvent, IParseStack, ColorIndex, ColorRequestType } from 'common/Types'; +import { IInputHandler, IAttributeData, IDisposable, IWindowOptions, IColorEvent, IParseStack, ColorIndex, ColorRequestType, IOscLinkData } from 'common/Types'; import { C0, C1 } from 'common/data/EscapeSequences'; import { CHARSETS, DEFAULT_CHARSET } from 'common/data/Charsets'; import { EscapeSequenceParser } from 'common/parser/EscapeSequenceParser'; @@ -214,8 +214,6 @@ class DECRQSS implements IDcsHandler { * @vt: #N DCS XTSETTCAP "Set Terminfo Data" "DCS + p Pt ST" "Set Terminfo Data." */ - - /** * The terminal's standard implementation of IInputHandler, this handles all * input from the Parser. @@ -230,6 +228,7 @@ export class InputHandler extends Disposable implements IInputHandler { private _workCell: CellData = new CellData(); private _windowTitle = ''; private _iconName = ''; + private _currentHyperlink?: IOscLinkData; protected _windowTitleStack: string[] = []; protected _iconNameStack: string[] = []; @@ -265,6 +264,10 @@ export class InputHandler extends Disposable implements IInputHandler { public get onTitleChange(): IEvent { return this._onTitleChange.event; } private _onColor = new EventEmitter(); public get onColor(): IEvent { return this._onColor.event; } + private _onStartHyperlink = new EventEmitter(); + public get onStartHyperlink(): IEvent { return this._onStartHyperlink.event; } + private _onFinishHyperlink = new EventEmitter(); + public get onFinishHyperlink(): IEvent { return this._onFinishHyperlink.event; } private _parseStack: IParseStack = { paused: false, @@ -403,6 +406,8 @@ export class InputHandler extends Disposable implements IInputHandler { // 5 - Change Special Color Number // 6 - Enable/disable Special Color Number c // 7 - current directory? (not in xterm spec, see https://gitlab.com/gnachman/iterm2/issues/3939) + // 8 - create hyperlink (not in xterm spec, see https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda) + this._parser.registerOscHandler(8, new OscHandler(data => this.setHyperlink(data))); // 10 - Change VT100 text foreground color to Pt. this._parser.registerOscHandler(10, new OscHandler(data => this.setOrReportFgColor(data))); // 11 - Change VT100 text background color to Pt. @@ -2889,6 +2894,66 @@ export class InputHandler extends Disposable implements IInputHandler { return true; } + /** + * OSC 8 ; ; ST - create hyperlink + * OSC 8 ; ; ST - finish hyperlink + * + * Test case: + * + * ```sh + * printf '\e]8;;http://example.com\e\\This is a link\e]8;;\e\\\n' + * ``` + * + * @vt: #Y OSC 8 "Create hyperlink" "OSC 8 ; params ; uri BEL" "Create a hyperlink to `uri` using `params`." + * `uri` is a hyperlink starting with `http://`, `https://`, `ftp://`, `file://` or `mailto://`. `params` is an + * optional list of key=value assignments, separated by the : character. Example: `id=xyz123:foo=bar:baz=quux`. + * Currently only the id key is defined. Cells that share the same ID and URI share hover feedback. + * Use `OSC 8 ; ; BEL` to finish the current hyperlink. + */ + public setHyperlink(data: string): boolean { + const args = data.split(';'); + console.log('hyperlink', args); + if (args.length < 2) { + return false; + } + if (args[1]) { + return this._createHyperlink(args[0], args[1]); + } + if (args[0]) { + return false; + } + return this._finishHyperlink(); + } + + private _createHyperlink(params: string, uri: string): boolean { + // It's legal to open a new hyperlink without explicitly finishing the previous one + if (this._currentHyperlink) { + this._finishHyperlink(); + } + const parsedParams = params.split(':'); + let id: string | undefined; + const idParamIndex = parsedParams.findIndex(e => e.startsWith('id=')); + if (idParamIndex !== -1) { + id = parsedParams[idParamIndex].slice(3) || undefined; + } + this._currentHyperlink = { id, uri }; + this._curAttrData.extended = this._curAttrData.extended.clone(); + this._curAttrData.extended.urlId = 1; + this._curAttrData.updateExtended(); + console.log('hasExtendedAttrs?', this._curAttrData.hasExtendedAttrs()); + this._onStartHyperlink.fire(this._currentHyperlink); + return true; + } + + private _finishHyperlink(): boolean { + this._curAttrData.extended = this._curAttrData.extended.clone(); + this._curAttrData.extended.urlId = 0; + this._curAttrData.updateExtended(); + this._onFinishHyperlink.fire(); + this._currentHyperlink = undefined; + return true; + } + // special colors - OSC 10 | 11 | 12 private _specialColors = [ColorIndex.FOREGROUND, ColorIndex.BACKGROUND, ColorIndex.CURSOR]; diff --git a/src/common/Types.d.ts b/src/common/Types.d.ts index 56815da0..129f8e1b 100644 --- a/src/common/Types.d.ts +++ b/src/common/Types.d.ts @@ -9,6 +9,7 @@ import { IDeleteEvent, IInsertEvent } from 'common/CircularList'; import { IParams } from 'common/parser/Types'; import { ICoreMouseService, ICoreService, IOptionsService, IUnicodeService } from 'common/services/Services'; import { IBufferSet } from 'common/buffer/Types'; +import { UnderlineStyle } from 'common/buffer/Constants'; export interface ICoreTerminal { coreMouseService: ICoreMouseService; @@ -114,12 +115,24 @@ export type IColorRGB = [number, number, number]; export interface IExtendedAttrs { ext: number; - underlineStyle: number; + underlineStyle: UnderlineStyle; underlineColor: number; + urlId: number; clone(): IExtendedAttrs; isEmpty(): boolean; } +/** + * Tracks the current hyperlink. Since these are treated as extended attirbutes, these get passed on + * to the linkifier when anything is printed. Doing it this way ensures that even when the cursor + * moves around unexpectedly the link is tracked, as opposed to using a start position and + * finalizing it at the end. + */ +export interface IOscLinkData { + id?: string; + uri: string; +} + /** Attribute data */ export interface IAttributeData { fg: number; diff --git a/src/common/buffer/AttributeData.ts b/src/common/buffer/AttributeData.ts index b51f7ecb..aac6a33d 100644 --- a/src/common/buffer/AttributeData.ts +++ b/src/common/buffer/AttributeData.ts @@ -35,7 +35,12 @@ export class AttributeData implements IAttributeData { // flags public isInverse(): number { return this.fg & FgFlags.INVERSE; } public isBold(): number { return this.fg & FgFlags.BOLD; } - public isUnderline(): number { return this.fg & FgFlags.UNDERLINE; } + public isUnderline(): number { + if (this.hasExtendedAttrs() && this.extended.underlineStyle !== UnderlineStyle.NONE) { + return 1; + } + return this.fg & FgFlags.UNDERLINE; + } public isBlink(): number { return this.fg & FgFlags.BLINK; } public isInvisible(): number { return this.fg & FgFlags.INVISIBLE; } public isItalic(): number { return this.bg & BgFlags.ITALIC; } @@ -128,10 +133,24 @@ export class AttributeData implements IAttributeData { */ export class ExtendedAttrs implements IExtendedAttrs { private _ext: number = 0; - public get ext(): number { return this._ext; } + public get ext(): number { + // TODO: How to handle previous underline style if link overrides it? + if (this._urlId) { + console.log('ext, has url'); + return ( + (this._ext & ~ExtFlags.UNDERLINE_STYLE) | + (this.underlineStyle << 26) + ); + } + return this._ext; + } public set ext(value: number) { this._ext = value; } public get underlineStyle(): UnderlineStyle { + // Always return the URL style if it has one + if (this._urlId) { + return UnderlineStyle.DASHED; + } return (this._ext & ExtFlags.UNDERLINE_STYLE) >> 26; } public set underlineStyle(value: UnderlineStyle) { @@ -140,6 +159,11 @@ export class ExtendedAttrs implements IExtendedAttrs { } public get underlineColor(): number { + // Always return the URL color if it has one + if (this._urlId) { + // TODO: fix + return 0; + } return this._ext & (Attributes.CM_MASK | Attributes.RGB_MASK); } public set underlineColor(value: number) { @@ -147,16 +171,24 @@ export class ExtendedAttrs implements IExtendedAttrs { this._ext |= value & (Attributes.CM_MASK | Attributes.RGB_MASK); } + private _urlId: number = 0; + public get urlId(): number { + return this._urlId; + } + public set urlId(value: number) { + this._urlId = value; + } + constructor( - underlineStyle: UnderlineStyle = UnderlineStyle.NONE, - underlineColor: number = Attributes.CM_DEFAULT + ext: number = 0, + urlId: number = 0 ) { - this.underlineStyle = underlineStyle; - this.underlineColor = underlineColor; + this._ext = ext; + this._urlId = urlId; } public clone(): IExtendedAttrs { - return new ExtendedAttrs(this.underlineStyle, this.underlineColor); + return new ExtendedAttrs(this._ext, this._urlId); } /** @@ -164,6 +196,6 @@ export class ExtendedAttrs implements IExtendedAttrs { * that needs to be persistant in the buffer. */ public isEmpty(): boolean { - return this.underlineStyle === UnderlineStyle.NONE; + return this.underlineStyle === UnderlineStyle.NONE && this._urlId === 0; } } diff --git a/src/common/services/OscLinkService.ts b/src/common/services/OscLinkService.ts new file mode 100644 index 00000000..58961c4f --- /dev/null +++ b/src/common/services/OscLinkService.ts @@ -0,0 +1,22 @@ +import { IBufferService, IOscLinkService } from 'common/services/Services'; +import { IOscLinkData } from 'common/Types'; + +export class OscLinkService implements IOscLinkService { + public serviceBrand: any; + + constructor( + @IBufferService private readonly _bufferService: IBufferService + ) { + } + + public registerLink(linkData: IOscLinkData): number { + // TODO: Add and return properly + return 1; + } + + public getLinkData(linkId: number): IOscLinkData | undefined { + return { + uri: 'https://github.com' + }; + } +} diff --git a/src/common/services/Services.ts b/src/common/services/Services.ts index 585b29ac..5f97a487 100644 --- a/src/common/services/Services.ts +++ b/src/common/services/Services.ts @@ -5,7 +5,7 @@ import { IEvent, IEventEmitter } from 'common/EventEmitter'; import { IBuffer, IBufferSet } from 'common/buffer/Types'; -import { IDecPrivateModes, ICoreMouseEvent, CoreMouseEncoding, ICoreMouseProtocol, CoreMouseEventType, ICharset, IWindowOptions, IModes, IAttributeData, ScrollSource, IDisposable, IColorRGB, IColor, CursorStyle } from 'common/Types'; +import { IDecPrivateModes, ICoreMouseEvent, CoreMouseEncoding, ICoreMouseProtocol, CoreMouseEventType, ICharset, IWindowOptions, IModes, IAttributeData, ScrollSource, IDisposable, IColorRGB, IColor, CursorStyle, IOscLinkData } from 'common/Types'; import { createDecorator } from 'common/services/ServiceRegistry'; import { IDecorationOptions, IDecoration } from 'xterm'; @@ -272,6 +272,18 @@ export interface ITheme { extendedAnsi?: string[]; } +export const IOscLinkService = createDecorator('OscLinkService'); +export interface IOscLinkService { + serviceBrand: undefined; + /** + * Registers a link to the service, returning the link ID. The link data is managed by this + * service and will be freed when this current cursor position is trimmed off the buffer. + */ + registerLink(linkData: IOscLinkData): number; + /** Get the link data associated with a link ID. */ + getLinkData(linkId: number): IOscLinkData | undefined; +} + export const IUnicodeService = createDecorator('UnicodeService'); export interface IUnicodeService { serviceBrand: undefined; From 7970b5c1e35726103ede881f5840453ad954a3e5 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 7 Aug 2022 06:05:42 -0700 Subject: [PATCH 03/55] Fix edge cases in link creation --- .../src/atlas/WebglCharAtlas.ts | 2 +- demo/client.ts | 14 +++++++++++++ demo/index.html | 1 + src/browser/OscLinkProvider.ts | 20 +++++++++++++++---- src/common/InputHandler.ts | 3 ++- 5 files changed, 34 insertions(+), 6 deletions(-) diff --git a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts index 182b3b3e..13dceae6 100644 --- a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts +++ b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts @@ -356,7 +356,7 @@ export class WebglCharAtlas implements IDisposable { private _drawToCache(codeOrChars: number | string, bg: number, fg: number, ext: number): IRasterizedGlyph { const chars = typeof codeOrChars === 'number' ? String.fromCharCode(codeOrChars) : codeOrChars; - console.log('_drawToCache', chars, ext); + this.hasCanvasChanged = true; // Allow 1 cell width per character, with a minimum of 2 (CJK), plus some padding. This is used diff --git a/demo/client.ts b/demo/client.ts index 64c694f5..8f858b39 100644 --- a/demo/client.ts +++ b/demo/client.ts @@ -189,6 +189,7 @@ if (document.location.pathname === '/test') { document.getElementById('powerline-symbol-test').addEventListener('click', powerlineSymbolTest); document.getElementById('underline-test').addEventListener('click', underlineTest); document.getElementById('ansi-colors').addEventListener('click', ansiColorsTest); + document.getElementById('osc-hyperlinks').addEventListener('click', addAnsiHyperlink); document.getElementById('add-decoration').addEventListener('click', addDecoration); document.getElementById('add-overview-ruler').addEventListener('click', addOverviewRuler); } @@ -827,6 +828,19 @@ function ansiColorsTest() { } } +function addAnsiHyperlink() { + term.write('\n\n\r'); + term.writeln(`Regular link with no id:`); + term.writeln('\x1b]8;;https://github.com\x07GitHub\x1b]8;;\x07'); + term.writeln('\x1b]8;;https://xtermjs.org\x07https://xtermjs.org\x1b]8;;\x07\x1b[C<- null cell'); + term.writeln(`\nShared ID links:`); + term.writeln('╔════╗ ╔════╗'); + term.writeln('║\x1b]8;;https://github.com\x07GitH\x1b]8;;\x07║ ║ ║'); + term.writeln('║\x1b]8;;https://github.com\x07ub\x1b]8;;\x07 ║ ║ ║'); + term.writeln('╚════╝ ╚════╝'); + term.write('\x1b[3A\x1b[8C\x1b]8;;https://xtermjs.org\x07xter\x1b[B\x1b[4Dm.js\x1b]8;;\x07\x1b[2B\x1b[12D'); +} + function addDecoration() { term.options['overviewRulerWidth'] = 15; const marker = term.registerMarker(1); diff --git a/demo/index.html b/demo/index.html index 836084b3..c38cb007 100644 --- a/demo/index.html +++ b/demo/index.html @@ -79,6 +79,7 @@
+
Decorations
diff --git a/src/browser/OscLinkProvider.ts b/src/browser/OscLinkProvider.ts index c7594479..7fee62af 100644 --- a/src/browser/OscLinkProvider.ts +++ b/src/browser/OscLinkProvider.ts @@ -28,12 +28,14 @@ export class OscLinkProvider implements ILinkProvider { let currentStart = -1; let finishLink = false; for (let x = 0; x < lineLength; x++) { - if (!line.hasContent(x)) { + // Minor optimization, only check for content if there isn't a link in case the link ends with + // a null cell + if (currentStart === -1 && !line.hasContent(x)) { continue; } line.loadCell(x, cell); - if (cell.extended.urlId) { + if (cell.hasExtendedAttrs() && cell.extended.urlId) { if (currentStart === -1) { currentStart = x; currentLinkId = cell.extended.urlId; @@ -55,8 +57,15 @@ export class OscLinkProvider implements ILinkProvider { text, // These ranges are 1-based range: { - start: { x: currentStart + 1, y }, - end: { x: x + 1, y } + start: { + x: currentStart + 1, + y + }, + end: { + // Offset end x if it's a link that ends on the last cell in the line + x: x + (!finishLink && x === lineLength - 1 ? 1 : 0), + y + } }, activate(e, text) { console.log('activate!', text); @@ -64,6 +73,9 @@ export class OscLinkProvider implements ILinkProvider { // TODO: Embedder API to handle hover }); } + currentStart = -1; + currentLinkId = -1; + finishLink = false; } } // TODO: Handle fetching and returning other link ranges to underline other links with the same id diff --git a/src/common/InputHandler.ts b/src/common/InputHandler.ts index b613a24a..e6d154b9 100644 --- a/src/common/InputHandler.ts +++ b/src/common/InputHandler.ts @@ -2937,15 +2937,16 @@ export class InputHandler extends Disposable implements IInputHandler { id = parsedParams[idParamIndex].slice(3) || undefined; } this._currentHyperlink = { id, uri }; + console.log('start hyperlink'); this._curAttrData.extended = this._curAttrData.extended.clone(); this._curAttrData.extended.urlId = 1; this._curAttrData.updateExtended(); - console.log('hasExtendedAttrs?', this._curAttrData.hasExtendedAttrs()); this._onStartHyperlink.fire(this._currentHyperlink); return true; } private _finishHyperlink(): boolean { + console.log('finish hyperlink'); this._curAttrData.extended = this._curAttrData.extended.clone(); this._curAttrData.extended.urlId = 0; this._curAttrData.updateExtended(); From fd79100d95daa54c727681a6b987df37180db7fd Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 7 Aug 2022 06:10:27 -0700 Subject: [PATCH 04/55] Fix test service injection --- src/common/CoreTerminal.ts | 2 +- src/common/InputHandler.test.ts | 18 ++++++++++++------ src/common/InputHandler.ts | 11 +++-------- src/common/TestUtils.test.ts | 14 ++++++++++++-- src/common/services/OscLinkService.ts | 1 + 5 files changed, 29 insertions(+), 17 deletions(-) diff --git a/src/common/CoreTerminal.ts b/src/common/CoreTerminal.ts index 4a1c99ff..6e318ce7 100644 --- a/src/common/CoreTerminal.ts +++ b/src/common/CoreTerminal.ts @@ -124,7 +124,7 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal { this._instantiationService.setService(IOscLinkService, this._oscLinkService); // Register input handler and handle/forward events - this._inputHandler = new InputHandler(this._bufferService, this._charsetService, this.coreService, this._dirtyRowService, this._logService, this.optionsService, this.coreMouseService, this.unicodeService); + this._inputHandler = new InputHandler(this._bufferService, this._charsetService, this.coreService, this._dirtyRowService, this._logService, this.optionsService, this._oscLinkService, this.coreMouseService, this.unicodeService); this.register(forwardEvent(this._inputHandler.onLineFeed, this._onLineFeed)); this.register(this._inputHandler); diff --git a/src/common/InputHandler.test.ts b/src/common/InputHandler.test.ts index eac41052..2fa4ac2f 100644 --- a/src/common/InputHandler.test.ts +++ b/src/common/InputHandler.test.ts @@ -11,7 +11,7 @@ import { CellData } from 'common/buffer/CellData'; import { Attributes, UnderlineStyle } from 'common/buffer/Constants'; import { AttributeData } from 'common/buffer/AttributeData'; import { Params } from 'common/parser/Params'; -import { MockCoreService, MockBufferService, MockDirtyRowService, MockOptionsService, MockLogService, MockCoreMouseService, MockCharsetService, MockUnicodeService } from 'common/TestUtils.test'; +import { MockCoreService, MockBufferService, MockDirtyRowService, MockOptionsService, MockLogService, MockCoreMouseService, MockCharsetService, MockUnicodeService, MockOscLinkService } from 'common/TestUtils.test'; import { IBufferService, ICoreService } from 'common/services/Services'; import { DEFAULT_OPTIONS } from 'common/services/OptionsService'; import { clone } from 'common/Clone'; @@ -67,7 +67,7 @@ describe('InputHandler', () => { bufferService.resize(80, 30); coreService = new CoreService(() => { }, bufferService, new MockLogService(), optionsService); - inputHandler = new TestInputHandler(bufferService, new MockCharsetService(), coreService, new MockDirtyRowService(), new MockLogService(), optionsService, new MockCoreMouseService(), new MockUnicodeService()); + inputHandler = new TestInputHandler(bufferService, new MockCharsetService(), coreService, new MockDirtyRowService(), new MockLogService(), optionsService, new MockOscLinkService(), new MockCoreMouseService(), new MockUnicodeService()); }); describe('SL/SR/DECIC/DECDC', () => { @@ -236,7 +236,7 @@ describe('InputHandler', () => { describe('setMode', () => { it('should toggle bracketedPasteMode', () => { const coreService = new MockCoreService(); - const inputHandler = new TestInputHandler(new MockBufferService(80, 30), new MockCharsetService(), coreService, new MockDirtyRowService(), new MockLogService(), new MockOptionsService(), new MockCoreMouseService(), new MockUnicodeService()); + const inputHandler = new TestInputHandler(new MockBufferService(80, 30), new MockCharsetService(), coreService, new MockDirtyRowService(), new MockLogService(), new MockOptionsService(), new MockOscLinkService(), new MockCoreMouseService(), new MockUnicodeService()); // Set bracketed paste mode inputHandler.setModePrivate(Params.fromArray([2004])); assert.equal(coreService.decPrivateModes.bracketedPasteMode, true); @@ -261,6 +261,7 @@ describe('InputHandler', () => { new MockDirtyRowService(), new MockLogService(), new MockOptionsService(), + new MockOscLinkService(), new MockCoreMouseService(), new MockUnicodeService() ); @@ -307,6 +308,7 @@ describe('InputHandler', () => { new MockDirtyRowService(), new MockLogService(), new MockOptionsService(), + new MockOscLinkService(), new MockCoreMouseService(), new MockUnicodeService() ); @@ -357,6 +359,7 @@ describe('InputHandler', () => { new MockDirtyRowService(), new MockLogService(), new MockOptionsService(), + new MockOscLinkService(), new MockCoreMouseService(), new MockUnicodeService() ); @@ -394,6 +397,7 @@ describe('InputHandler', () => { new MockDirtyRowService(), new MockLogService(), new MockOptionsService(), + new MockOscLinkService(), new MockCoreMouseService(), new MockUnicodeService() ); @@ -444,6 +448,7 @@ describe('InputHandler', () => { new MockDirtyRowService(), new MockLogService(), new MockOptionsService(), + new MockOscLinkService(), new MockCoreMouseService(), new MockUnicodeService() ); @@ -570,6 +575,7 @@ describe('InputHandler', () => { new MockDirtyRowService(), new MockLogService(), new MockOptionsService(), + new MockOscLinkService(), new MockCoreMouseService(), new MockUnicodeService() ); @@ -593,7 +599,7 @@ describe('InputHandler', () => { beforeEach(() => { bufferService = new MockBufferService(80, 30); - handler = new TestInputHandler(bufferService, new MockCharsetService(), new MockCoreService(), new MockDirtyRowService(), new MockLogService(), new MockOptionsService(), new MockCoreMouseService(), new MockUnicodeService()); + handler = new TestInputHandler(bufferService, new MockCharsetService(), new MockCoreService(), new MockDirtyRowService(), new MockLogService(), new MockOptionsService(), new MockOscLinkService(), new MockCoreMouseService(), new MockUnicodeService()); }); it('should handle DECSET/DECRST 47 (alt screen buffer)', async () => { await handler.parseP('\x1b[?47h\r\n\x1b[31mJUNK\x1b[?47lTEST'); @@ -790,7 +796,7 @@ describe('InputHandler', () => { describe('colon notation', () => { let inputHandler2: TestInputHandler; beforeEach(() => { - inputHandler2 = new TestInputHandler(bufferService, new MockCharsetService(), coreService, new MockDirtyRowService(), new MockLogService(), optionsService, new MockCoreMouseService(), new MockUnicodeService()); + inputHandler2 = new TestInputHandler(bufferService, new MockCharsetService(), coreService, new MockDirtyRowService(), new MockLogService(), optionsService, new MockOscLinkService(), new MockCoreMouseService(), new MockUnicodeService()); }); describe('should equal to semicolon', () => { it('CSI 38:2::50:100:150 m', async () => { @@ -2156,7 +2162,7 @@ describe('InputHandler - async handlers', () => { coreService = new CoreService(() => { }, bufferService, new MockLogService(), optionsService); coreService.onData(data => { console.log(data); }); - inputHandler = new TestInputHandler(bufferService, new MockCharsetService(), coreService, new MockDirtyRowService(), new MockLogService(), optionsService, new MockCoreMouseService(), new MockUnicodeService()); + inputHandler = new TestInputHandler(bufferService, new MockCharsetService(), coreService, new MockDirtyRowService(), new MockLogService(), optionsService, new MockOscLinkService(), new MockCoreMouseService(), new MockUnicodeService()); }); it('async CUP with CPR check', async () => { diff --git a/src/common/InputHandler.ts b/src/common/InputHandler.ts index e6d154b9..76d35fd4 100644 --- a/src/common/InputHandler.ts +++ b/src/common/InputHandler.ts @@ -17,7 +17,7 @@ import { IParsingState, IDcsHandler, IEscapeSequenceParser, IParams, IFunctionId import { NULL_CELL_CODE, NULL_CELL_WIDTH, Attributes, FgFlags, BgFlags, Content, UnderlineStyle } from 'common/buffer/Constants'; import { CellData } from 'common/buffer/CellData'; import { AttributeData } from 'common/buffer/AttributeData'; -import { ICoreService, IBufferService, IOptionsService, ILogService, IDirtyRowService, ICoreMouseService, ICharsetService, IUnicodeService, LogLevelEnum } from 'common/services/Services'; +import { ICoreService, IBufferService, IOptionsService, ILogService, IDirtyRowService, ICoreMouseService, ICharsetService, IUnicodeService, LogLevelEnum, IOscLinkService } from 'common/services/Services'; import { OscHandler } from 'common/parser/OscParser'; import { DcsHandler } from 'common/parser/DcsParser'; import { IBuffer } from 'common/buffer/Types'; @@ -264,10 +264,6 @@ export class InputHandler extends Disposable implements IInputHandler { public get onTitleChange(): IEvent { return this._onTitleChange.event; } private _onColor = new EventEmitter(); public get onColor(): IEvent { return this._onColor.event; } - private _onStartHyperlink = new EventEmitter(); - public get onStartHyperlink(): IEvent { return this._onStartHyperlink.event; } - private _onFinishHyperlink = new EventEmitter(); - public get onFinishHyperlink(): IEvent { return this._onFinishHyperlink.event; } private _parseStack: IParseStack = { paused: false, @@ -284,6 +280,7 @@ export class InputHandler extends Disposable implements IInputHandler { private readonly _dirtyRowService: IDirtyRowService, private readonly _logService: ILogService, private readonly _optionsService: IOptionsService, + private readonly _oscLinkService: IOscLinkService, private readonly _coreMouseService: ICoreMouseService, private readonly _unicodeService: IUnicodeService, private readonly _parser: IEscapeSequenceParser = new EscapeSequenceParser() @@ -2937,11 +2934,10 @@ export class InputHandler extends Disposable implements IInputHandler { id = parsedParams[idParamIndex].slice(3) || undefined; } this._currentHyperlink = { id, uri }; - console.log('start hyperlink'); + this._oscLinkService.registerLink(this._currentHyperlink); this._curAttrData.extended = this._curAttrData.extended.clone(); this._curAttrData.extended.urlId = 1; this._curAttrData.updateExtended(); - this._onStartHyperlink.fire(this._currentHyperlink); return true; } @@ -2950,7 +2946,6 @@ export class InputHandler extends Disposable implements IInputHandler { this._curAttrData.extended = this._curAttrData.extended.clone(); this._curAttrData.extended.urlId = 0; this._curAttrData.updateExtended(); - this._onFinishHyperlink.fire(); this._currentHyperlink = undefined; return true; } diff --git a/src/common/TestUtils.test.ts b/src/common/TestUtils.test.ts index 48f3a69e..1d353b91 100644 --- a/src/common/TestUtils.test.ts +++ b/src/common/TestUtils.test.ts @@ -3,13 +3,13 @@ * @license MIT */ -import { IBufferService, ICoreService, ILogService, IOptionsService, ITerminalOptions, IDirtyRowService, ICoreMouseService, ICharsetService, IUnicodeService, IUnicodeVersionProvider, LogLevelEnum, IDecorationService, IInternalDecoration } from 'common/services/Services'; +import { IBufferService, ICoreService, ILogService, IOptionsService, ITerminalOptions, IDirtyRowService, ICoreMouseService, ICharsetService, IUnicodeService, IUnicodeVersionProvider, LogLevelEnum, IDecorationService, IInternalDecoration, IOscLinkService } from 'common/services/Services'; import { IEvent, EventEmitter } from 'common/EventEmitter'; 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 } from 'common/Types'; +import { IDecPrivateModes, ICoreMouseEvent, CoreMouseEventType, ICharset, IModes, IAttributeData, IOscLinkData } from 'common/Types'; import { UnicodeV6 } from 'common/input/UnicodeV6'; import { IDecorationOptions, IDecoration } from 'xterm'; @@ -138,6 +138,16 @@ export class MockOptionsService implements IOptionsService { } } +export class MockOscLinkService implements IOscLinkService { + public serviceBrand: any; + public registerLink(linkData: IOscLinkData): number { + return 1; + } + public getLinkData(linkId: number): IOscLinkData | undefined { + return undefined; + } +} + // defaults to V6 always to keep tests passing export class MockUnicodeService implements IUnicodeService { public serviceBrand: any; diff --git a/src/common/services/OscLinkService.ts b/src/common/services/OscLinkService.ts index 58961c4f..b67b9846 100644 --- a/src/common/services/OscLinkService.ts +++ b/src/common/services/OscLinkService.ts @@ -10,6 +10,7 @@ export class OscLinkService implements IOscLinkService { } public registerLink(linkData: IOscLinkData): number { + console.log('register link'); // TODO: Add and return properly return 1; } From ace99d125db2ac7d1cdc782ede16fcfd8315aaf7 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 7 Aug 2022 07:02:42 -0700 Subject: [PATCH 05/55] Progress on osc link service --- demo/client.ts | 19 ++++-- src/common/InputHandler.ts | 15 +++-- src/common/TestUtils.test.ts | 2 + src/common/services/OscLinkService.ts | 85 ++++++++++++++++++++++++--- src/common/services/Services.ts | 4 ++ 5 files changed, 105 insertions(+), 20 deletions(-) diff --git a/demo/client.ts b/demo/client.ts index 8f858b39..96f6b335 100644 --- a/demo/client.ts +++ b/demo/client.ts @@ -833,12 +833,19 @@ function addAnsiHyperlink() { term.writeln(`Regular link with no id:`); term.writeln('\x1b]8;;https://github.com\x07GitHub\x1b]8;;\x07'); term.writeln('\x1b]8;;https://xtermjs.org\x07https://xtermjs.org\x1b]8;;\x07\x1b[C<- null cell'); - term.writeln(`\nShared ID links:`); - term.writeln('╔════╗ ╔════╗'); - term.writeln('║\x1b]8;;https://github.com\x07GitH\x1b]8;;\x07║ ║ ║'); - term.writeln('║\x1b]8;;https://github.com\x07ub\x1b]8;;\x07 ║ ║ ║'); - term.writeln('╚════╝ ╚════╝'); - term.write('\x1b[3A\x1b[8C\x1b]8;;https://xtermjs.org\x07xter\x1b[B\x1b[4Dm.js\x1b]8;;\x07\x1b[2B\x1b[12D'); + term.writeln(`\nAdjacent links:`); + term.writeln('\x1b]8;;https://github.com\x07GitHub\x1b]8;;https://xtermjs.org\x07xterm.js\x1b]8;;\x07'); + term.writeln(`\nShared ID link:`); + term.writeln('╔════╗'); + term.writeln('║\x1b]8;id=testid;https://github.com\x07GitH\x1b]8;;\x07║'); + term.writeln('║\x1b]8;id=testid;https://github.com\x07ub\x1b]8;;\x07 ║'); + term.writeln('╚════╝'); + term.writeln(`\nWrapped link with no ID (not meant to share underline):`); + term.writeln('╔════╗'); + term.writeln('║ ║'); + term.writeln('║ ║'); + term.writeln('╚════╝'); + term.write('\x1b[3A\x1b[1C\x1b]8;;https://xtermjs.org\x07xter\x1b[B\x1b[4Dm.js\x1b]8;;\x07\x1b[2B\x1b[5D'); } function addDecoration() { diff --git a/src/common/InputHandler.ts b/src/common/InputHandler.ts index 76d35fd4..37a0a127 100644 --- a/src/common/InputHandler.ts +++ b/src/common/InputHandler.ts @@ -228,7 +228,7 @@ export class InputHandler extends Disposable implements IInputHandler { private _workCell: CellData = new CellData(); private _windowTitle = ''; private _iconName = ''; - private _currentHyperlink?: IOscLinkData; + private _currentLinkId?: number; protected _windowTitleStack: string[] = []; protected _iconNameStack: string[] = []; @@ -639,6 +639,9 @@ export class InputHandler extends Disposable implements IInputHandler { if (screenReaderMode) { this._onA11yChar.fire(stringFromCodePoint(code)); } + if (this._currentLinkId !== undefined) { + this._oscLinkService.addLineToLink(this._currentLinkId, this._activeBuffer.ybase + this._activeBuffer.y); + } // insert combining char at last cursor position // this._activeBuffer.x should never be 0 for a combining char @@ -2924,7 +2927,7 @@ export class InputHandler extends Disposable implements IInputHandler { private _createHyperlink(params: string, uri: string): boolean { // It's legal to open a new hyperlink without explicitly finishing the previous one - if (this._currentHyperlink) { + if (this._currentLinkId !== undefined) { this._finishHyperlink(); } const parsedParams = params.split(':'); @@ -2933,10 +2936,10 @@ export class InputHandler extends Disposable implements IInputHandler { if (idParamIndex !== -1) { id = parsedParams[idParamIndex].slice(3) || undefined; } - this._currentHyperlink = { id, uri }; - this._oscLinkService.registerLink(this._currentHyperlink); this._curAttrData.extended = this._curAttrData.extended.clone(); - this._curAttrData.extended.urlId = 1; + this._currentLinkId = this._oscLinkService.registerLink({ id, uri }); + this._curAttrData.extended.urlId = this._currentLinkId; + console.log('register', uri, `id=${this._curAttrData.extended.urlId}`); this._curAttrData.updateExtended(); return true; } @@ -2946,7 +2949,7 @@ export class InputHandler extends Disposable implements IInputHandler { this._curAttrData.extended = this._curAttrData.extended.clone(); this._curAttrData.extended.urlId = 0; this._curAttrData.updateExtended(); - this._currentHyperlink = undefined; + this._currentLinkId = undefined; return true; } diff --git a/src/common/TestUtils.test.ts b/src/common/TestUtils.test.ts index 1d353b91..215481af 100644 --- a/src/common/TestUtils.test.ts +++ b/src/common/TestUtils.test.ts @@ -146,6 +146,8 @@ export class MockOscLinkService implements IOscLinkService { public getLinkData(linkId: number): IOscLinkData | undefined { return undefined; } + public addLineToLink(linkId: number, y: number): void { + } } // defaults to V6 always to keep tests passing diff --git a/src/common/services/OscLinkService.ts b/src/common/services/OscLinkService.ts index b67b9846..a3f8af1d 100644 --- a/src/common/services/OscLinkService.ts +++ b/src/common/services/OscLinkService.ts @@ -1,23 +1,92 @@ +/** + * Copyright (c) 2022 The xterm.js authors. All rights reserved. + * @license MIT + */ import { IBufferService, IOscLinkService } from 'common/services/Services'; -import { IOscLinkData } from 'common/Types'; +import { IMarker, IOscLinkData } from 'common/Types'; export class OscLinkService implements IOscLinkService { public serviceBrand: any; + private _nextId = 1; + + // TODO: Evict on marker dispose + private _entriesNoId: IOscLinkEntryNoId[] = []; + private _entriesWithId: Map = new Map(); + + // The "link id" (number) which is the numberic representation of a unique link should not be + // confused with "id" (string) which comes in with "id=" in the OSC link's properties + private _dataByLinkId: Map = new Map(); + constructor( @IBufferService private readonly _bufferService: IBufferService ) { } - public registerLink(linkData: IOscLinkData): number { - console.log('register link'); - // TODO: Add and return properly - return 1; + public registerLink(data: IOscLinkData): number { + // TODO: Extend range where appropriate + const buffer = this._bufferService.buffer; + + // Links with no id will only ever be registered a single time + if (data.id === undefined) { + const entry: IOscLinkEntryNoId = { + data, + id: this._nextId++, + lines: [buffer.addMarker(buffer.ybase + buffer.y)] + }; + this._entriesNoId.push(entry); + this._dataByLinkId.set(entry.id, entry); + return entry.id; + } + + const castData = data as Required; + const key = this._getEntryIdKey(castData); + const match = this._entriesWithId.get(key); + if (match) { + this.addLineToLink(match.id, buffer.ybase + buffer.y); + return match.id; + } + + const entry: IOscLinkEntryWithId = { + id: this._nextId++, + key: this._getEntryIdKey(castData), + data: castData, + lines: [buffer.addMarker(buffer.ybase + buffer.y)] + }; + this._entriesWithId.set(entry.key, entry); + this._dataByLinkId.set(entry.id, entry); + return entry.id; + } + + public addLineToLink(linkId: number, y: number): void { + const link = this._dataByLinkId.get(linkId); + if (!link) { + return; + } + if (link.lines.every(e => e.line !== y)) { + console.log(' add new line', y); + link.lines.push(this._bufferService.buffer.addMarker(y)); + } } public getLinkData(linkId: number): IOscLinkData | undefined { - return { - uri: 'https://github.com' - }; + return this._dataByLinkId.get(linkId)?.data; + } + + private _getEntryIdKey(linkData: Required): string { + return `${linkData.id};;${linkData.uri}`; } } + +interface IOscLinkEntry { + data: T; + id: number; + lines: IMarker[]; +} + +interface IOscLinkEntryNoId extends IOscLinkEntry { +} + +interface IOscLinkEntryWithId extends IOscLinkEntry> { + key: string; +} diff --git a/src/common/services/Services.ts b/src/common/services/Services.ts index 5f97a487..88a21252 100644 --- a/src/common/services/Services.ts +++ b/src/common/services/Services.ts @@ -280,6 +280,10 @@ export interface IOscLinkService { * service and will be freed when this current cursor position is trimmed off the buffer. */ registerLink(linkData: IOscLinkData): number; + /** + * Adds a line to a link if needed. + */ + addLineToLink(linkId: number, y: number): void; /** Get the link data associated with a link ID. */ getLinkData(linkId: number): IOscLinkData | undefined; } From d9b8c6a838ed2e03972f418065858f2494ee5310 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 7 Aug 2022 07:35:46 -0700 Subject: [PATCH 06/55] linkHandler option and default handler --- demo/client.ts | 4 +-- src/browser/OscLinkProvider.ts | 34 +++++++++++++++++++++---- src/common/services/OptionsService.ts | 1 + src/common/services/OscLinkService.ts | 4 +-- src/common/services/Services.ts | 3 ++- typings/xterm.d.ts | 36 +++++++++++++++++++++++++++ 6 files changed, 72 insertions(+), 10 deletions(-) diff --git a/demo/client.ts b/demo/client.ts index 96f6b335..cfebf384 100644 --- a/demo/client.ts +++ b/demo/client.ts @@ -835,12 +835,12 @@ function addAnsiHyperlink() { term.writeln('\x1b]8;;https://xtermjs.org\x07https://xtermjs.org\x1b]8;;\x07\x1b[C<- null cell'); term.writeln(`\nAdjacent links:`); term.writeln('\x1b]8;;https://github.com\x07GitHub\x1b]8;;https://xtermjs.org\x07xterm.js\x1b]8;;\x07'); - term.writeln(`\nShared ID link:`); + term.writeln(`\nShared ID link (underline should be shared):`); term.writeln('╔════╗'); term.writeln('║\x1b]8;id=testid;https://github.com\x07GitH\x1b]8;;\x07║'); term.writeln('║\x1b]8;id=testid;https://github.com\x07ub\x1b]8;;\x07 ║'); term.writeln('╚════╝'); - term.writeln(`\nWrapped link with no ID (not meant to share underline):`); + term.writeln(`\nWrapped link with no ID (not necessarily meant to share underline):`); term.writeln('╔════╗'); term.writeln('║ ║'); term.writeln('║ ║'); diff --git a/src/browser/OscLinkProvider.ts b/src/browser/OscLinkProvider.ts index 7fee62af..96a2c395 100644 --- a/src/browser/OscLinkProvider.ts +++ b/src/browser/OscLinkProvider.ts @@ -5,16 +5,22 @@ import { ILink, ILinkProvider } from 'browser/Types'; import { CellData } from 'common/buffer/CellData'; -import { IBufferService, IOscLinkService } from 'common/services/Services'; +import { IBufferService, IOptionsService, IOscLinkService } from 'common/services/Services'; export class OscLinkProvider implements ILinkProvider { constructor( @IBufferService private readonly _bufferService: IBufferService, + @IOptionsService private readonly _optionsService: IOptionsService, @IOscLinkService private readonly _oscLinkService: IOscLinkService ) { } public provideLinks(y: number, callback: (links: ILink[] | undefined) => void): void { + // OSC links only work when a link handler is set + // if (this._optionsService.rawOptions.linkHandler === null) { + // return; + // } + const line = this._bufferService.buffer.lines.get(y - 1); if (!line) { callback(undefined); @@ -52,12 +58,14 @@ export class OscLinkProvider implements ILinkProvider { if (finishLink || (currentStart !== -1 && x === lineLength - 1)) { const text = this._oscLinkService.getLinkData(currentLinkId)?.uri; if (text) { + const linkHandler = this._optionsService.rawOptions.linkHandler; // OSC links always use underline and pointer decorations result.push({ text, // These ranges are 1-based range: { start: { + // TODO: Adjacent links aren't working correctly x: currentStart + 1, y }, @@ -67,10 +75,9 @@ export class OscLinkProvider implements ILinkProvider { y } }, - activate(e, text) { - console.log('activate!', text); - } - // TODO: Embedder API to handle hover + activate: linkHandler?.activate || defaultActivate, + hover: linkHandler?.hover, + leave: linkHandler?.leave }); } currentStart = -1; @@ -82,3 +89,20 @@ export class OscLinkProvider implements ILinkProvider { callback(result); } } + +function defaultActivate(e: MouseEvent, uri: string): void { + const answer = confirm(`Do you want to navigate to ${uri}?`); + if (answer) { + const newWindow = window.open(); + if (newWindow) { + try { + newWindow.opener = null; + } catch { + // no-op, Electron can throw + } + newWindow.location.href = uri; + } else { + console.warn('Opening link blocked as opener could not be cleared'); + } + } +} diff --git a/src/common/services/OptionsService.ts b/src/common/services/OptionsService.ts index ab9edfbf..744903f1 100644 --- a/src/common/services/OptionsService.ts +++ b/src/common/services/OptionsService.ts @@ -24,6 +24,7 @@ export const DEFAULT_OPTIONS: Readonly = { fontWeightBold: 'bold', lineHeight: 1.0, letterSpacing: 0, + linkHandler: null, logLevel: 'info', scrollback: 1000, scrollSensitivity: 1, diff --git a/src/common/services/OscLinkService.ts b/src/common/services/OscLinkService.ts index a3f8af1d..d3744229 100644 --- a/src/common/services/OscLinkService.ts +++ b/src/common/services/OscLinkService.ts @@ -24,7 +24,6 @@ export class OscLinkService implements IOscLinkService { } public registerLink(data: IOscLinkData): number { - // TODO: Extend range where appropriate const buffer = this._bufferService.buffer; // Links with no id will only ever be registered a single time @@ -39,6 +38,7 @@ export class OscLinkService implements IOscLinkService { return entry.id; } + // Add the line to the link if it already exists const castData = data as Required; const key = this._getEntryIdKey(castData); const match = this._entriesWithId.get(key); @@ -47,6 +47,7 @@ export class OscLinkService implements IOscLinkService { return match.id; } + // Create the link const entry: IOscLinkEntryWithId = { id: this._nextId++, key: this._getEntryIdKey(castData), @@ -64,7 +65,6 @@ export class OscLinkService implements IOscLinkService { return; } if (link.lines.every(e => e.line !== y)) { - console.log(' add new line', y); link.lines.push(this._bufferService.buffer.addMarker(y)); } } diff --git a/src/common/services/Services.ts b/src/common/services/Services.ts index 88a21252..63543c56 100644 --- a/src/common/services/Services.ts +++ b/src/common/services/Services.ts @@ -7,7 +7,7 @@ import { IEvent, IEventEmitter } from 'common/EventEmitter'; import { IBuffer, IBufferSet } from 'common/buffer/Types'; import { IDecPrivateModes, ICoreMouseEvent, CoreMouseEncoding, ICoreMouseProtocol, CoreMouseEventType, ICharset, IWindowOptions, IModes, IAttributeData, ScrollSource, IDisposable, IColorRGB, IColor, CursorStyle, IOscLinkData } from 'common/Types'; import { createDecorator } from 'common/services/ServiceRegistry'; -import { IDecorationOptions, IDecoration } from 'xterm'; +import { IDecorationOptions, IDecoration, ILinkHandler } from 'xterm'; export const IBufferService = createDecorator('BufferService'); export interface IBufferService { @@ -223,6 +223,7 @@ export interface ITerminalOptions { fontWeightBold: FontWeight; letterSpacing: number; lineHeight: number; + linkHandler: ILinkHandler | null; logLevel: LogLevel; macOptionIsMeta: boolean; macOptionClickForcesSelection: boolean; diff --git a/typings/xterm.d.ts b/typings/xterm.d.ts index 05058d04..2fa17421 100644 --- a/typings/xterm.d.ts +++ b/typings/xterm.d.ts @@ -128,6 +128,14 @@ declare module 'xterm' { */ lineHeight?: number; + /** + * The handler for OSC 8 hyperlinks. Links will use the `confirm` browser + * API if no link handler is set. Consider the security of users when using + * this, there should be some tooltip or prompt when hovering or activating + * the link. + */ + linkHandler?: ILinkHandler | null; + /** * What log level to use, this will log for all levels below and including * what is set: @@ -1101,6 +1109,34 @@ declare module 'xterm' { y: number; } + /** + * A link handler for OSC 8 hyperlinks. + */ + interface ILinkHandler { + /** + * Calls when the link is activated. + * @param event The mouse event triggering the callback. + * @param text The text of the link. + */ + activate(event: MouseEvent, text: string): void; + + /** + * Called when the mouse hovers the link. To use this to create a DOM-based hover tooltip, + * create the hover element within `Terminal.element` and add the `xterm-hover` class to it, + * that will cause mouse events to not fall through and activate other links. + * @param event The mouse event triggering the callback. + * @param text The text of the link. + */ + hover?(event: MouseEvent, text: string): void; + + /** + * Called when the mouse leaves the link. + * @param event The mouse event triggering the callback. + * @param text The text of the link. + */ + leave?(event: MouseEvent, text: string): void; + } + /** * A custom link provider. */ From c39d2351683008e22c5766458ec066da337ba4b8 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 7 Aug 2022 07:42:21 -0700 Subject: [PATCH 07/55] Fix adjacent link edge case --- demo/client.ts | 2 +- src/browser/OscLinkProvider.ts | 13 ++++++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/demo/client.ts b/demo/client.ts index cfebf384..507663a8 100644 --- a/demo/client.ts +++ b/demo/client.ts @@ -834,7 +834,7 @@ function addAnsiHyperlink() { term.writeln('\x1b]8;;https://github.com\x07GitHub\x1b]8;;\x07'); term.writeln('\x1b]8;;https://xtermjs.org\x07https://xtermjs.org\x1b]8;;\x07\x1b[C<- null cell'); term.writeln(`\nAdjacent links:`); - term.writeln('\x1b]8;;https://github.com\x07GitHub\x1b]8;;https://xtermjs.org\x07xterm.js\x1b]8;;\x07'); + term.writeln('\x1b]8;;https://github.com\x07GitHub\x1b]8;;https://xtermjs.org\x07\x1b[32mxterm.js\x1b[0m\x1b]8;;\x07'); term.writeln(`\nShared ID link (underline should be shared):`); term.writeln('╔════╗'); term.writeln('║\x1b]8;id=testid;https://github.com\x07GitH\x1b]8;;\x07║'); diff --git a/src/browser/OscLinkProvider.ts b/src/browser/OscLinkProvider.ts index 96a2c395..8d3977c6 100644 --- a/src/browser/OscLinkProvider.ts +++ b/src/browser/OscLinkProvider.ts @@ -65,7 +65,6 @@ export class OscLinkProvider implements ILinkProvider { // These ranges are 1-based range: { start: { - // TODO: Adjacent links aren't working correctly x: currentStart + 1, y }, @@ -80,12 +79,20 @@ export class OscLinkProvider implements ILinkProvider { leave: linkHandler?.leave }); } - currentStart = -1; - currentLinkId = -1; finishLink = false; + + // Clear link or start a new link if one starts immediately + if (cell.hasExtendedAttrs() && cell.extended.urlId) { + currentStart = x; + currentLinkId = cell.extended.urlId; + } else { + currentStart = -1; + currentLinkId = -1; + } } } // TODO: Handle fetching and returning other link ranges to underline other links with the same id + console.log('result', result); callback(result); } } From 9c9ff2b38c6e8f3f0b4693f11527a08339414789 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 7 Aug 2022 07:46:16 -0700 Subject: [PATCH 08/55] Clean up --- src/browser/OscLinkProvider.ts | 9 ++------- src/common/InputHandler.ts | 3 --- src/common/buffer/AttributeData.ts | 7 ------- 3 files changed, 2 insertions(+), 17 deletions(-) diff --git a/src/browser/OscLinkProvider.ts b/src/browser/OscLinkProvider.ts index 8d3977c6..38c07106 100644 --- a/src/browser/OscLinkProvider.ts +++ b/src/browser/OscLinkProvider.ts @@ -16,11 +16,6 @@ export class OscLinkProvider implements ILinkProvider { } public provideLinks(y: number, callback: (links: ILink[] | undefined) => void): void { - // OSC links only work when a link handler is set - // if (this._optionsService.rawOptions.linkHandler === null) { - // return; - // } - const line = this._bufferService.buffer.lines.get(y - 1); if (!line) { callback(undefined); @@ -28,6 +23,7 @@ export class OscLinkProvider implements ILinkProvider { } const result: ILink[] = []; + const linkHandler = this._optionsService.rawOptions.linkHandler; const cell = new CellData(); const lineLength = line.getTrimmedLength(); let currentLinkId = -1; @@ -58,7 +54,6 @@ export class OscLinkProvider implements ILinkProvider { if (finishLink || (currentStart !== -1 && x === lineLength - 1)) { const text = this._oscLinkService.getLinkData(currentLinkId)?.uri; if (text) { - const linkHandler = this._optionsService.rawOptions.linkHandler; // OSC links always use underline and pointer decorations result.push({ text, @@ -91,8 +86,8 @@ export class OscLinkProvider implements ILinkProvider { } } } + // TODO: Handle fetching and returning other link ranges to underline other links with the same id - console.log('result', result); callback(result); } } diff --git a/src/common/InputHandler.ts b/src/common/InputHandler.ts index 37a0a127..34c99f84 100644 --- a/src/common/InputHandler.ts +++ b/src/common/InputHandler.ts @@ -2912,7 +2912,6 @@ export class InputHandler extends Disposable implements IInputHandler { */ public setHyperlink(data: string): boolean { const args = data.split(';'); - console.log('hyperlink', args); if (args.length < 2) { return false; } @@ -2939,13 +2938,11 @@ export class InputHandler extends Disposable implements IInputHandler { this._curAttrData.extended = this._curAttrData.extended.clone(); this._currentLinkId = this._oscLinkService.registerLink({ id, uri }); this._curAttrData.extended.urlId = this._currentLinkId; - console.log('register', uri, `id=${this._curAttrData.extended.urlId}`); this._curAttrData.updateExtended(); return true; } private _finishHyperlink(): boolean { - console.log('finish hyperlink'); this._curAttrData.extended = this._curAttrData.extended.clone(); this._curAttrData.extended.urlId = 0; this._curAttrData.updateExtended(); diff --git a/src/common/buffer/AttributeData.ts b/src/common/buffer/AttributeData.ts index aac6a33d..3af3d293 100644 --- a/src/common/buffer/AttributeData.ts +++ b/src/common/buffer/AttributeData.ts @@ -134,9 +134,7 @@ export class AttributeData implements IAttributeData { export class ExtendedAttrs implements IExtendedAttrs { private _ext: number = 0; public get ext(): number { - // TODO: How to handle previous underline style if link overrides it? if (this._urlId) { - console.log('ext, has url'); return ( (this._ext & ~ExtFlags.UNDERLINE_STYLE) | (this.underlineStyle << 26) @@ -159,11 +157,6 @@ export class ExtendedAttrs implements IExtendedAttrs { } public get underlineColor(): number { - // Always return the URL color if it has one - if (this._urlId) { - // TODO: fix - return 0; - } return this._ext & (Attributes.CM_MASK | Attributes.RGB_MASK); } public set underlineColor(value: number) { From 8191483fbd9b5a68c47cad2b6d66738adc5fa66b Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 7 Aug 2022 08:05:42 -0700 Subject: [PATCH 09/55] Evict links from maps when markers are disposed --- src/common/services/OscLinkService.ts | 45 ++++++++++++++++++++------- 1 file changed, 34 insertions(+), 11 deletions(-) diff --git a/src/common/services/OscLinkService.ts b/src/common/services/OscLinkService.ts index d3744229..13bd8aa4 100644 --- a/src/common/services/OscLinkService.ts +++ b/src/common/services/OscLinkService.ts @@ -10,12 +10,16 @@ export class OscLinkService implements IOscLinkService { private _nextId = 1; - // TODO: Evict on marker dispose - private _entriesNoId: IOscLinkEntryNoId[] = []; + /** + * A map of the link key to link entry. This is used to add additional lines to links with ids. + */ private _entriesWithId: Map = new Map(); - // The "link id" (number) which is the numberic representation of a unique link should not be - // confused with "id" (string) which comes in with "id=" in the OSC link's properties + /** + * A map of the link id to the link entry. The "link id" (number) which is the numberic + * representation of a unique link should not be confused with "id" (string) which comes in with + * `id=` in the OSC link's properties. + */ private _dataByLinkId: Map = new Map(); constructor( @@ -28,12 +32,13 @@ export class OscLinkService implements IOscLinkService { // Links with no id will only ever be registered a single time if (data.id === undefined) { + const marker = buffer.addMarker(buffer.ybase + buffer.y); const entry: IOscLinkEntryNoId = { data, id: this._nextId++, - lines: [buffer.addMarker(buffer.ybase + buffer.y)] + lines: [marker] }; - this._entriesNoId.push(entry); + marker.onDispose(() => this._removeMarkerFromLink(entry, marker)); this._dataByLinkId.set(entry.id, entry); return entry.id; } @@ -48,24 +53,28 @@ export class OscLinkService implements IOscLinkService { } // Create the link + const marker = buffer.addMarker(buffer.ybase + buffer.y); const entry: IOscLinkEntryWithId = { id: this._nextId++, key: this._getEntryIdKey(castData), data: castData, - lines: [buffer.addMarker(buffer.ybase + buffer.y)] + lines: [marker] }; + marker.onDispose(() => this._removeMarkerFromLink(entry, marker)); this._entriesWithId.set(entry.key, entry); this._dataByLinkId.set(entry.id, entry); return entry.id; } public addLineToLink(linkId: number, y: number): void { - const link = this._dataByLinkId.get(linkId); - if (!link) { + const entry = this._dataByLinkId.get(linkId); + if (!entry) { return; } - if (link.lines.every(e => e.line !== y)) { - link.lines.push(this._bufferService.buffer.addMarker(y)); + if (entry.lines.every(e => e.line !== y)) { + const marker = this._bufferService.buffer.addMarker(y); + entry.lines.push(marker); + marker.onDispose(() => this._removeMarkerFromLink(entry, marker)); } } @@ -76,6 +85,20 @@ export class OscLinkService implements IOscLinkService { private _getEntryIdKey(linkData: Required): string { return `${linkData.id};;${linkData.uri}`; } + + private _removeMarkerFromLink(entry: IOscLinkEntryNoId | IOscLinkEntryWithId, marker: IMarker): void { + const index = entry.lines.indexOf(marker); + if (index === -1) { + return; + } + entry.lines.splice(index, 1); + if (entry.lines.length === 0) { + if (entry.data.id !== undefined) { + this._entriesWithId.delete((entry as IOscLinkEntryWithId).key); + } + this._dataByLinkId.delete(entry.id); + } + } } interface IOscLinkEntry { From e87e15e87bc6a7f8dda32b3047e9ce7c0b996b60 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 7 Aug 2022 10:36:42 -0700 Subject: [PATCH 10/55] Only override AttributeData.isUnderline when urlId is set --- src/common/buffer/AttributeData.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/buffer/AttributeData.ts b/src/common/buffer/AttributeData.ts index 3af3d293..e4f464cd 100644 --- a/src/common/buffer/AttributeData.ts +++ b/src/common/buffer/AttributeData.ts @@ -36,7 +36,7 @@ export class AttributeData implements IAttributeData { public isInverse(): number { return this.fg & FgFlags.INVERSE; } public isBold(): number { return this.fg & FgFlags.BOLD; } public isUnderline(): number { - if (this.hasExtendedAttrs() && this.extended.underlineStyle !== UnderlineStyle.NONE) { + if (this.hasExtendedAttrs() && this.extended.urlId) { return 1; } return this.fg & FgFlags.UNDERLINE; From 2cfbee413ff2289506f4a6135865273d0700b539 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 7 Aug 2022 11:30:08 -0700 Subject: [PATCH 11/55] Add some OscLinkService unit tests --- src/common/services/BufferService.ts | 10 ++--- src/common/services/OscLinkService.test.ts | 44 ++++++++++++++++++++++ 2 files changed, 48 insertions(+), 6 deletions(-) create mode 100644 src/common/services/OscLinkService.test.ts diff --git a/src/common/services/BufferService.ts b/src/common/services/BufferService.ts index bba60dd8..e3b7dcd8 100644 --- a/src/common/services/BufferService.ts +++ b/src/common/services/BufferService.ts @@ -32,13 +32,11 @@ export class BufferService extends Disposable implements IBufferService { /** An IBufferline to clone/copy from for new blank lines */ private _cachedBlankLine: IBufferLine | undefined; - constructor( - @IOptionsService private _optionsService: IOptionsService - ) { + constructor(@IOptionsService optionsService: IOptionsService) { super(); - this.cols = Math.max(_optionsService.rawOptions.cols || 0, MINIMUM_COLS); - this.rows = Math.max(_optionsService.rawOptions.rows || 0, MINIMUM_ROWS); - this.buffers = new BufferSet(_optionsService, this); + this.cols = Math.max(optionsService.rawOptions.cols || 0, MINIMUM_COLS); + this.rows = Math.max(optionsService.rawOptions.rows || 0, MINIMUM_ROWS); + this.buffers = new BufferSet(optionsService, this); } public dispose(): void { diff --git a/src/common/services/OscLinkService.test.ts b/src/common/services/OscLinkService.test.ts new file mode 100644 index 00000000..5000e8e2 --- /dev/null +++ b/src/common/services/OscLinkService.test.ts @@ -0,0 +1,44 @@ +/** + * Copyright (c) 2020 The xterm.js authors. All rights reserved. + * @license MIT + */ + +import { assert } from 'chai'; +import { AttributeData } from 'common/buffer/AttributeData'; +import { BufferService } from 'common/services/BufferService'; +import { OptionsService } from 'common/services/OptionsService'; +import { OscLinkService } from 'common/services/OscLinkService'; +import { IBufferService, IOptionsService, IOscLinkService } from 'common/services/Services'; + +describe('OscLinkService', () => { + describe('constructor', () => { + let bufferService: IBufferService; + let optionsService: IOptionsService; + let oscLinkService: IOscLinkService; + beforeEach(() => { + optionsService = new OptionsService({ rows: 3, cols: 10 }); + bufferService = new BufferService(optionsService); + oscLinkService = new OscLinkService(bufferService); + }); + + it('link IDs are created and fetched consistently', () => { + const linkId = oscLinkService.registerLink({ id: 'foo', uri: 'bar' }); + assert.ok(linkId); + assert.equal(oscLinkService.registerLink({ id: 'foo', uri: 'bar' }), linkId); + }); + + it('should dispose the link ID when the last marker is trimmed from the buffer', () => { + // Activate the alt buffer to get 0 scrollback + bufferService.buffers.activateAltBuffer(); + const linkId = oscLinkService.registerLink({ id: 'foo', uri: 'bar' }); + assert.ok(linkId); + bufferService.scroll(new AttributeData()); + assert.notStrictEqual(oscLinkService.registerLink({ id: 'foo', uri: 'bar' }), linkId); + }); + + it('should fetch link data from link id', () => { + const linkId = oscLinkService.registerLink({ id: 'foo', uri: 'bar' }); + assert.deepStrictEqual(oscLinkService.getLinkData(linkId), { id: 'foo', uri: 'bar' }); + }); + }); +}); From bc528cd870e52c75787137e06568343de1029155 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 7 Aug 2022 11:51:52 -0700 Subject: [PATCH 12/55] Ensure extended underline style is cleared on SGR 24 --- src/common/InputHandler.ts | 1 + src/common/buffer/AttributeData.ts | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/common/InputHandler.ts b/src/common/InputHandler.ts index 34c99f84..5a0725a4 100644 --- a/src/common/InputHandler.ts +++ b/src/common/InputHandler.ts @@ -2500,6 +2500,7 @@ export class InputHandler extends Disposable implements IInputHandler { } else if (p === 24) { // not underlined attr.fg &= ~FgFlags.UNDERLINE; + this._processUnderline(UnderlineStyle.NONE, attr); } else if (p === 25) { // not blink attr.fg &= ~FgFlags.BLINK; diff --git a/src/common/buffer/AttributeData.ts b/src/common/buffer/AttributeData.ts index e4f464cd..3af3d293 100644 --- a/src/common/buffer/AttributeData.ts +++ b/src/common/buffer/AttributeData.ts @@ -36,7 +36,7 @@ export class AttributeData implements IAttributeData { public isInverse(): number { return this.fg & FgFlags.INVERSE; } public isBold(): number { return this.fg & FgFlags.BOLD; } public isUnderline(): number { - if (this.hasExtendedAttrs() && this.extended.urlId) { + if (this.hasExtendedAttrs() && this.extended.underlineStyle !== UnderlineStyle.NONE) { return 1; } return this.fg & FgFlags.UNDERLINE; From 81a62306afebe463a192e7d4b85f345ab007c9f7 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Wed, 10 Aug 2022 05:59:29 -0700 Subject: [PATCH 13/55] Ensure canvas device dims are > 0x0 when firing callback See microsoft/vscode#157444 --- src/browser/renderer/DevicePixelObserver.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/browser/renderer/DevicePixelObserver.ts b/src/browser/renderer/DevicePixelObserver.ts index 611eb910..caf1b21d 100644 --- a/src/browser/renderer/DevicePixelObserver.ts +++ b/src/browser/renderer/DevicePixelObserver.ts @@ -24,10 +24,12 @@ export function observeDevicePixelDimensions(element: HTMLElement, callback: (de return; } - callback( - entry.devicePixelContentBoxSize[0].inlineSize, - entry.devicePixelContentBoxSize[0].blockSize - ); + // Fire the callback, ignore events where the dimensions are 0x0 as the canvas is likely hidden + const width = entry.devicePixelContentBoxSize[0].inlineSize; + const height = entry.devicePixelContentBoxSize[0].blockSize; + if (width > 0 && height > 0) { + callback(width, height); + } }); observer.observe(element, { box: ['device-pixel-content-box'] } as any); return toDisposable(() => observer?.disconnect()); From 9a119a7292d43a18118f2fbb727c5729609e8f54 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Wed, 10 Aug 2022 06:27:32 -0700 Subject: [PATCH 14/55] Whitespace change to trigger release --- addons/xterm-addon-webgl/README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/addons/xterm-addon-webgl/README.md b/addons/xterm-addon-webgl/README.md index a431170f..5dee6b62 100644 --- a/addons/xterm-addon-webgl/README.md +++ b/addons/xterm-addon-webgl/README.md @@ -2,7 +2,6 @@ An addon for [xterm.js](https://github.com/xtermjs/xterm.js) that enables a WebGL2-based renderer. This addon requires xterm.js v4+. - ### Install ```bash From 79d47893df3e734b25225318a0d0a45d4020d02f Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 19 Aug 2022 13:19:08 -0700 Subject: [PATCH 15/55] Move webgl underline rendering down See microsoft/vscode#158326 --- addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts index 13dceae6..72421b2d 100644 --- a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts +++ b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts @@ -439,8 +439,7 @@ export class WebglCharAtlas implements IDisposable { if (underline) { this._tmpCtx.save(); const lineWidth = Math.max(1, Math.floor(this._config.fontSize * window.devicePixelRatio / 15)); - // When the width is odd, draw at 0.5 position. Offset by an additional 1 dpr to bring the - // underline closer to the character + // When the line width is odd, draw at a 0.5 position const yOffset = (lineWidth % 2 === 1 ? 0.5 : 0) + window.devicePixelRatio; this._tmpCtx.lineWidth = lineWidth; @@ -463,9 +462,9 @@ export class WebglCharAtlas implements IDisposable { this._tmpCtx.beginPath(); const xLeft = padding; const xRight = padding + this._config.scaledCellWidth; - const yTop = Math.ceil(padding + this._config.scaledCharHeight - lineWidth) - yOffset; - const yMid = padding + this._config.scaledCharHeight - yOffset; - const yBot = Math.ceil(padding + this._config.scaledCharHeight + lineWidth) - yOffset; + const yTop = Math.ceil(padding + this._config.scaledCharHeight) - yOffset; + const yMid = padding + this._config.scaledCharHeight + lineWidth - yOffset; + const yBot = Math.ceil(padding + this._config.scaledCharHeight + lineWidth * 2) - yOffset; switch (this._workAttributeData.extended.underlineStyle) { case UnderlineStyle.DOUBLE: this._tmpCtx.moveTo(xLeft, yTop); From 0698afaf12c89df34b6c72edca9a7e229b4ff0fe Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 19 Aug 2022 13:24:39 -0700 Subject: [PATCH 16/55] Only stroke glyph when font size >= 12 --- addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts index 72421b2d..a7e92f94 100644 --- a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts +++ b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts @@ -440,7 +440,7 @@ export class WebglCharAtlas implements IDisposable { this._tmpCtx.save(); const lineWidth = Math.max(1, Math.floor(this._config.fontSize * window.devicePixelRatio / 15)); // When the line width is odd, draw at a 0.5 position - const yOffset = (lineWidth % 2 === 1 ? 0.5 : 0) + window.devicePixelRatio; + const yOffset = lineWidth % 2 === 1 ? 0.5 : 0; this._tmpCtx.lineWidth = lineWidth; // Underline color @@ -527,8 +527,9 @@ export class WebglCharAtlas implements IDisposable { this._tmpCtx.restore(); // Draw stroke in the background color for non custom characters in order to give an outline - // between the text and the underline - if (!customGlyph) { + // between the text and the underline. Only do this when font size is >= 12 as the underline + // looks odd when the font size is too small + if (!customGlyph && this._config.fontSize >= 12) { // This only works when transparency is disabled because it's not clear how to clear stroked // text if (!this._config.allowTransparency && chars !== ' ') { From b59de22c36ac376ff83bec563c0cb2a2dcb5429c Mon Sep 17 00:00:00 2001 From: greenmashimaro Date: Tue, 23 Aug 2022 15:09:43 +0800 Subject: [PATCH 17/55] fix typo --- src/common/buffer/BufferSet.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/common/buffer/BufferSet.ts b/src/common/buffer/BufferSet.ts index 7d07cec8..f940bb8f 100644 --- a/src/common/buffer/BufferSet.ts +++ b/src/common/buffer/BufferSet.ts @@ -58,14 +58,14 @@ export class BufferSet extends Disposable implements IBufferSet { } /** - * Returns the normal Buffer of the BufferSet + * Returns the currently active Buffer of the BufferSet */ public get active(): Buffer { return this._activeBuffer; } /** - * Returns the currently active Buffer of the BufferSet + * Returns the normal Buffer of the BufferSet */ public get normal(): Buffer { return this._normal; From 559b6950b2ba4318ca44948350779e5f364a5f58 Mon Sep 17 00:00:00 2001 From: greenmashimaro Date: Tue, 23 Aug 2022 15:09:52 +0800 Subject: [PATCH 18/55] remove unnecessary eslint comments --- src/browser/LocalizableStrings.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/browser/LocalizableStrings.ts b/src/browser/LocalizableStrings.ts index c0a904cb..e34eaa48 100644 --- a/src/browser/LocalizableStrings.ts +++ b/src/browser/LocalizableStrings.ts @@ -3,8 +3,5 @@ * @license MIT */ -// eslint-disable-next-line prefer-const -export let promptLabel = 'Terminal input'; - -// eslint-disable-next-line prefer-const -export let tooMuchOutput = 'Too much output to announce, navigate to rows manually to read'; +export const promptLabel = 'Terminal input'; +export const tooMuchOutput = 'Too much output to announce, navigate to rows manually to read'; From ac81d3f72dd6a57e169dbf57b2942a7d1f87eadf Mon Sep 17 00:00:00 2001 From: greenmashimaro Date: Wed, 24 Aug 2022 22:41:45 +0800 Subject: [PATCH 19/55] feat: restore LocalizableStrings.ts const export --- src/browser/LocalizableStrings.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/browser/LocalizableStrings.ts b/src/browser/LocalizableStrings.ts index e34eaa48..fcf677c0 100644 --- a/src/browser/LocalizableStrings.ts +++ b/src/browser/LocalizableStrings.ts @@ -3,5 +3,12 @@ * @license MIT */ -export const promptLabel = 'Terminal input'; -export const tooMuchOutput = 'Too much output to announce, navigate to rows manually to read'; +/* + * note: intentional design, it's exposed to embedders of xterm.js + * so they can change the strings to localize xterm.js. #4055 + */ +// eslint-disable-next-line prefer-const +export let promptLabel = 'Terminal input'; + +// eslint-disable-next-line prefer-const +export let tooMuchOutput = 'Too much output to announce, navigate to rows manually to read'; From 86ba6cd2882fb0dfff2ac151b6f75f0a2bac76d5 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Wed, 24 Aug 2022 08:55:11 -0700 Subject: [PATCH 20/55] Tweak comment --- src/browser/LocalizableStrings.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/browser/LocalizableStrings.ts b/src/browser/LocalizableStrings.ts index fcf677c0..d8bcc2c6 100644 --- a/src/browser/LocalizableStrings.ts +++ b/src/browser/LocalizableStrings.ts @@ -3,10 +3,8 @@ * @license MIT */ -/* - * note: intentional design, it's exposed to embedders of xterm.js - * so they can change the strings to localize xterm.js. #4055 - */ +// This file contains strings that get exported in the API so they can be localized + // eslint-disable-next-line prefer-const export let promptLabel = 'Terminal input'; From e40cf0684eba2314af92467768b6546255760438 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Wed, 24 Aug 2022 15:36:03 -0700 Subject: [PATCH 21/55] Properly clear underscore glyph's background See microsoft/vscode#158497 --- addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts index 13dceae6..33b514b9 100644 --- a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts +++ b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts @@ -560,7 +560,10 @@ export class WebglCharAtlas implements IDisposable { let isBeyondCellBounds = clearColor(this._tmpCtx.getImageData(padding, padding, this._config.scaledCellWidth, this._config.scaledCellHeight), backgroundColor, foregroundColor, enableClearThresholdCheck); if (isBeyondCellBounds) { for (let offset = 1; offset <= 5; offset++) { - this._tmpCtx.clearRect(0, 0, this._tmpCanvas.width, this._tmpCanvas.height); + this._tmpCtx.save(); + this._tmpCtx.fillStyle = backgroundColor.css; + this._tmpCtx.fillRect(0, 0, this._tmpCanvas.width, this._tmpCanvas.height); + this._tmpCtx.restore(); this._tmpCtx.fillText(chars, padding, padding + this._config.scaledCharHeight - offset); isBeyondCellBounds = clearColor(this._tmpCtx.getImageData(padding, padding, this._config.scaledCellWidth, this._config.scaledCellHeight), backgroundColor, foregroundColor, enableClearThresholdCheck); if (!isBeyondCellBounds) { From 774ebe40e6b91cf1e81e0834876f7932266502c8 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 26 Aug 2022 07:26:36 -0700 Subject: [PATCH 22/55] Improve glyph padding, especially in large font sizes Fixes #4066 --- .../xterm-addon-canvas/src/BaseRenderLayer.ts | 4 +- .../src/atlas/WebglCharAtlas.ts | 2 +- src/browser/renderer/CustomGlyphs.ts | 49 ++++++++++++------- 3 files changed, 35 insertions(+), 20 deletions(-) diff --git a/addons/xterm-addon-canvas/src/BaseRenderLayer.ts b/addons/xterm-addon-canvas/src/BaseRenderLayer.ts index a69968cc..dfd814e3 100644 --- a/addons/xterm-addon-canvas/src/BaseRenderLayer.ts +++ b/addons/xterm-addon-canvas/src/BaseRenderLayer.ts @@ -344,7 +344,7 @@ export abstract class BaseRenderLayer implements IRenderLayer { // Draw custom characters if applicable let drawSuccess = false; if (this._optionsService.rawOptions.customGlyphs !== false) { - drawSuccess = tryDrawCustomChar(this._ctx, cell.getChars(), x * this._scaledCellWidth, y * this._scaledCellHeight, this._scaledCellWidth, this._scaledCellHeight); + drawSuccess = tryDrawCustomChar(this._ctx, cell.getChars(), x * this._scaledCellWidth, y * this._scaledCellHeight, this._scaledCellWidth, this._scaledCellHeight, this._optionsService.rawOptions.fontSize); } // Draw the character @@ -473,7 +473,7 @@ export abstract class BaseRenderLayer implements IRenderLayer { // Draw custom characters if applicable let drawSuccess = false; if (this._optionsService.rawOptions.customGlyphs !== false) { - drawSuccess = tryDrawCustomChar(this._ctx, cell.getChars(), x * this._scaledCellWidth, y * this._scaledCellHeight, this._scaledCellWidth, this._scaledCellHeight); + drawSuccess = tryDrawCustomChar(this._ctx, cell.getChars(), x * this._scaledCellWidth, y * this._scaledCellHeight, this._scaledCellWidth, this._scaledCellHeight, this._optionsService.rawOptions.fontSize); } // Draw the character diff --git a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts index 90d13e55..2321e546 100644 --- a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts +++ b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts @@ -427,7 +427,7 @@ export class WebglCharAtlas implements IDisposable { // Draw custom characters if applicable let customGlyph = false; if (this._config.customGlyphs !== false) { - customGlyph = tryDrawCustomChar(this._tmpCtx, chars, padding, padding, this._config.scaledCellWidth, this._config.scaledCellHeight); + customGlyph = tryDrawCustomChar(this._tmpCtx, chars, padding, padding, this._config.scaledCellWidth, this._config.scaledCellHeight, this._config.fontSize); } // Whether to clear pixels based on a threshold difference between the glyph color and the diff --git a/src/browser/renderer/CustomGlyphs.ts b/src/browser/renderer/CustomGlyphs.ts index 883b4d5d..2f32f981 100644 --- a/src/browser/renderer/CustomGlyphs.ts +++ b/src/browser/renderer/CustomGlyphs.ts @@ -349,25 +349,27 @@ const enum VectorType { * not been patched with powerline characters and also to get pixel perfect rendering as rendering * issues can occur around AA/SPAA. * + * The line variants draw beyond the cell and get clipped to ensure the end of the line is not visible. + * * Original symbols defined in https://github.com/powerline/fontpatcher */ export const powerlineDefinitions: { [index: string]: IVectorShape } = { // Right triangle solid - '\u{E0B0}': { d: 'M0,0 L1,.5 L0,1', type: VectorType.FILL }, + '\u{E0B0}': { d: 'M0,0 L1,.5 L0,1', type: VectorType.FILL, rightPadding: 2 }, // Right triangle line - '\u{E0B1}': { d: 'M0,0 L1,.5 L0,1', type: VectorType.STROKE, leftPadding: window.devicePixelRatio / 2, rightPadding: window.devicePixelRatio / 2 }, + '\u{E0B1}': { d: 'M-1,-.5 L1,.5 L-1,1.5', type: VectorType.STROKE, leftPadding: 1, rightPadding: 1 }, // Left triangle solid - '\u{E0B2}': { d: 'M1,0 L0,.5 L1,1', type: VectorType.FILL }, + '\u{E0B2}': { d: 'M1,0 L0,.5 L1,1', type: VectorType.FILL, leftPadding: 2 }, // Left triangle line - '\u{E0B3}': { d: 'M1,0 L0,.5 L1,1', type: VectorType.STROKE, leftPadding: window.devicePixelRatio / 2, rightPadding: window.devicePixelRatio / 2 }, + '\u{E0B3}': { d: 'M2,-.5 L0,.5 L2,1.5', type: VectorType.STROKE, leftPadding: 1, rightPadding: 1 }, // Right semi-circle solid, - '\u{E0B4}': { d: 'M0,0 L0,1 C0.552,1,1,0.776,1,.5 C1,0.224,0.552,0,0,0', type: VectorType.FILL }, + '\u{E0B4}': { d: 'M0,0 L0,1 C0.552,1,1,0.776,1,.5 C1,0.224,0.552,0,0,0', type: VectorType.FILL, rightPadding: 1 }, // Right semi-circle line, - '\u{E0B5}': { d: 'M0,1 C0.552,1,1,0.776,1,.5 C1,0.224,0.552,0,0,0', type: VectorType.STROKE, rightPadding: window.devicePixelRatio / 2 }, + '\u{E0B5}': { d: 'M0,1 C0.552,1,1,0.776,1,.5 C1,0.224,0.552,0,0,0', type: VectorType.STROKE, rightPadding: 1 }, // Left semi-circle solid, - '\u{E0B6}': { d: 'M1,0 L1,1 C0.448,1,0,0.776,0,.5 C0,0.224,0.448,0,1,0', type: VectorType.FILL }, + '\u{E0B6}': { d: 'M1,0 L1,1 C0.448,1,0,0.776,0,.5 C0,0.224,0.448,0,1,0', type: VectorType.FILL, leftPadding: 1 }, // Left semi-circle line, - '\u{E0B7}': { d: 'M1,1 C0.448,1,0,0.776,0,.5 C0,0.224,0.448,0,1,0', type: VectorType.STROKE, leftPadding: window.devicePixelRatio / 2 } + '\u{E0B7}': { d: 'M1,1 C0.448,1,0,0.776,0,.5 C0,0.224,0.448,0,1,0', type: VectorType.STROKE, leftPadding: 1 } }; /** @@ -380,7 +382,8 @@ export function tryDrawCustomChar( xOffset: number, yOffset: number, scaledCellWidth: number, - scaledCellHeight: number + scaledCellHeight: number, + fontSize: number ): boolean { const blockElementDefinition = blockElementDefinitions[c]; if (blockElementDefinition) { @@ -402,7 +405,7 @@ export function tryDrawCustomChar( const powerlineDefinition = powerlineDefinitions[c]; if (powerlineDefinition) { - drawPowerlineChar(ctx, powerlineDefinition, xOffset, yOffset, scaledCellWidth, scaledCellHeight); + drawPowerlineChar(ctx, powerlineDefinition, xOffset, yOffset, scaledCellWidth, scaledCellHeight, fontSize); return true; } @@ -562,7 +565,7 @@ function drawBoxDrawingChar( if (!args[0] || !args[1]) { continue; } - f(ctx, translateArgs(args, scaledCellWidth, scaledCellHeight, xOffset, yOffset)); + f(ctx, translateArgs(args, scaledCellWidth, scaledCellHeight, xOffset, yOffset, true)); } ctx.stroke(); ctx.closePath(); @@ -575,10 +578,13 @@ function drawPowerlineChar( xOffset: number, yOffset: number, scaledCellWidth: number, - scaledCellHeight: number + scaledCellHeight: number, + fontSize: number ): void { ctx.beginPath(); - ctx.lineWidth = window.devicePixelRatio; + // Scale the stroke with DPR and font size + const cssLineWidth = fontSize / 12; + ctx.lineWidth = window.devicePixelRatio * cssLineWidth; for (const instruction of charDefinition.d.split(' ')) { const type = instruction[0]; const f = svgToCanvasInstructionMap[type]; @@ -590,7 +596,16 @@ function drawPowerlineChar( if (!args[0] || !args[1]) { continue; } - f(ctx, translateArgs(args, scaledCellWidth, scaledCellHeight, xOffset, yOffset, charDefinition.leftPadding, charDefinition.rightPadding)); + f(ctx, translateArgs( + args, + scaledCellWidth, + scaledCellHeight, + xOffset, + yOffset, + false, + (charDefinition.leftPadding ?? 0) * (cssLineWidth / 2), + (charDefinition.rightPadding ?? 0) * (cssLineWidth / 2) + )); } if (charDefinition.type === VectorType.STROKE) { ctx.strokeStyle = ctx.fillStyle; @@ -611,7 +626,7 @@ const svgToCanvasInstructionMap: { [index: string]: any } = { 'M': (ctx: CanvasRenderingContext2D, args: number[]) => ctx.moveTo(args[0], args[1]) }; -function translateArgs(args: string[], cellWidth: number, cellHeight: number, xOffset: number, yOffset: number, leftPadding: number = 0, rightPadding: number = 0): number[] { +function translateArgs(args: string[], cellWidth: number, cellHeight: number, xOffset: number, yOffset: number, doClamp: boolean, leftPadding: number = 0, rightPadding: number = 0): number[] { const result = args.map(e => parseFloat(e) || parseInt(e)); if (result.length < 2) { @@ -623,7 +638,7 @@ function translateArgs(args: string[], cellWidth: number, cellHeight: number, xO result[x] *= cellWidth - (leftPadding * window.devicePixelRatio) - (rightPadding * window.devicePixelRatio); // Ensure coordinate doesn't escape cell bounds and round to the nearest 0.5 to ensure a crisp // line at 100% devicePixelRatio - if (result[x] !== 0) { + if (doClamp && result[x] !== 0) { result[x] = clamp(Math.round(result[x] + 0.5) - 0.5, cellWidth, 0); } // Apply the cell's offset (ie. x*cellWidth) @@ -635,7 +650,7 @@ function translateArgs(args: string[], cellWidth: number, cellHeight: number, xO result[y] *= cellHeight; // Ensure coordinate doesn't escape cell bounds and round to the nearest 0.5 to ensure a crisp // line at 100% devicePixelRatio - if (result[y] !== 0) { + if (doClamp && result[y] !== 0) { result[y] = clamp(Math.round(result[y] + 0.5) - 0.5, cellHeight, 0); } // Apply the cell's offset (ie. x*cellHeight) From 4382a551d146a806a896634d25c53210c3e0035b Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 26 Aug 2022 07:53:20 -0700 Subject: [PATCH 23/55] Use powerline extra symbols in demo font family --- demo/client.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/demo/client.ts b/demo/client.ts index 64c694f5..bcc79a45 100644 --- a/demo/client.ts +++ b/demo/client.ts @@ -203,7 +203,7 @@ function createTerminal(): void { term = new Terminal({ allowProposedApi: true, windowsMode: isWindows, - fontFamily: 'Fira Code, courier-new, courier, monospace', + fontFamily: '"Fira Code", courier-new, courier, monospace, "Powerline Extra Symbols"', theme: xtermjsTheme } as ITerminalOptions); From f6860442fb259e0c33b88b20d8d0911da372d782 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 26 Aug 2022 08:12:00 -0700 Subject: [PATCH 24/55] Show entire glyph for extra powerline symbols Fixes #4072 --- addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts | 5 +++-- src/browser/renderer/RendererUtils.ts | 8 ++++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts index 90d13e55..4be02aab 100644 --- a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts +++ b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts @@ -13,7 +13,7 @@ import { IDisposable } from 'xterm'; import { AttributeData } from 'common/buffer/AttributeData'; import { color, rgba } from 'common/Color'; import { tryDrawCustomChar } from 'browser/renderer/CustomGlyphs'; -import { excludeFromContrastRatioDemands, isPowerlineGlyph } from 'browser/renderer/RendererUtils'; +import { excludeFromContrastRatioDemands, isPowerlineGlyph, isRestrictedPowerlineGlyph } from 'browser/renderer/RendererUtils'; // For debugging purposes, it can be useful to set this to a really tiny value, // to verify that LRU eviction works. @@ -418,6 +418,7 @@ export class WebglCharAtlas implements IDisposable { this._tmpCtx.textBaseline = TEXT_BASELINE; const powerlineGlyph = chars.length === 1 && isPowerlineGlyph(chars.charCodeAt(0)); + const restrictedPowerlineGlyph = chars.length === 1 && isRestrictedPowerlineGlyph(chars.charCodeAt(0)); const foregroundColor = this._getForegroundColor(bg, bgColorMode, bgColor, fg, fgColorMode, fgColor, inverse, dim, bold, excludeFromContrastRatioDemands(chars.charCodeAt(0))); this._tmpCtx.fillStyle = foregroundColor.css; @@ -606,7 +607,7 @@ export class WebglCharAtlas implements IDisposable { return NULL_RASTERIZED_GLYPH; } - const rasterizedGlyph = this._findGlyphBoundingBox(imageData, this._workBoundingBox, allowedWidth, powerlineGlyph, customGlyph, padding); + 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 diff --git a/src/browser/renderer/RendererUtils.ts b/src/browser/renderer/RendererUtils.ts index 0a4a77e8..0f60dc29 100644 --- a/src/browser/renderer/RendererUtils.ts +++ b/src/browser/renderer/RendererUtils.ts @@ -14,11 +14,15 @@ export function isPowerlineGlyph(codepoint: number): boolean { // Only return true for Powerline symbols which require // different padding and should be excluded from minimum contrast // ratio standards - return 0xE0A4 <= codepoint && codepoint <= 0xE0D6; + return 0xE0A4 <= codepoint && codepoint <= 0xE0D6; +} + +export function isRestrictedPowerlineGlyph(codepoint: number): boolean { + return 0xE0B0 <= codepoint && codepoint <= 0xE0B7; } function isBoxOrBlockGlyph(codepoint: number): boolean { - return (0x2500 <= codepoint && codepoint <= 0x259F); + return 0x2500 <= codepoint && codepoint <= 0x259F; } export function excludeFromContrastRatioDemands(codepoint: number): boolean { From 4979e9d2be746ff526647726e8615cba87c5c4bf Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 26 Aug 2022 13:26:00 -0700 Subject: [PATCH 25/55] Show test characters in underline test --- demo/client.ts | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/demo/client.ts b/demo/client.ts index 64c694f5..a053b59e 100644 --- a/demo/client.ts +++ b/demo/client.ts @@ -766,12 +766,23 @@ function underlineTest() { term.write('\n\n\r'); term.writeln('Underline styles:'); term.writeln(''); - term.writeln(`${u(0)}4:0m - No underline`); - term.writeln(`${u(1)}4:1m - Straight`); - term.writeln(`${u(2)}4:2m - Double`); - term.writeln(`${u(3)}4:3m - Curly`); - term.writeln(`${u(4)}4:4m - Dotted`); - term.writeln(`${u(5)}4:5m - Dashed\x1b[0m`); + function showSequence(id: number, name: string) { + let alphabet = ''; + for (let i = 97; i < 123; i++) { + alphabet += String.fromCharCode(i); + } + let numbers = ''; + for (let i = 0; i < 10; i++) { + numbers += i.toString(); + } + return `${u(id)}4:${id}m - ${name}\x1b[4:0m`.padEnd(33, ' ') + `${u(id)}${alphabet} ${numbers}\x1b[4:0m`; + } + term.writeln(showSequence(0, 'No underline')); + term.writeln(showSequence(1, 'Straight')); + term.writeln(showSequence(2, 'Double')); + term.writeln(showSequence(3, 'Curly')); + term.writeln(showSequence(4, 'Dotted')); + term.writeln(showSequence(5, 'Dashed')); term.writeln(''); term.writeln(`Underline colors (256 color mode):`); term.writeln(''); From 7ffd9e1f7f24e35d79796118535177d59cc96f97 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 26 Aug 2022 14:16:02 -0700 Subject: [PATCH 26/55] Only draw character outline if it has a descent Fixes #4059 --- .../src/atlas/WebglCharAtlas.ts | 28 ++++++++++++------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts index 90d13e55..2af88b92 100644 --- a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts +++ b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts @@ -533,18 +533,26 @@ export class WebglCharAtlas implements IDisposable { // This only works when transparency is disabled because it's not clear how to clear stroked // text if (!this._config.allowTransparency && chars !== ' ') { - // This translates to 1/2 the line width in either direction + // Measure the text, only draw the stroke if there is a descent beyond an alphabetic text + // baseline this._tmpCtx.save(); - // Clip the region to only draw in valid pixels near the underline to avoid a slight - // outline around the whole glyph, as well as additional pixels in the glyph at the top - // which would increase GPU memory demands - const clipRegion = new Path2D(); - clipRegion.rect(xLeft, yTop - Math.ceil(lineWidth / 2), this._config.scaledCellWidth, yBot - yTop + Math.ceil(lineWidth / 2)); - this._tmpCtx.clip(clipRegion); - this._tmpCtx.lineWidth = window.devicePixelRatio * 3; - this._tmpCtx.strokeStyle = backgroundColor.css; - this._tmpCtx.strokeText(chars, padding, padding + this._config.scaledCharHeight); + this._tmpCtx.textBaseline = 'alphabetic'; + const metrics = this._tmpCtx.measureText(chars); this._tmpCtx.restore(); + if ('actualBoundingBoxDescent' in metrics && metrics.actualBoundingBoxDescent > 0) { + // This translates to 1/2 the line width in either direction + this._tmpCtx.save(); + // Clip the region to only draw in valid pixels near the underline to avoid a slight + // outline around the whole glyph, as well as additional pixels in the glyph at the top + // which would increase GPU memory demands + const clipRegion = new Path2D(); + clipRegion.rect(xLeft, yTop - Math.ceil(lineWidth / 2), this._config.scaledCellWidth, yBot - yTop + Math.ceil(lineWidth / 2)); + this._tmpCtx.clip(clipRegion); + this._tmpCtx.lineWidth = window.devicePixelRatio * 3; + this._tmpCtx.strokeStyle = backgroundColor.css; + this._tmpCtx.strokeText(chars, padding, padding + this._config.scaledCharHeight); + this._tmpCtx.restore(); + } } } } From 903830ce1f29f9f3a6842d45a3e8b6180af353aa Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 27 Aug 2022 06:06:17 -0700 Subject: [PATCH 27/55] Add CJK chars to underline test --- demo/client.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/demo/client.ts b/demo/client.ts index e06eb0a2..cd39b8dd 100644 --- a/demo/client.ts +++ b/demo/client.ts @@ -775,7 +775,7 @@ function underlineTest() { for (let i = 0; i < 10; i++) { numbers += i.toString(); } - return `${u(id)}4:${id}m - ${name}\x1b[4:0m`.padEnd(33, ' ') + `${u(id)}${alphabet} ${numbers}\x1b[4:0m`; + return `${u(id)}4:${id}m - ${name}\x1b[4:0m`.padEnd(33, ' ') + `${u(id)}${alphabet} ${numbers} 汉语 한국어\x1b[4:0m`; } term.writeln(showSequence(0, 'No underline')); term.writeln(showSequence(1, 'Straight')); From e76918eddc824054fc15182123e5550f98cf11a7 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 27 Aug 2022 06:34:38 -0700 Subject: [PATCH 28/55] Take char width into account when drawing underlines/strokethrough Fixes #4063 --- .../src/atlas/CharAtlasCache.ts | 5 +- .../src/atlas/WebglCharAtlas.ts | 136 ++++++++++-------- 2 files changed, 78 insertions(+), 63 deletions(-) diff --git a/addons/xterm-addon-webgl/src/atlas/CharAtlasCache.ts b/addons/xterm-addon-webgl/src/atlas/CharAtlasCache.ts index 41114de0..6aba2125 100644 --- a/addons/xterm-addon-webgl/src/atlas/CharAtlasCache.ts +++ b/addons/xterm-addon-webgl/src/atlas/CharAtlasCache.ts @@ -7,7 +7,7 @@ import { generateConfig, configEquals } from './CharAtlasUtils'; import { WebglCharAtlas } from './WebglCharAtlas'; import { ICharAtlasConfig } from './Types'; import { Terminal } from 'xterm'; -import { IColorSet } from 'browser/Types'; +import { IColorSet, ITerminal } from 'browser/Types'; interface ICharAtlasCacheEntry { atlas: WebglCharAtlas; @@ -64,8 +64,9 @@ export function acquireCharAtlas( } } + const core: ITerminal = (terminal as any)._core; const newEntry: ICharAtlasCacheEntry = { - atlas: new WebglCharAtlas(document, newConfig), + atlas: new WebglCharAtlas(document, newConfig, core.unicodeService), config: newConfig, ownedBy: [terminal] }; diff --git a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts index 9ede81c5..5685012d 100644 --- a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts +++ b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts @@ -14,6 +14,7 @@ import { AttributeData } from 'common/buffer/AttributeData'; import { color, rgba } from 'common/Color'; import { tryDrawCustomChar } from 'browser/renderer/CustomGlyphs'; import { excludeFromContrastRatioDemands, isPowerlineGlyph, isRestrictedPowerlineGlyph } from 'browser/renderer/RendererUtils'; +import { IUnicodeService } from 'common/services/Services'; // For debugging purposes, it can be useful to set this to a really tiny value, // to verify that LRU eviction works. @@ -89,7 +90,8 @@ export class WebglCharAtlas implements IDisposable { constructor( document: Document, - private _config: ICharAtlasConfig + private readonly _config: ICharAtlasConfig, + private readonly _unicodeService: IUnicodeService ) { this.cacheCanvas = document.createElement('canvas'); this.cacheCanvas.width = TEXTURE_WIDTH; @@ -436,6 +438,11 @@ export class WebglCharAtlas implements IDisposable { // underline colors to prevent important colors could get cleared. let enableClearThresholdCheck = !powerlineGlyph; + let chWidth: number = 1; + if (typeof codeOrChars === 'number' && (underline || strikethrough)) { + chWidth = this._unicodeService.wcwidth(codeOrChars); + } + // Draw underline if (underline) { this._tmpCtx.save(); @@ -462,69 +469,76 @@ export class WebglCharAtlas implements IDisposable { // Underline style/stroke this._tmpCtx.beginPath(); const xLeft = padding; - const xRight = padding + this._config.scaledCellWidth; + const xRight = padding + this._config.scaledCellWidth * chWidth; const yTop = Math.ceil(padding + this._config.scaledCharHeight) - yOffset; const yMid = padding + this._config.scaledCharHeight + lineWidth - yOffset; const yBot = Math.ceil(padding + this._config.scaledCharHeight + lineWidth * 2) - yOffset; - switch (this._workAttributeData.extended.underlineStyle) { - case UnderlineStyle.DOUBLE: - this._tmpCtx.moveTo(xLeft, yTop); - this._tmpCtx.lineTo(xRight, yTop); - this._tmpCtx.moveTo(xLeft, yBot); - this._tmpCtx.lineTo(xRight, yBot); - break; - case UnderlineStyle.CURLY: - const xMid = padding + this._config.scaledCellWidth / 2; - // Choose the bezier top and bottom based on the device pixel ratio, the curly line is - // made taller when the line width is as otherwise it's not very clear otherwise. - const yCurlyBot = lineWidth <= 1 ? yBot : Math.ceil(padding + this._config.scaledCharHeight - lineWidth / 2) - yOffset; - const yCurlyTop = lineWidth <= 1 ? yTop : Math.ceil(padding + this._config.scaledCharHeight + lineWidth / 2) - yOffset; - // Clip the left and right edges of the underline such that it can be drawn just outside - // the edge of the cell to ensure a continuous stroke when there are multiple underlined - // glyphs adjacent to one another. - const clipRegion = new Path2D(); - clipRegion.rect(xLeft, yTop, this._config.scaledCellWidth, yBot - yTop); - this._tmpCtx.clip(clipRegion); - // Start 1/2 cell before and end 1/2 cells after to ensure a smooth curve with other cells - this._tmpCtx.moveTo(xLeft - this._config.scaledCellWidth / 2, yMid); - this._tmpCtx.bezierCurveTo( - xLeft - this._config.scaledCellWidth / 2, yCurlyTop, - xLeft, yCurlyTop, - xLeft, yMid - ); - this._tmpCtx.bezierCurveTo( - xLeft, yCurlyBot, - xMid, yCurlyBot, - xMid, yMid - ); - this._tmpCtx.bezierCurveTo( - xMid, yCurlyTop, - xRight, yCurlyTop, - xRight, yMid - ); - this._tmpCtx.bezierCurveTo( - xRight, yCurlyBot, - xRight + this._config.scaledCellWidth / 2, yCurlyBot, - xRight + this._config.scaledCellWidth / 2, yMid - ); - break; - case UnderlineStyle.DOTTED: - this._tmpCtx.setLineDash([window.devicePixelRatio * 2, window.devicePixelRatio]); - this._tmpCtx.moveTo(xLeft, yTop); - this._tmpCtx.lineTo(xRight, yTop); - break; - case UnderlineStyle.DASHED: - this._tmpCtx.setLineDash([window.devicePixelRatio * 4, window.devicePixelRatio * 3]); - this._tmpCtx.moveTo(xLeft, yTop); - this._tmpCtx.lineTo(xRight, yTop); - break; - case UnderlineStyle.SINGLE: - default: - this._tmpCtx.moveTo(xLeft, yTop); - this._tmpCtx.lineTo(xRight, yTop); - break; + + for (let i = 0; i < chWidth; i++) { + this._tmpCtx.save(); + const xChLeft = xLeft + i * this._config.scaledCellWidth; + const xChRight = xLeft + (i + 1) * this._config.scaledCellWidth; + const xChMid = xChLeft + this._config.scaledCellWidth / 2; + switch (this._workAttributeData.extended.underlineStyle) { + case UnderlineStyle.DOUBLE: + this._tmpCtx.moveTo(xChLeft, yTop); + this._tmpCtx.lineTo(xChRight, yTop); + this._tmpCtx.moveTo(xChLeft, yBot); + this._tmpCtx.lineTo(xChRight, yBot); + break; + case UnderlineStyle.CURLY: + // Choose the bezier top and bottom based on the device pixel ratio, the curly line is + // made taller when the line width is as otherwise it's not very clear otherwise. + const yCurlyBot = lineWidth <= 1 ? yBot : Math.ceil(padding + this._config.scaledCharHeight - lineWidth / 2) - yOffset; + const yCurlyTop = lineWidth <= 1 ? yTop : Math.ceil(padding + this._config.scaledCharHeight + lineWidth / 2) - yOffset; + // Clip the left and right edges of the underline such that it can be drawn just outside + // the edge of the cell to ensure a continuous stroke when there are multiple underlined + // glyphs adjacent to one another. + const clipRegion = new Path2D(); + clipRegion.rect(xChLeft, yTop, this._config.scaledCellWidth, yBot - yTop); + this._tmpCtx.clip(clipRegion); + // Start 1/2 cell before and end 1/2 cells after to ensure a smooth curve with other cells + this._tmpCtx.moveTo(xChLeft - this._config.scaledCellWidth / 2, yMid); + this._tmpCtx.bezierCurveTo( + xChLeft - this._config.scaledCellWidth / 2, yCurlyTop, + xChLeft, yCurlyTop, + xChLeft, yMid + ); + this._tmpCtx.bezierCurveTo( + xChLeft, yCurlyBot, + xChMid, yCurlyBot, + xChMid, yMid + ); + this._tmpCtx.bezierCurveTo( + xChMid, yCurlyTop, + xChRight, yCurlyTop, + xChRight, yMid + ); + this._tmpCtx.bezierCurveTo( + xChRight, yCurlyBot, + xChRight + this._config.scaledCellWidth / 2, yCurlyBot, + xChRight + this._config.scaledCellWidth / 2, yMid + ); + break; + case UnderlineStyle.DOTTED: + this._tmpCtx.setLineDash([window.devicePixelRatio * 2, window.devicePixelRatio]); + this._tmpCtx.moveTo(xChLeft, yTop); + this._tmpCtx.lineTo(xChRight, yTop); + break; + case UnderlineStyle.DASHED: + this._tmpCtx.setLineDash([window.devicePixelRatio * 4, window.devicePixelRatio * 3]); + this._tmpCtx.moveTo(xChLeft, yTop); + this._tmpCtx.lineTo(xChRight, yTop); + break; + case UnderlineStyle.SINGLE: + default: + this._tmpCtx.moveTo(xChLeft, yTop); + this._tmpCtx.lineTo(xChRight, yTop); + break; + } + this._tmpCtx.stroke(); + this._tmpCtx.restore(); } - this._tmpCtx.stroke(); this._tmpCtx.restore(); // Draw stroke in the background color for non custom characters in order to give an outline @@ -590,7 +604,7 @@ export class WebglCharAtlas implements IDisposable { this._tmpCtx.strokeStyle = this._tmpCtx.fillStyle; this._tmpCtx.beginPath(); this._tmpCtx.moveTo(padding, padding + Math.floor(this._config.scaledCharHeight / 2) - yOffset); - this._tmpCtx.lineTo(padding + this._config.scaledCharWidth, padding + Math.floor(this._config.scaledCharHeight / 2) - yOffset); + this._tmpCtx.lineTo(padding + this._config.scaledCharWidth * chWidth, padding + Math.floor(this._config.scaledCharHeight / 2) - yOffset); this._tmpCtx.stroke(); } From 4ecc098de42dcb5f07e5d96696dccc6e70a5b727 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 27 Aug 2022 06:38:15 -0700 Subject: [PATCH 29/55] Get width of emoji/combined chars --- addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts index 5685012d..e1d56b78 100644 --- a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts +++ b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts @@ -438,9 +438,11 @@ export class WebglCharAtlas implements IDisposable { // underline colors to prevent important colors could get cleared. let enableClearThresholdCheck = !powerlineGlyph; - let chWidth: number = 1; - if (typeof codeOrChars === 'number' && (underline || strikethrough)) { + let chWidth: number; + if (typeof codeOrChars === 'number') { chWidth = this._unicodeService.wcwidth(codeOrChars); + } else { + chWidth = this._unicodeService.getStringCellWidth(codeOrChars); } // Draw underline From 4d837e280222c3cf101fbfc31b25bb895681ecc5 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 27 Aug 2022 06:39:07 -0700 Subject: [PATCH 30/55] Add an emoji to underline test --- demo/client.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/demo/client.ts b/demo/client.ts index cd39b8dd..8e03ec3c 100644 --- a/demo/client.ts +++ b/demo/client.ts @@ -775,7 +775,7 @@ function underlineTest() { for (let i = 0; i < 10; i++) { numbers += i.toString(); } - return `${u(id)}4:${id}m - ${name}\x1b[4:0m`.padEnd(33, ' ') + `${u(id)}${alphabet} ${numbers} 汉语 한국어\x1b[4:0m`; + return `${u(id)}4:${id}m - ${name}\x1b[4:0m`.padEnd(33, ' ') + `${u(id)}${alphabet} ${numbers} 汉语 한국어 👽\x1b[4:0m`; } term.writeln(showSequence(0, 'No underline')); term.writeln(showSequence(1, 'Straight')); From 0237b2965e8b5831bdb31aeefc2b45d97e95cc75 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 27 Aug 2022 19:28:20 -0700 Subject: [PATCH 31/55] Fix resize on demo --- demo/client.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/demo/client.ts b/demo/client.ts index 8e03ec3c..c8fcfb79 100644 --- a/demo/client.ts +++ b/demo/client.ts @@ -406,7 +406,11 @@ function initOptions(term: TerminalType): void { const input = document.getElementById(`opt-${o}`); addDomListener(input, 'change', () => { console.log('change', o, input.value); - if (o === 'lineHeight') { + if (o === 'rows') { + term.resize(term.cols, parseInt(input.value)); + } else if (o === 'cols') { + term.resize(parseInt(input.value), term.rows); + } else if (o === 'lineHeight') { term.options.lineHeight = parseFloat(input.value); } else if (o === 'scrollSensitivity') { term.options.scrollSensitivity = parseFloat(input.value); From 5e3f63e876dcf875b7509c951c01818699a42370 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 27 Aug 2022 20:06:58 -0700 Subject: [PATCH 32/55] Avoid garbage collection in webgl decoration hot code Part of #4079 --- addons/xterm-addon-webgl/src/WebglRenderer.ts | 81 ++++++++++++------- src/common/SortedList.ts | 25 +++++- src/common/TestUtils.test.ts | 1 + src/common/services/DecorationService.ts | 16 ++++ src/common/services/Services.ts | 5 ++ 5 files changed, 94 insertions(+), 34 deletions(-) diff --git a/addons/xterm-addon-webgl/src/WebglRenderer.ts b/addons/xterm-addon-webgl/src/WebglRenderer.ts index 9e90df02..efb80f66 100644 --- a/addons/xterm-addon-webgl/src/WebglRenderer.ts +++ b/addons/xterm-addon-webgl/src/WebglRenderer.ts @@ -11,7 +11,7 @@ import { WebglCharAtlas } from './atlas/WebglCharAtlas'; import { RectangleRenderer } from './RectangleRenderer'; import { IWebGL2RenderingContext } from './Types'; import { RenderModel, COMBINED_CHAR_BIT_MASK, RENDER_MODEL_BG_OFFSET, RENDER_MODEL_FG_OFFSET, RENDER_MODEL_EXT_OFFSET, RENDER_MODEL_INDICIES_PER_CELL } from './RenderModel'; -import { Disposable, toDisposable } from 'common/Lifecycle'; +import { Disposable } from 'common/Lifecycle'; import { Attributes, BgFlags, Content, FgFlags, NULL_CELL_CHAR, NULL_CELL_CODE } from 'common/buffer/Constants'; import { Terminal, IEvent } from 'xterm'; import { IRenderLayer } from './renderLayer/Types'; @@ -26,6 +26,15 @@ import { CharData, ICellData } from 'common/Types'; import { AttributeData } from 'common/buffer/AttributeData'; import { ICoreService, IDecorationService } from 'common/services/Services'; +/** Work variables to avoid garbage collection. */ +const w: { fg: number, bg: number, hasFg: boolean, hasBg: boolean, isSelected: boolean } = { + fg: 0, + bg: 0, + hasFg: false, + hasBg: false, + isSelected: false +}; + export class WebglRenderer extends Disposable implements IRenderer { private _renderLayers: IRenderLayer[]; private _charAtlas: WebglCharAtlas | undefined; @@ -404,79 +413,89 @@ export class WebglRenderer extends Disposable implements IRenderer { this._workColors.ext = this._workCell.bg & BgFlags.HAS_EXTENDED ? this._workCell.extended.ext : 0; // Get any foreground/background overrides, this happens on the model to avoid spreading // override logic throughout the different sub-renderers - let bgOverride: number | undefined; - let fgOverride: number | undefined; - let isSelected: boolean = false; + + // Reset overrides work variables + w.bg = 0; + w.fg = 0; + w.hasBg = false; + w.hasFg = false; + w.isSelected = false; // Apply decorations on the bottom layer - for (const d of this._decorationService.getDecorationsAtCell(x, y, 'bottom')) { + this._decorationService.forEachDecorationAtCell(x, y, 'bottom', d => { if (d.backgroundColorRGB) { - bgOverride = d.backgroundColorRGB.rgba >> 8 & 0xFFFFFF; + w.bg = d.backgroundColorRGB.rgba >> 8 & 0xFFFFFF; + w.hasBg = true; } if (d.foregroundColorRGB) { - fgOverride = d.foregroundColorRGB.rgba >> 8 & 0xFFFFFF; + w.fg = d.foregroundColorRGB.rgba >> 8 & 0xFFFFFF; + w.hasFg = true; } - } + }); // Apply the selection color if needed - isSelected = this._isCellSelected(x, y); - if (isSelected) { - bgOverride = (this._coreBrowserService.isFocused ? this._colors.selectionBackgroundOpaque : this._colors.selectionInactiveBackgroundOpaque).rgba >> 8 & 0xFFFFFF; + w.isSelected = this._isCellSelected(x, y); + if (w.isSelected) { + w.bg = (this._coreBrowserService.isFocused ? this._colors.selectionBackgroundOpaque : this._colors.selectionInactiveBackgroundOpaque).rgba >> 8 & 0xFFFFFF; + w.hasBg = true; if (this._colors.selectionForeground) { - fgOverride = this._colors.selectionForeground.rgba >> 8 & 0xFFFFFF; + w.fg = this._colors.selectionForeground.rgba >> 8 & 0xFFFFFF; + w.hasFg = true; } } // Apply decorations on the top layer - for (const d of this._decorationService.getDecorationsAtCell(x, y, 'top')) { + this._decorationService.forEachDecorationAtCell(x, y, 'top', d => { if (d.backgroundColorRGB) { - bgOverride = d.backgroundColorRGB.rgba >> 8 & 0xFFFFFF; + w.bg = d.backgroundColorRGB.rgba >> 8 & 0xFFFFFF; + w.hasBg = true; } if (d.foregroundColorRGB) { - fgOverride = d.foregroundColorRGB.rgba >> 8 & 0xFFFFFF; + w.fg = d.foregroundColorRGB.rgba >> 8 & 0xFFFFFF; + w.hasFg = true; } - } + }); // Convert any overrides from rgba to the fg/bg packed format. This resolves the inverse flag // ahead of time in order to use the correct cache key - if (bgOverride !== undefined) { - if (isSelected) { + if (w.hasBg) { + if (w.isSelected) { // Non-RGB attributes from model + force non-dim + override + force RGB color mode - bgOverride = (this._workCell.bg & ~Attributes.RGB_MASK & ~BgFlags.DIM) | bgOverride | Attributes.CM_RGB; + w.bg = (this._workCell.bg & ~Attributes.RGB_MASK & ~BgFlags.DIM) | w.bg | Attributes.CM_RGB; } else { // Non-RGB attributes from model + override + force RGB color mode - bgOverride = (this._workCell.bg & ~Attributes.RGB_MASK) | bgOverride | Attributes.CM_RGB; + w.bg = (this._workCell.bg & ~Attributes.RGB_MASK) | w.bg | Attributes.CM_RGB; } } - if (fgOverride !== undefined) { + if (w.hasFg) { // Non-RGB attributes from model + force disable inverse + override + force RGB color mode - fgOverride = (this._workCell.fg & ~Attributes.RGB_MASK & ~FgFlags.INVERSE) | fgOverride | Attributes.CM_RGB; + w.fg = (this._workCell.fg & ~Attributes.RGB_MASK & ~FgFlags.INVERSE) | w.fg | Attributes.CM_RGB; } - // Handle case where inverse was specified by only one of bgOverride or fgOverride was set, + // Handle case where inverse was specified by only one of bg override or fg override was set, // resolving the other inverse color and setting the inverse flag if needed. if (this._workColors.fg & FgFlags.INVERSE) { - if (bgOverride !== undefined && fgOverride === undefined) { + if (w.hasBg && w.hasFg) { // Resolve bg color type (default color has a different meaning in fg vs bg) if ((this._workColors.bg & Attributes.CM_MASK) === Attributes.CM_DEFAULT) { - fgOverride = (this._workColors.fg & ~(Attributes.RGB_MASK | FgFlags.INVERSE | Attributes.CM_MASK)) | ((this._colors.background.rgba >> 8 & 0xFFFFFF) & Attributes.RGB_MASK) | Attributes.CM_RGB; + w.fg = (this._workColors.fg & ~(Attributes.RGB_MASK | FgFlags.INVERSE | Attributes.CM_MASK)) | ((this._colors.background.rgba >> 8 & 0xFFFFFF) & Attributes.RGB_MASK) | Attributes.CM_RGB; } else { - fgOverride = (this._workColors.fg & ~(Attributes.RGB_MASK | FgFlags.INVERSE | Attributes.CM_MASK)) | this._workColors.bg & (Attributes.RGB_MASK | Attributes.CM_MASK); + w.fg = (this._workColors.fg & ~(Attributes.RGB_MASK | FgFlags.INVERSE | Attributes.CM_MASK)) | this._workColors.bg & (Attributes.RGB_MASK | Attributes.CM_MASK); } } - if (bgOverride === undefined && fgOverride !== undefined) { + if (w.hasBg && w.hasFg) { // Resolve bg color type (default color has a different meaning in fg vs bg) if ((this._workColors.fg & Attributes.CM_MASK) === Attributes.CM_DEFAULT) { - bgOverride = (this._workColors.bg & ~(Attributes.RGB_MASK | Attributes.CM_MASK)) | ((this._colors.foreground.rgba >> 8 & 0xFFFFFF) & Attributes.RGB_MASK) | Attributes.CM_RGB; + w.bg = (this._workColors.bg & ~(Attributes.RGB_MASK | Attributes.CM_MASK)) | ((this._colors.foreground.rgba >> 8 & 0xFFFFFF) & Attributes.RGB_MASK) | Attributes.CM_RGB; } else { - bgOverride = (this._workColors.bg & ~(Attributes.RGB_MASK | Attributes.CM_MASK)) | this._workColors.fg & (Attributes.RGB_MASK | Attributes.CM_MASK); + w.bg = (this._workColors.bg & ~(Attributes.RGB_MASK | Attributes.CM_MASK)) | this._workColors.fg & (Attributes.RGB_MASK | Attributes.CM_MASK); } } } // Use the override if it exists - this._workColors.bg = bgOverride ?? this._workColors.bg; - this._workColors.fg = fgOverride ?? this._workColors.fg; + this._workColors.bg = w.bg ?? this._workColors.bg; + this._workColors.fg = w.fg ?? this._workColors.fg; } private _isCellSelected(x: number, y: number): boolean { diff --git a/src/common/SortedList.ts b/src/common/SortedList.ts index 9c819959..737b9acb 100644 --- a/src/common/SortedList.ts +++ b/src/common/SortedList.ts @@ -3,6 +3,9 @@ * @license MIT */ +// Work variables to avoid garbage collection. +let i = 0; + /** * A generic list that is maintained in sorted order and allows values with duplicate keys. This * list is based on binary search and as such locating a key will take O(log n) amortized, this @@ -25,7 +28,7 @@ export class SortedList { this._array.push(value); return; } - const i = this._search(this._getKey(value), 0, this._array.length - 1); + i = this._search(this._getKey(value), 0, this._array.length - 1); this._array.splice(i, 0, value); } @@ -37,7 +40,7 @@ export class SortedList { if (key === undefined) { return false; } - let i = this._search(key, 0, this._array.length - 1); + i = this._search(key, 0, this._array.length - 1); if (i === -1) { return false; } @@ -57,7 +60,7 @@ export class SortedList { if (this._array.length === 0) { return; } - let i = this._search(key, 0, this._array.length - 1); + i = this._search(key, 0, this._array.length - 1); if (i < 0 || i >= this._array.length) { return; } @@ -69,6 +72,22 @@ export class SortedList { } while (++i < this._array.length && this._getKey(this._array[i]) === key); } + public forEachByKey(key: number, callback: (value: T) => void): void { + if (this._array.length === 0) { + return; + } + i = this._search(key, 0, this._array.length - 1); + if (i < 0 || i >= this._array.length) { + return; + } + if (this._getKey(this._array[i]) !== key) { + return; + } + do { + callback(this._array[i]); + } while (++i < this._array.length && this._getKey(this._array[i]) === key); + } + public values(): IterableIterator { return this._array.values(); } diff --git a/src/common/TestUtils.test.ts b/src/common/TestUtils.test.ts index 48f3a69e..cbcdd36e 100644 --- a/src/common/TestUtils.test.ts +++ b/src/common/TestUtils.test.ts @@ -163,5 +163,6 @@ export class MockDecorationService implements IDecorationService { public reset(): void { } public *getDecorationsAtLine(line: number): IterableIterator { } public *getDecorationsAtCell(x: number, line: number): IterableIterator { } + public forEachDecorationAtCell(x: number, line: number, layer: 'bottom' | 'top' | undefined, callback: (decoration: IInternalDecoration) => void): void { } public dispose(): void { } } diff --git a/src/common/services/DecorationService.ts b/src/common/services/DecorationService.ts index 58bff729..89571363 100644 --- a/src/common/services/DecorationService.ts +++ b/src/common/services/DecorationService.ts @@ -11,6 +11,12 @@ import { SortedList } from 'common/SortedList'; import { IColor } from 'common/Types'; import { IDecorationOptions, IDecoration, IMarker, IEvent } from 'xterm'; +/** Work variables to avoid garbage collection. */ +const w = { + xmin: 0, + xmax: 0 +}; + export class DecorationService extends Disposable implements IDecorationService { public serviceBrand: any; @@ -72,6 +78,16 @@ export class DecorationService extends Disposable implements IDecorationService } } + public forEachDecorationAtCell(x: number, line: number, layer: 'bottom' | 'top' | undefined, callback: (decoration: IInternalDecoration) => void): void { + this._decorations.forEachByKey(line, d => { + w.xmin = d.options.x ?? 0; + w.xmax = w.xmin + (d.options.width ?? 1); + if (x >= w.xmin && x < w.xmax && (!layer || (d.options.layer ?? 'bottom') === layer)) { + callback(d); + } + }); + } + public dispose(): void { for (const d of this._decorations.values()) { this._onDecorationRemoved.fire(d); diff --git a/src/common/services/Services.ts b/src/common/services/Services.ts index 585b29ac..b8e227b3 100644 --- a/src/common/services/Services.ts +++ b/src/common/services/Services.ts @@ -308,6 +308,11 @@ export interface IDecorationService extends IDisposable { getDecorationsAtLine(line: number): IterableIterator; /** Iterates over the decorations at a cell (in no particular order). */ getDecorationsAtCell(x: number, line: number, layer?: 'bottom' | 'top'): IterableIterator; + /** + * Trigger a callback over the decoration at a cell (in no particular order). This is a high + * performance, but less ergonomic, version of {@link getDecorationsAtCell}. + */ + forEachDecorationAtCell(x: number, line: number, layer: 'bottom' | 'top' | undefined, callback: (decoration: IInternalDecoration) => void): void; } export interface IInternalDecoration extends IDecoration { readonly options: IDecorationOptions; From 0727741ca1f061ca7d3fd8b94c6f290872c6d236 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 27 Aug 2022 20:50:16 -0700 Subject: [PATCH 33/55] Remove iterators from IDecorationService They're too low performance for typical use cases Fixes #4079 --- addons/xterm-addon-canvas/src/BaseRenderLayer.ts | 11 +++++------ addons/xterm-addon-canvas/src/TextRenderLayer.ts | 6 +++--- src/browser/renderer/dom/DomRendererRowFactory.ts | 6 +++--- src/common/TestUtils.test.ts | 2 -- src/common/services/DecorationService.ts | 4 ---- src/common/services/Services.ts | 8 ++------ 6 files changed, 13 insertions(+), 24 deletions(-) diff --git a/addons/xterm-addon-canvas/src/BaseRenderLayer.ts b/addons/xterm-addon-canvas/src/BaseRenderLayer.ts index dfd814e3..ae396016 100644 --- a/addons/xterm-addon-canvas/src/BaseRenderLayer.ts +++ b/addons/xterm-addon-canvas/src/BaseRenderLayer.ts @@ -404,12 +404,11 @@ export abstract class BaseRenderLayer implements IRenderLayer { // Don't try cache the glyph if it uses any decoration foreground/background override. let hasOverrides = false; - for (const d of this._decorationService.getDecorationsAtCell(x, y)) { + this._decorationService.forEachDecorationAtCell(x, y, undefined, d => { if (d.backgroundColorRGB || d.foregroundColorRGB) { hasOverrides = true; - break; } - } + }); const atlasDidDraw = hasOverrides ? false : this._charAtlas?.draw(this._ctx, this._currentGlyphIdentifier, x * this._scaledCellWidth + this._scaledCharLeft, y * this._scaledCellHeight + this._scaledCharTop); @@ -519,9 +518,9 @@ export abstract class BaseRenderLayer implements IRenderLayer { let bgOverride: number | undefined; let fgOverride: number | undefined; let isTop = false; - for (const d of this._decorationService.getDecorationsAtCell(x, y)) { + this._decorationService.forEachDecorationAtCell(x, y, undefined, d => { if (d.options.layer !== 'top' && isTop) { - continue; + return; } if (d.backgroundColorRGB) { bgOverride = d.backgroundColorRGB.rgba; @@ -530,7 +529,7 @@ export abstract class BaseRenderLayer implements IRenderLayer { fgOverride = d.foregroundColorRGB.rgba; } isTop = d.options.layer === 'top'; - } + }); // Apply selection foreground if applicable if (!isTop) { diff --git a/addons/xterm-addon-canvas/src/TextRenderLayer.ts b/addons/xterm-addon-canvas/src/TextRenderLayer.ts index 0308f125..86e21ad5 100644 --- a/addons/xterm-addon-canvas/src/TextRenderLayer.ts +++ b/addons/xterm-addon-canvas/src/TextRenderLayer.ts @@ -187,15 +187,15 @@ export class TextRenderLayer extends BaseRenderLayer { // Get any decoration foreground/background overrides, this must be fetched before the early // exist but applied after inverse let isTop = false; - for (const d of this._decorationService.getDecorationsAtCell(x, this._bufferService.buffer.ydisp + y)) { + this._decorationService.forEachDecorationAtCell(x, this._bufferService.buffer.ydisp + y, undefined, d => { if (d.options.layer !== 'top' && isTop) { - continue; + return; } if (d.backgroundColorRGB) { nextFillStyle = d.backgroundColorRGB.css; } isTop = d.options.layer === 'top'; - } + }); if (prevFillStyle === null) { // This is either the first iteration, or the default background was set. Either way, we diff --git a/src/browser/renderer/dom/DomRendererRowFactory.ts b/src/browser/renderer/dom/DomRendererRowFactory.ts index cf4b3680..d3eb9e8e 100644 --- a/src/browser/renderer/dom/DomRendererRowFactory.ts +++ b/src/browser/renderer/dom/DomRendererRowFactory.ts @@ -204,9 +204,9 @@ export class DomRendererRowFactory { let bgOverride: IColor | undefined; let fgOverride: IColor | undefined; let isTop = false; - for (const d of this._decorationService.getDecorationsAtCell(x, row)) { + this._decorationService.forEachDecorationAtCell(x, row, undefined, d => { if (d.options.layer !== 'top' && isTop) { - continue; + return; } if (d.backgroundColorRGB) { bgColorMode = Attributes.CM_RGB; @@ -219,7 +219,7 @@ export class DomRendererRowFactory { fgOverride = d.foregroundColorRGB; } isTop = d.options.layer === 'top'; - } + }); // Apply selection foreground if applicable const isInSelection = this._isCellInSelection(x, row); diff --git a/src/common/TestUtils.test.ts b/src/common/TestUtils.test.ts index cbcdd36e..ff1e1b46 100644 --- a/src/common/TestUtils.test.ts +++ b/src/common/TestUtils.test.ts @@ -161,8 +161,6 @@ export class MockDecorationService implements IDecorationService { public onDecorationRemoved = new EventEmitter().event; public registerDecoration(decorationOptions: IDecorationOptions): IDecoration | undefined { return undefined; } public reset(): void { } - public *getDecorationsAtLine(line: number): IterableIterator { } - public *getDecorationsAtCell(x: number, line: number): IterableIterator { } public forEachDecorationAtCell(x: number, line: number, layer: 'bottom' | 'top' | undefined, callback: (decoration: IInternalDecoration) => void): void { } public dispose(): void { } } diff --git a/src/common/services/DecorationService.ts b/src/common/services/DecorationService.ts index 89571363..e5d115a1 100644 --- a/src/common/services/DecorationService.ts +++ b/src/common/services/DecorationService.ts @@ -62,10 +62,6 @@ export class DecorationService extends Disposable implements IDecorationService this._decorations.clear(); } - public *getDecorationsAtLine(line: number): IterableIterator { - return this._decorations.getKeyIterator(line); - } - public *getDecorationsAtCell(x: number, line: number, layer?: 'bottom' | 'top'): IterableIterator { let xmin = 0; let xmax = 0; diff --git a/src/common/services/Services.ts b/src/common/services/Services.ts index b8e227b3..817a7680 100644 --- a/src/common/services/Services.ts +++ b/src/common/services/Services.ts @@ -304,13 +304,9 @@ export interface IDecorationService extends IDisposable { readonly onDecorationRemoved: IEvent; registerDecoration(decorationOptions: IDecorationOptions): IDecoration | undefined; reset(): void; - /** Iterates over the decorations at a line (in no particular order). */ - getDecorationsAtLine(line: number): IterableIterator; - /** Iterates over the decorations at a cell (in no particular order). */ - getDecorationsAtCell(x: number, line: number, layer?: 'bottom' | 'top'): IterableIterator; /** - * Trigger a callback over the decoration at a cell (in no particular order). This is a high - * performance, but less ergonomic, version of {@link getDecorationsAtCell}. + * Trigger a callback over the decoration at a cell (in no particular order). This uses a callback + * instead of an iterator as it's typically used in hot code paths. */ forEachDecorationAtCell(x: number, line: number, layer: 'bottom' | 'top' | undefined, callback: (decoration: IInternalDecoration) => void): void; } From 1b64a09f50bcc1edda5dc16e2ff7fcd3de7602d5 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 27 Aug 2022 21:35:56 -0700 Subject: [PATCH 34/55] Correct conditions after moving to work object --- addons/xterm-addon-webgl/src/WebglRenderer.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/addons/xterm-addon-webgl/src/WebglRenderer.ts b/addons/xterm-addon-webgl/src/WebglRenderer.ts index efb80f66..ea10b368 100644 --- a/addons/xterm-addon-webgl/src/WebglRenderer.ts +++ b/addons/xterm-addon-webgl/src/WebglRenderer.ts @@ -475,27 +475,29 @@ export class WebglRenderer extends Disposable implements IRenderer { // Handle case where inverse was specified by only one of bg override or fg override was set, // resolving the other inverse color and setting the inverse flag if needed. if (this._workColors.fg & FgFlags.INVERSE) { - if (w.hasBg && w.hasFg) { + if (w.hasBg && !w.hasFg) { // Resolve bg color type (default color has a different meaning in fg vs bg) if ((this._workColors.bg & Attributes.CM_MASK) === Attributes.CM_DEFAULT) { w.fg = (this._workColors.fg & ~(Attributes.RGB_MASK | FgFlags.INVERSE | Attributes.CM_MASK)) | ((this._colors.background.rgba >> 8 & 0xFFFFFF) & Attributes.RGB_MASK) | Attributes.CM_RGB; } else { w.fg = (this._workColors.fg & ~(Attributes.RGB_MASK | FgFlags.INVERSE | Attributes.CM_MASK)) | this._workColors.bg & (Attributes.RGB_MASK | Attributes.CM_MASK); } + w.hasFg = true; } - if (w.hasBg && w.hasFg) { + if (!w.hasBg && w.hasFg) { // Resolve bg color type (default color has a different meaning in fg vs bg) if ((this._workColors.fg & Attributes.CM_MASK) === Attributes.CM_DEFAULT) { w.bg = (this._workColors.bg & ~(Attributes.RGB_MASK | Attributes.CM_MASK)) | ((this._colors.foreground.rgba >> 8 & 0xFFFFFF) & Attributes.RGB_MASK) | Attributes.CM_RGB; } else { w.bg = (this._workColors.bg & ~(Attributes.RGB_MASK | Attributes.CM_MASK)) | this._workColors.fg & (Attributes.RGB_MASK | Attributes.CM_MASK); } + w.hasBg = true; } } // Use the override if it exists - this._workColors.bg = w.bg ?? this._workColors.bg; - this._workColors.fg = w.fg ?? this._workColors.fg; + this._workColors.bg = w.hasBg ? w.bg : this._workColors.bg; + this._workColors.fg = w.hasFg ? w.fg : this._workColors.fg; } private _isCellSelected(x: number, y: number): boolean { From 23ee719f6ed399e01f9abac7a50146936abbba3f Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 28 Aug 2022 08:02:35 -0700 Subject: [PATCH 35/55] Remove more garbage collection issues from cell render loop Part of #4079 --- addons/xterm-addon-webgl/src/GlyphRenderer.ts | 67 ++++++++++--------- .../src/atlas/WebglCharAtlas.ts | 49 +++++++++----- 2 files changed, 66 insertions(+), 50 deletions(-) diff --git a/addons/xterm-addon-webgl/src/GlyphRenderer.ts b/addons/xterm-addon-webgl/src/GlyphRenderer.ts index 6817cb21..6c8df65c 100644 --- a/addons/xterm-addon-webgl/src/GlyphRenderer.ts +++ b/addons/xterm-addon-webgl/src/GlyphRenderer.ts @@ -70,6 +70,14 @@ const INDICES_PER_CELL = 10; const BYTES_PER_CELL = INDICES_PER_CELL * Float32Array.BYTES_PER_ELEMENT; const CELL_POSITION_INDICES = 2; +/** Work variables to avoid garbage collection. */ +const w: { i: number, glyph: IRasterizedGlyph | undefined, leftCellPadding: number, clippedPixels: number } = { + i: 0, + glyph: undefined, + leftCellPadding: 0, + clippedPixels: 0 +}; + export class GlyphRenderer extends Disposable { private _atlas: WebglCharAtlas | undefined; @@ -170,18 +178,20 @@ export class GlyphRenderer extends Disposable { } public updateCell(x: number, y: number, code: number, bg: number, fg: number, ext: number, chars: string, lastBg: number): void { + // Since this function is called for every cell (`rows*cols`), it must be very optimized. It + // should not instantiate any variables unless a new glyph is drawn to the cache where the + // slight slowdown is acceptable for the developer ergonomics provided as it's a once of for + // each glyph. this._updateCell(this._vertices.attributes, x, y, code, bg, fg, ext, chars, lastBg); } private _updateCell(array: Float32Array, x: number, y: number, code: number | undefined, bg: number, fg: number, ext: number, chars: string, lastBg: number): void { - const terminal = this._terminal; - - const i = (y * terminal.cols + x) * INDICES_PER_CELL; + w.i = (y * this._terminal.cols + x) * INDICES_PER_CELL; // Exit early if this is a null character, allow space character to continue as it may have // underline/strikethrough styles if (code === NULL_CELL_CODE || code === undefined/* This is used for the right side of wide chars */) { - fill(array, 0, i, i + INDICES_PER_CELL - 1 - CELL_POSITION_INDICES); + fill(array, 0, w.i, w.i + INDICES_PER_CELL - 1 - CELL_POSITION_INDICES); return; } @@ -190,47 +200,40 @@ export class GlyphRenderer extends Disposable { } // Get the glyph - let rasterizedGlyph: IRasterizedGlyph; if (chars && chars.length > 1) { - rasterizedGlyph = this._atlas.getRasterizedGlyphCombinedChar(chars, bg, fg, ext); + w.glyph = this._atlas.getRasterizedGlyphCombinedChar(chars, bg, fg, ext); } else { - rasterizedGlyph = this._atlas.getRasterizedGlyph(code, bg, fg, ext); + w.glyph = this._atlas.getRasterizedGlyph(code, bg, fg, ext); } - // Fill empty if no glyph was found - if (!rasterizedGlyph) { - fill(array, 0, i, i + INDICES_PER_CELL - 1 - CELL_POSITION_INDICES); - return; - } - - const leftCellPadding = Math.floor((this._dimensions.scaledCellWidth - this._dimensions.scaledCharWidth) / 2); - if (bg !== lastBg && rasterizedGlyph.offset.x > leftCellPadding) { - const clippedPixels = rasterizedGlyph.offset.x - leftCellPadding; + w.leftCellPadding = Math.floor((this._dimensions.scaledCellWidth - this._dimensions.scaledCharWidth) / 2); + if (bg !== lastBg && w.glyph.offset.x > w.leftCellPadding) { + w.clippedPixels = w.glyph.offset.x - w.leftCellPadding; // a_origin - array[i ] = -(rasterizedGlyph.offset.x - clippedPixels) + this._dimensions.scaledCharLeft; - array[i + 1] = -rasterizedGlyph.offset.y + this._dimensions.scaledCharTop; + array[w.i ] = -(w.glyph.offset.x - w.clippedPixels) + this._dimensions.scaledCharLeft; + array[w.i + 1] = -w.glyph.offset.y + this._dimensions.scaledCharTop; // a_size - array[i + 2] = (rasterizedGlyph.size.x - clippedPixels) / this._dimensions.scaledCanvasWidth; - array[i + 3] = rasterizedGlyph.size.y / this._dimensions.scaledCanvasHeight; + array[w.i + 2] = (w.glyph.size.x - w.clippedPixels) / this._dimensions.scaledCanvasWidth; + array[w.i + 3] = w.glyph.size.y / this._dimensions.scaledCanvasHeight; // a_texcoord - array[i + 4] = rasterizedGlyph.texturePositionClipSpace.x + clippedPixels / this._atlas.cacheCanvas.width; - array[i + 5] = rasterizedGlyph.texturePositionClipSpace.y; + array[w.i + 4] = w.glyph.texturePositionClipSpace.x + w.clippedPixels / this._atlas.cacheCanvas.width; + array[w.i + 5] = w.glyph.texturePositionClipSpace.y; // a_texsize - array[i + 6] = rasterizedGlyph.sizeClipSpace.x - clippedPixels / this._atlas.cacheCanvas.width; - array[i + 7] = rasterizedGlyph.sizeClipSpace.y; + array[w.i + 6] = w.glyph.sizeClipSpace.x - w.clippedPixels / this._atlas.cacheCanvas.width; + array[w.i + 7] = w.glyph.sizeClipSpace.y; } else { // a_origin - array[i ] = -rasterizedGlyph.offset.x + this._dimensions.scaledCharLeft; - array[i + 1] = -rasterizedGlyph.offset.y + this._dimensions.scaledCharTop; + array[w.i ] = -w.glyph.offset.x + this._dimensions.scaledCharLeft; + array[w.i + 1] = -w.glyph.offset.y + this._dimensions.scaledCharTop; // a_size - array[i + 2] = rasterizedGlyph.size.x / this._dimensions.scaledCanvasWidth; - array[i + 3] = rasterizedGlyph.size.y / this._dimensions.scaledCanvasHeight; + array[w.i + 2] = w.glyph.size.x / this._dimensions.scaledCanvasWidth; + array[w.i + 3] = w.glyph.size.y / this._dimensions.scaledCanvasHeight; // a_texcoord - array[i + 4] = rasterizedGlyph.texturePositionClipSpace.x; - array[i + 5] = rasterizedGlyph.texturePositionClipSpace.y; + array[w.i + 4] = w.glyph.texturePositionClipSpace.x; + array[w.i + 5] = w.glyph.texturePositionClipSpace.y; // a_texsize - array[i + 6] = rasterizedGlyph.sizeClipSpace.x; - array[i + 7] = rasterizedGlyph.sizeClipSpace.y; + array[w.i + 6] = w.glyph.sizeClipSpace.x; + array[w.i + 7] = w.glyph.sizeClipSpace.y; } // a_cellpos only changes on resize } diff --git a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts index e1d56b78..dda4c2fc 100644 --- a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts +++ b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts @@ -52,6 +52,19 @@ interface ICharAtlasActiveRow { height: number; } +/** Work variables to avoid garbage collection. */ +const w: { + glyphSet: IRasterizedGlyphSet | undefined; + glyphSetBg: { [fg: number]: { [ext: number]: IRasterizedGlyph } } | undefined; + glyphSetFg: { [ext: number]: IRasterizedGlyph } | undefined; + glyph: IRasterizedGlyph | undefined; +} = { + glyphSet: undefined, + glyphSetBg: undefined, + glyphSetFg: undefined, + glyph: undefined +}; + export class WebglCharAtlas implements IDisposable { private _didWarmUp: boolean = false; @@ -175,33 +188,33 @@ export class WebglCharAtlas implements IDisposable { fg: number, ext: number ): IRasterizedGlyph { - let rasterizedGlyphSet = cacheMap[key]; - if (!rasterizedGlyphSet) { - rasterizedGlyphSet = {}; - cacheMap[key] = rasterizedGlyphSet; + w.glyphSet = cacheMap[key]; + if (!w.glyphSet) { + w.glyphSet = {}; + cacheMap[key] = w.glyphSet; } - let rasterizedGlyphSetBg = rasterizedGlyphSet[bg]; - if (!rasterizedGlyphSetBg) { - rasterizedGlyphSetBg = {}; - rasterizedGlyphSet[bg] = rasterizedGlyphSetBg; + w.glyphSetBg = w.glyphSet[bg]; + if (!w.glyphSetBg) { + w.glyphSetBg = {}; + w.glyphSet[bg] = w.glyphSetBg; } - let rasterizedGlyph: IRasterizedGlyph | undefined; - let rasterizedGlyphSetFg = rasterizedGlyphSetBg[fg]; - if (!rasterizedGlyphSetFg) { - rasterizedGlyphSetFg = {}; - rasterizedGlyphSetBg[fg] = rasterizedGlyphSetFg; + w.glyph = undefined; + w.glyphSetFg = w.glyphSetBg[fg]; + if (!w.glyphSetFg) { + w.glyphSetFg = {}; + w.glyphSetBg[fg] = w.glyphSetFg; } else { - rasterizedGlyph = rasterizedGlyphSetFg[ext]; + w.glyph = w.glyphSetFg[ext]; } - if (!rasterizedGlyph) { - rasterizedGlyph = this._drawToCache(key, bg, fg, ext); - rasterizedGlyphSetFg[ext] = rasterizedGlyph; + if (!w.glyph) { + w.glyph = this._drawToCache(key, bg, fg, ext); + w.glyphSetFg[ext] = w.glyph; } - return rasterizedGlyph; + return w.glyph; } private _getColorFromAnsiIndex(idx: number): IColor { From 1d945fdffce1d86dcaf9acf4b5538ef6aa10a80f Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 28 Aug 2022 08:34:51 -0700 Subject: [PATCH 36/55] Create 2 and 4 key map objects, use in color and glyph caches --- .../src/atlas/WebglCharAtlas.ts | 54 ++++--------------- src/browser/ColorContrastCache.ts | 29 +++++----- src/common/MultiKeyMap.ts | 42 +++++++++++++++ 3 files changed, 64 insertions(+), 61 deletions(-) create mode 100644 src/common/MultiKeyMap.ts diff --git a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts index dda4c2fc..c17c54a5 100644 --- a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts +++ b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts @@ -15,6 +15,7 @@ import { color, rgba } from 'common/Color'; import { tryDrawCustomChar } from 'browser/renderer/CustomGlyphs'; import { excludeFromContrastRatioDemands, isPowerlineGlyph, isRestrictedPowerlineGlyph } from 'browser/renderer/RendererUtils'; import { IUnicodeService } from 'common/services/Services'; +import { FourKeyMap } from 'common/MultiKeyMap'; // For debugging purposes, it can be useful to set this to a really tiny value, // to verify that LRU eviction works. @@ -53,23 +54,15 @@ interface ICharAtlasActiveRow { } /** Work variables to avoid garbage collection. */ -const w: { - glyphSet: IRasterizedGlyphSet | undefined; - glyphSetBg: { [fg: number]: { [ext: number]: IRasterizedGlyph } } | undefined; - glyphSetFg: { [ext: number]: IRasterizedGlyph } | undefined; - glyph: IRasterizedGlyph | undefined; -} = { - glyphSet: undefined, - glyphSetBg: undefined, - glyphSetFg: undefined, +const w: { glyph: IRasterizedGlyph | undefined } = { glyph: undefined }; export class WebglCharAtlas implements IDisposable { private _didWarmUp: boolean = false; - private _cacheMap: { [code: number]: IRasterizedGlyphSet } = {}; - private _cacheMapCombined: { [chars: string]: IRasterizedGlyphSet } = {}; + private _cacheMap: FourKeyMap = new FourKeyMap(); + private _cacheMapCombined: FourKeyMap = new FourKeyMap(); // The texture that the atlas is drawn to public cacheCanvas: HTMLCanvasElement; @@ -137,13 +130,7 @@ export class WebglCharAtlas implements IDisposable { // Pre-fill with ASCII 33-126 for (let i = 33; i < 126; i++) { const rasterizedGlyph = this._drawToCache(i, DEFAULT_COLOR, DEFAULT_COLOR, DEFAULT_EXT); - this._cacheMap[i] = { - [DEFAULT_COLOR]: { - [DEFAULT_COLOR]: { - [DEFAULT_EXT]: rasterizedGlyph - } - } - }; + this._cacheMap.set(i, DEFAULT_COLOR, DEFAULT_COLOR, DEFAULT_EXT, rasterizedGlyph); } } @@ -161,8 +148,8 @@ export class WebglCharAtlas implements IDisposable { return; } this._cacheCtx.clearRect(0, 0, TEXTURE_WIDTH, TEXTURE_HEIGHT); - this._cacheMap = {}; - this._cacheMapCombined = {}; + this._cacheMap.clear(); + this._cacheMapCombined.clear(); this._currentRow.x = 0; this._currentRow.y = 0; this._currentRow.height = 0; @@ -182,38 +169,17 @@ export class WebglCharAtlas implements IDisposable { * Gets the glyphs texture coords, drawing the texture if it's not already */ private _getFromCacheMap( - cacheMap: { [key: string | number]: IRasterizedGlyphSet }, + cacheMap: FourKeyMap, key: string | number, bg: number, fg: number, ext: number ): IRasterizedGlyph { - w.glyphSet = cacheMap[key]; - if (!w.glyphSet) { - w.glyphSet = {}; - cacheMap[key] = w.glyphSet; - } - - w.glyphSetBg = w.glyphSet[bg]; - if (!w.glyphSetBg) { - w.glyphSetBg = {}; - w.glyphSet[bg] = w.glyphSetBg; - } - - w.glyph = undefined; - w.glyphSetFg = w.glyphSetBg[fg]; - if (!w.glyphSetFg) { - w.glyphSetFg = {}; - w.glyphSetBg[fg] = w.glyphSetFg; - } else { - w.glyph = w.glyphSetFg[ext]; - } - + w.glyph = cacheMap.get(key, bg, fg, ext); if (!w.glyph) { w.glyph = this._drawToCache(key, bg, fg, ext); - w.glyphSetFg[ext] = w.glyph; + cacheMap.set(key, bg, fg, ext, w.glyph); } - return w.glyph; } diff --git a/src/browser/ColorContrastCache.ts b/src/browser/ColorContrastCache.ts index 73b7a0b7..0c60e8db 100644 --- a/src/browser/ColorContrastCache.ts +++ b/src/browser/ColorContrastCache.ts @@ -5,35 +5,30 @@ import { IColorContrastCache } from 'browser/Types'; import { IColor } from 'common/Types'; +import { TwoKeyMap } from 'common/MultiKeyMap'; export class ColorContrastCache implements IColorContrastCache { - private _color: { [bg: number]: { [fg: number]: IColor | null | undefined } | undefined } = {}; - private _rgba: { [bg: number]: { [fg: number]: string | null | undefined } | undefined } = {}; - - public clear(): void { - this._color = {}; - this._rgba = {}; - } + private _color: TwoKeyMap = new TwoKeyMap(); + private _css: TwoKeyMap = new TwoKeyMap(); public setCss(bg: number, fg: number, value: string | null): void { - if (!this._rgba[bg]) { - this._rgba[bg] = {}; - } - this._rgba[bg]![fg] = value; + this._css.set(bg, fg, value); } public getCss(bg: number, fg: number): string | null | undefined { - return this._rgba[bg] ? this._rgba[bg]![fg] : undefined; + return this._css.get(bg, fg); } public setColor(bg: number, fg: number, value: IColor | null): void { - if (!this._color[bg]) { - this._color[bg] = {}; - } - this._color[bg]![fg] = value; + this._color.set(bg, fg, value); } public getColor(bg: number, fg: number): IColor | null | undefined { - return this._color[bg] ? this._color[bg]![fg] : undefined; + return this._color.get(bg, fg); + } + + public clear(): void { + this._color.clear(); + this._css.clear(); } } diff --git a/src/common/MultiKeyMap.ts b/src/common/MultiKeyMap.ts new file mode 100644 index 00000000..6287a8f2 --- /dev/null +++ b/src/common/MultiKeyMap.ts @@ -0,0 +1,42 @@ +/** + * Copyright (c) 2022 The xterm.js authors. All rights reserved. + * @license MIT + */ + +export class TwoKeyMap { + private _data: { [bg: string | number]: { [fg: string | number]: TValue | undefined } | undefined } = {}; + + public set(first: TFirst, second: TSecond, value: TValue): void { + if (!this._data[first]) { + this._data[first] = {}; + } + this._data[first as string | number]![second] = value; + } + + public get(first: TFirst, second: TSecond): TValue | undefined { + return this._data[first as string | number] ? this._data[first as string | number]![second] : undefined; + } + + public clear(): void { + this._data = {}; + } +} + +export class FourKeyMap { + private _data: TwoKeyMap> = new TwoKeyMap(); + + public set(first: TFirst, second: TSecond, third: TThird, fourth: TFourth, value: TValue): void { + if (!this._data.get(first, second)) { + this._data.set(first, second, new TwoKeyMap()); + } + this._data.get(first, second)!.set(third, fourth, value); + } + + public get(first: TFirst, second: TSecond, third: TThird, fourth: TFourth): TValue | undefined { + return this._data.get(first, second)?.get(third, fourth); + } + + public clear(): void { + this._data.clear(); + } +} From caaca169eb6c8c56f009b671b170ccac709d1374 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 28 Aug 2022 08:45:36 -0700 Subject: [PATCH 37/55] Add multi key map tests --- src/common/MultiKeyMap.test.ts | 69 ++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 src/common/MultiKeyMap.test.ts diff --git a/src/common/MultiKeyMap.test.ts b/src/common/MultiKeyMap.test.ts new file mode 100644 index 00000000..b908321c --- /dev/null +++ b/src/common/MultiKeyMap.test.ts @@ -0,0 +1,69 @@ +/** + * Copyright (c) 2018 The xterm.js authors. All rights reserved. + * @license MIT + */ + +import { assert } from 'chai'; +import { FourKeyMap, TwoKeyMap } from 'common/MultiKeyMap'; + +const strictEqual = assert.strictEqual; + +describe('TwoKeyMap', () => { + let map: TwoKeyMap; + + beforeEach(() => { + map = new TwoKeyMap(); + }); + + it('set, get', () => { + strictEqual(map.get(1, 2), undefined); + map.set(1, 2, 'foo'); + strictEqual(map.get(1, 2), 'foo'); + map.set(1, 3, 'bar'); + strictEqual(map.get(1, 2), 'foo'); + strictEqual(map.get(1, 3), 'bar'); + map.set(2, 2, 'foo2'); + map.set(2, 3, 'bar2'); + strictEqual(map.get(1, 2), 'foo'); + strictEqual(map.get(1, 3), 'bar'); + strictEqual(map.get(2, 2), 'foo2'); + strictEqual(map.get(2, 3), 'bar2'); + }); + it('clear', () => { + strictEqual(map.get(1, 2), undefined); + map.set(1, 2, 'foo'); + strictEqual(map.get(1, 2), 'foo'); + map.clear(); + strictEqual(map.get(1, 2), undefined); + }); +}); + +describe('FourKeyMap', () => { + let map: FourKeyMap; + + beforeEach(() => { + map = new FourKeyMap(); + }); + + it('set, get', () => { + strictEqual(map.get(1, 2, 3, 4), undefined); + map.set(1, 2, 3, 4, 'foo'); + strictEqual(map.get(1, 2, 3, 4), 'foo'); + map.set(1, 3, 3, 4, 'bar'); + strictEqual(map.get(1, 2, 3, 4), 'foo'); + strictEqual(map.get(1, 3, 3, 4), 'bar'); + map.set(2, 2, 3, 4, 'foo2'); + map.set(2, 3, 3, 4, 'bar2'); + strictEqual(map.get(1, 2, 3, 4), 'foo'); + strictEqual(map.get(1, 3, 3, 4), 'bar'); + strictEqual(map.get(2, 2, 3, 4), 'foo2'); + strictEqual(map.get(2, 3, 3, 4), 'bar2'); + }); + it('clear', () => { + strictEqual(map.get(1, 2, 3, 4), undefined); + map.set(1, 2, 3, 4, 'foo'); + strictEqual(map.get(1, 2, 3, 4), 'foo'); + map.clear(); + strictEqual(map.get(1, 2, 3, 4), undefined); + }); +}); From 4a50ca81c62528e4171ba8654a08184eb846e4bf Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 28 Aug 2022 09:06:42 -0700 Subject: [PATCH 38/55] Optimize gc for WebglRenderer._updateModel and BufferLine.loadCell RectangleRenderer not done yet --- addons/xterm-addon-webgl/src/WebglRenderer.ts | 42 ++++++++++++------- src/common/buffer/BufferLine.ts | 13 ++++-- 2 files changed, 37 insertions(+), 18 deletions(-) diff --git a/addons/xterm-addon-webgl/src/WebglRenderer.ts b/addons/xterm-addon-webgl/src/WebglRenderer.ts index ea10b368..81817a80 100644 --- a/addons/xterm-addon-webgl/src/WebglRenderer.ts +++ b/addons/xterm-addon-webgl/src/WebglRenderer.ts @@ -22,7 +22,7 @@ import { EventEmitter } from 'common/EventEmitter'; import { CellData } from 'common/buffer/CellData'; import { addDisposableDomListener } from 'browser/Lifecycle'; import { ICharacterJoinerService, ICoreBrowserService } from 'browser/services/Services'; -import { CharData, ICellData } from 'common/Types'; +import { CharData, IBufferLine, ICellData } from 'common/Types'; import { AttributeData } from 'common/buffer/AttributeData'; import { ICoreService, IDecorationService } from 'common/services/Services'; @@ -314,14 +314,28 @@ export class WebglRenderer extends Disposable implements IRenderer { private _updateModel(start: number, end: number): void { const terminal = this._core; let cell: ICellData = this._workCell; - let lastBg: number = 0; - for (let y = start; y <= end; y++) { - const row = y + terminal.buffer.ydisp; - const line = terminal.buffer.lines.get(row)!; + // Declare variable ahead of time to avoid garbage collection + let lastBg: number; + let y: number; + let row: number; + let line: IBufferLine; + let joinedRanges: [number, number][]; + let isJoined: boolean; + let lastCharX: number; + let range: [number, number]; + let chars: string; + let code: number; + let i: number; + let x: number; + let j: number; + + for (y = start; y <= end; y++) { + row = y + terminal.buffer.ydisp; + line = terminal.buffer.lines.get(row)!; this._model.lineLengths[y] = 0; - const joinedRanges = this._characterJoinerService.getJoinedCharacters(row); - for (let x = 0; x < terminal.cols; x++) { + joinedRanges = this._characterJoinerService.getJoinedCharacters(row); + for (x = 0; x < terminal.cols; x++) { lastBg = this._workColors.bg; line.loadCell(x, cell); @@ -330,15 +344,15 @@ export class WebglRenderer extends Disposable implements IRenderer { } // If true, indicates that the current character(s) to draw were joined. - let isJoined = false; - let lastCharX = x; + isJoined = false; + lastCharX = x; // Process any joined character ranges as needed. Because of how the // ranges are produced, we know that they are valid for the characters // and attributes of our input. if (joinedRanges.length > 0 && x === joinedRanges[0][0]) { isJoined = true; - const range = joinedRanges.shift()!; + range = joinedRanges.shift()!; // We already know the exact start and end column of the joined range, // so we get the string and width representing it directly. @@ -352,9 +366,9 @@ export class WebglRenderer extends Disposable implements IRenderer { lastCharX = range[1] - 1; } - const chars = cell.getChars(); - let code = cell.getCode(); - const i = ((y * terminal.cols) + x) * RENDER_MODEL_INDICIES_PER_CELL; + chars = cell.getChars(); + code = cell.getCode(); + i = ((y * terminal.cols) + x) * RENDER_MODEL_INDICIES_PER_CELL; // Load colors/resolve overrides into work colors this._loadColorsForCell(x, row); @@ -390,7 +404,7 @@ export class WebglRenderer extends Disposable implements IRenderer { // Null out non-first cells for (x++; x < lastCharX; x++) { - const j = ((y * terminal.cols) + x) * RENDER_MODEL_INDICIES_PER_CELL; + j = ((y * terminal.cols) + x) * RENDER_MODEL_INDICIES_PER_CELL; this._glyphRenderer.updateCell(x, y, NULL_CELL_CODE, 0, 0, 0, NULL_CELL_CHAR, 0); this._model.cells[j] = NULL_CELL_CODE; this._model.cells[j + RENDER_MODEL_BG_OFFSET] = this._workColors.bg; diff --git a/src/common/buffer/BufferLine.ts b/src/common/buffer/BufferLine.ts index 6d2a442f..eed87e6b 100644 --- a/src/common/buffer/BufferLine.ts +++ b/src/common/buffer/BufferLine.ts @@ -37,6 +37,11 @@ const enum Cell { export const DEFAULT_ATTR_DATA = Object.freeze(new AttributeData()); +/** Work variables to avoid garbage collection. */ +const w: { startIndex: number } = { + startIndex: 0 +}; + /** * Typed array based bufferline implementation. * @@ -168,10 +173,10 @@ export class BufferLine implements IBufferLine { * to GC as it significantly reduced the amount of new objects/references needed. */ public loadCell(index: number, cell: ICellData): ICellData { - const startIndex = index * CELL_SIZE; - cell.content = this._data[startIndex + Cell.CONTENT]; - cell.fg = this._data[startIndex + Cell.FG]; - cell.bg = this._data[startIndex + Cell.BG]; + w.startIndex = index * CELL_SIZE; + cell.content = this._data[w.startIndex + Cell.CONTENT]; + cell.fg = this._data[w.startIndex + Cell.FG]; + cell.bg = this._data[w.startIndex + Cell.BG]; if (cell.content & Content.IS_COMBINED_MASK) { cell.combinedData = this._combined[index]; } From 3b4ecd0709baeac86404a5609b3ff7c9f4ff0963 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 28 Aug 2022 09:20:24 -0700 Subject: [PATCH 39/55] Optimize gc in rectangle renderer --- .../src/RectangleRenderer.ts | 79 ++++++++++++------- 1 file changed, 51 insertions(+), 28 deletions(-) diff --git a/addons/xterm-addon-webgl/src/RectangleRenderer.ts b/addons/xterm-addon-webgl/src/RectangleRenderer.ts index f16fdec0..acaea56a 100644 --- a/addons/xterm-addon-webgl/src/RectangleRenderer.ts +++ b/addons/xterm-addon-webgl/src/RectangleRenderer.ts @@ -59,6 +59,18 @@ const BYTES_PER_RECTANGLE = INDICES_PER_RECTANGLE * Float32Array.BYTES_PER_ELEME const INITIAL_BUFFER_RECTANGLE_CAPACITY = 20 * INDICES_PER_RECTANGLE; +/** Work variables to avoid garbage collection. */ +const w: { rgba: number, isDefault: boolean, x1: number, y1: number, r: number, g: number, b: number, a: number } = { + rgba: 0, + isDefault: false, + x1: 0, + y1: 0, + r: 0, + g: 0, + b: 0, + a: 0 +}; + export class RectangleRenderer extends Disposable { private _program: WebGLProgram; @@ -174,22 +186,34 @@ export class RectangleRenderer extends Disposable { const terminal = this._terminal; const vertices = this._vertices; + // Declare variable ahead of time to avoid garbage collection let rectangleCount = 1; + let y: number; + let x: number; + let currentStartX: number; + let currentBg: number; + let currentFg: number; + let currentInverse: boolean; + let modelIndex: number; + let bg: number; + let fg: number; + let inverse: boolean; + let offset: number; - for (let y = 0; y < terminal.rows; y++) { - let currentStartX = -1; - let currentBg = 0; - let currentFg = 0; - let currentInverse = false; - for (let x = 0; x < terminal.cols; x++) { - const modelIndex = ((y * terminal.cols) + x) * RENDER_MODEL_INDICIES_PER_CELL; - const bg = model.cells[modelIndex + RENDER_MODEL_BG_OFFSET]; - const fg = model.cells[modelIndex + RENDER_MODEL_FG_OFFSET]; - const inverse = !!(fg & FgFlags.INVERSE); + for (y = 0; y < terminal.rows; y++) { + currentStartX = -1; + currentBg = 0; + currentFg = 0; + currentInverse = false; + for (x = 0; x < terminal.cols; x++) { + modelIndex = ((y * terminal.cols) + x) * RENDER_MODEL_INDICIES_PER_CELL; + bg = model.cells[modelIndex + RENDER_MODEL_BG_OFFSET]; + fg = model.cells[modelIndex + RENDER_MODEL_FG_OFFSET]; + inverse = !!(fg & FgFlags.INVERSE); if (bg !== currentBg || (fg !== currentFg && (currentInverse || inverse))) { // A rectangle needs to be drawn if going from non-default to another color if (currentBg !== 0 || (currentInverse && currentFg !== 0)) { - const offset = rectangleCount++ * INDICES_PER_RECTANGLE; + offset = rectangleCount++ * INDICES_PER_RECTANGLE; this._updateRectangle(vertices, offset, currentFg, currentBg, currentStartX, x, y); } currentStartX = x; @@ -200,7 +224,7 @@ export class RectangleRenderer extends Disposable { } // Finish rectangle if it's still going if (currentBg !== 0 || (currentInverse && currentFg !== 0)) { - const offset = rectangleCount++ * INDICES_PER_RECTANGLE; + offset = rectangleCount++ * INDICES_PER_RECTANGLE; this._updateRectangle(vertices, offset, currentFg, currentBg, currentStartX, terminal.cols, y); } } @@ -208,48 +232,47 @@ export class RectangleRenderer extends Disposable { } private _updateRectangle(vertices: IVertices, offset: number, fg: number, bg: number, startX: number, endX: number, y: number): void { - let rgba: number | undefined; - let isDefault = false; + w.isDefault = false; if (fg & FgFlags.INVERSE) { switch (fg & Attributes.CM_MASK) { case Attributes.CM_P16: case Attributes.CM_P256: - rgba = this._colors.ansi[fg & Attributes.PCOLOR_MASK].rgba; + w.rgba = this._colors.ansi[fg & Attributes.PCOLOR_MASK].rgba; break; case Attributes.CM_RGB: - rgba = (fg & Attributes.RGB_MASK) << 8; + w.rgba = (fg & Attributes.RGB_MASK) << 8; break; case Attributes.CM_DEFAULT: default: - rgba = this._colors.foreground.rgba; + w.rgba = this._colors.foreground.rgba; } } else { switch (bg & Attributes.CM_MASK) { case Attributes.CM_P16: case Attributes.CM_P256: - rgba = this._colors.ansi[bg & Attributes.PCOLOR_MASK].rgba; + w.rgba = this._colors.ansi[bg & Attributes.PCOLOR_MASK].rgba; break; case Attributes.CM_RGB: - rgba = (bg & Attributes.RGB_MASK) << 8; + w.rgba = (bg & Attributes.RGB_MASK) << 8; break; case Attributes.CM_DEFAULT: default: - rgba = this._colors.background.rgba; - isDefault = true; + w.rgba = this._colors.background.rgba; + w.isDefault = true; } } if (vertices.attributes.length < offset + 4) { vertices.attributes = expandFloat32Array(vertices.attributes, this._terminal.rows * this._terminal.cols * INDICES_PER_RECTANGLE); } - const x1 = startX * this._dimensions.scaledCellWidth; - const y1 = y * this._dimensions.scaledCellHeight; - const r = ((rgba >> 24) & 0xFF) / 255; - const g = ((rgba >> 16) & 0xFF) / 255; - const b = ((rgba >> 8 ) & 0xFF) / 255; - const a = (!isDefault && bg & BgFlags.DIM) ? DIM_OPACITY : 1; + w.x1 = startX * this._dimensions.scaledCellWidth; + w.y1 = y * this._dimensions.scaledCellHeight; + w.r = ((w.rgba >> 24) & 0xFF) / 255; + w.g = ((w.rgba >> 16) & 0xFF) / 255; + w.b = ((w.rgba >> 8 ) & 0xFF) / 255; + w.a = (!w.isDefault && bg & BgFlags.DIM) ? DIM_OPACITY : 1; - this._addRectangle(vertices.attributes, offset, x1, y1, (endX - startX) * this._dimensions.scaledCellWidth, this._dimensions.scaledCellHeight, r, g, b, a); + this._addRectangle(vertices.attributes, offset, w.x1, w.y1, (endX - startX) * this._dimensions.scaledCellWidth, this._dimensions.scaledCellHeight, w.r, w.g, w.b, w.a); } private _addRectangle(array: Float32Array, offset: number, x1: number, y1: number, width: number, height: number, r: number, g: number, b: number, a: number): void { From f7e0f69598dee372ec94a6a09e343afc5c4f0385 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 28 Aug 2022 13:22:29 -0700 Subject: [PATCH 40/55] Fix rounding error gap in webgl renderer Fixes #4054 Fixes #4076 --- addons/xterm-addon-webgl/src/GlyphRenderer.ts | 2 +- .../src/RectangleRenderer.ts | 28 +++++++++---------- addons/xterm-addon-webgl/src/WebglRenderer.ts | 7 +++-- 3 files changed, 19 insertions(+), 18 deletions(-) diff --git a/addons/xterm-addon-webgl/src/GlyphRenderer.ts b/addons/xterm-addon-webgl/src/GlyphRenderer.ts index 6c8df65c..5d3fb9ac 100644 --- a/addons/xterm-addon-webgl/src/GlyphRenderer.ts +++ b/addons/xterm-addon-webgl/src/GlyphRenderer.ts @@ -130,7 +130,7 @@ export class GlyphRenderer extends Disposable { gl.vertexAttribPointer(VertexAttribLocations.UNIT_QUAD, 2, this._gl.FLOAT, false, 0, 0); // Setup the unit quad element array buffer, this points to indices in - // unitQuadVertuces to allow is to draw 2 triangles from the vertices + // unitQuadVertices to allow is to draw 2 triangles from the vertices const unitQuadElementIndices = new Uint8Array([0, 1, 3, 0, 2, 3]); const elementIndicesBuffer = gl.createBuffer(); this.register(toDisposable(() => gl.deleteBuffer(elementIndicesBuffer))); diff --git a/addons/xterm-addon-webgl/src/RectangleRenderer.ts b/addons/xterm-addon-webgl/src/RectangleRenderer.ts index acaea56a..ef08fb7c 100644 --- a/addons/xterm-addon-webgl/src/RectangleRenderer.ts +++ b/addons/xterm-addon-webgl/src/RectangleRenderer.ts @@ -28,12 +28,11 @@ layout (location = ${VertexAttribLocations.COLOR}) in vec4 a_color; layout (location = ${VertexAttribLocations.UNIT_QUAD}) in vec2 a_unitquad; uniform mat4 u_projection; -uniform vec2 u_resolution; out vec4 v_color; void main() { - vec2 zeroToOne = (a_position + (a_unitquad * a_size)) / u_resolution; + vec2 zeroToOne = a_position + (a_unitquad * a_size); gl_Position = u_projection * vec4(zeroToOne, 0.0, 1.0); v_color = a_color; }`; @@ -75,7 +74,6 @@ export class RectangleRenderer extends Disposable { private _program: WebGLProgram; private _vertexArrayObject: IWebGLVertexArrayObject; - private _resolutionLocation: WebGLUniformLocation; private _attributesBuffer: WebGLBuffer; private _projectionLocation: WebGLUniformLocation; private _bgFloat!: Float32Array; @@ -99,7 +97,6 @@ export class RectangleRenderer extends Disposable { this.register(toDisposable(() => gl.deleteProgram(this._program))); // Uniform locations - this._resolutionLocation = throwIfFalsy(gl.getUniformLocation(this._program, 'u_resolution')); this._projectionLocation = throwIfFalsy(gl.getUniformLocation(this._program, 'u_projection')); // Create and set the vertex array object @@ -116,7 +113,7 @@ export class RectangleRenderer extends Disposable { gl.vertexAttribPointer(VertexAttribLocations.UNIT_QUAD, 2, this._gl.FLOAT, false, 0, 0); // Setup the unit quad element array buffer, this points to indices in - // unitQuadVertuces to allow is to draw 2 triangles from the vertices + // unitQuadVertices to allow is to draw 2 triangles from the vertices const unitQuadElementIndices = new Uint8Array([0, 1, 3, 0, 2, 3]); const elementIndicesBuffer = gl.createBuffer(); this.register(toDisposable(() => gl.deleteBuffer(elementIndicesBuffer))); @@ -148,7 +145,6 @@ export class RectangleRenderer extends Disposable { gl.bindVertexArray(this._vertexArrayObject); gl.uniformMatrix4fv(this._projectionLocation, false, PROJECTION_MATRIX); - gl.uniform2f(this._resolutionLocation, gl.canvas.width, gl.canvas.height); // Bind attributes buffer and draw gl.bindBuffer(gl.ARRAY_BUFFER, this._attributesBuffer); @@ -165,6 +161,10 @@ export class RectangleRenderer extends Disposable { this._updateViewportRectangle(); } + public setDimensions(dimensions: IRenderDimensions): void { + this._dimensions = dimensions; + } + private _updateCachedColors(): void { this._bgFloat = this._colorToFloat32Array(this._colors.background); } @@ -276,10 +276,10 @@ export class RectangleRenderer extends Disposable { } private _addRectangle(array: Float32Array, offset: number, x1: number, y1: number, width: number, height: number, r: number, g: number, b: number, a: number): void { - array[offset ] = x1; - array[offset + 1] = y1; - array[offset + 2] = width; - array[offset + 3] = height; + array[offset ] = x1 / this._dimensions.scaledCanvasWidth; + array[offset + 1] = y1 / this._dimensions.scaledCanvasHeight; + array[offset + 2] = width / this._dimensions.scaledCanvasWidth; + array[offset + 3] = height / this._dimensions.scaledCanvasHeight; array[offset + 4] = r; array[offset + 5] = g; array[offset + 6] = b; @@ -287,10 +287,10 @@ export class RectangleRenderer extends Disposable { } private _addRectangleFloat(array: Float32Array, offset: number, x1: number, y1: number, width: number, height: number, color: Float32Array): void { - array[offset ] = x1; - array[offset + 1] = y1; - array[offset + 2] = width; - array[offset + 3] = height; + array[offset ] = x1 / this._dimensions.scaledCanvasWidth; + array[offset + 1] = y1 / this._dimensions.scaledCanvasHeight; + array[offset + 2] = width / this._dimensions.scaledCanvasWidth; + array[offset + 3] = height / this._dimensions.scaledCanvasHeight; array[offset + 4] = color[0]; array[offset + 5] = color[1]; array[offset + 6] = color[2]; diff --git a/addons/xterm-addon-webgl/src/WebglRenderer.ts b/addons/xterm-addon-webgl/src/WebglRenderer.ts index 81817a80..43de1c98 100644 --- a/addons/xterm-addon-webgl/src/WebglRenderer.ts +++ b/addons/xterm-addon-webgl/src/WebglRenderer.ts @@ -182,6 +182,7 @@ export class WebglRenderer extends Disposable implements IRenderer { this._core.screenElement!.style.width = `${this.dimensions.canvasWidth}px`; this._core.screenElement!.style.height = `${this.dimensions.canvasHeight}px`; + this._rectangleRenderer.setDimensions(this.dimensions); this._rectangleRenderer.onResize(); this._glyphRenderer.setDimensions(this.dimensions); this._glyphRenderer.onResize(); @@ -622,11 +623,11 @@ export class WebglRenderer extends Disposable implements IRenderer { } private _setCanvasDevicePixelDimensions(width: number, height: number): void { - if (this.dimensions.scaledCanvasWidth === width && this.dimensions.scaledCanvasHeight === height) { + if (this._canvas.width === width && this._canvas.height === height) { return; } - this.dimensions.scaledCanvasWidth = width; - this.dimensions.scaledCanvasHeight = height; + // While the actual canvas size has changed, keep scaledCanvasWidth/Height as the value before + // the change as it's an exact multiple of the cell sizes. this._canvas.width = width; this._canvas.height = height; this._requestRedrawViewport(); From 5c6d16ef0a805469f2e2628b2a428dc9285eae38 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 28 Aug 2022 13:33:09 -0700 Subject: [PATCH 41/55] Only remove padding from restricted powerline chars --- addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts index c17c54a5..d4e58c9f 100644 --- a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts +++ b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts @@ -404,7 +404,7 @@ export class WebglCharAtlas implements IDisposable { this._tmpCtx.fillStyle = foregroundColor.css; // For powerline glyphs left/top padding is excluded (https://github.com/microsoft/vscode/issues/120129) - const padding = powerlineGlyph ? 0 : TMP_CANVAS_GLYPH_PADDING * 2; + const padding = restrictedPowerlineGlyph ? 0 : TMP_CANVAS_GLYPH_PADDING * 2; // Draw custom characters if applicable let customGlyph = false; From 22cca3495ae7658460a2df22c8b11d52aff152a7 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Tue, 30 Aug 2022 07:00:33 -0700 Subject: [PATCH 42/55] Expose range on link handler This is needed for proper tooltip positioning Part of #1134 --- src/browser/OscLinkProvider.ts | 32 +++++++++++++++++--------------- typings/xterm.d.ts | 9 ++++++--- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/src/browser/OscLinkProvider.ts b/src/browser/OscLinkProvider.ts index 38c07106..fdae9b0e 100644 --- a/src/browser/OscLinkProvider.ts +++ b/src/browser/OscLinkProvider.ts @@ -6,6 +6,7 @@ import { ILink, ILinkProvider } from 'browser/Types'; import { CellData } from 'common/buffer/CellData'; import { IBufferService, IOptionsService, IOscLinkService } from 'common/services/Services'; +import { IBufferRange } from 'xterm'; export class OscLinkProvider implements ILinkProvider { constructor( @@ -54,24 +55,25 @@ export class OscLinkProvider implements ILinkProvider { if (finishLink || (currentStart !== -1 && x === lineLength - 1)) { const text = this._oscLinkService.getLinkData(currentLinkId)?.uri; if (text) { + // These ranges are 1-based + const range: IBufferRange = { + start: { + x: currentStart + 1, + y + }, + end: { + // Offset end x if it's a link that ends on the last cell in the line + x: x + (!finishLink && x === lineLength - 1 ? 1 : 0), + y + } + }; // OSC links always use underline and pointer decorations result.push({ text, - // These ranges are 1-based - range: { - start: { - x: currentStart + 1, - y - }, - end: { - // Offset end x if it's a link that ends on the last cell in the line - x: x + (!finishLink && x === lineLength - 1 ? 1 : 0), - y - } - }, - activate: linkHandler?.activate || defaultActivate, - hover: linkHandler?.hover, - leave: linkHandler?.leave + range, + activate: (e, text) => (linkHandler?.activate(e, text, range) || defaultActivate(e, text)), + hover: (e, text) => linkHandler?.hover?.(e, text, range), + leave: (e, text) => linkHandler?.leave?.(e, text, range) }); } finishLink = false; diff --git a/typings/xterm.d.ts b/typings/xterm.d.ts index 2fa17421..bd48afd4 100644 --- a/typings/xterm.d.ts +++ b/typings/xterm.d.ts @@ -1117,8 +1117,9 @@ declare module 'xterm' { * Calls when the link is activated. * @param event The mouse event triggering the callback. * @param text The text of the link. + * @param range The buffer range of the link. */ - activate(event: MouseEvent, text: string): void; + activate(event: MouseEvent, text: string, range: IBufferRange): void; /** * Called when the mouse hovers the link. To use this to create a DOM-based hover tooltip, @@ -1126,15 +1127,17 @@ declare module 'xterm' { * that will cause mouse events to not fall through and activate other links. * @param event The mouse event triggering the callback. * @param text The text of the link. + * @param range The buffer range of the link. */ - hover?(event: MouseEvent, text: string): void; + hover?(event: MouseEvent, text: string, range: IBufferRange): void; /** * Called when the mouse leaves the link. * @param event The mouse event triggering the callback. * @param text The text of the link. + * @param range The buffer range of the link. */ - leave?(event: MouseEvent, text: string): void; + leave?(event: MouseEvent, text: string, range: IBufferRange): void; } /** From c5861d512c41b179fe944c1c032ad13d3a070dc0 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Tue, 30 Aug 2022 07:01:23 -0700 Subject: [PATCH 43/55] Import from Types --- src/browser/OscLinkProvider.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/browser/OscLinkProvider.ts b/src/browser/OscLinkProvider.ts index fdae9b0e..9af0cf6f 100644 --- a/src/browser/OscLinkProvider.ts +++ b/src/browser/OscLinkProvider.ts @@ -3,10 +3,9 @@ * @license MIT */ -import { ILink, ILinkProvider } from 'browser/Types'; +import { IBufferRange, ILink, ILinkProvider } from 'browser/Types'; import { CellData } from 'common/buffer/CellData'; import { IBufferService, IOptionsService, IOscLinkService } from 'common/services/Services'; -import { IBufferRange } from 'xterm'; export class OscLinkProvider implements ILinkProvider { constructor( From 284d9f98933c615e24f528bee027e98df5f1a898 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Tue, 30 Aug 2022 07:51:32 -0700 Subject: [PATCH 44/55] Fix ternary in activate call Part of #1134 --- src/browser/OscLinkProvider.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/browser/OscLinkProvider.ts b/src/browser/OscLinkProvider.ts index 9af0cf6f..3ca93b09 100644 --- a/src/browser/OscLinkProvider.ts +++ b/src/browser/OscLinkProvider.ts @@ -70,7 +70,7 @@ export class OscLinkProvider implements ILinkProvider { result.push({ text, range, - activate: (e, text) => (linkHandler?.activate(e, text, range) || defaultActivate(e, text)), + activate: (e, text) => (linkHandler ? linkHandler.activate(e, text, range) : defaultActivate(e, text)), hover: (e, text) => linkHandler?.hover?.(e, text, range), leave: (e, text) => linkHandler?.leave?.(e, text, range) }); From 4a6e7bee211de163438adab78a18668d721475ab Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Tue, 30 Aug 2022 14:09:39 -0700 Subject: [PATCH 45/55] Fix lgtm recommendations --- .../src/SelectionRenderLayer.ts | 3 +- addons/xterm-addon-webgl/src/GlyphRenderer.ts | 2 +- addons/xterm-addon-webgl/src/Types.d.ts | 4 --- .../src/atlas/WebglCharAtlas.ts | 3 +- .../src/renderLayer/CursorRenderLayer.ts | 2 +- bin/extract_vtfeatures.js | 2 +- demo/index.html | 4 +-- src/browser/Terminal.ts | 30 +++++++++---------- src/browser/TestUtils.test.ts | 2 +- .../decorations/OverviewRulerRenderer.ts | 2 +- src/browser/public/Terminal.ts | 2 +- src/browser/renderer/dom/DomRenderer.ts | 2 +- src/browser/services/RenderService.ts | 2 +- src/browser/services/Services.ts | 4 +-- src/common/CoreTerminal.ts | 2 +- src/common/services/OptionsService.ts | 1 - src/common/services/Services.ts | 2 +- 17 files changed, 29 insertions(+), 40 deletions(-) diff --git a/addons/xterm-addon-canvas/src/SelectionRenderLayer.ts b/addons/xterm-addon-canvas/src/SelectionRenderLayer.ts index 61fc4783..82aa056b 100644 --- a/addons/xterm-addon-canvas/src/SelectionRenderLayer.ts +++ b/addons/xterm-addon-canvas/src/SelectionRenderLayer.ts @@ -3,12 +3,11 @@ * @license MIT */ -import { IRenderDimensions, IRequestRedrawEvent } from 'browser/renderer/Types'; +import { IRenderDimensions } from 'browser/renderer/Types'; import { BaseRenderLayer } from './BaseRenderLayer'; import { IColorSet } from 'browser/Types'; import { IBufferService, IDecorationService, IOptionsService } from 'common/services/Services'; import { ICoreBrowserService } from 'browser/services/Services'; -import { IEventEmitter } from 'common/EventEmitter'; interface ISelectionState { start?: [number, number]; diff --git a/addons/xterm-addon-webgl/src/GlyphRenderer.ts b/addons/xterm-addon-webgl/src/GlyphRenderer.ts index 5d3fb9ac..85ef25f6 100644 --- a/addons/xterm-addon-webgl/src/GlyphRenderer.ts +++ b/addons/xterm-addon-webgl/src/GlyphRenderer.ts @@ -8,7 +8,7 @@ import { WebglCharAtlas } from './atlas/WebglCharAtlas'; import { IWebGL2RenderingContext, IWebGLVertexArrayObject, IRenderModel, IRasterizedGlyph } from './Types'; import { fill } from 'common/TypedArrayUtils'; import { NULL_CELL_CODE } from 'common/buffer/Constants'; -import { Terminal, IBufferLine } from 'xterm'; +import { Terminal } from 'xterm'; import { IColorSet } from 'browser/Types'; import { IRenderDimensions } from 'browser/renderer/Types'; import { Disposable, toDisposable } from 'common/Lifecycle'; diff --git a/addons/xterm-addon-webgl/src/Types.d.ts b/addons/xterm-addon-webgl/src/Types.d.ts index c803d3e4..bcfa11c8 100644 --- a/addons/xterm-addon-webgl/src/Types.d.ts +++ b/addons/xterm-addon-webgl/src/Types.d.ts @@ -3,10 +3,6 @@ * @license MIT */ -export interface IRasterizedGlyphSet { - [bg: number]: { [fg: number]: { [ext: number]: IRasterizedGlyph } } | undefined; -} - /** * Represents a rasterized glyph within a texture atlas. Some numbers are * tracked in CSS pixels as well in order to reduce calculations during the diff --git a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts index d4e58c9f..e7dbfaa2 100644 --- a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts +++ b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts @@ -5,7 +5,7 @@ import { ICharAtlasConfig } from './Types'; import { DIM_OPACITY, TEXT_BASELINE } from 'browser/renderer/Constants'; -import { IRasterizedGlyph, IBoundingBox, IRasterizedGlyphSet } from '../Types'; +import { IRasterizedGlyph, IBoundingBox } from '../Types'; import { DEFAULT_COLOR, Attributes, DEFAULT_EXT, UnderlineStyle } from 'common/buffer/Constants'; import { throwIfFalsy } from '../WebglUtils'; import { IColor } from 'common/Types'; @@ -450,7 +450,6 @@ export class WebglCharAtlas implements IDisposable { // Underline style/stroke this._tmpCtx.beginPath(); const xLeft = padding; - const xRight = padding + this._config.scaledCellWidth * chWidth; const yTop = Math.ceil(padding + this._config.scaledCharHeight) - yOffset; const yMid = padding + this._config.scaledCharHeight + lineWidth - yOffset; const yBot = Math.ceil(padding + this._config.scaledCharHeight + lineWidth * 2) - yOffset; diff --git a/addons/xterm-addon-webgl/src/renderLayer/CursorRenderLayer.ts b/addons/xterm-addon-webgl/src/renderLayer/CursorRenderLayer.ts index 04e2b387..8a16a387 100644 --- a/addons/xterm-addon-webgl/src/renderLayer/CursorRenderLayer.ts +++ b/addons/xterm-addon-webgl/src/renderLayer/CursorRenderLayer.ts @@ -7,7 +7,7 @@ import { Terminal } from 'xterm'; import { BaseRenderLayer } from './BaseRenderLayer'; import { ICellData } from 'common/Types'; import { CellData } from 'common/buffer/CellData'; -import { IColorSet, ITerminal } from 'browser/Types'; +import { IColorSet } from 'browser/Types'; import { IRenderDimensions, IRequestRedrawEvent } from 'browser/renderer/Types'; import { IEventEmitter } from 'common/EventEmitter'; import { ICoreBrowserService } from 'browser/services/Services'; diff --git a/bin/extract_vtfeatures.js b/bin/extract_vtfeatures.js index 9b5ab898..1ccfb636 100644 --- a/bin/extract_vtfeatures.js +++ b/bin/extract_vtfeatures.js @@ -362,7 +362,7 @@ function* parseMultiLineGen(filename, s) { if (!s.includes('@vt:')) { return; } - const lines = s.split('\n').map(el => el.trim().replace(/\*/, '').replace(/\s/, '')); + const lines = s.split('\n').map(el => el.trim().replace(/\*/, '').replace(/\s/, '')); // lgtm [js/incomplete-sanitization] let grabLine = false; let longDescription = []; let feature = undefined; diff --git a/demo/index.html b/demo/index.html index c38cb007..d6c20851 100644 --- a/demo/index.html +++ b/demo/index.html @@ -10,8 +10,8 @@ - - + +

xterm.js: A terminal for the web

diff --git a/src/browser/Terminal.ts b/src/browser/Terminal.ts index aacd6cf7..e45d2355 100644 --- a/src/browser/Terminal.ts +++ b/src/browser/Terminal.ts @@ -213,22 +213,20 @@ export class Terminal extends CoreTerminal implements ITerminal { acc = 'ansi'; ident = '4;' + req.index; } - if (acc) { - switch (req.type) { - case ColorRequestType.REPORT: - const channels = color.toColorRGB(acc === 'ansi' - ? this._colorManager.colors.ansi[req.index] - : this._colorManager.colors[acc]); - this.coreService.triggerDataEvent(`${C0.ESC}]${ident};${toRgbString(channels)}${C1_ESCAPED.ST}`); - break; - case ColorRequestType.SET: - if (acc === 'ansi') this._colorManager.colors.ansi[req.index] = rgba.toColor(...req.color); - else this._colorManager.colors[acc] = rgba.toColor(...req.color); - break; - case ColorRequestType.RESTORE: - this._colorManager.restoreColor(req.index); - break; - } + switch (req.type) { + case ColorRequestType.REPORT: + const channels = color.toColorRGB(acc === 'ansi' + ? this._colorManager.colors.ansi[req.index] + : this._colorManager.colors[acc]); + this.coreService.triggerDataEvent(`${C0.ESC}]${ident};${toRgbString(channels)}${C1_ESCAPED.ST}`); + break; + case ColorRequestType.SET: + if (acc === 'ansi') this._colorManager.colors.ansi[req.index] = rgba.toColor(...req.color); + else this._colorManager.colors[acc] = rgba.toColor(...req.color); + break; + case ColorRequestType.RESTORE: + this._colorManager.restoreColor(req.index); + break; } } this._renderService?.setColors(this._colorManager.colors); diff --git a/src/browser/TestUtils.test.ts b/src/browser/TestUtils.test.ts index e09097a2..a3b5fdb0 100644 --- a/src/browser/TestUtils.test.ts +++ b/src/browser/TestUtils.test.ts @@ -7,7 +7,7 @@ import { IDisposable, IMarker, ILinkProvider, IDecorationOptions, IDecoration } import { IEvent, EventEmitter } from 'common/EventEmitter'; import { ICharacterJoinerService, ICharSizeService, ICoreBrowserService, IMouseService, IRenderService, ISelectionService } from 'browser/services/Services'; import { IRenderDimensions, IRenderer, IRequestRedrawEvent } from 'browser/renderer/Types'; -import { IColorSet, ITerminal, ILinkifier2, IBrowser, IViewport, IColorManager, ICompositionHelper, CharacterJoinerHandler, IRenderDebouncer, IBufferRange } from 'browser/Types'; +import { IColorSet, ITerminal, ILinkifier2, IBrowser, IViewport, IColorManager, ICompositionHelper, CharacterJoinerHandler, IBufferRange } from 'browser/Types'; import { IBuffer, IBufferStringIterator, IBufferSet } from 'common/buffer/Types'; import { IBufferLine, ICellData, IAttributeData, ICircularList, XtermListener, ICharset, ITerminalOptions } from 'common/Types'; import { Buffer } from 'common/buffer/Buffer'; diff --git a/src/browser/decorations/OverviewRulerRenderer.ts b/src/browser/decorations/OverviewRulerRenderer.ts index 22f3ddf5..7c284a61 100644 --- a/src/browser/decorations/OverviewRulerRenderer.ts +++ b/src/browser/decorations/OverviewRulerRenderer.ts @@ -7,7 +7,7 @@ import { ColorZoneStore, IColorZone, IColorZoneStore } from 'browser/decorations import { addDisposableDomListener } from 'browser/Lifecycle'; import { IRenderService } from 'browser/services/Services'; import { Disposable } from 'common/Lifecycle'; -import { IBufferService, IDecorationService, IInternalDecoration, IOptionsService } from 'common/services/Services'; +import { IBufferService, IDecorationService, IOptionsService } from 'common/services/Services'; // Helper objects to avoid excessive calculation and garbage collection during rendering. These are // static values for each render and can be accessed using the decoration position as the key. diff --git a/src/browser/public/Terminal.ts b/src/browser/public/Terminal.ts index 57efdbd0..2e39cd63 100644 --- a/src/browser/public/Terminal.ts +++ b/src/browser/public/Terminal.ts @@ -3,7 +3,7 @@ * @license MIT */ -import { Terminal as ITerminalApi, IMarker, IDisposable, ILocalizableStrings, ITerminalAddon, IBufferNamespace as IBufferNamespaceApi, IParser, ILinkProvider, IUnicodeHandling, FontWeight, IModes, IDecorationOptions, IDecoration } from 'xterm'; +import { Terminal as ITerminalApi, IMarker, IDisposable, ILocalizableStrings, ITerminalAddon, IBufferNamespace as IBufferNamespaceApi, IParser, ILinkProvider, IUnicodeHandling, IModes, IDecorationOptions, IDecoration } from 'xterm'; import { IBufferRange, ITerminal } from 'browser/Types'; import { Terminal as TerminalCore } from 'browser/Terminal'; import * as Strings from 'browser/LocalizableStrings'; diff --git a/src/browser/renderer/dom/DomRenderer.ts b/src/browser/renderer/dom/DomRenderer.ts index 050f9396..ec9b835c 100644 --- a/src/browser/renderer/dom/DomRenderer.ts +++ b/src/browser/renderer/dom/DomRenderer.ts @@ -9,7 +9,7 @@ import { INVERTED_DEFAULT_COLOR } from 'browser/renderer/Constants'; import { Disposable } from 'common/Lifecycle'; import { IColorSet, ILinkifierEvent, ILinkifier2 } from 'browser/Types'; import { ICharSizeService } from 'browser/services/Services'; -import { IOptionsService, IBufferService, IInstantiationService, IDecorationService } from 'common/services/Services'; +import { IOptionsService, IBufferService, IInstantiationService } from 'common/services/Services'; import { EventEmitter, IEvent } from 'common/EventEmitter'; import { color } from 'common/Color'; import { removeElementFromParent } from 'browser/Dom'; diff --git a/src/browser/services/RenderService.ts b/src/browser/services/RenderService.ts index 78bd7f56..13cdaf9c 100644 --- a/src/browser/services/RenderService.ts +++ b/src/browser/services/RenderService.ts @@ -9,7 +9,7 @@ import { EventEmitter, IEvent } from 'common/EventEmitter'; import { Disposable } from 'common/Lifecycle'; import { ScreenDprMonitor } from 'browser/ScreenDprMonitor'; import { addDisposableDomListener } from 'browser/Lifecycle'; -import { IColorSet, IRenderDebouncer, IRenderDebouncerWithCallback } from 'browser/Types'; +import { IColorSet, IRenderDebouncerWithCallback } from 'browser/Types'; import { IOptionsService, IBufferService, IDecorationService } from 'common/services/Services'; import { ICharSizeService, IRenderService } from 'browser/services/Services'; diff --git a/src/browser/services/Services.ts b/src/browser/services/Services.ts index 9f226338..165d0c7e 100644 --- a/src/browser/services/Services.ts +++ b/src/browser/services/Services.ts @@ -5,12 +5,10 @@ import { IEvent } from 'common/EventEmitter'; import { IRenderDimensions, IRenderer } from 'browser/renderer/Types'; -import { IColorSet, IRenderDebouncer } from 'browser/Types'; +import { IColorSet } from 'browser/Types'; import { ISelectionRedrawRequestEvent as ISelectionRequestRedrawEvent, ISelectionRequestScrollLinesEvent } from 'browser/selection/Types'; import { createDecorator } from 'common/services/ServiceRegistry'; import { IDisposable } from 'common/Types'; -import { IDecorationOptions, IDecoration } from 'xterm'; -import { IBufferService } from 'common/services/Services'; export const ICharSizeService = createDecorator('CharSizeService'); export interface ICharSizeService { diff --git a/src/common/CoreTerminal.ts b/src/common/CoreTerminal.ts index 6e318ce7..7960858d 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 { IDisposable, IBufferLine, IAttributeData, ICoreTerminal, IKeyboardEvent, IScrollEvent, ScrollSource, ITerminalOptions as IPublicTerminalOptions } from 'common/Types'; +import { IDisposable, IAttributeData, ICoreTerminal, IScrollEvent, ScrollSource } from 'common/Types'; import { CoreService } from 'common/services/CoreService'; import { EventEmitter, IEvent, forwardEvent } from 'common/EventEmitter'; import { CoreMouseService } from 'common/services/CoreMouseService'; diff --git a/src/common/services/OptionsService.ts b/src/common/services/OptionsService.ts index 744903f1..46e5919c 100644 --- a/src/common/services/OptionsService.ts +++ b/src/common/services/OptionsService.ts @@ -120,7 +120,6 @@ export class OptionsService implements IOptionsService { throw new Error(`"${value}" is not a valid value for ${key}`); } break; - case 'cursorStyle': case 'wordSeparator': if (!value) { value = DEFAULT_OPTIONS[key]; diff --git a/src/common/services/Services.ts b/src/common/services/Services.ts index 82ad7355..9312cd46 100644 --- a/src/common/services/Services.ts +++ b/src/common/services/Services.ts @@ -5,7 +5,7 @@ import { IEvent, IEventEmitter } from 'common/EventEmitter'; import { IBuffer, IBufferSet } from 'common/buffer/Types'; -import { IDecPrivateModes, ICoreMouseEvent, CoreMouseEncoding, ICoreMouseProtocol, CoreMouseEventType, ICharset, IWindowOptions, IModes, IAttributeData, ScrollSource, IDisposable, IColorRGB, IColor, CursorStyle, IOscLinkData } from 'common/Types'; +import { IDecPrivateModes, ICoreMouseEvent, CoreMouseEncoding, ICoreMouseProtocol, CoreMouseEventType, ICharset, IWindowOptions, IModes, IAttributeData, ScrollSource, IDisposable, IColor, CursorStyle, IOscLinkData } from 'common/Types'; import { createDecorator } from 'common/services/ServiceRegistry'; import { IDecorationOptions, IDecoration, ILinkHandler } from 'xterm'; From 9459553665e94713bb8dd3415b8638c0f61eabdb Mon Sep 17 00:00:00 2001 From: Kenneth Russell Date: Fri, 26 Aug 2022 16:50:42 -0700 Subject: [PATCH 46/55] Handle WebGL context loss/restore events in WebGL renderer. If the context is restored within a brief period of time (3 seconds), recreate members dependent on WebGL state, and fire onRequestRedraw to the renderer's listeners. Handle changes to the char atlas better in the demo client. Tested in Chrome Canary on macOS by running the demo in one window, and visiting the internal URL about:gpucrash in another window. The terminal now recovers and redraws properly. --- addons/xterm-addon-webgl/src/WebglRenderer.ts | 52 ++++++++++++++++--- demo/client.ts | 2 +- 2 files changed, 45 insertions(+), 9 deletions(-) diff --git a/addons/xterm-addon-webgl/src/WebglRenderer.ts b/addons/xterm-addon-webgl/src/WebglRenderer.ts index 43de1c98..23c763a2 100644 --- a/addons/xterm-addon-webgl/src/WebglRenderer.ts +++ b/addons/xterm-addon-webgl/src/WebglRenderer.ts @@ -46,8 +46,8 @@ export class WebglRenderer extends Disposable implements IRenderer { private _canvas: HTMLCanvasElement; private _gl: IWebGL2RenderingContext; - private _rectangleRenderer: RectangleRenderer; - private _glyphRenderer: GlyphRenderer; + private _rectangleRenderer!: RectangleRenderer; + private _glyphRenderer!: GlyphRenderer; public dimensions: IRenderDimensions; @@ -62,6 +62,8 @@ export class WebglRenderer extends Disposable implements IRenderer { private _onContextLoss = new EventEmitter(); public get onContextLoss(): IEvent { return this._onContextLoss.event; } + private _contextRestorationTimeout: number | undefined; + constructor( private _terminal: Terminal, private _colors: IColorSet, @@ -108,16 +110,35 @@ export class WebglRenderer extends Disposable implements IRenderer { throw new Error('WebGL2 not supported ' + this._gl); } - this.register(addDisposableDomListener(this._canvas, 'webglcontextlost', (e) => { this._onContextLoss.fire(e); })); + this.register(addDisposableDomListener(this._canvas, 'webglcontextlost', (e) => { + console.log('webglcontextlost event received'); + // Prevent the default behavior in order to enable WebGL context restoration. + e.preventDefault(); + // Wait a few seconds to see if the 'webglcontextrestored' event is fired. + // If not, dispatch the onContextLoss notification to observers. + this._contextRestorationTimeout = setTimeout(() => { + if (this._contextRestorationTimeout !== 0) { + console.log('webgl context not restored; firing onContextLoss'); + this._onContextLoss.fire(e); + } + }, 3000 /* ms */); + })); + this.register(addDisposableDomListener(this._canvas, 'webglcontextrestored', (e) => { + console.log('webglcontextrestored event received'); + clearTimeout(this._contextRestorationTimeout); + this._contextRestorationTimeout = 0; + // The texture atlas and glyph renderer must be fully reinitialized + // because their contents have been lost. + removeTerminalFromCache(this._terminal); + this._initializeWebGLState(); + this._requestRedrawViewport(); + })); + this.register(observeDevicePixelDimensions(this._canvas, (w, h) => this._setCanvasDevicePixelDimensions(w, h))); this._core.screenElement!.appendChild(this._canvas); - this._rectangleRenderer = this.register(new RectangleRenderer(this._terminal, this._colors, this._gl, this.dimensions)); - this._glyphRenderer = this.register(new GlyphRenderer(this._terminal, this._colors, this._gl, this.dimensions)); - - // Update dimensions and acquire char atlas - this.onCharSizeChanged(); + this._initializeWebGLState(); this._isAttached = document.body.contains(this._core.screenElement!); } @@ -235,6 +256,21 @@ export class WebglRenderer extends Disposable implements IRenderer { this._refreshCharAtlas(); } + /** + * Initializes members dependent on WebGL context state. + */ + private _initializeWebGLState(): void { + // Dispose any previous rectangle and glyph renderers before creating new ones. + this._rectangleRenderer?.dispose(); + this._glyphRenderer?.dispose(); + + this._rectangleRenderer = new RectangleRenderer(this._terminal, this._colors, this._gl, this.dimensions); + this._glyphRenderer = new GlyphRenderer(this._terminal, this._colors, this._gl, this.dimensions); + + // Update dimensions and acquire char atlas + this.onCharSizeChanged(); + } + /** * Refreshes the char atlas, aquiring a new one if necessary. * @param terminal The terminal. diff --git a/demo/client.ts b/demo/client.ts index 3593004a..2557fb44 100644 --- a/demo/client.ts +++ b/demo/client.ts @@ -606,7 +606,7 @@ function htmlSerializeButtonHandler(): void { } function addTextureAtlas(e: HTMLCanvasElement) { - document.querySelector('#texture-atlas').appendChild(e); + document.querySelector('#texture-atlas').replaceChildren(e); } function writeCustomGlyphHandler() { From f309affff2d7482188a67cd3c56cabdcb69fe543 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Wed, 31 Aug 2022 05:44:48 -0700 Subject: [PATCH 47/55] Tweak handling of _contextRestorationTimeout --- addons/xterm-addon-webgl/src/WebglRenderer.ts | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/addons/xterm-addon-webgl/src/WebglRenderer.ts b/addons/xterm-addon-webgl/src/WebglRenderer.ts index 23c763a2..6dcd3061 100644 --- a/addons/xterm-addon-webgl/src/WebglRenderer.ts +++ b/addons/xterm-addon-webgl/src/WebglRenderer.ts @@ -53,6 +53,7 @@ export class WebglRenderer extends Disposable implements IRenderer { private _core: ITerminal; private _isAttached: boolean; + private _contextRestorationTimeout: number | undefined; private _onChangeTextureAtlas = new EventEmitter(); public get onChangeTextureAtlas(): IEvent { return this._onChangeTextureAtlas.event; } @@ -62,8 +63,6 @@ export class WebglRenderer extends Disposable implements IRenderer { private _onContextLoss = new EventEmitter(); public get onContextLoss(): IEvent { return this._onContextLoss.event; } - private _contextRestorationTimeout: number | undefined; - constructor( private _terminal: Terminal, private _colors: IColorSet, @@ -117,16 +116,15 @@ export class WebglRenderer extends Disposable implements IRenderer { // Wait a few seconds to see if the 'webglcontextrestored' event is fired. // If not, dispatch the onContextLoss notification to observers. this._contextRestorationTimeout = setTimeout(() => { - if (this._contextRestorationTimeout !== 0) { - console.log('webgl context not restored; firing onContextLoss'); - this._onContextLoss.fire(e); - } + this._contextRestorationTimeout = undefined; + console.warn('webgl context not restored; firing onContextLoss'); + this._onContextLoss.fire(e); }, 3000 /* ms */); })); this.register(addDisposableDomListener(this._canvas, 'webglcontextrestored', (e) => { - console.log('webglcontextrestored event received'); + console.warn('webglcontextrestored event received'); clearTimeout(this._contextRestorationTimeout); - this._contextRestorationTimeout = 0; + this._contextRestorationTimeout = undefined; // The texture atlas and glyph renderer must be fully reinitialized // because their contents have been lost. removeTerminalFromCache(this._terminal); From 6d2d4555e7cbb64dbd61ee64a38f2eb5e4f443c9 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Wed, 31 Aug 2022 06:28:58 -0700 Subject: [PATCH 48/55] Create codeql-analysis.yml --- .github/workflows/codeql-analysis.yml | 72 +++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 .github/workflows/codeql-analysis.yml diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml new file mode 100644 index 00000000..9afca6e8 --- /dev/null +++ b/.github/workflows/codeql-analysis.yml @@ -0,0 +1,72 @@ +# For most projects, this workflow file will not need changing; you simply need +# to commit it to your repository. +# +# You may wish to alter this file to override the set of languages analyzed, +# or to provide custom queries or build logic. +# +# ******** NOTE ******** +# We have attempted to detect the languages in your repository. Please check +# the `language` matrix defined below to confirm you have the correct set of +# supported CodeQL languages. +# +name: "CodeQL" + +on: + push: + branches: [ "master" ] + pull_request: + # The branches below must be a subset of the branches above + branches: [ "master" ] + schedule: + - cron: '41 17 * * 0' + +jobs: + analyze: + name: Analyze + runs-on: ubuntu-latest + permissions: + actions: read + contents: read + security-events: write + + strategy: + fail-fast: false + matrix: + language: [ 'javascript', 'python' ] + # CodeQL supports [ 'cpp', 'csharp', 'go', 'java', 'javascript', 'python', 'ruby' ] + # Learn more about CodeQL language support at https://aka.ms/codeql-docs/language-support + + steps: + - name: Checkout repository + uses: actions/checkout@v3 + + # Initializes the CodeQL tools for scanning. + - name: Initialize CodeQL + uses: github/codeql-action/init@v2 + with: + languages: ${{ matrix.language }} + # If you wish to specify custom queries, you can do so here or in a config file. + # By default, queries listed here will override any specified in a config file. + # Prefix the list here with "+" to use these queries and those in the config file. + + # Details on CodeQL's query packs refer to : https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-code-scanning#using-queries-in-ql-packs + # queries: security-extended,security-and-quality + + + # Autobuild attempts to build any compiled languages (C/C++, C#, or Java). + # If this step fails, then you should remove it and run the build manually (see below) + - name: Autobuild + uses: github/codeql-action/autobuild@v2 + + # ℹ️ Command-line programs to run using the OS shell. + # 📚 See https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsrun + + # If the Autobuild fails above, remove it and uncomment the following three lines. + # modify them (or add more) to build your code if your project, please refer to the EXAMPLE below for guidance. + + # - run: | + # echo "Run, Build Application using script" + # ./location_of_script_within_repo/buildscript.sh + + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@v2 From c5a6b997d155988ecebc8acad11e919beb7fefb9 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Wed, 31 Aug 2022 07:25:03 -0700 Subject: [PATCH 49/55] Remove lgtm comments in favor of codeql dismissable alerts --- bin/extract_vtfeatures.js | 2 +- demo/server.js | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/bin/extract_vtfeatures.js b/bin/extract_vtfeatures.js index 1ccfb636..9b5ab898 100644 --- a/bin/extract_vtfeatures.js +++ b/bin/extract_vtfeatures.js @@ -362,7 +362,7 @@ function* parseMultiLineGen(filename, s) { if (!s.includes('@vt:')) { return; } - const lines = s.split('\n').map(el => el.trim().replace(/\*/, '').replace(/\s/, '')); // lgtm [js/incomplete-sanitization] + const lines = s.split('\n').map(el => el.trim().replace(/\*/, '').replace(/\s/, '')); let grabLine = false; let longDescription = []; let feature = undefined; diff --git a/demo/server.js b/demo/server.js index 71a9d36a..8d295942 100644 --- a/demo/server.js +++ b/demo/server.js @@ -20,19 +20,19 @@ function startServer() { logs = {}; app.use('/xterm.css', express.static(__dirname + '/../css/xterm.css')); - app.get('/logo.png', (req, res) => { // lgtm [js/missing-rate-limiting] + app.get('/logo.png', (req, res) => { res.sendFile(__dirname + '/logo.png'); }); - app.get('/', (req, res) => { // lgtm [js/missing-rate-limiting] + app.get('/', (req, res) => { res.sendFile(__dirname + '/index.html'); }); - app.get('/test', (req, res) => { // lgtm [js/missing-rate-limiting] + app.get('/test', (req, res) => { res.sendFile(__dirname + '/test.html'); }); - app.get('/style.css', (req, res) => { // lgtm [js/missing-rate-limiting] + app.get('/style.css', (req, res) => { res.sendFile(__dirname + '/style.css'); }); From ab8ce61e56945a834ceb2cf086dfc61a88c352f9 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Wed, 31 Aug 2022 07:26:23 -0700 Subject: [PATCH 50/55] Remove python scanning --- .github/workflows/codeql-analysis.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index 9afca6e8..f8c8362a 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -32,7 +32,7 @@ jobs: strategy: fail-fast: false matrix: - language: [ 'javascript', 'python' ] + language: [ 'javascript' ] # CodeQL supports [ 'cpp', 'csharp', 'go', 'java', 'javascript', 'python', 'ruby' ] # Learn more about CodeQL language support at https://aka.ms/codeql-docs/language-support @@ -48,11 +48,11 @@ jobs: # If you wish to specify custom queries, you can do so here or in a config file. # By default, queries listed here will override any specified in a config file. # Prefix the list here with "+" to use these queries and those in the config file. - + # Details on CodeQL's query packs refer to : https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-code-scanning#using-queries-in-ql-packs # queries: security-extended,security-and-quality - + # Autobuild attempts to build any compiled languages (C/C++, C#, or Java). # If this step fails, then you should remove it and run the build manually (see below) - name: Autobuild @@ -61,7 +61,7 @@ jobs: # ℹ️ Command-line programs to run using the OS shell. # 📚 See https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsrun - # If the Autobuild fails above, remove it and uncomment the following three lines. + # If the Autobuild fails above, remove it and uncomment the following three lines. # modify them (or add more) to build your code if your project, please refer to the EXAMPLE below for guidance. # - run: | From 06d6e08347b051955a29b3378c1a47b37acf372a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Thu, 1 Sep 2022 00:50:11 +0200 Subject: [PATCH 51/55] sgr_pixels mouse report --- bin/test_mousemodes.js | 56 +++++--- src/browser/Terminal.ts | 8 +- src/browser/TestUtils.test.ts | 2 +- src/browser/input/Mouse.ts | 14 -- src/browser/services/MouseService.ts | 31 ++++- src/browser/services/Services.ts | 2 +- src/common/InputHandler.ts | 6 + src/common/Types.d.ts | 3 + src/common/services/CoreMouseService.test.ts | 132 ++++++++++--------- src/common/services/CoreMouseService.ts | 34 +++-- 10 files changed, 172 insertions(+), 116 deletions(-) diff --git a/bin/test_mousemodes.js b/bin/test_mousemodes.js index 6a547ab1..b73cd455 100644 --- a/bin/test_mousemodes.js +++ b/bin/test_mousemodes.js @@ -123,19 +123,19 @@ const ENC = { row: report[5] - 32 }) ], - 'UTF8' : [ - '\x1b[?1005h', - // format: CSI M