From f4ffb13ece1f06234e75bf538b4668fc81060ded Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 24 Jul 2022 10:59:24 -0700 Subject: [PATCH 1/4] Render dim bg in all renderers Fixes #3534 --- addons/xterm-addon-webgl/src/WebglRenderer.ts | 43 ++++++++++++++++++- src/browser/renderer/TextRenderLayer.ts | 9 ++++ .../renderer/dom/DomRendererRowFactory.ts | 7 +++ src/common/Color.ts | 5 +++ 4 files changed, 63 insertions(+), 1 deletion(-) diff --git a/addons/xterm-addon-webgl/src/WebglRenderer.ts b/addons/xterm-addon-webgl/src/WebglRenderer.ts index 1e385b3399..1e255a0340 100644 --- a/addons/xterm-addon-webgl/src/WebglRenderer.ts +++ b/addons/xterm-addon-webgl/src/WebglRenderer.ts @@ -12,7 +12,7 @@ import { RectangleRenderer } from './RectangleRenderer'; import { IWebGL2RenderingContext } from './Types'; import { RenderModel, COMBINED_CHAR_BIT_MASK, RENDER_MODEL_BG_OFFSET, RENDER_MODEL_FG_OFFSET, RENDER_MODEL_INDICIES_PER_CELL } from './RenderModel'; import { Disposable } from 'common/Lifecycle'; -import { Attributes, Content, FgFlags, NULL_CELL_CHAR, NULL_CELL_CODE } from 'common/buffer/Constants'; +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'; import { IRenderDimensions, IRenderer, IRequestRedrawEvent } from 'browser/renderer/Types'; @@ -24,6 +24,7 @@ import { ICharacterJoinerService } from 'browser/services/Services'; import { CharData, ICellData } from 'common/Types'; import { AttributeData } from 'common/buffer/AttributeData'; import { IDecorationService } from 'common/services/Services'; +import { color, rgba as rgbaNs } from 'common/Color'; export class WebglRenderer extends Disposable implements IRenderer { private _renderLayers: IRenderLayer[]; @@ -418,6 +419,46 @@ export class WebglRenderer extends Disposable implements IRenderer { } } + // Apply dim if there is no override yet as it's the default color, this requires resolving the + // attribute color ahead of time because it's a derivative of it + if (this._workColors.bg & BgFlags.DIM) { + // TODO: Share logic + let rgba: number | undefined; + if (this._workColors.fg & FgFlags.INVERSE) { + switch (this._workColors.fg & Attributes.CM_MASK) { + case Attributes.CM_P16: + case Attributes.CM_P256: + rgba = this._colors.ansi[this._workColors.fg & Attributes.PCOLOR_MASK].rgba; + break; + case Attributes.CM_RGB: + rgba = (this._workColors.fg & Attributes.RGB_MASK) << 8; + break; + case Attributes.CM_DEFAULT: + default: + rgba = this._colors.foreground.rgba; + } + } else { + switch (this._workColors.bg& Attributes.CM_MASK) { + case Attributes.CM_P16: + case Attributes.CM_P256: + rgba = this._colors.ansi[this._workColors.bg& Attributes.PCOLOR_MASK].rgba; + break; + case Attributes.CM_RGB: + rgba = (this._workColors.bg& Attributes.RGB_MASK) << 8; + break; + case Attributes.CM_DEFAULT: + default: + rgba = this._colors.background.rgba; + } + } + bgOverride = color.blend(this._colors.background, rgbaNs.toColor( + (rgba >> 24) & 0xFF, + (rgba >> 16) & 0xFF, + (rgba >> 8) & 0xFF, + 0x80 // 50% opacity + )).rgba >> 8 & 0xFFFFFF; + } + // 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) { diff --git a/src/browser/renderer/TextRenderLayer.ts b/src/browser/renderer/TextRenderLayer.ts index ef5a9b62ea..f62819d02f 100644 --- a/src/browser/renderer/TextRenderLayer.ts +++ b/src/browser/renderer/TextRenderLayer.ts @@ -14,6 +14,7 @@ import { CellData } from 'common/buffer/CellData'; import { IOptionsService, IBufferService, IDecorationService } from 'common/services/Services'; import { ICharacterJoinerService } from 'browser/services/Services'; import { JoinedCellData } from 'browser/services/CharacterJoinerService'; +import { color, css } from 'common/Color'; /** * This CharData looks like a null character, which will forc a clear and render @@ -177,6 +178,14 @@ export class TextRenderLayer extends BaseRenderLayer { nextFillStyle = this._colors.ansi[cell.getBgColor()].css; } + // Apply dim to the background, this is relatively slow as the CSS is re-parsed but dim is + // rarely used + if (nextFillStyle && cell.isDim()) { + console.log('old', nextFillStyle); + nextFillStyle = color.multiplyOpacity(css.toColor(nextFillStyle), 0.5).css; + console.log('new', nextFillStyle); + } + // Get any decoration foreground/background overrides, this must be fetched before the early // exist but applied after inverse let isTop = false; diff --git a/src/browser/renderer/dom/DomRendererRowFactory.ts b/src/browser/renderer/dom/DomRendererRowFactory.ts index fadf50325f..c9e1ff2b8e 100644 --- a/src/browser/renderer/dom/DomRendererRowFactory.ts +++ b/src/browser/renderer/dom/DomRendererRowFactory.ts @@ -249,6 +249,13 @@ export class DomRendererRowFactory { } } + // If there is no background override by now it's the original color, so apply dim if needed + if (!bgOverride) { + if (cell.isDim()) { + bgOverride = color.multiplyOpacity(resolvedBg, 0.5); + } + } + // Foreground switch (fgColorMode) { case Attributes.CM_P16: diff --git a/src/common/Color.ts b/src/common/Color.ts index e5c7e3eb44..f564d66d24 100644 --- a/src/common/Color.ts +++ b/src/common/Color.ts @@ -84,6 +84,11 @@ export namespace color { }; } + export function multiplyOpacity(color: IColor, factor: number): IColor { + const a = color.rgba & 0xFF; + return opacity(color, (a * factor) / 0xFF); + } + export function toColorRGB(color: IColor): IColorRGB { return [(color.rgba >> 24) & 0xFF, (color.rgba >> 16) & 0xFF, (color.rgba >> 8) & 0xFF]; } From ee2e54487df67d909641d5cf4ee170776530b9e0 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 24 Jul 2022 13:13:26 -0700 Subject: [PATCH 2/4] Fix inverse dim for webgl Fixes #3534 --- .../src/RectangleRenderer.ts | 5 +- addons/xterm-addon-webgl/src/WebglRenderer.ts | 40 ------------ .../src/atlas/WebglCharAtlas.ts | 65 ++++++++++++------- 3 files changed, 46 insertions(+), 64 deletions(-) diff --git a/addons/xterm-addon-webgl/src/RectangleRenderer.ts b/addons/xterm-addon-webgl/src/RectangleRenderer.ts index 16b1b9c0b3..bc08ad4a0e 100644 --- a/addons/xterm-addon-webgl/src/RectangleRenderer.ts +++ b/addons/xterm-addon-webgl/src/RectangleRenderer.ts @@ -5,7 +5,7 @@ import { createProgram, expandFloat32Array, PROJECTION_MATRIX, throwIfFalsy } from './WebglUtils'; import { IRenderModel, IWebGLVertexArrayObject, IWebGL2RenderingContext } from './Types'; -import { Attributes, FgFlags } from 'common/buffer/Constants'; +import { Attributes, BgFlags, FgFlags } from 'common/buffer/Constants'; import { Terminal } from 'xterm'; import { IColor } from 'common/Types'; import { IColorSet } from 'browser/Types'; @@ -244,8 +244,9 @@ export class RectangleRenderer extends Disposable { const r = ((rgba >> 24) & 0xFF) / 255; const g = ((rgba >> 16) & 0xFF) / 255; const b = ((rgba >> 8 ) & 0xFF) / 255; + const a = bg & BgFlags.DIM ? 0.5 : 1; - this._addRectangle(vertices.attributes, offset, x1, y1, (endX - startX) * this._dimensions.scaledCellWidth, this._dimensions.scaledCellHeight, r, g, b, 1); + this._addRectangle(vertices.attributes, offset, x1, y1, (endX - startX) * this._dimensions.scaledCellWidth, this._dimensions.scaledCellHeight, r, g, b, a); } private _addRectangle(array: Float32Array, offset: number, x1: number, y1: number, width: number, height: number, r: number, g: number, b: number, a: number): void { diff --git a/addons/xterm-addon-webgl/src/WebglRenderer.ts b/addons/xterm-addon-webgl/src/WebglRenderer.ts index 1e255a0340..e782efab7e 100644 --- a/addons/xterm-addon-webgl/src/WebglRenderer.ts +++ b/addons/xterm-addon-webgl/src/WebglRenderer.ts @@ -419,46 +419,6 @@ export class WebglRenderer extends Disposable implements IRenderer { } } - // Apply dim if there is no override yet as it's the default color, this requires resolving the - // attribute color ahead of time because it's a derivative of it - if (this._workColors.bg & BgFlags.DIM) { - // TODO: Share logic - let rgba: number | undefined; - if (this._workColors.fg & FgFlags.INVERSE) { - switch (this._workColors.fg & Attributes.CM_MASK) { - case Attributes.CM_P16: - case Attributes.CM_P256: - rgba = this._colors.ansi[this._workColors.fg & Attributes.PCOLOR_MASK].rgba; - break; - case Attributes.CM_RGB: - rgba = (this._workColors.fg & Attributes.RGB_MASK) << 8; - break; - case Attributes.CM_DEFAULT: - default: - rgba = this._colors.foreground.rgba; - } - } else { - switch (this._workColors.bg& Attributes.CM_MASK) { - case Attributes.CM_P16: - case Attributes.CM_P256: - rgba = this._colors.ansi[this._workColors.bg& Attributes.PCOLOR_MASK].rgba; - break; - case Attributes.CM_RGB: - rgba = (this._workColors.bg& Attributes.RGB_MASK) << 8; - break; - case Attributes.CM_DEFAULT: - default: - rgba = this._colors.background.rgba; - } - } - bgOverride = color.blend(this._colors.background, rgbaNs.toColor( - (rgba >> 24) & 0xFF, - (rgba >> 16) & 0xFF, - (rgba >> 8) & 0xFF, - 0x80 // 50% opacity - )).rgba >> 8 & 0xFFFFFF; - } - // 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) { diff --git a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts index 05751c4216..50736dbc46 100644 --- a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts +++ b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts @@ -189,7 +189,7 @@ export class WebglCharAtlas implements IDisposable { return this._config.colors.ansi[idx]; } - private _getBackgroundColor(bgColorMode: number, bgColor: number, inverse: boolean): IColor { + private _getBackgroundColor(bgColorMode: number, bgColor: number, inverse: boolean, dim: boolean): IColor { if (this._config.allowTransparency) { // The background color might have some transparency, so we need to render it as fully // transparent in the atlas. Otherwise we'd end up drawing the transparent background twice @@ -197,50 +197,76 @@ export class WebglCharAtlas implements IDisposable { return TRANSPARENT_COLOR; } + let result: IColor; switch (bgColorMode) { case Attributes.CM_P16: case Attributes.CM_P256: - return this._getColorFromAnsiIndex(bgColor); + result = this._getColorFromAnsiIndex(bgColor); + break; case Attributes.CM_RGB: const arr = AttributeData.toColorRGB(bgColor); // TODO: This object creation is slow - return { - rgba: bgColor << 8, - css: `#${toPaddedHex(arr[0])}${toPaddedHex(arr[1])}${toPaddedHex(arr[2])}` - }; + result = rgba.toColor(arr[0], arr[1], arr[2]); + break; case Attributes.CM_DEFAULT: default: if (inverse) { - return this._config.colors.foreground; + result = this._config.colors.foreground; + } else { + result = this._config.colors.background; } - return this._config.colors.background; + break; + } + + if (dim) { + // Blend here instead of using opacity because transparent colors mess with clipping the + // glyph's bounding box + result = color.blend(this._config.colors.background, color.multiplyOpacity(result, 0.5)); } + + return result; } - private _getForegroundColor(bg: number, bgColorMode: number, bgColor: number, fg: number, fgColorMode: number, fgColor: number, inverse: boolean, bold: boolean, excludeFromContrastRatioDemands: boolean): IColor { - const minimumContrastColor = this._getMinimumContrastColor(bg, bgColorMode, bgColor, fg, fgColorMode, fgColor, inverse, bold, excludeFromContrastRatioDemands); + private _getForegroundColor(bg: number, bgColorMode: number, bgColor: number, fg: number, fgColorMode: number, fgColor: number, inverse: boolean, dim: boolean, bold: boolean, excludeFromContrastRatioDemands: boolean): IColor { + // TODO: Pass dim along to get min contrast? + const minimumContrastColor = this._getMinimumContrastColor(bg, bgColorMode, bgColor, fg, fgColorMode, fgColor, false, bold, excludeFromContrastRatioDemands); if (minimumContrastColor) { return minimumContrastColor; } + let result: IColor; switch (fgColorMode) { case Attributes.CM_P16: case Attributes.CM_P256: if (this._config.drawBoldTextInBrightColors && bold && fgColor < 8) { fgColor += 8; } - return this._getColorFromAnsiIndex(fgColor); + result = this._getColorFromAnsiIndex(fgColor); + break; case Attributes.CM_RGB: const arr = AttributeData.toColorRGB(fgColor); - return rgba.toColor(arr[0], arr[1], arr[2]); + result = rgba.toColor(arr[0], arr[1], arr[2]); + break; case Attributes.CM_DEFAULT: default: if (inverse) { - // Inverse should always been opaque, even when transparency is used - return color.opaque(this._config.colors.background); + result = this._config.colors.background; + } else { + result = this._config.colors.foreground; } - return this._config.colors.foreground; } + + // Always use an opaque color regardless of allowTransparency + if (this._config.allowTransparency) { + result = color.opaque(result); + } + + // Apply dim to the color, opacity is fine to use for the foreground color + if (dim) { + result = color.multiplyOpacity(result, 0.5); + } + + return result; } private _resolveBackgroundRgba(bgColorMode: number, bgColor: number, inverse: boolean): number { @@ -357,7 +383,7 @@ export class WebglCharAtlas implements IDisposable { } // draw the background - const backgroundColor = this._getBackgroundColor(bgColorMode, bgColor, inverse); + const backgroundColor = this._getBackgroundColor(bgColorMode, bgColor, inverse, dim); // Use a 'copy' composite operation to clear any existing glyph out of _tmpCtxWithAlpha, regardless of // transparency in backgroundColor this._tmpCtx.globalCompositeOperation = 'copy'; @@ -373,14 +399,9 @@ export class WebglCharAtlas implements IDisposable { this._tmpCtx.textBaseline = TEXT_BASELINE; const powerLineGlyph = chars.length === 1 && isPowerlineGlyph(chars.charCodeAt(0)); - const foregroundColor = this._getForegroundColor(bg, bgColorMode, bgColor, fg, fgColorMode, fgColor, inverse, bold, excludeFromContrastRatioDemands(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; - // Apply alpha to dim the character - if (dim) { - this._tmpCtx.globalAlpha = DIM_OPACITY; - } - // For powerline glyphs left/top padding is excluded (https://github.com/microsoft/vscode/issues/120129) const padding = powerLineGlyph ? 0 : TMP_CANVAS_GLYPH_PADDING; From a4f7e2d8e9cc679e40a47d29ee977e448fb16f9e Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 24 Jul 2022 13:20:04 -0700 Subject: [PATCH 3/4] Fix inverse dim on canvas renderer --- src/browser/renderer/BaseRenderLayer.ts | 1 + src/browser/renderer/TextRenderLayer.ts | 2 -- src/browser/renderer/atlas/DynamicCharAtlas.ts | 14 ++++++++++---- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/browser/renderer/BaseRenderLayer.ts b/src/browser/renderer/BaseRenderLayer.ts index 0a9b805701..aa9e3ded1f 100644 --- a/src/browser/renderer/BaseRenderLayer.ts +++ b/src/browser/renderer/BaseRenderLayer.ts @@ -345,6 +345,7 @@ export abstract class BaseRenderLayer implements IRenderLayer { } } + console.log(`draw char ${cell.getChars()} with style`, this._ctx.fillStyle); const atlasDidDraw = hasOverrides ? false : this._charAtlas?.draw(this._ctx, this._currentGlyphIdentifier, x * this._scaledCellWidth + this._scaledCharLeft, y * this._scaledCellHeight + this._scaledCharTop); if (!atlasDidDraw) { diff --git a/src/browser/renderer/TextRenderLayer.ts b/src/browser/renderer/TextRenderLayer.ts index f62819d02f..6771fdb4e7 100644 --- a/src/browser/renderer/TextRenderLayer.ts +++ b/src/browser/renderer/TextRenderLayer.ts @@ -181,9 +181,7 @@ export class TextRenderLayer extends BaseRenderLayer { // Apply dim to the background, this is relatively slow as the CSS is re-parsed but dim is // rarely used if (nextFillStyle && cell.isDim()) { - console.log('old', nextFillStyle); nextFillStyle = color.multiplyOpacity(css.toColor(nextFillStyle), 0.5).css; - console.log('new', nextFillStyle); } // Get any decoration foreground/background overrides, this must be fetched before the early diff --git a/src/browser/renderer/atlas/DynamicCharAtlas.ts b/src/browser/renderer/atlas/DynamicCharAtlas.ts index 590698798e..ea3181174a 100644 --- a/src/browser/renderer/atlas/DynamicCharAtlas.ts +++ b/src/browser/renderer/atlas/DynamicCharAtlas.ts @@ -225,13 +225,18 @@ export class DynamicCharAtlas extends BaseCharAtlas { // around the anti-aliased edges of the glyph, and it would look too dark. return TRANSPARENT_COLOR; } + let result: IColor; if (glyph.bg === INVERTED_DEFAULT_COLOR) { - return this._config.colors.foreground; + result = this._config.colors.foreground; + } else if (glyph.bg < 256) { + result = this._getColorFromAnsiIndex(glyph.bg); + } else { + result = this._config.colors.background; } - if (glyph.bg < 256) { - return this._getColorFromAnsiIndex(glyph.bg); + if (glyph.dim) { + result = color.blend(this._config.colors.background, color.multiplyOpacity(result, 0.5)); } - return this._config.colors.background; + return result; } private _getForegroundColor(glyph: IGlyphIdentifier): IColor { @@ -274,6 +279,7 @@ export class DynamicCharAtlas extends BaseCharAtlas { if (glyph.dim) { this._tmpCtx.globalAlpha = DIM_OPACITY; } + // Draw the character this._tmpCtx.fillText(glyph.chars, 0, this._config.scaledCharHeight); From 2ceec348092b55b39fbec32a79d97a8f8ac583e6 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 24 Jul 2022 17:20:41 -0700 Subject: [PATCH 4/4] Remove log --- src/browser/renderer/BaseRenderLayer.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/browser/renderer/BaseRenderLayer.ts b/src/browser/renderer/BaseRenderLayer.ts index aa9e3ded1f..0a9b805701 100644 --- a/src/browser/renderer/BaseRenderLayer.ts +++ b/src/browser/renderer/BaseRenderLayer.ts @@ -345,7 +345,6 @@ export abstract class BaseRenderLayer implements IRenderLayer { } } - console.log(`draw char ${cell.getChars()} with style`, this._ctx.fillStyle); const atlasDidDraw = hasOverrides ? false : this._charAtlas?.draw(this._ctx, this._currentGlyphIdentifier, x * this._scaledCellWidth + this._scaledCharLeft, y * this._scaledCellHeight + this._scaledCharTop); if (!atlasDidDraw) {