Skip to content

Commit

Permalink
Merge pull request xtermjs#4672 from Tyriar/4262
Browse files Browse the repository at this point in the history 
Enforce half minimum contrast ratio for dim text
  • Loading branch information
@Tyriar
Tyriar committed Aug 13, 2023
2 parents af787e1 + 28c4732 commit 6c93460
Show file tree
Hide file tree
Showing 7 changed files with 119 additions and 17 deletions.
73 changes: 71 additions & 2 deletions addons/xterm-addon-webgl/test/WebglRenderer.api.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -789,7 +789,7 @@ describe('WebGL Renderer Integration Tests', async () => {
await pollFor(page, () => getCellColor(6, 2), [0xad, 0x7f, 0xa8, 255]);
await pollFor(page, () => getCellColor(7, 2), [0x34, 0xe2, 0xe2, 255]);
await pollFor(page, () => getCellColor(8, 2), [0xee, 0xee, 0xec, 255]);
// Setting and check for minimum contrast values, note that these are note
// Setting and check for minimum contrast values, note that these are not
// exact to the contrast ratio, if the increase luminance algorithm
// changes then these will probably fail
await page.evaluate(`window.term.options.minimumContrastRatio = 10;`);
Expand Down Expand Up @@ -858,7 +858,7 @@ describe('WebGL Renderer Integration Tests', async () => {
await pollFor(page, () => getCellColor(6, 2), [0xad, 0x7f, 0xa8, 255]);
await pollFor(page, () => getCellColor(7, 2), [0x34, 0xe2, 0xe2, 255]);
await pollFor(page, () => getCellColor(8, 2), [0xee, 0xee, 0xec, 255]);
// Setting and check for minimum contrast values, note that these are note
// Setting and check for minimum contrast values, note that these are not
// exact to the contrast ratio, if the increase luminance algorithm
// changes then these will probably fail
await page.evaluate(`window.term.options.minimumContrastRatio = 10;`);
Expand All @@ -879,6 +879,75 @@ describe('WebGL Renderer Integration Tests', async () => {
await pollFor(page, () => getCellColor(7, 2), [13, 67, 67, 255]);
await pollFor(page, () => getCellColor(8, 2), [64, 64, 64, 255]);
});

itWebgl('should enforce half the contrast for dim cells', async () => {
const theme: ITheme = {
background: '#ffffff',
black: '#2e3436',
red: '#cc0000',
green: '#4e9a06',
yellow: '#c4a000',
blue: '#3465a4',
magenta: '#75507b',
cyan: '#06989a',
white: '#d3d7cf',
brightBlack: '#555753',
brightRed: '#ef2929',
brightGreen: '#8ae234',
brightYellow: '#fce94f',
brightBlue: '#729fcf',
brightMagenta: '#ad7fa8',
brightCyan: '#34e2e2',
brightWhite: '#eeeeec'
};
await page.evaluate(`
window.term.options.theme = ${JSON.stringify(theme)};
window.term.options.minimumContrastRatio = 1;
`);
// Block characters ignore block elements so a different char is used here
await writeSync(page,
'\\x1b[2m' +
`\\x1b[30m■\\x1b[31m■\\x1b[32m■\\x1b[33m■\\x1b[34m■\\x1b[35m■\\x1b[36m■\\x1b[37m■\\r\\n` +
`\\x1b[90m■\\x1b[91m■\\x1b[92m■\\x1b[93m■\\x1b[94m■\\x1b[95m■\\x1b[96m■\\x1b[97m■`
);
// Validate before minimumContrastRatio is applied
await pollFor(page, () => getCellColor(1, 1), [Math.floor((255 + 0x2e) / 2), Math.floor((255 + 0x34) / 2), Math.floor((255 + 0x36) / 2), 255]);
await pollFor(page, () => getCellColor(2, 1), [Math.floor((255 + 0xcc) / 2), Math.floor((255 + 0x00) / 2), Math.floor((255 + 0x00) / 2), 255]);
await pollFor(page, () => getCellColor(3, 1), [Math.floor((255 + 0x4e) / 2), Math.floor((255 + 0x9a) / 2), Math.floor((255 + 0x06) / 2), 255]);
await pollFor(page, () => getCellColor(4, 1), [Math.floor((255 + 0xc4) / 2), Math.floor((255 + 0xa0) / 2), Math.floor((255 + 0x00) / 2), 255]);
await pollFor(page, () => getCellColor(5, 1), [Math.floor((255 + 0x34) / 2), Math.floor((255 + 0x65) / 2), Math.floor((255 + 0xa4) / 2), 255]);
await pollFor(page, () => getCellColor(6, 1), [Math.floor((255 + 0x75) / 2), Math.floor((255 + 0x50) / 2), Math.floor((255 + 0x7b) / 2), 255]);
await pollFor(page, () => getCellColor(7, 1), [Math.floor((255 + 0x06) / 2), Math.floor((255 + 0x98) / 2), Math.floor((255 + 0x9a) / 2), 255]);
await pollFor(page, () => getCellColor(8, 1), [Math.floor((255 + 0xd3) / 2), Math.floor((255 + 0xd7) / 2), Math.floor((255 + 0xcf) / 2), 255]);
await pollFor(page, () => getCellColor(1, 2), [Math.floor((255 + 0x55) / 2), Math.floor((255 + 0x57) / 2), Math.floor((255 + 0x53) / 2), 255]);
await pollFor(page, () => getCellColor(2, 2), [Math.floor((255 + 0xef) / 2), Math.floor((255 + 0x29) / 2), Math.floor((255 + 0x29) / 2), 255]);
await pollFor(page, () => getCellColor(3, 2), [Math.floor((255 + 0x8a) / 2), Math.floor((255 + 0xe2) / 2), Math.floor((255 + 0x34) / 2), 255]);
await pollFor(page, () => getCellColor(4, 2), [Math.floor((255 + 0xfc) / 2), Math.floor((255 + 0xe9) / 2), Math.floor((255 + 0x4f) / 2), 255]);
await pollFor(page, () => getCellColor(5, 2), [Math.floor((255 + 0x72) / 2), Math.floor((255 + 0x9f) / 2), Math.floor((255 + 0xcf) / 2), 255]);
await pollFor(page, () => getCellColor(6, 2), [Math.floor((255 + 0xad) / 2), Math.floor((255 + 0x7f) / 2), Math.floor((255 + 0xa8) / 2), 255]);
await pollFor(page, () => getCellColor(7, 2), [Math.floor((255 + 0x34) / 2), Math.floor((255 + 0xe2) / 2), Math.floor((255 + 0xe2) / 2), 255]);
await pollFor(page, () => getCellColor(8, 2), [Math.floor((255 + 0xee) / 2), Math.floor((255 + 0xee) / 2), Math.floor((255 + 0xec) / 2), 255]);
// Setting and check for minimum contrast values, note that these are not
// exact to the contrast ratio, if the increase luminance algorithm
// changes then these will probably fail
await page.evaluate(`window.term.options.minimumContrastRatio = 10;`);
await pollFor(page, () => getCellColor(1, 1), [150, 153, 154, 255]);
await pollFor(page, () => getCellColor(2, 1), [229, 127, 127, 255]);
await pollFor(page, () => getCellColor(3, 1), [63, 124, 4, 255]);
await pollFor(page, () => getCellColor(4, 1), [127, 104, 0, 255]);
await pollFor(page, () => getCellColor(5, 1), [153, 178, 209, 255]);
await pollFor(page, () => getCellColor(6, 1), [186, 167, 189, 255]);
await pollFor(page, () => getCellColor(7, 1), [4, 122, 124, 255]);
await pollFor(page, () => getCellColor(8, 1), [110, 112, 108, 255]);
await pollFor(page, () => getCellColor(1, 2), [170, 171, 169, 255]);
await pollFor(page, () => getCellColor(2, 2), [215, 36, 36, 255]);
await pollFor(page, () => getCellColor(3, 2), [72, 117, 25, 255]);
await pollFor(page, () => getCellColor(4, 2), [117, 109, 36, 255]);
await pollFor(page, () => getCellColor(5, 2), [72, 103, 135, 255]);
await pollFor(page, () => getCellColor(6, 2), [125, 91, 121, 255]);
await pollFor(page, () => getCellColor(7, 2), [25, 117, 117, 255]);
await pollFor(page, () => getCellColor(8, 2), [111, 111, 110, 255]);
});
});

describe('selectionBackground', async () => {
Expand Down
3 changes: 3 additions & 0 deletions src/browser/Types.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,10 @@ export interface IColorSet {
selectionInactiveBackgroundTransparent: IColor;
selectionInactiveBackgroundOpaque: IColor;
ansi: IColor[];
/** Maps original colors to colors that respect minimum contrast ratio. */
contrastCache: IColorContrastCache;
/** Maps original colors to colors that respect _half_ of the minimum contrast ratio. */
halfContrastCache: IColorContrastCache;
}

export type ReadonlyColorSet = Readonly<Omit<IColorSet, 'ansi'>> & { ansi: Readonly<Pick<IColorSet, 'ansi'>['ansi']> };
Expand Down
18 changes: 15 additions & 3 deletions src/browser/renderer/dom/DomRendererRowFactory.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { JoinedCellData } from 'browser/services/CharacterJoinerService';
import { excludeFromContrastRatioDemands } from 'browser/renderer/shared/RendererUtils';
import { AttributeData } from 'common/buffer/AttributeData';
import { WidthCache } from 'browser/renderer/dom/WidthCache';
import { IColorContrastCache } from 'browser/Types';


export const enum RowCss {
Expand Down Expand Up @@ -444,15 +445,19 @@ export class DomRendererRowFactory {
}

// Try get from cache first, only use the cache when there are no decoration overrides
const cache = this._getContrastCache(cell);
let adjustedColor: IColor | undefined | null = undefined;
if (!bgOverride && !fgOverride) {
adjustedColor = this._themeService.colors.contrastCache.getColor(bg.rgba, fg.rgba);
adjustedColor = cache.getColor(bg.rgba, fg.rgba);
}

// Calculate and store in cache
if (adjustedColor === undefined) {
adjustedColor = color.ensureContrastRatio(bgOverride || bg, fgOverride || fg, this._optionsService.rawOptions.minimumContrastRatio);
this._themeService.colors.contrastCache.setColor((bgOverride || bg).rgba, (fgOverride || fg).rgba, adjustedColor ?? null);
// Dim cells only require half the contrast, otherwise they wouldn't be distinguishable from
// non-dim cells
const ratio = this._optionsService.rawOptions.minimumContrastRatio / (cell.isDim() ? 2 : 1);
adjustedColor = color.ensureContrastRatio(bgOverride || bg, fgOverride || fg, ratio);
cache.setColor((bgOverride || bg).rgba, (fgOverride || fg).rgba, adjustedColor ?? null);
}

if (adjustedColor) {
Expand All @@ -463,6 +468,13 @@ export class DomRendererRowFactory {
return false;
}

private _getContrastCache(cell: ICellData): IColorContrastCache {
if (cell.isDim()) {
return this._themeService.colors.halfContrastCache;
}
return this._themeService.colors.contrastCache;
}

private _addStyle(element: HTMLElement, style: string): void {
element.setAttribute('style', `${element.getAttribute('style') || ''}${style};`);
}
Expand Down
3 changes: 2 additions & 1 deletion src/browser/renderer/shared/CharAtlasUtils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ export function generateConfig(deviceCellWidth: number, deviceCellHeight: number
// For the static char atlas, we only use the first 16 colors, but we need all 256 for the
// dynamic character atlas.
ansi: colors.ansi.slice(),
contrastCache: colors.contrastCache
contrastCache: colors.contrastCache,
halfContrastCache: colors.halfContrastCache
};
return {
customGlyphs: options.customGlyphs,
Expand Down
24 changes: 17 additions & 7 deletions src/browser/renderer/shared/TextureAtlas.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { FourKeyMap } from 'common/MultiKeyMap';
import { IdleTaskQueue } from 'common/TaskQueue';
import { IBoundingBox, ICharAtlasConfig, IRasterizedGlyph, IRequestRedrawEvent, ITextureAtlas } from 'browser/renderer/shared/Types';
import { EventEmitter } from 'common/EventEmitter';
import { IColorContrastCache } from 'browser/Types';

/**
* A shared object which is used to draw nothing for a particular cell.
Expand Down Expand Up @@ -309,8 +310,7 @@ export class TextureAtlas implements ITextureAtlas {
}

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);
const minimumContrastColor = this._getMinimumContrastColor(bg, bgColorMode, bgColor, fg, fgColorMode, fgColor, false, bold, dim, excludeFromContrastRatioDemands);
if (minimumContrastColor) {
return minimumContrastColor;
}
Expand Down Expand Up @@ -385,23 +385,26 @@ export class TextureAtlas implements ITextureAtlas {
}
}

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

// Try get from cache first
const adjustedColor = this._config.colors.contrastCache.getColor(bg, fg);
const cache = this._getContrastCache(dim);
const adjustedColor = cache.getColor(bg, fg);
if (adjustedColor !== undefined) {
return adjustedColor || undefined;
}

const bgRgba = this._resolveBackgroundRgba(bgColorMode, bgColor, inverse);
const fgRgba = this._resolveForegroundRgba(fgColorMode, fgColor, inverse, bold);
const result = rgba.ensureContrastRatio(bgRgba, fgRgba, this._config.minimumContrastRatio);
// Dim cells only require half the contrast, otherwise they wouldn't be distinguishable from
// non-dim cells
const result = rgba.ensureContrastRatio(bgRgba, fgRgba, this._config.minimumContrastRatio / (dim ? 2 : 1));

if (!result) {
this._config.colors.contrastCache.setColor(bg, fg, null);
cache.setColor(bg, fg, null);
return undefined;
}

Expand All @@ -410,11 +413,18 @@ export class TextureAtlas implements ITextureAtlas {
(result >> 16) & 0xFF,
(result >> 8) & 0xFF
);
this._config.colors.contrastCache.setColor(bg, fg, color);
cache.setColor(bg, fg, color);

return color;
}

private _getContrastCache(dim: boolean): IColorContrastCache {
if (dim) {
return this._config.colors.halfContrastCache;
}
return this._config.colors.contrastCache;
}

private _drawToCache(codeOrChars: number | string, bg: number, fg: number, ext: number, restrictToCellHeight: boolean = false): IRasterizedGlyph {
const chars = typeof codeOrChars === 'number' ? String.fromCharCode(codeOrChars) : codeOrChars;

Expand Down
7 changes: 6 additions & 1 deletion src/browser/services/ThemeService.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,12 @@ describe('ThemeService', () => {
describe('constructor', () => {
it('should fill all colors with values', () => {
for (const key of Object.keys(themeService.colors)) {
if (key !== 'ansi' && key !== 'contrastCache' && key !== 'selectionForeground') {
if (![
'ansi',
'contrastCache',
'halfContrastCache',
'selectionForeground'
].includes(key)) {
// A #rrggbb or rgba(...)
assert.ok((themeService.colors as any)[key].css.length >= 7);
}
Expand Down
8 changes: 5 additions & 3 deletions src/browser/services/ThemeService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ export class ThemeService extends Disposable implements IThemeService {
public serviceBrand: undefined;

private _colors: IColorSet;
private _contrastCache: IColorContrastCache;
private _contrastCache: IColorContrastCache = new ColorContrastCache();
private _halfContrastCache: IColorContrastCache = new ColorContrastCache();
private _restoreColors!: IRestoreColorSet;

public get colors(): ReadonlyColorSet { return this._colors; }
Expand All @@ -94,7 +95,6 @@ export class ThemeService extends Disposable implements IThemeService {
) {
super();

this._contrastCache = new ColorContrastCache();
this._colors = {
foreground: DEFAULT_FOREGROUND,
background: DEFAULT_BACKGROUND,
Expand All @@ -106,7 +106,8 @@ export class ThemeService extends Disposable implements IThemeService {
selectionInactiveBackgroundTransparent: DEFAULT_SELECTION,
selectionInactiveBackgroundOpaque: color.blend(DEFAULT_BACKGROUND, DEFAULT_SELECTION),
ansi: DEFAULT_ANSI_COLORS.slice(),
contrastCache: this._contrastCache
contrastCache: this._contrastCache,
halfContrastCache: this._halfContrastCache
};
this._updateRestoreColors();
this._setTheme(this._optionsService.rawOptions.theme);
Expand Down Expand Up @@ -172,6 +173,7 @@ export class ThemeService extends Disposable implements IThemeService {
}
// Clear our the cache
this._contrastCache.clear();
this._halfContrastCache.clear();
this._updateRestoreColors();
this._onChangeColors.fire(this.colors);
}
Expand Down

0 comments on commit 6c93460

Please sign in to comment.