From 61432ab65d457a8d4ed21b22cec23b0c48a73828 Mon Sep 17 00:00:00 2001 From: Puneethnaik Date: Tue, 9 Jun 2020 23:34:53 +0530 Subject: [PATCH] Made changes according as per Review Comments. --- .../src/atlas/CharAtlasUtils.ts | 2 +- src/browser/Color.ts | 22 +++++++++---------- src/browser/ColorManager.ts | 16 ++++++++++---- src/browser/Types.d.ts | 2 +- src/browser/renderer/Renderer.ts | 2 -- src/browser/renderer/SelectionRenderLayer.ts | 2 +- src/browser/renderer/dom/DomRenderer.ts | 3 +-- 7 files changed, 27 insertions(+), 22 deletions(-) diff --git a/addons/xterm-addon-webgl/src/atlas/CharAtlasUtils.ts b/addons/xterm-addon-webgl/src/atlas/CharAtlasUtils.ts index 8e26b1d6a..5496a500b 100644 --- a/addons/xterm-addon-webgl/src/atlas/CharAtlasUtils.ts +++ b/addons/xterm-addon-webgl/src/atlas/CharAtlasUtils.ts @@ -20,7 +20,7 @@ export function generateConfig(scaledCharWidth: number, scaledCharHeight: number background: colors.background, cursor: NULL_COLOR, cursorAccent: NULL_COLOR, - selection: NULL_COLOR, + selectionTransparent: NULL_COLOR, selectionOpaque: NULL_COLOR, // For the static char atlas, we only use the first 16 colors, but we need all 256 for the // dynamic character atlas. diff --git a/src/browser/Color.ts b/src/browser/Color.ts index f2ef590fc..2effe515e 100644 --- a/src/browser/Color.ts +++ b/src/browser/Color.ts @@ -3,7 +3,7 @@ * @license MIT */ -import { IColor, IColorSet } from 'browser/Types'; +import { IColor } from 'browser/Types'; /** * Helper functions where the source type is "channels" (individual color channels as numbers). @@ -47,16 +47,8 @@ export namespace color { const rgba = channels.toRgba(r, g, b); return { css, rgba }; } - export function blendSelectionWithBgWithOpacity(colors: IColorSet, opacity: number=0.3): void { - const [selectionR, selectionG, selectionB, selectionA] = rgba.toChannels(colors.selection.rgba); - // The selection color is opaque. It needs to be blended with background color at 0.3 opacity Issue #2737 - if (selectionA === 0xFF) { - const newAlpha: number = Math.round(opacity*255); - colors.selection = { - css: channels.toCss(selectionR, selectionG, selectionB, newAlpha), - rgba: channels.toRgba(selectionR, selectionG, selectionB, newAlpha) - }; - } + export function isOpaque(color: IColor): boolean { + return (color.rgba & 0xFF) === 0xFF; } export function ensureContrastRatio(bg: IColor, fg: IColor, ratio: number): IColor | undefined { @@ -79,6 +71,14 @@ export namespace color { rgba: rgbaColor }; } + export function opacity(color: IColor, opacity: number): IColor { + const a = Math.round(opacity * 0xFF); + const [r, g, b] = rgba.toChannels(color.rgba); + return { + css: channels.toCss(r, g, b, a), + rgba: channels.toRgba(r, g, b, a) + }; + } } /** diff --git a/src/browser/ColorManager.ts b/src/browser/ColorManager.ts index 7d5ad598e..9a0fd7ee4 100644 --- a/src/browser/ColorManager.ts +++ b/src/browser/ColorManager.ts @@ -5,7 +5,7 @@ import { IColorManager, IColor, IColorSet, IColorContrastCache } from 'browser/Types'; import { ITheme } from 'common/services/Services'; -import { channels, color, css, rgba } from 'browser/Color'; +import { channels, color, css } from 'browser/Color'; import { ColorContrastCache } from 'browser/ColorContrastCache'; const DEFAULT_FOREGROUND = css.toColor('#ffffff'); @@ -92,7 +92,7 @@ export class ColorManager implements IColorManager { background: DEFAULT_BACKGROUND, cursor: DEFAULT_CURSOR, cursorAccent: DEFAULT_CURSOR_ACCENT, - selection: DEFAULT_SELECTION, + selectionTransparent: DEFAULT_SELECTION, selectionOpaque: color.blend(DEFAULT_BACKGROUND, DEFAULT_SELECTION), ansi: DEFAULT_ANSI_COLORS.slice(), contrastCache: this._contrastCache @@ -115,8 +115,16 @@ export class ColorManager implements IColorManager { this.colors.background = this._parseColor(theme.background, DEFAULT_BACKGROUND); this.colors.cursor = this._parseColor(theme.cursor, DEFAULT_CURSOR, true); this.colors.cursorAccent = this._parseColor(theme.cursorAccent, DEFAULT_CURSOR_ACCENT, true); - this.colors.selection = this._parseColor(theme.selection, DEFAULT_SELECTION, true); - this.colors.selectionOpaque = color.blend(this.colors.background, this.colors.selection); + this.colors.selectionTransparent = this._parseColor(theme.selection, DEFAULT_SELECTION, true); + this.colors.selectionOpaque = color.blend(this.colors.background, this.colors.selectionTransparent); + /** + * If selection color is opaque, blend it with background with 0.3 opacity + * Issue #2737 + */ + if (color.isOpaque(this.colors.selectionTransparent)) { + const opacity = 0.3; + this.colors.selectionTransparent = color.opacity(this.colors.selectionTransparent, opacity); + } this.colors.ansi[0] = this._parseColor(theme.black, DEFAULT_ANSI_COLORS[0]); this.colors.ansi[1] = this._parseColor(theme.red, DEFAULT_ANSI_COLORS[1]); this.colors.ansi[2] = this._parseColor(theme.green, DEFAULT_ANSI_COLORS[2]); diff --git a/src/browser/Types.d.ts b/src/browser/Types.d.ts index d0f8da152..1245ab043 100644 --- a/src/browser/Types.d.ts +++ b/src/browser/Types.d.ts @@ -120,7 +120,7 @@ export interface IColorSet { background: IColor; cursor: IColor; cursorAccent: IColor; - selection: IColor; + selectionTransparent: IColor; /** The selection blended on top of background. */ selectionOpaque: IColor; ansi: IColor[]; diff --git a/src/browser/renderer/Renderer.ts b/src/browser/renderer/Renderer.ts index b00b4116a..b9d02ff86 100644 --- a/src/browser/renderer/Renderer.ts +++ b/src/browser/renderer/Renderer.ts @@ -15,7 +15,6 @@ import { ICharSizeService, ICoreBrowserService } from 'browser/services/Services import { IBufferService, IOptionsService, ICoreService } from 'common/services/Services'; import { removeTerminalFromCache } from 'browser/renderer/atlas/CharAtlasCache'; import { EventEmitter, IEvent } from 'common/EventEmitter'; -import { color } from 'browser/Color'; let nextRendererId = 1; @@ -45,7 +44,6 @@ export class Renderer extends Disposable implements IRenderer { super(); const allowTransparency = this._optionsService.options.allowTransparency; this._characterJoinerRegistry = new CharacterJoinerRegistry(this._bufferService); - color.blendSelectionWithBgWithOpacity(this._colors); this._renderLayers = [ new TextRenderLayer(this._screenElement, 0, this._colors, this._characterJoinerRegistry, allowTransparency, this._id, this._bufferService, _optionsService), new SelectionRenderLayer(this._screenElement, 1, this._colors, this._id, this._bufferService, _optionsService), diff --git a/src/browser/renderer/SelectionRenderLayer.ts b/src/browser/renderer/SelectionRenderLayer.ts index 5689ce932..685111e8b 100644 --- a/src/browser/renderer/SelectionRenderLayer.ts +++ b/src/browser/renderer/SelectionRenderLayer.ts @@ -78,7 +78,7 @@ export class SelectionRenderLayer extends BaseRenderLayer { return; } - this._ctx.fillStyle = this._colors.selection.css; + this._ctx.fillStyle = this._colors.selectionTransparent.css; if (columnSelectMode) { const startCol = start[0]; diff --git a/src/browser/renderer/dom/DomRenderer.ts b/src/browser/renderer/dom/DomRenderer.ts index 64e2fc687..f42ff684c 100644 --- a/src/browser/renderer/dom/DomRenderer.ts +++ b/src/browser/renderer/dom/DomRenderer.ts @@ -54,7 +54,6 @@ export class DomRenderer extends Disposable implements IRenderer { @IBufferService private readonly _bufferService: IBufferService ) { super(); - color.blendSelectionWithBgWithOpacity(this._colors); this._rowContainer = document.createElement('div'); this._rowContainer.classList.add(ROW_CONTAINER_CLASS); this._rowContainer.style.lineHeight = 'normal'; @@ -226,7 +225,7 @@ export class DomRenderer extends Disposable implements IRenderer { `}` + `${this._terminalSelector} .${SELECTION_CLASS} div {` + ` position: absolute;` + - ` background-color: ${this._colors.selection.css};` + + ` background-color: ${this._colors.selectionTransparent.css};` + `}`; // Colors this._colors.ansi.forEach((c, i) => {