Skip to content

exclude powerline chars from min contrast for webgl and dom #3742

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Apr 20, 2022
Merged

exclude powerline chars from min contrast for webgl and dom #3742

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b20ce44
apply to dom and webgl too
meganrogge Apr 20, 2022
341ca24
Merge branch 'master' into merogge/contrast
meganrogge Apr 20, 2022
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
21 changes: 10 additions & 11 deletions addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,8 +216,8 @@ export class WebglCharAtlas implements IDisposable {
}
}

private _getForegroundCss(bg: number, bgColorMode: number, bgColor: number, fg: number, fgColorMode: number, fgColor: number, inverse: boolean, bold: boolean): string {
const minimumContrastCss = this._getMinimumContrastCss(bg, bgColorMode, bgColor, fg, fgColorMode, fgColor, inverse, bold);
private _getForegroundCss(bg: number, bgColorMode: number, bgColor: number, fg: number, fgColorMode: number, fgColor: number, inverse: boolean, bold: boolean, isPowerLineGlyph: boolean): string {
const minimumContrastCss = this._getMinimumContrastCss(bg, bgColorMode, bgColor, fg, fgColorMode, fgColor, inverse, bold, isPowerLineGlyph);
if (minimumContrastCss) {
return minimumContrastCss;
}
Expand Down Expand Up @@ -281,8 +281,8 @@ export class WebglCharAtlas implements IDisposable {
}
}

private _getMinimumContrastCss(bg: number, bgColorMode: number, bgColor: number, fg: number, fgColorMode: number, fgColor: number, inverse: boolean, bold: boolean): string | undefined {
if (this._config.minimumContrastRatio === 1) {
private _getMinimumContrastCss(bg: number, bgColorMode: number, bgColor: number, fg: number, fgColorMode: number, fgColor: number, inverse: boolean, bold: boolean, isPowerLineGlyph: boolean): string | undefined {
if (this._config.minimumContrastRatio === 1 || isPowerLineGlyph) {
return undefined;
}

Expand Down Expand Up @@ -370,13 +370,6 @@ export class WebglCharAtlas implements IDisposable {
`${fontStyle} ${fontWeight} ${this._config.fontSize * this._config.devicePixelRatio}px ${this._config.fontFamily}`;
this._tmpCtx.textBaseline = TEXT_BASELINE;

this._tmpCtx.fillStyle = this._getForegroundCss(bg, bgColorMode, bgColor, fg, fgColorMode, fgColor, inverse, bold);

// Apply alpha to dim the character
if (dim) {
this._tmpCtx.globalAlpha = DIM_OPACITY;
}

// 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
Expand All @@ -387,6 +380,12 @@ export class WebglCharAtlas implements IDisposable {
isPowerlineGlyph = true;
}
}
this._tmpCtx.fillStyle = this._getForegroundCss(bg, bgColorMode, bgColor, fg, fgColorMode, fgColor, inverse, bold, isPowerlineGlyph);

// 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;
Expand Down
13 changes: 7 additions & 6 deletions src/browser/renderer/dom/DomRendererRowFactory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* @license MIT
*/

import { IBufferLine } from 'common/Types';
import { IBufferLine, ICellData } from 'common/Types';
import { INVERTED_DEFAULT_COLOR } from 'browser/renderer/atlas/Constants';
import { NULL_CELL_CODE, WHITESPACE_CELL_CHAR, Attributes } from 'common/buffer/Constants';
import { CellData } from 'common/buffer/CellData';
Expand Down Expand Up @@ -178,7 +178,7 @@ export class DomRendererRowFactory {
if (cell.isBold() && fg < 8 && this._optionsService.rawOptions.drawBoldTextInBrightColors) {
fg += 8;
}
if (!this._applyMinimumContrast(charElement, this._colors.background, this._colors.ansi[fg])) {
if (!this._applyMinimumContrast(charElement, this._colors.background, this._colors.ansi[fg], cell)) {
charElement.classList.add(`xterm-fg-${fg}`);
}
break;
Expand All @@ -188,13 +188,13 @@ export class DomRendererRowFactory {
(fg >> 8) & 0xFF,
(fg ) & 0xFF
);
if (!this._applyMinimumContrast(charElement, this._colors.background, color)) {
if (!this._applyMinimumContrast(charElement, this._colors.background, color, cell)) {
this._addStyle(charElement, `color:#${padStart(fg.toString(16), '0', 6)}`);
}
break;
case Attributes.CM_DEFAULT:
default:
if (!this._applyMinimumContrast(charElement, this._colors.background, this._colors.foreground)) {
if (!this._applyMinimumContrast(charElement, this._colors.background, this._colors.foreground, cell)) {
if (isInverse) {
charElement.classList.add(`xterm-fg-${INVERTED_DEFAULT_COLOR}`);
}
Expand Down Expand Up @@ -224,8 +224,9 @@ export class DomRendererRowFactory {
return fragment;
}

private _applyMinimumContrast(element: HTMLElement, bg: IColor, fg: IColor): boolean {
if (this._optionsService.rawOptions.minimumContrastRatio === 1) {
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) {
return false;
}

Expand Down