From 851c2e39af2303a241b3f5c88b45bd19e95f3ada Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 9 Sep 2023 08:37:55 -0700 Subject: [PATCH 1/4] Fix webgl not respecting MCR for inverse Part of #4759 --- src/browser/renderer/shared/TextureAtlas.ts | 2 +- test/playwright/SharedRendererTests.ts | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/browser/renderer/shared/TextureAtlas.ts b/src/browser/renderer/shared/TextureAtlas.ts index dd059574..56c77b68 100644 --- a/src/browser/renderer/shared/TextureAtlas.ts +++ b/src/browser/renderer/shared/TextureAtlas.ts @@ -309,7 +309,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 { - const minimumContrastColor = this._getMinimumContrastColor(bg, bgColorMode, bgColor, fg, fgColorMode, fgColor, false, bold, dim, excludeFromContrastRatioDemands); + const minimumContrastColor = this._getMinimumContrastColor(bg, bgColorMode, bgColor, fg, fgColorMode, fgColor, inverse, bold, dim, excludeFromContrastRatioDemands); if (minimumContrastColor) { return minimumContrastColor; } diff --git a/test/playwright/SharedRendererTests.ts b/test/playwright/SharedRendererTests.ts index 889bfc40..d746ca4c 100644 --- a/test/playwright/SharedRendererTests.ts +++ b/test/playwright/SharedRendererTests.ts @@ -1102,6 +1102,19 @@ export function injectSharedRendererTests(ctx: ISharedRendererTestContext): void await pollFor(ctx.value.page, () => getCellColor(ctx.value, 2, 1), [0, 0, 0, 255]); await pollFor(ctx.value.page, () => getCellColor(ctx.value, 3, 1), [0, 0, 0, 255]); }); + test.only('#4759: minimum contrast ratio should be respected on inverse text', async () => { + const theme: ITheme = { + foreground: '#aaaaaa', + background: '#333333' + }; + await ctx.value.page.evaluate(`window.term.options.theme = ${JSON.stringify(theme)};`); + await ctx.value.proxy.write(`\x1b[7m■■`); + // Validate before minimumContrastRatio is applied + await pollFor(ctx.value.page, () => getCellColor(ctx.value, 1, 1), [0x33, 0x33, 0x33, 255]); + await ctx.value.page.evaluate(`window.term.options.minimumContrastRatio = 10;`); + frameDetails = undefined; + await pollFor(ctx.value.page, () => getCellColor(ctx.value, 1, 1), [0, 0, 0, 255]); + }); }); } From 1aac37e14f9741061fed2dc06d7449fe2bc2a6ff Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 9 Sep 2023 08:51:50 -0700 Subject: [PATCH 2/4] Improve regression tests for 4759 --- test/playwright/SharedRendererTests.ts | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/test/playwright/SharedRendererTests.ts b/test/playwright/SharedRendererTests.ts index d746ca4c..849da743 100644 --- a/test/playwright/SharedRendererTests.ts +++ b/test/playwright/SharedRendererTests.ts @@ -1102,7 +1102,7 @@ export function injectSharedRendererTests(ctx: ISharedRendererTestContext): void await pollFor(ctx.value.page, () => getCellColor(ctx.value, 2, 1), [0, 0, 0, 255]); await pollFor(ctx.value.page, () => getCellColor(ctx.value, 3, 1), [0, 0, 0, 255]); }); - test.only('#4759: minimum contrast ratio should be respected on inverse text', async () => { + test('#4759: minimum contrast ratio should be respected on inverse text', async () => { const theme: ITheme = { foreground: '#aaaaaa', background: '#333333' @@ -1110,10 +1110,32 @@ export function injectSharedRendererTests(ctx: ISharedRendererTestContext): void await ctx.value.page.evaluate(`window.term.options.theme = ${JSON.stringify(theme)};`); await ctx.value.proxy.write(`\x1b[7m■■`); // Validate before minimumContrastRatio is applied - await pollFor(ctx.value.page, () => getCellColor(ctx.value, 1, 1), [0x33, 0x33, 0x33, 255]); + await pollFor(ctx.value.page, () => getCellColor(ctx.value, 1, 1), [51, 51, 51, 255]); + await pollFor(ctx.value.page, () => getCellColor(ctx.value, 2, 1), [51, 51, 51, 255]); await ctx.value.page.evaluate(`window.term.options.minimumContrastRatio = 10;`); frameDetails = undefined; await pollFor(ctx.value.page, () => getCellColor(ctx.value, 1, 1), [0, 0, 0, 255]); + await pollFor(ctx.value.page, () => getCellColor(ctx.value, 2, 1), [0, 0, 0, 255]); + }); + test('#4759: minimum contrast ratio should be respected on selected inverse text', async () => { + const theme: ITheme = { + foreground: '#777777', + background: '#555555', + selectionBackground: '#666666' // Slightly more contrast needed for selection + }; + await ctx.value.page.evaluate(`window.term.options.theme = ${JSON.stringify(theme)};`); + await ctx.value.proxy.write(`\x1b[7m■■`); + await ctx.value.proxy.selectAll(); + // Validate before minimumContrastRatio is applied + await pollFor(ctx.value.page, () => getCellColor(ctx.value, 1, 1), [85, 85, 85, 255]); + await pollFor(ctx.value.page, () => getCellColor(ctx.value, 2, 1), [85, 85, 85, 255]); + await pollFor(ctx.value.page, () => getCellColor(ctx.value, 3, 1), [102, 102, 102, 255]); + await ctx.value.page.evaluate(`window.term.options.minimumContrastRatio = 10;`); + await ctx.value.proxy.selectAll(); + frameDetails = undefined; + await pollFor(ctx.value.page, () => getCellColor(ctx.value, 1, 1), [255, 255, 255, 255]); + await pollFor(ctx.value.page, () => getCellColor(ctx.value, 2, 1), [255, 255, 255, 255]); + await pollFor(ctx.value.page, () => getCellColor(ctx.value, 3, 1), [102, 102, 102, 255]); }); }); } From b0868967cbe2348a30826e1ed2f06abae59d81ce Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 9 Sep 2023 08:52:43 -0700 Subject: [PATCH 3/4] Fix inverse MCR with selection for dom renderer Fixes #4759 --- src/browser/renderer/dom/DomRendererRowFactory.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/browser/renderer/dom/DomRendererRowFactory.ts b/src/browser/renderer/dom/DomRendererRowFactory.ts index 2d21a647..614b2301 100644 --- a/src/browser/renderer/dom/DomRendererRowFactory.ts +++ b/src/browser/renderer/dom/DomRendererRowFactory.ts @@ -419,7 +419,7 @@ export class DomRendererRowFactory { break; case Attributes.CM_DEFAULT: default: - if (!this._applyMinimumContrast(charElement, resolvedBg, colors.foreground, cell, bgOverride, undefined)) { + if (!this._applyMinimumContrast(charElement, resolvedBg, colors.foreground, cell, bgOverride, fgOverride)) { if (isInverse) { classes.push(`xterm-fg-${INVERTED_DEFAULT_COLOR}`); } From fe4528c9bed849476e4ac57e56f432e3840f1ff3 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 9 Sep 2023 09:00:57 -0700 Subject: [PATCH 4/4] Add one regression test to canvas exceptions --- test/playwright/SharedRendererTests.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/playwright/SharedRendererTests.ts b/test/playwright/SharedRendererTests.ts index 849da743..a19a7537 100644 --- a/test/playwright/SharedRendererTests.ts +++ b/test/playwright/SharedRendererTests.ts @@ -1117,7 +1117,7 @@ export function injectSharedRendererTests(ctx: ISharedRendererTestContext): void await pollFor(ctx.value.page, () => getCellColor(ctx.value, 1, 1), [0, 0, 0, 255]); await pollFor(ctx.value.page, () => getCellColor(ctx.value, 2, 1), [0, 0, 0, 255]); }); - test('#4759: minimum contrast ratio should be respected on selected inverse text', async () => { + (ctx.skipCanvasExceptions ? test.skip : test)('#4759: minimum contrast ratio should be respected on selected inverse text', async () => { const theme: ITheme = { foreground: '#777777', background: '#555555',