From 6ced3014cf2989acb22877aa755c68c3c479bb04 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 27 May 2022 10:37:04 -0700 Subject: [PATCH 1/2] Support opaque selections in DOM renderer Brings the DOM renderer closer to the webgl renderer which draws the text on top of the selection. This is done by leveraging the override system similar to in webgl and as well as forcing the element above the selection. This also improves contrast color caching for the DOM renderer. This should result in a slight performance hit but only when a selection is being made. The improved contrast seems worth it. Fixes #3838 --- src/browser/renderer/dom/DomRenderer.ts | 2 +- .../renderer/dom/DomRendererRowFactory.ts | 22 ++++++++++++------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/browser/renderer/dom/DomRenderer.ts b/src/browser/renderer/dom/DomRenderer.ts index fdeef91c..a966e6f9 100644 --- a/src/browser/renderer/dom/DomRenderer.ts +++ b/src/browser/renderer/dom/DomRenderer.ts @@ -226,7 +226,7 @@ export class DomRenderer extends Disposable implements IRenderer { `}` + `${this._terminalSelector} .${SELECTION_CLASS} div {` + ` position: absolute;` + - ` background-color: ${this._colors.selectionTransparent.css};` + + ` background-color: ${this._colors.selectionOpaque.css};` + `}`; // Colors this._colors.ansi.forEach((c, i) => { diff --git a/src/browser/renderer/dom/DomRendererRowFactory.ts b/src/browser/renderer/dom/DomRendererRowFactory.ts index 266d7587..39fd4fdd 100644 --- a/src/browser/renderer/dom/DomRendererRowFactory.ts +++ b/src/browser/renderer/dom/DomRendererRowFactory.ts @@ -206,14 +206,22 @@ export class DomRendererRowFactory { } // Apply selection foreground if applicable + const isInSelection = this._isCellInSelection(x, row); if (!isTop) { - if (this._colors.selectionForeground && this._isCellInSelection(x, row)) { + if (this._colors.selectionForeground && isInSelection) { fgColorMode = Attributes.CM_RGB; fg = this._colors.selectionForeground.rgba >> 8 & 0xFFFFFF; fgOverride = this._colors.selectionForeground; } } + // If in the selection, force the element to be above the selection to improve contrast and + // support opaque selections + if (isInSelection) { + bgOverride = this._colors.selectionOpaque; + isTop = true; + } + // If it's a top decoration, render above the selection if (isTop) { charElement.classList.add(`xterm-decoration-top`); @@ -226,7 +234,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], cell, undefined, undefined)) { + if (!this._applyMinimumContrast(charElement, this._colors.background, this._colors.ansi[fg], cell, bgOverride, undefined)) { charElement.classList.add(`xterm-fg-${fg}`); } break; @@ -242,7 +250,7 @@ export class DomRendererRowFactory { break; case Attributes.CM_DEFAULT: default: - if (!this._applyMinimumContrast(charElement, this._colors.background, this._colors.foreground, cell, undefined, undefined)) { + if (!this._applyMinimumContrast(charElement, this._colors.background, this._colors.foreground, cell, bgOverride, undefined)) { if (isInverse) { charElement.classList.add(`xterm-fg-${INVERTED_DEFAULT_COLOR}`); } @@ -279,16 +287,14 @@ export class DomRendererRowFactory { // Try get from cache first, only use the cache when there are no decoration overrides let adjustedColor: IColor | undefined | null = undefined; - if (!bgOverride || !fgOverride) { - adjustedColor = this._colors.contrastCache.getColor(this._workCell.bg, this._workCell.fg); + if (!bgOverride && !fgOverride) { + adjustedColor = this._colors.contrastCache.getColor(bg.rgba, fg.rgba); } // Calculate and store in cache if (adjustedColor === undefined) { adjustedColor = color.ensureContrastRatio(bgOverride || bg, fgOverride || fg, this._optionsService.rawOptions.minimumContrastRatio); - if (!bgOverride || !fgOverride) { - this._colors.contrastCache.setColor(this._workCell.bg, this._workCell.fg, adjustedColor ?? null); - } + this._colors.contrastCache.setColor((bgOverride || bg).rgba, (fgOverride || fg).rgba, adjustedColor ?? null); } if (adjustedColor) { From 98c40e3f8e85437d7a9987b0a286a4b76d3746da Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 27 May 2022 10:46:23 -0700 Subject: [PATCH 2/2] Add opaque selection dom renderer tests --- .../dom/DomRendererRowFactory.test.ts | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/src/browser/renderer/dom/DomRendererRowFactory.test.ts b/src/browser/renderer/dom/DomRendererRowFactory.test.ts index 5a374b14..7c48cce8 100644 --- a/src/browser/renderer/dom/DomRendererRowFactory.test.ts +++ b/src/browser/renderer/dom/DomRendererRowFactory.test.ts @@ -12,7 +12,7 @@ import { IBufferLine } from 'common/Types'; import { CellData } from 'common/buffer/CellData'; import { MockCoreService, MockDecorationService, MockOptionsService } from 'common/TestUtils.test'; import { css } from 'common/Color'; -import { MockCharacterJoinerService, MockSelectionService } from 'browser/TestUtils.test'; +import { MockCharacterJoinerService } from 'browser/TestUtils.test'; describe('DomRendererRowFactory', () => { let dom: jsdom.JSDOM; @@ -245,6 +245,26 @@ describe('DomRendererRowFactory', () => { ); }); }); + + describe('selectionForeground', () => { + it('should force selected cells with content to be rendered above the background', () => { + lineData.setCell(0, CellData.fromCharData([DEFAULT_ATTR, 'a', 1, 'a'.charCodeAt(0)])); + lineData.setCell(1, CellData.fromCharData([DEFAULT_ATTR, 'b', 1, 'b'.charCodeAt(0)])); + rowFactory.onSelectionChanged([1, 0], [2, 0], false); + const fragment = rowFactory.createRow(lineData, 0, false, undefined, 0, false, 5, 20); + assert.equal(getFragmentHtml(fragment), + 'ab' + ); + }); + it('should force whitespace cells to be rendered above the background', () => { + lineData.setCell(1, CellData.fromCharData([DEFAULT_ATTR, 'a', 1, 'a'.charCodeAt(0)])); + rowFactory.onSelectionChanged([0, 0], [2, 0], false); + const fragment = rowFactory.createRow(lineData, 0, false, undefined, 0, false, 5, 20); + assert.equal(getFragmentHtml(fragment), + ' a' + ); + }); + }); }); function getFragmentHtml(fragment: DocumentFragment): string {