Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

align isPowerlineGlyph across renderers #3743

Merged
merged 2 commits into from
Apr 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 5 additions & 13 deletions addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -370,25 +371,16 @@ 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) {
this._tmpCtx.globalAlpha = DIM_OPACITY;
}

// 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;
Expand Down Expand Up @@ -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
Expand Down
9 changes: 2 additions & 7 deletions src/browser/renderer/BaseRenderLayer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
}

Expand Down
6 changes: 6 additions & 0 deletions src/browser/renderer/RendererUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,9 @@ export function throwIfFalsy<T>(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;
}
4 changes: 2 additions & 2 deletions src/browser/renderer/dom/DomRendererRowFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
}

Expand Down