From da33543ffb96f43e1904bc335d8cacd4eed407d6 Mon Sep 17 00:00:00 2001 From: Megan Rogge Date: Wed, 20 Apr 2022 14:25:46 -0700 Subject: [PATCH] align isPowerlineGlyph across renderers (#3743) --- .../src/atlas/WebglCharAtlas.ts | 18 +++++------------- src/browser/renderer/BaseRenderLayer.ts | 9 ++------- src/browser/renderer/RendererUtils.ts | 6 ++++++ .../renderer/dom/DomRendererRowFactory.ts | 4 ++-- 4 files changed, 15 insertions(+), 22 deletions(-) diff --git a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts index 911ea3c0..3194d397 100644 --- a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts +++ b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts @@ -13,6 +13,7 @@ import { IDisposable } from 'xterm'; import { AttributeData } from 'common/buffer/AttributeData'; import { channels, rgba } from 'browser/Color'; import { tryDrawCustomChar } from 'browser/renderer/CustomGlyphs'; +import { isPowerlineGlyph } 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. @@ -370,17 +371,8 @@ export class WebglCharAtlas implements IDisposable { `${fontStyle} ${fontWeight} ${this._config.fontSize * this._config.devicePixelRatio}px ${this._config.fontFamily}`; this._tmpCtx.textBaseline = TEXT_BASELINE; - // Check if the char is a powerline glyph, these will be restricted to a single cell glyph, no - // padding on either side that are allowed for other glyphs since they are designed to be pixel - // perfect but may render with "bad" anti-aliasing - let isPowerlineGlyph = false; - if (chars.length === 1) { - const code = chars.charCodeAt(0); - if (code >= 0xE0A0 && code <= 0xE0D6) { - isPowerlineGlyph = true; - } - } - this._tmpCtx.fillStyle = this._getForegroundCss(bg, bgColorMode, bgColor, fg, fgColorMode, fgColor, inverse, bold, isPowerlineGlyph); + const powerLineGlyph = chars.length === 1 && isPowerlineGlyph(chars.charCodeAt(0)); + this._tmpCtx.fillStyle = this._getForegroundCss(bg, bgColorMode, bgColor, fg, fgColorMode, fgColor, inverse, bold, powerLineGlyph); // Apply alpha to dim the character if (dim) { @@ -388,7 +380,7 @@ export class WebglCharAtlas implements IDisposable { } // For powerline glyphs left/top padding is excluded (https://github.com/microsoft/vscode/issues/120129) - const padding = isPowerlineGlyph ? 0 : TMP_CANVAS_GLYPH_PADDING; + const padding = powerLineGlyph ? 0 : TMP_CANVAS_GLYPH_PADDING; // Draw custom characters if applicable let drawSuccess = false; @@ -458,7 +450,7 @@ export class WebglCharAtlas implements IDisposable { return NULL_RASTERIZED_GLYPH; } - const rasterizedGlyph = this._findGlyphBoundingBox(imageData, this._workBoundingBox, allowedWidth, isPowerlineGlyph, drawSuccess); + 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 diff --git a/src/browser/renderer/BaseRenderLayer.ts b/src/browser/renderer/BaseRenderLayer.ts index a95df5fa..90d4f82f 100644 --- a/src/browser/renderer/BaseRenderLayer.ts +++ b/src/browser/renderer/BaseRenderLayer.ts @@ -14,7 +14,7 @@ import { AttributeData } from 'common/buffer/AttributeData'; import { IColorSet, IColor } from 'browser/Types'; import { CellData } from 'common/buffer/CellData'; import { IBufferService, IOptionsService } from 'common/services/Services'; -import { throwIfFalsy } from 'browser/renderer/RendererUtils'; +import { isPowerlineGlyph, throwIfFalsy } from 'browser/renderer/RendererUtils'; import { channels, color, rgba } from 'browser/Color'; import { removeElementFromParent } from 'browser/Dom'; import { tryDrawCustomChar } from 'browser/renderer/CustomGlyphs'; @@ -428,12 +428,7 @@ export abstract class BaseRenderLayer implements IRenderLayer { } private _getContrastColor(cell: CellData): IColor | undefined { - const codepoint = cell.getCode(); - if (57344 <= codepoint && codepoint <= 63743) { - // powerline chars #3739 - return undefined; - } - if (this._optionsService.rawOptions.minimumContrastRatio === 1) { + if (this._optionsService.rawOptions.minimumContrastRatio === 1 || isPowerlineGlyph(cell.getCode())) { return undefined; } diff --git a/src/browser/renderer/RendererUtils.ts b/src/browser/renderer/RendererUtils.ts index 48fd26a4..174556bd 100644 --- a/src/browser/renderer/RendererUtils.ts +++ b/src/browser/renderer/RendererUtils.ts @@ -9,3 +9,9 @@ export function throwIfFalsy(value: T | undefined | null): T { } return value; } + +export function isPowerlineGlyph(codepoint: number): boolean { + // This range was established via + // https://apw-bash-settings.readthedocs.io/en/latest/fontpatching.html + return 0xE000 <= codepoint && codepoint <= 0xF8FF; +} diff --git a/src/browser/renderer/dom/DomRendererRowFactory.ts b/src/browser/renderer/dom/DomRendererRowFactory.ts index e24c5fe5..9822ec36 100644 --- a/src/browser/renderer/dom/DomRendererRowFactory.ts +++ b/src/browser/renderer/dom/DomRendererRowFactory.ts @@ -12,6 +12,7 @@ import { color, rgba } from 'browser/Color'; import { IColorSet, IColor } from 'browser/Types'; import { ICharacterJoinerService } from 'browser/services/Services'; import { JoinedCellData } from 'browser/services/CharacterJoinerService'; +import { isPowerlineGlyph } from 'browser/renderer/RendererUtils'; export const BOLD_CLASS = 'xterm-bold'; export const DIM_CLASS = 'xterm-dim'; @@ -225,8 +226,7 @@ export class DomRendererRowFactory { } private _applyMinimumContrast(element: HTMLElement, bg: IColor, fg: IColor, cell: ICellData): boolean { - const codepoint = cell.getCode(); - if (this._optionsService.rawOptions.minimumContrastRatio === 1 || 57344 <= codepoint && codepoint <= 63743) { + if (this._optionsService.rawOptions.minimumContrastRatio === 1 || isPowerlineGlyph(cell.getCode())) { return false; }