From 7c5853bf0051b4938690546fb8d67c717b160979 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 1 Apr 2022 05:30:01 -0700 Subject: [PATCH 1/7] Create color zone system Part of #3703 --- src/browser/Decorations/ColorZoneStore.ts | 94 +++++++++++++++++++ .../Decorations/OverviewRulerRenderer.ts | 25 ++++- 2 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 src/browser/Decorations/ColorZoneStore.ts diff --git a/src/browser/Decorations/ColorZoneStore.ts b/src/browser/Decorations/ColorZoneStore.ts new file mode 100644 index 0000000000..c582323499 --- /dev/null +++ b/src/browser/Decorations/ColorZoneStore.ts @@ -0,0 +1,94 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { IInternalDecoration } from 'common/services/Services'; +import { IDecorationOverviewRulerOptions } from 'xterm'; + +export interface IColorZoneStore { + readonly zones: IColorZone[]; + clear(): void; + addDecoration(decoration: IInternalDecoration): void; + /** + * Sets the amount of padding in lines that will be added between zones, if new lines intersect + * the padding they will be merged into the same zone. + */ + setPadding(padding: { [position: string]: number }): void; +} + +export interface IColorZone { + /** Color in a format supported by canvas' fillStyle. */ + color: string; + position: 'full' | 'left' | 'center' | 'right' | undefined; + startBufferLine: number; + endBufferLine: number; +} + +export class ColorZoneStore implements IColorZoneStore { + private _zones: IColorZone[] = []; + public get zones(): IColorZone[] { return this._zones; } + + private _linePadding: { [position: string]: number } = { + full: 0, + left: 0, + center: 0, + right: 0 + }; + + public clear(): void { + this._zones.length = 0; + } + + public addDecoration(decoration: IInternalDecoration): void { + if (!decoration.options.overviewRulerOptions) { + return; + } + for (const z of this._zones) { + if (z.color === decoration.options.overviewRulerOptions.color && + z.position === decoration.options.overviewRulerOptions.position) { + if (this._lineIntersectsZone(z, decoration.marker.line)) { + console.log('intersects, skip'); + // this._addLineToZone(z, decoration.marker.line); + return; + } + if (this._lineAdjacentToZone(z, decoration.marker.line, decoration.options.overviewRulerOptions.position)) { + this._addLineToZone(z, decoration.marker.line); + console.log('adjacent, add'); + return; + } + } + } + // TODO: Track zones in an object pool to reduce GC + this._zones.push({ + color: decoration.options.overviewRulerOptions.color, + position: decoration.options.overviewRulerOptions.position, + startBufferLine: decoration.marker.line, + endBufferLine: decoration.marker.line + }); + } + + public setPadding(padding: { [position: string]: number }): void { + console.log('padding', padding); + this._linePadding = padding; + } + + private _lineIntersectsZone(zone: IColorZone, line: number): boolean { + return ( + line >= zone.startBufferLine && + line <= zone.endBufferLine + ); + } + + private _lineAdjacentToZone(zone: IColorZone, line: number, position: IColorZone['position']): boolean { + return ( + (line >= zone.startBufferLine - 1 - this._linePadding[position || 'full'] * 2) && + (line <= zone.endBufferLine + 1 + this._linePadding[position || 'full'] * 2) + ); + } + + private _addLineToZone(zone: IColorZone, line: number): void { + zone.startBufferLine = Math.min(zone.startBufferLine, line); + zone.endBufferLine = Math.max(zone.endBufferLine, line); + } +} diff --git a/src/browser/Decorations/OverviewRulerRenderer.ts b/src/browser/Decorations/OverviewRulerRenderer.ts index 1407f4bbe2..8e55dec5e8 100644 --- a/src/browser/Decorations/OverviewRulerRenderer.ts +++ b/src/browser/Decorations/OverviewRulerRenderer.ts @@ -3,6 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import { ColorZoneStore, IColorZoneStore } from 'browser/Decorations/ColorZoneStore'; import { addDisposableDomListener } from 'browser/Lifecycle'; import { IRenderService } from 'browser/services/Services'; import { Disposable } from 'common/Lifecycle'; @@ -33,6 +34,7 @@ export class OverviewRulerRenderer extends Disposable { private readonly _canvas: HTMLCanvasElement; private readonly _ctx: CanvasRenderingContext2D; private readonly _decorationElements: Map = new Map(); + private readonly _colorZoneStore: IColorZoneStore = new ColorZoneStore(); private get _width(): number { return this._optionsService.options.overviewRulerWidth || 0; } @@ -40,6 +42,7 @@ export class OverviewRulerRenderer extends Disposable { private _shouldUpdateDimensions: boolean | undefined = true; private _shouldUpdateAnchor: boolean | undefined = true; + private _lastKnownBufferLength: number = 0; private _containerHeight: number | undefined; @@ -84,6 +87,11 @@ export class OverviewRulerRenderer extends Disposable { this.register(this._bufferService.buffers.onBufferActivate(() => { this._canvas!.style.display = this._bufferService.buffer === this._bufferService.buffers.alt ? 'none' : 'block'; })); + this.register(this._bufferService.onScroll(() => { + if (this._lastKnownBufferLength !== this._bufferService.buffers.normal.lines.length) { + this._refreshColorZonePadding(); + } + })); } /** * On dimension change, update canvas dimensions @@ -140,6 +148,17 @@ export class OverviewRulerRenderer extends Disposable { drawX.right = drawWidth.left + drawWidth.center; } + private _refreshColorZonePadding(): void { + const nonFullPadding = Math.ceil(this._bufferService.buffers.active.lines.length / (this._canvas.height - 1) * (drawHeight.full / 2)); + this._colorZoneStore.setPadding({ + full: Math.ceil(this._bufferService.buffers.active.lines.length / (this._canvas.height - 1) * (drawHeight.full / 2)), + left: nonFullPadding, + center: nonFullPadding, + right: nonFullPadding + }); + this._lastKnownBufferLength = this._bufferService.buffers.normal.lines.length; + } + private _refreshStyle(decoration: IInternalDecoration): void { if (!decoration.options.overviewRulerOptions) { this._decorationElements.delete(decoration); @@ -151,7 +170,7 @@ export class OverviewRulerRenderer extends Disposable { /* x */ drawX[decoration.options.overviewRulerOptions.position!], /* y */ Math.round( (this._canvas.height - 1) * // -1 to ensure at least 2px are allowed for decoration on last line - (decoration.options.marker.line / this._bufferService.buffers.active.lines.length) - drawHeight[decoration.options.overviewRulerOptions.position!] / 2 + (decoration.options.marker.line / this._bufferService.buffers.active.lines.length) - drawHeight[decoration.options.overviewRulerOptions.position!] / 2 ), /* w */ drawWidth[decoration.options.overviewRulerOptions.position!], /* h */ drawHeight[decoration.options.overviewRulerOptions.position!] @@ -164,6 +183,7 @@ export class OverviewRulerRenderer extends Disposable { this._canvas.style.height = `${this._screenElement.clientHeight}px`; this._canvas.height = Math.round(this._screenElement.clientHeight * window.devicePixelRatio); this._refreshDrawConstants(); + this._refreshColorZonePadding(); } private _refreshDecorations(): void { @@ -171,7 +191,9 @@ export class OverviewRulerRenderer extends Disposable { this._refreshCanvasDimensions(); } this._ctx.clearRect(0, 0, this._canvas.width, this._canvas.height); + this._colorZoneStore.clear(); for (const decoration of this._decorationService.decorations) { + this._colorZoneStore.addDecoration(decoration); if (decoration.options.overviewRulerOptions && decoration.options.overviewRulerOptions.position !== 'full') { this._renderDecoration(decoration); } @@ -181,6 +203,7 @@ export class OverviewRulerRenderer extends Disposable { this._renderDecoration(decoration); } } + console.log('zones', this._colorZoneStore.zones); this._shouldUpdateDimensions = false; this._shouldUpdateAnchor = false; } From 8524c23deb34c10e06ea0428996afa81f4d0c55a Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 1 Apr 2022 05:55:53 -0700 Subject: [PATCH 2/7] Render overview ruler using color zones Fixes #3703 --- src/browser/Decorations/ColorZoneStore.ts | 8 +-- .../Decorations/OverviewRulerRenderer.ts | 64 ++++++++----------- 2 files changed, 27 insertions(+), 45 deletions(-) diff --git a/src/browser/Decorations/ColorZoneStore.ts b/src/browser/Decorations/ColorZoneStore.ts index c582323499..457749654e 100644 --- a/src/browser/Decorations/ColorZoneStore.ts +++ b/src/browser/Decorations/ColorZoneStore.ts @@ -4,7 +4,6 @@ *--------------------------------------------------------------------------------------------*/ import { IInternalDecoration } from 'common/services/Services'; -import { IDecorationOverviewRulerOptions } from 'xterm'; export interface IColorZoneStore { readonly zones: IColorZone[]; @@ -48,13 +47,10 @@ export class ColorZoneStore implements IColorZoneStore { if (z.color === decoration.options.overviewRulerOptions.color && z.position === decoration.options.overviewRulerOptions.position) { if (this._lineIntersectsZone(z, decoration.marker.line)) { - console.log('intersects, skip'); - // this._addLineToZone(z, decoration.marker.line); return; } if (this._lineAdjacentToZone(z, decoration.marker.line, decoration.options.overviewRulerOptions.position)) { this._addLineToZone(z, decoration.marker.line); - console.log('adjacent, add'); return; } } @@ -82,8 +78,8 @@ export class ColorZoneStore implements IColorZoneStore { private _lineAdjacentToZone(zone: IColorZone, line: number, position: IColorZone['position']): boolean { return ( - (line >= zone.startBufferLine - 1 - this._linePadding[position || 'full'] * 2) && - (line <= zone.endBufferLine + 1 + this._linePadding[position || 'full'] * 2) + (line >= zone.startBufferLine - this._linePadding[position || 'full'] * 2) && + (line <= zone.endBufferLine + this._linePadding[position || 'full'] * 2) ); } diff --git a/src/browser/Decorations/OverviewRulerRenderer.ts b/src/browser/Decorations/OverviewRulerRenderer.ts index 8e55dec5e8..d2ffdc2d28 100644 --- a/src/browser/Decorations/OverviewRulerRenderer.ts +++ b/src/browser/Decorations/OverviewRulerRenderer.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { ColorZoneStore, IColorZoneStore } from 'browser/Decorations/ColorZoneStore'; +import { ColorZoneStore, IColorZone, IColorZoneStore } from 'browser/Decorations/ColorZoneStore'; import { addDisposableDomListener } from 'browser/Lifecycle'; import { IRenderService } from 'browser/services/Services'; import { Disposable } from 'common/Lifecycle'; @@ -149,34 +149,15 @@ export class OverviewRulerRenderer extends Disposable { } private _refreshColorZonePadding(): void { - const nonFullPadding = Math.ceil(this._bufferService.buffers.active.lines.length / (this._canvas.height - 1) * (drawHeight.full / 2)); this._colorZoneStore.setPadding({ - full: Math.ceil(this._bufferService.buffers.active.lines.length / (this._canvas.height - 1) * (drawHeight.full / 2)), - left: nonFullPadding, - center: nonFullPadding, - right: nonFullPadding + full: this._bufferService.buffers.active.lines.length / (this._canvas.height - 1) * (drawHeight.full / 2), + left: this._bufferService.buffers.active.lines.length / (this._canvas.height - 1) * (drawHeight.left / 2), + center: this._bufferService.buffers.active.lines.length / (this._canvas.height - 1) * (drawHeight.center / 2), + right: this._bufferService.buffers.active.lines.length / (this._canvas.height - 1) * (drawHeight.right / 2) }); this._lastKnownBufferLength = this._bufferService.buffers.normal.lines.length; } - private _refreshStyle(decoration: IInternalDecoration): void { - if (!decoration.options.overviewRulerOptions) { - this._decorationElements.delete(decoration); - return; - } - this._ctx.lineWidth = 1; - this._ctx.fillStyle = decoration.options.overviewRulerOptions.color; - this._ctx.fillRect( - /* x */ drawX[decoration.options.overviewRulerOptions.position!], - /* y */ Math.round( - (this._canvas.height - 1) * // -1 to ensure at least 2px are allowed for decoration on last line - (decoration.options.marker.line / this._bufferService.buffers.active.lines.length) - drawHeight[decoration.options.overviewRulerOptions.position!] / 2 - ), - /* w */ drawWidth[decoration.options.overviewRulerOptions.position!], - /* h */ drawHeight[decoration.options.overviewRulerOptions.position!] - ); - } - private _refreshCanvasDimensions(): void { this._canvas.style.width = `${this._width}px`; this._canvas.width = Math.round(this._width * window.devicePixelRatio); @@ -194,27 +175,32 @@ export class OverviewRulerRenderer extends Disposable { this._colorZoneStore.clear(); for (const decoration of this._decorationService.decorations) { this._colorZoneStore.addDecoration(decoration); - if (decoration.options.overviewRulerOptions && decoration.options.overviewRulerOptions.position !== 'full') { - this._renderDecoration(decoration); - } } - for (const decoration of this._decorationService.decorations) { - if (decoration.options.overviewRulerOptions && decoration.options.overviewRulerOptions.position === 'full') { - this._renderDecoration(decoration); - } + this._ctx.lineWidth = 1; + for (const zone of this._colorZoneStore.zones) { + this._renderColorZone(zone); } - console.log('zones', this._colorZoneStore.zones); this._shouldUpdateDimensions = false; this._shouldUpdateAnchor = false; } - private _renderDecoration(decoration: IInternalDecoration): void { - const element = this._decorationElements.get(decoration); - if (!element) { - this._decorationElements.set(decoration, this._canvas); - decoration.onDispose(() => this._queueRefresh()); - } - this._refreshStyle(decoration); + private _renderColorZone(zone: IColorZone): void { + // TODO: Is _decorationElements needed? + + this._ctx.fillStyle = zone.color; + console.log('zone height', drawHeight[zone.position || 'full']); + this._ctx.fillRect( + /* x */ drawX[zone.position || 'full'], + /* y */ Math.round( + (this._canvas.height - 1) * // -1 to ensure at least 2px are allowed for decoration on last line + (zone.startBufferLine / this._bufferService.buffers.active.lines.length) - drawHeight[zone.position || 'full'] / 2 + ), + /* w */ drawWidth[zone.position || 'full'], + /* h */ Math.round( + (this._canvas.height - 1) * // -1 to ensure at least 2px are allowed for decoration on last line + ((zone.endBufferLine - zone.startBufferLine) / this._bufferService.buffers.active.lines.length) + drawHeight[zone.position || 'full'] + ) + ); } private _queueRefresh(updateCanvasDimensions?: boolean, updateAnchor?: boolean): void { From 6b26a1bcf8bda287b269884df80ba8f8206bf489 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 1 Apr 2022 06:05:30 -0700 Subject: [PATCH 3/7] Stick to integers for padding and render full after --- .../Decorations/OverviewRulerRenderer.ts | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/browser/Decorations/OverviewRulerRenderer.ts b/src/browser/Decorations/OverviewRulerRenderer.ts index d2ffdc2d28..de2342f1fb 100644 --- a/src/browser/Decorations/OverviewRulerRenderer.ts +++ b/src/browser/Decorations/OverviewRulerRenderer.ts @@ -150,10 +150,10 @@ export class OverviewRulerRenderer extends Disposable { private _refreshColorZonePadding(): void { this._colorZoneStore.setPadding({ - full: this._bufferService.buffers.active.lines.length / (this._canvas.height - 1) * (drawHeight.full / 2), - left: this._bufferService.buffers.active.lines.length / (this._canvas.height - 1) * (drawHeight.left / 2), - center: this._bufferService.buffers.active.lines.length / (this._canvas.height - 1) * (drawHeight.center / 2), - right: this._bufferService.buffers.active.lines.length / (this._canvas.height - 1) * (drawHeight.right / 2) + full: Math.floor(this._bufferService.buffers.active.lines.length / (this._canvas.height - 1) * (drawHeight.full / 2)), + left: Math.floor(this._bufferService.buffers.active.lines.length / (this._canvas.height - 1) * (drawHeight.left / 2)), + center: Math.floor(this._bufferService.buffers.active.lines.length / (this._canvas.height - 1) * (drawHeight.center / 2)), + right: Math.floor(this._bufferService.buffers.active.lines.length / (this._canvas.height - 1) * (drawHeight.right / 2)) }); this._lastKnownBufferLength = this._bufferService.buffers.normal.lines.length; } @@ -177,8 +177,16 @@ export class OverviewRulerRenderer extends Disposable { this._colorZoneStore.addDecoration(decoration); } this._ctx.lineWidth = 1; - for (const zone of this._colorZoneStore.zones) { - this._renderColorZone(zone); + const zones = this._colorZoneStore.zones; + for (const zone of zones) { + if (zone.position !== 'full') { + this._renderColorZone(zone); + } + } + for (const zone of zones) { + if (zone.position === 'full') { + this._renderColorZone(zone); + } } this._shouldUpdateDimensions = false; this._shouldUpdateAnchor = false; From d072a628ca9bcafac4ae4b68b9eabc413e1164fc Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 1 Apr 2022 06:25:55 -0700 Subject: [PATCH 4/7] Object pool, correct merging of zones --- demo/client.ts | 4 +-- src/browser/Decorations/ColorZoneStore.ts | 33 ++++++++++++++++--- .../Decorations/OverviewRulerRenderer.ts | 9 +++-- 3 files changed, 34 insertions(+), 12 deletions(-) diff --git a/demo/client.ts b/demo/client.ts index 55ff8d62d1..7c21956aa7 100644 --- a/demo/client.ts +++ b/demo/client.ts @@ -556,8 +556,8 @@ function loadTest() { function addDecoration() { term.options['overviewRulerWidth'] = 15; const marker = term.addMarker(1); - const decoration = term.registerDecoration({ marker, overviewRulerOptions: { color: '#ef2929'} }); - decoration.onRender((e) => e.style.backgroundColor = '#ef2929'); + const decoration = term.registerDecoration({ marker, overviewRulerOptions: { color: '#ef292980', position: 'left' } }); + decoration.onRender((e) => e.style.backgroundColor = '#ef292980'); } function addOverviewRuler() { diff --git a/src/browser/Decorations/ColorZoneStore.ts b/src/browser/Decorations/ColorZoneStore.ts index 457749654e..bf68dc1482 100644 --- a/src/browser/Decorations/ColorZoneStore.ts +++ b/src/browser/Decorations/ColorZoneStore.ts @@ -26,7 +26,12 @@ export interface IColorZone { export class ColorZoneStore implements IColorZoneStore { private _zones: IColorZone[] = []; - public get zones(): IColorZone[] { return this._zones; } + + // The zone pool is used to keep zone objects from being freed between clearing the color zone + // store and fetching the zones. This helps reduce GC pressure since the color zones are + // accumulated on potentially every scroll event. + private _zonePool: IColorZone[] = []; + private _zonePoolIndex = 0; private _linePadding: { [position: string]: number } = { full: 0, @@ -35,8 +40,15 @@ export class ColorZoneStore implements IColorZoneStore { right: 0 }; + public get zones(): IColorZone[] { + // Trim the zone pool to free unused memory + this._zonePool.length = Math.min(this._zonePool.length, this._zones.length); + return this._zones; + } + public clear(): void { this._zones.length = 0; + this._zonePoolIndex = 0; } public addDecoration(decoration: IInternalDecoration): void { @@ -50,22 +62,33 @@ export class ColorZoneStore implements IColorZoneStore { return; } if (this._lineAdjacentToZone(z, decoration.marker.line, decoration.options.overviewRulerOptions.position)) { + console.log('add line to zone'); this._addLineToZone(z, decoration.marker.line); return; } } } - // TODO: Track zones in an object pool to reduce GC + // Create using zone pool if possible + if (this._zonePoolIndex < this._zonePool.length) { + this._zonePool[this._zonePoolIndex].color = decoration.options.overviewRulerOptions.color; + this._zonePool[this._zonePoolIndex].position = decoration.options.overviewRulerOptions.position; + this._zonePool[this._zonePoolIndex].startBufferLine = decoration.marker.line; + this._zonePool[this._zonePoolIndex].endBufferLine = decoration.marker.line; + this._zones.push(this._zonePool[this._zonePoolIndex++]); + return; + } + // Create this._zones.push({ color: decoration.options.overviewRulerOptions.color, position: decoration.options.overviewRulerOptions.position, startBufferLine: decoration.marker.line, endBufferLine: decoration.marker.line }); + this._zonePool.push(this._zones[this._zones.length - 1]); + this._zonePoolIndex++; } public setPadding(padding: { [position: string]: number }): void { - console.log('padding', padding); this._linePadding = padding; } @@ -78,8 +101,8 @@ export class ColorZoneStore implements IColorZoneStore { private _lineAdjacentToZone(zone: IColorZone, line: number, position: IColorZone['position']): boolean { return ( - (line >= zone.startBufferLine - this._linePadding[position || 'full'] * 2) && - (line <= zone.endBufferLine + this._linePadding[position || 'full'] * 2) + (line >= zone.startBufferLine - this._linePadding[position || 'full']) && + (line <= zone.endBufferLine + this._linePadding[position || 'full']) ); } diff --git a/src/browser/Decorations/OverviewRulerRenderer.ts b/src/browser/Decorations/OverviewRulerRenderer.ts index de2342f1fb..2b5f2dc4eb 100644 --- a/src/browser/Decorations/OverviewRulerRenderer.ts +++ b/src/browser/Decorations/OverviewRulerRenderer.ts @@ -150,10 +150,10 @@ export class OverviewRulerRenderer extends Disposable { private _refreshColorZonePadding(): void { this._colorZoneStore.setPadding({ - full: Math.floor(this._bufferService.buffers.active.lines.length / (this._canvas.height - 1) * (drawHeight.full / 2)), - left: Math.floor(this._bufferService.buffers.active.lines.length / (this._canvas.height - 1) * (drawHeight.left / 2)), - center: Math.floor(this._bufferService.buffers.active.lines.length / (this._canvas.height - 1) * (drawHeight.center / 2)), - right: Math.floor(this._bufferService.buffers.active.lines.length / (this._canvas.height - 1) * (drawHeight.right / 2)) + full: Math.floor(this._bufferService.buffers.active.lines.length / (this._canvas.height - 1) * drawHeight.full), + left: Math.floor(this._bufferService.buffers.active.lines.length / (this._canvas.height - 1) * drawHeight.left), + center: Math.floor(this._bufferService.buffers.active.lines.length / (this._canvas.height - 1) * drawHeight.center), + right: Math.floor(this._bufferService.buffers.active.lines.length / (this._canvas.height - 1) * drawHeight.right) }); this._lastKnownBufferLength = this._bufferService.buffers.normal.lines.length; } @@ -196,7 +196,6 @@ export class OverviewRulerRenderer extends Disposable { // TODO: Is _decorationElements needed? this._ctx.fillStyle = zone.color; - console.log('zone height', drawHeight[zone.position || 'full']); this._ctx.fillRect( /* x */ drawX[zone.position || 'full'], /* y */ Math.round( From e98a20ba9ed29e8d1313151a238903b14bfe254c Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 1 Apr 2022 07:04:52 -0700 Subject: [PATCH 5/7] Fix typo in test name --- src/browser/Terminal.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/browser/Terminal.test.ts b/src/browser/Terminal.test.ts index d039b17b88..3eafb26150 100644 --- a/src/browser/Terminal.test.ts +++ b/src/browser/Terminal.test.ts @@ -231,7 +231,7 @@ describe('Terminal', () => { }); term.paste('foo'); }); - it('should sanitize \n chars', done => { + it('should sanitize \\n chars', done => { term.onData(e => { assert.equal(e, '\rfoo\rbar\r'); done(); From 7eace4d4799516f3f563cb3da5066c2eca6f700f Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 1 Apr 2022 07:05:05 -0700 Subject: [PATCH 6/7] Add tests for ColorZoneStore --- .../Decorations/ColorZoneStore.test.ts | 88 +++++++++++++++++++ src/browser/Decorations/ColorZoneStore.ts | 8 +- 2 files changed, 94 insertions(+), 2 deletions(-) create mode 100644 src/browser/Decorations/ColorZoneStore.test.ts diff --git a/src/browser/Decorations/ColorZoneStore.test.ts b/src/browser/Decorations/ColorZoneStore.test.ts new file mode 100644 index 0000000000..73e3402f65 --- /dev/null +++ b/src/browser/Decorations/ColorZoneStore.test.ts @@ -0,0 +1,88 @@ +/** + * Copyright (c) 2017 The xterm.js authors. All rights reserved. + * @license MIT + */ + +import { assert } from 'chai'; +import { ColorZoneStore } from 'browser/Decorations/ColorZoneStore'; + +const optionsRedFull = { + overviewRulerOptions: { + color: 'red', + position: 'full' as 'full' + } +}; + +describe('ColorZoneStore', () => { + let store: ColorZoneStore; + + beforeEach(() => { + store = new ColorZoneStore(); + store.setPadding({ + full: 1, + left: 1, + center: 1, + right: 1 + }); + }); + + it('should merge adjacent zones', () => { + store.addDecoration({ + marker: { line: 0 }, + options: optionsRedFull + }); + store.addDecoration({ + marker: { line: 1 }, + options: optionsRedFull + }); + assert.deepStrictEqual(store.zones, [ + { + color: 'red', + position: 'full', + startBufferLine: 0, + endBufferLine: 1 + } + ]); + }); + + it('should not merge non-adjacent zones', () => { + store.addDecoration({ + marker: { line: 0 }, + options: optionsRedFull + }); + store.addDecoration({ + marker: { line: 2 }, + options: optionsRedFull + }); + assert.deepStrictEqual(store.zones, [ + { + color: 'red', + position: 'full', + startBufferLine: 0, + endBufferLine: 0 + }, + { + color: 'red', + position: 'full', + startBufferLine: 2, + endBufferLine: 2 + } + ]); + }); + + it('should reuse zone objects', () => { + const obj = { + marker: { line: 0 }, + options: optionsRedFull + }; + store.addDecoration(obj); + const zone = store.zones[0]; + store.clear(); + store.addDecoration({ + marker: { line: 1 }, + options: optionsRedFull + }); + // The object reference should be the same + assert.equal(zone, store.zones[0]); + }); +}); diff --git a/src/browser/Decorations/ColorZoneStore.ts b/src/browser/Decorations/ColorZoneStore.ts index bf68dc1482..d066bedb80 100644 --- a/src/browser/Decorations/ColorZoneStore.ts +++ b/src/browser/Decorations/ColorZoneStore.ts @@ -24,6 +24,11 @@ export interface IColorZone { endBufferLine: number; } +interface IMinimalDecorationForColorZone { + marker: Pick; + options: Pick; +} + export class ColorZoneStore implements IColorZoneStore { private _zones: IColorZone[] = []; @@ -51,7 +56,7 @@ export class ColorZoneStore implements IColorZoneStore { this._zonePoolIndex = 0; } - public addDecoration(decoration: IInternalDecoration): void { + public addDecoration(decoration: IMinimalDecorationForColorZone): void { if (!decoration.options.overviewRulerOptions) { return; } @@ -62,7 +67,6 @@ export class ColorZoneStore implements IColorZoneStore { return; } if (this._lineAdjacentToZone(z, decoration.marker.line, decoration.options.overviewRulerOptions.position)) { - console.log('add line to zone'); this._addLineToZone(z, decoration.marker.line); return; } From 8158a16629dd1da39e43e953c5b24a4b8c7204b6 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 1 Apr 2022 07:07:13 -0700 Subject: [PATCH 7/7] Remove unused _decorationElements --- src/browser/Decorations/OverviewRulerRenderer.ts | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/browser/Decorations/OverviewRulerRenderer.ts b/src/browser/Decorations/OverviewRulerRenderer.ts index 2b5f2dc4eb..f34338cee5 100644 --- a/src/browser/Decorations/OverviewRulerRenderer.ts +++ b/src/browser/Decorations/OverviewRulerRenderer.ts @@ -33,7 +33,6 @@ const drawX = { export class OverviewRulerRenderer extends Disposable { private readonly _canvas: HTMLCanvasElement; private readonly _ctx: CanvasRenderingContext2D; - private readonly _decorationElements: Map = new Map(); private readonly _colorZoneStore: IColorZoneStore = new ColorZoneStore(); private get _width(): number { return this._optionsService.options.overviewRulerWidth || 0; @@ -75,7 +74,6 @@ export class OverviewRulerRenderer extends Disposable { */ private _registerDecorationListeners(): void { this.register(this._decorationService.onDecorationRegistered(() => this._queueRefresh(undefined, true))); - this.register(this._decorationService.onDecorationRemoved(decoration => this._removeDecoration(decoration))); } /** @@ -120,10 +118,6 @@ export class OverviewRulerRenderer extends Disposable { } public override dispose(): void { - for (const decoration of this._decorationElements) { - decoration[0].dispose(); - } - this._decorationElements.clear(); this._canvas?.remove(); super.dispose(); } @@ -221,9 +215,4 @@ export class OverviewRulerRenderer extends Disposable { this._animationFrame = undefined; }); } - - private _removeDecoration(decoration: IInternalDecoration): void { - this._decorationElements.get(decoration)?.remove(); - this._decorationElements.delete(decoration); - } }