From c7122e8447beea0dac15cd25982f7cf68adc67a2 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 30 Jul 2022 10:29:41 -0700 Subject: [PATCH 1/8] Fix dim flag applying to selection in webgl Fixes #3970 --- addons/xterm-addon-webgl/src/WebglRenderer.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/addons/xterm-addon-webgl/src/WebglRenderer.ts b/addons/xterm-addon-webgl/src/WebglRenderer.ts index d2d4f478..6616db6e 100644 --- a/addons/xterm-addon-webgl/src/WebglRenderer.ts +++ b/addons/xterm-addon-webgl/src/WebglRenderer.ts @@ -402,6 +402,7 @@ export class WebglRenderer extends Disposable implements IRenderer { // override logic throughout the different sub-renderers let bgOverride: number | undefined; let fgOverride: number | undefined; + let isSelected: boolean = false; // Apply decorations on the bottom layer for (const d of this._decorationService.getDecorationsAtCell(x, y, 'bottom')) { @@ -414,7 +415,8 @@ export class WebglRenderer extends Disposable implements IRenderer { } // Apply the selection color if needed - if (this._isCellSelected(x, y)) { + isSelected = this._isCellSelected(x, y); + if (isSelected) { bgOverride = (this._coreBrowserService.isFocused ? this._colors.selectionBackgroundOpaque : this._colors.selectionInactiveBackgroundOpaque).rgba >> 8 & 0xFFFFFF; if (this._colors.selectionForeground) { fgOverride = this._colors.selectionForeground.rgba >> 8 & 0xFFFFFF; @@ -434,8 +436,13 @@ export class WebglRenderer extends Disposable implements IRenderer { // 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) { - // Non-RGB attributes from model + override + force RGB color mode - bgOverride = (this._workCell.bg & ~Attributes.RGB_MASK) | bgOverride | Attributes.CM_RGB; + if (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; + } else { + // Non-RGB attributes from model + override + force RGB color mode + bgOverride = (this._workCell.bg & ~Attributes.RGB_MASK) | bgOverride | Attributes.CM_RGB; + } } if (fgOverride !== undefined) { // Non-RGB attributes from model + force disable inverse + override + force RGB color mode From ce4a85d07d23bc5e1d015b32e7c8283e07900280 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 30 Jul 2022 17:52:00 -0700 Subject: [PATCH 2/8] Clip outline drawing near underline Fixes #3975 --- addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts index e5f0fe6c..bd8aa2ae 100644 --- a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts +++ b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts @@ -516,9 +516,17 @@ export class WebglCharAtlas implements IDisposable { // text if (!this._config.allowTransparency && chars !== ' ') { // 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 ed492f72536f28136ca7afcd883fa487dae044fc Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 30 Jul 2022 19:24:09 -0700 Subject: [PATCH 3/8] Implement improved texture packing strategy Fixes #3977 --- .../src/atlas/WebglCharAtlas.ts | 108 ++++++++++++++---- 1 file changed, 84 insertions(+), 24 deletions(-) diff --git a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts index e5f0fe6c..008e65a8 100644 --- a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts +++ b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts @@ -45,6 +45,12 @@ const NULL_RASTERIZED_GLYPH: IRasterizedGlyph = { const TMP_CANVAS_GLYPH_PADDING = 2; +interface ICharAtlasActiveRow { + x: number; + y: number; + height: number; +} + export class WebglCharAtlas implements IDisposable { private _didWarmUp: boolean = false; @@ -59,13 +65,22 @@ export class WebglCharAtlas implements IDisposable { // A temporary context that glyphs are drawn to before being transfered to the atlas. private _tmpCtx: CanvasRenderingContext2D; - // Since glyphs are expected to be around the same height, the packing - // strategy used it to fill a row with glyphs while keeping track of the - // tallest glyph in the row. Once the row is full a new row is started at - // (0,lastRow+lastRowTallestGlyph). - private _currentRowY: number = 0; - private _currentRowX: number = 0; - private _currentRowHeight: number = 0; + // Texture atlas current positioning data. The texture packing strategy used is to fill from + // left-to-right and top-to-bottom. When the glyph being written is less than half of the current + // row's height, the following happens: + // + // - The current row becomes the fixed height row A + // - A new fixed height row B the exact size of the glyph is created below the current row + // - A new dynamic height current row is created below B + // + // This strategy does a good job preventing space being wasted for very short glyphs such as + // underscores, hyphens etc. or those with underlines rendered. + private _currentRow: ICharAtlasActiveRow = { + x: 0, + y: 0, + height: 0 + }; + private readonly _fixedRows: ICharAtlasActiveRow[] = []; public hasCanvasChanged = false; @@ -118,7 +133,7 @@ export class WebglCharAtlas implements IDisposable { } public beginFrame(): boolean { - if (this._currentRowY > TEXTURE_CAPACITY) { + if (this._currentRow.y > TEXTURE_CAPACITY) { this.clearTexture(); this.warmUp(); return true; @@ -127,15 +142,16 @@ export class WebglCharAtlas implements IDisposable { } public clearTexture(): void { - if (this._currentRowX === 0 && this._currentRowY === 0) { + if (this._currentRow.x === 0 && this._currentRow.y === 0) { return; } this._cacheCtx.clearRect(0, 0, TEXTURE_WIDTH, TEXTURE_HEIGHT); this._cacheMap = {}; this._cacheMapCombined = {}; - this._currentRowHeight = 0; - this._currentRowX = 0; - this._currentRowY = 0; + this._currentRow.x = 0; + this._currentRow.y = 0; + this._currentRow.height = 0; + this._fixedRows.length = 0; this._didWarmUp = false; } @@ -580,22 +596,66 @@ export class WebglCharAtlas implements IDisposable { const rasterizedGlyph = this._findGlyphBoundingBox(imageData, this._workBoundingBox, allowedWidth, powerLineGlyph, drawSuccess); const clippedImageData = this._clipImageData(imageData, this._workBoundingBox); - // Check if there is enough room in the current row and go to next if needed - if (this._currentRowX + rasterizedGlyph.size.x > TEXTURE_WIDTH) { - this._currentRowX = 0; - this._currentRowY += this._currentRowHeight; - this._currentRowHeight = 0; + // Find the best atlas row to use + let activeRow: ICharAtlasActiveRow; + while (true) { + // Select the ideal existing row, preferring fixed rows over the current row + activeRow = this._currentRow; + for (const row of this._fixedRows) { + if ((activeRow === this._currentRow || row.height < activeRow.height) && rasterizedGlyph.size.y <= row.height) { + activeRow = row; + } + } + + // Create a new one if vertical space would be wasted, fixing the previously active row in the + // process as it now has a fixed height + if (activeRow.height > rasterizedGlyph.size.y * 2) { + // Fix the current row as the new row is being added below + if (this._currentRow.height > 0) { + this._fixedRows.push(this._currentRow); + } + + // Create the new fixed height row + activeRow = { + x: 0, + y: this._currentRow.y + this._currentRow.height, + height: rasterizedGlyph.size.y + }; + this._fixedRows.push(activeRow); + + // Create the new current row below the new fixed height row + this._currentRow = { + x: 0, + y: activeRow.y + activeRow.height, + height: 0 + }; + } + + // Exit the loop if there is enough room in the row + if (activeRow.x + rasterizedGlyph.size.x <= TEXTURE_WIDTH) { + break; + } + + // If there is enough room in the current row, finish it and try again + if (activeRow === this._currentRow) { + activeRow.x = 0; + activeRow.y += activeRow.height; + activeRow.height = 0; + } else { + this._fixedRows.splice(this._fixedRows.indexOf(activeRow), 1); + } } // Record texture position - rasterizedGlyph.texturePosition.x = this._currentRowX; - rasterizedGlyph.texturePosition.y = this._currentRowY; - rasterizedGlyph.texturePositionClipSpace.x = this._currentRowX / TEXTURE_WIDTH; - rasterizedGlyph.texturePositionClipSpace.y = this._currentRowY / TEXTURE_HEIGHT; + rasterizedGlyph.texturePosition.x = activeRow.x; + rasterizedGlyph.texturePosition.y = activeRow.y; + rasterizedGlyph.texturePositionClipSpace.x = activeRow.x / TEXTURE_WIDTH; + rasterizedGlyph.texturePositionClipSpace.y = activeRow.y / TEXTURE_HEIGHT; - // Update atlas current row - this._currentRowHeight = Math.max(this._currentRowHeight, rasterizedGlyph.size.y); - this._currentRowX += rasterizedGlyph.size.x; + // Update atlas current row, for fixed rows the glyph height will never be larger than the row + // height + activeRow.height = Math.max(activeRow.height, rasterizedGlyph.size.y); + activeRow.x += rasterizedGlyph.size.x; // putImageData doesn't do any blending, so it will overwrite any existing cache entry for us this._cacheCtx.putImageData(clippedImageData, rasterizedGlyph.texturePosition.x, rasterizedGlyph.texturePosition.y); From 0514b559361f5b79339636ad716f5464da9282b6 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 30 Jul 2022 23:16:47 -0700 Subject: [PATCH 4/8] Fix default underline color Fixes #3971 --- 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 6878069a..b51f7ecb 100644 --- a/src/common/buffer/AttributeData.ts +++ b/src/common/buffer/AttributeData.ts @@ -149,7 +149,7 @@ export class ExtendedAttrs implements IExtendedAttrs { constructor( underlineStyle: UnderlineStyle = UnderlineStyle.NONE, - underlineColor: number = -1 + underlineColor: number = Attributes.CM_DEFAULT ) { this.underlineStyle = underlineStyle; this.underlineColor = underlineColor; From 686105942c98c1fcd7fed7c76a9c74b96cd18a25 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 30 Jul 2022 23:25:18 -0700 Subject: [PATCH 5/8] Reduce line width of webgl underline --- addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts index 008e65a8..6ec81ce6 100644 --- a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts +++ b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts @@ -438,8 +438,8 @@ export class WebglCharAtlas implements IDisposable { // Draw underline if (underline) { this._tmpCtx.save(); - const lineWidth = Math.max(1, Math.floor(this._config.fontSize * window.devicePixelRatio / 10)); - const yOffset = this._tmpCtx.lineWidth % 2 === 1 ? 0.5 : 0; // When the width is odd, draw at 0.5 position + const lineWidth = Math.max(1, Math.floor(this._config.fontSize * window.devicePixelRatio / 15)); + const yOffset = lineWidth % 2 === 1 ? 0.5 : 0; // When the width is odd, draw at 0.5 position this._tmpCtx.lineWidth = lineWidth; // Underline color From f4618b229f151d551f63ddbdfa61cfa9e9a48209 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 30 Jul 2022 23:30:52 -0700 Subject: [PATCH 6/8] Bring underlines closer to characters --- .../src/atlas/WebglCharAtlas.ts | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts index 6ec81ce6..c5069e64 100644 --- a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts +++ b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts @@ -439,7 +439,9 @@ export class WebglCharAtlas implements IDisposable { if (underline) { this._tmpCtx.save(); const lineWidth = Math.max(1, Math.floor(this._config.fontSize * window.devicePixelRatio / 15)); - const yOffset = lineWidth % 2 === 1 ? 0.5 : 0; // When the width is odd, draw at 0.5 position + // When the width is odd, draw at 0.5 position. Offset by an additional 1 dpr to bring the + // underline closer to the character + const yOffset = (lineWidth % 2 === 1 ? 0.5 : 0) + window.devicePixelRatio; this._tmpCtx.lineWidth = lineWidth; // Underline color @@ -508,18 +510,18 @@ export class WebglCharAtlas implements IDisposable { break; case UnderlineStyle.DOTTED: this._tmpCtx.setLineDash([window.devicePixelRatio * 2, window.devicePixelRatio]); - this._tmpCtx.moveTo(xLeft, yMid); - this._tmpCtx.lineTo(xRight, yMid); + 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, yMid); - this._tmpCtx.lineTo(xRight, yMid); + this._tmpCtx.moveTo(xLeft, yTop); + this._tmpCtx.lineTo(xRight, yTop); break; case UnderlineStyle.SINGLE: default: - this._tmpCtx.moveTo(xLeft, yMid); - this._tmpCtx.lineTo(xRight, yMid); + this._tmpCtx.moveTo(xLeft, yTop); + this._tmpCtx.lineTo(xRight, yTop); break; } this._tmpCtx.stroke(); From bff529e1e2d1840d3f4dfc1d5bc0dcca899a5da6 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 30 Jul 2022 23:41:08 -0700 Subject: [PATCH 7/8] Simplify dom tests and fix assertion --- src/browser/renderer/dom/DomRendererRowFactory.test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/browser/renderer/dom/DomRendererRowFactory.test.ts b/src/browser/renderer/dom/DomRendererRowFactory.test.ts index df359dac..776ff701 100644 --- a/src/browser/renderer/dom/DomRendererRowFactory.test.ts +++ b/src/browser/renderer/dom/DomRendererRowFactory.test.ts @@ -139,7 +139,7 @@ describe('DomRendererRowFactory', () => { lineData.setCell(0, cell); const fragment = rowFactory.createRow(lineData, 0, false, undefined, 0, false, 5, 20); assert.equal(getFragmentHtml(fragment), - 'a' + 'a' ); }); it('should add class for double underline style', () => { @@ -150,7 +150,7 @@ describe('DomRendererRowFactory', () => { lineData.setCell(0, cell); const fragment = rowFactory.createRow(lineData, 0, false, undefined, 0, false, 5, 20); assert.equal(getFragmentHtml(fragment), - 'a' + 'a' ); }); it('should add class for curly underline style', () => { @@ -161,7 +161,7 @@ describe('DomRendererRowFactory', () => { lineData.setCell(0, cell); const fragment = rowFactory.createRow(lineData, 0, false, undefined, 0, false, 5, 20); assert.equal(getFragmentHtml(fragment), - 'a' + 'a' ); }); it('should add class for double dotted style', () => { @@ -172,7 +172,7 @@ describe('DomRendererRowFactory', () => { lineData.setCell(0, cell); const fragment = rowFactory.createRow(lineData, 0, false, undefined, 0, false, 5, 20); assert.equal(getFragmentHtml(fragment), - 'a' + 'a' ); }); it('should add class for dashed underline style', () => { @@ -183,7 +183,7 @@ describe('DomRendererRowFactory', () => { lineData.setCell(0, cell); const fragment = rowFactory.createRow(lineData, 0, false, undefined, 0, false, 5, 20); assert.equal(getFragmentHtml(fragment), - 'a' + 'a' ); }); }); From 8d57c64d29080de015d051617ad4f41d48edff70 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 30 Jul 2022 23:48:37 -0700 Subject: [PATCH 8/8] Include purpose of canvas addon in readme Fixes #3978 --- addons/xterm-addon-canvas/README.md | 5 +++++ addons/xterm-addon-webgl/README.md | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/addons/xterm-addon-canvas/README.md b/addons/xterm-addon-canvas/README.md index ed65967d..bffac96e 100644 --- a/addons/xterm-addon-canvas/README.md +++ b/addons/xterm-addon-canvas/README.md @@ -2,6 +2,7 @@ An addon for [xterm.js](https://github.com/xtermjs/xterm.js) that enables a canvas-based renderer using a 2d context to draw. This addon requires xterm.js v5+. +The purpose of this addon is to be used as a fallback for the [webgl addon](https://www.npmjs.com/package/xterm-addon-webgl) when better performance is desired over the default DOM renderer, but WebGL2 isn't supported or performant for some reason. ### Install @@ -21,3 +22,7 @@ terminal.loadAddon(new CanvasAddon()); ``` See the full [API](https://github.com/xtermjs/xterm.js/blob/master/addons/xterm-addon-canvas/typings/xterm-addon-canvas.d.ts) for more advanced usage. + +### See also + +- [xterm-addon-webgl](https://www.npmjs.com/package/xterm-addon-webgl) A renderer for xterm.js that uses WebGL diff --git a/addons/xterm-addon-webgl/README.md b/addons/xterm-addon-webgl/README.md index 2519fb7b..a431170f 100644 --- a/addons/xterm-addon-webgl/README.md +++ b/addons/xterm-addon-webgl/README.md @@ -36,3 +36,7 @@ terminal.loadAddon(addon); ``` Read more about handling WebGL context losses on the [Khronos wiki](https://www.khronos.org/webgl/wiki/HandlingContextLost). + +### See also + +- [xterm-addon-canvas](https://www.npmjs.com/package/xterm-addon-canvas) A renderer for xterm.js that uses a 2d canvas that can be used as a fallback when WebGL is not available