Merge pull request #4525 from Tyriar/4161_dim

Don't apply dim to background color

Author: Tyriar

addons/xterm-addon-canvas/src/CanvasAddon.ts

@@ -4,7 +4,7 @@
  */
 
 import { ICharacterJoinerService, ICharSizeService, ICoreBrowserService, IRenderService, ISelectionService, IThemeService } from 'browser/services/Services';
-import { IColorSet, ITerminal } from 'browser/Types';
+import { ITerminal } from 'browser/Types';
 import { CanvasRenderer } from './CanvasRenderer';
 import { IBufferService, ICoreService, IDecorationService, IOptionsService } from 'common/services/Services';
 import { ITerminalAddon, Terminal } from 'xterm';

addons/xterm-addon-canvas/src/TextRenderLayer.ts

@@ -188,12 +188,6 @@ export class TextRenderLayer extends BaseRenderLayer {
         nextFillStyle = this._themeService.colors.ansi[cell.getBgColor()].css;
       }
 
-      // Apply dim to the background, this is relatively slow as the CSS is re-parsed but dim is
-      // rarely used
-      if (nextFillStyle && cell.isDim()) {
-        nextFillStyle = color.multiplyOpacity(css.toColor(nextFillStyle), 0.5).css;
-      }
-
       // Get any decoration foreground/background overrides, this must be fetched before the early
       // exist but applied after inverse
       let isTop = false;

addons/xterm-addon-webgl/src/RectangleRenderer.ts

@@ -270,7 +270,7 @@ export class RectangleRenderer extends Disposable {
     $r = (($rgba >> 24) & 0xFF) / 255;
     $g = (($rgba >> 16) & 0xFF) / 255;
     $b = (($rgba >> 8 ) & 0xFF) / 255;
-    $a = (!$isDefault && bg & BgFlags.DIM) ? DIM_OPACITY : 1;
+    $a = 1;
 
     this._addRectangle(vertices.attributes, offset, $x1, $y1, (endX - startX) * this._dimensions.device.cell.width, this._dimensions.device.cell.height, $r, $g, $b, $a);
   }

addons/xterm-addon-webgl/test/WebglRenderer.api.ts

@@ -364,6 +364,55 @@ describe('WebGL Renderer Integration Tests', async () => {
       }
     });
 
+    itWebgl('foreground 16-255 dim', async () => {
+      let data = '';
+      for (let y = 0; y < 240 / 16; y++) {
+        for (let x = 0; x < 16; x++) {
+          data += `\\x1b[2;38;5;${16 + y * 16 + x}m█\x1b[0m`;
+        }
+        data += '\\r\\n';
+      }
+      await writeSync(page, data);
+      for (let y = 0; y < 240 / 16; y++) {
+        for (let x = 0; x < 16; x++) {
+          const cssColor = COLORS_16_TO_255[y * 16 + x];
+          const r = parseInt(cssColor.slice(1, 3), 16);
+          const g = parseInt(cssColor.slice(3, 5), 16);
+          const b = parseInt(cssColor.slice(5, 7), 16);
+          // It's difficult to assert the exact color due to rounding, just ensure the color differs
+          // to the regular color
+          await pollFor(page, async () => {
+            const c = await getCellColor(x + 1, y + 1);
+            return (
+              (c[0] === 0 || c[0] !== r) &&
+              (c[1] === 0 || c[1] !== g) &&
+              (c[2] === 0 || c[2] !== b)
+            );
+          }, true);
+        }
+      }
+    });
+
+    itWebgl('background 16-255 dim', async () => {
+      let data = '';
+      for (let y = 0; y < 240 / 16; y++) {
+        for (let x = 0; x < 16; x++) {
+          data += `\\x1b[2;48;5;${16 + y * 16 + x}m \\x1b[0m`;
+        }
+        data += '\\r\\n';
+      }
+      await writeSync(page, data);
+      for (let y = 0; y < 240 / 16; y++) {
+        for (let x = 0; x < 16; x++) {
+          const cssColor = COLORS_16_TO_255[y * 16 + x];
+          const r = parseInt(cssColor.slice(1, 3), 16);
+          const g = parseInt(cssColor.slice(3, 5), 16);
+          const b = parseInt(cssColor.slice(5, 7), 16);
+          await pollFor(page, () => getCellColor(x + 1, y + 1), [r, g, b, 255]);
+        }
+      }
+    });
+
     itWebgl('foreground true color red', async () => {
       let data = '';
       for (let y = 0; y < 16; y++) {

css/xterm.css

@@ -161,7 +161,9 @@
 }
 
 .xterm-dim {
-    opacity: 0.5;
+    /* Dim should not apply to background, so the opacity of the foreground color is applied
+     * explicitly in the generated class and reset to 1 here */
+    opacity: 1 !important;
 }
 
 .xterm-underline-1 { text-decoration: underline; }

src/browser/renderer/dom/DomRenderer.ts

@@ -3,7 +3,7 @@
  * @license MIT
  */
 
-import { BOLD_CLASS, CURSOR_BLINK_CLASS, CURSOR_CLASS, CURSOR_STYLE_BAR_CLASS, CURSOR_STYLE_BLOCK_CLASS, CURSOR_STYLE_UNDERLINE_CLASS, DomRendererRowFactory, ITALIC_CLASS } from 'browser/renderer/dom/DomRendererRowFactory';
+import { BOLD_CLASS, CURSOR_BLINK_CLASS, CURSOR_CLASS, CURSOR_STYLE_BAR_CLASS, CURSOR_STYLE_BLOCK_CLASS, CURSOR_STYLE_UNDERLINE_CLASS, DIM_CLASS, DomRendererRowFactory, ITALIC_CLASS } from 'browser/renderer/dom/DomRendererRowFactory';
 import { INVERTED_DEFAULT_COLOR } from 'browser/renderer/shared/Constants';
 import { createRenderDimensions } from 'browser/renderer/shared/RendererUtils';
 import { IRenderDimensions, IRenderer, IRequestRedrawEvent } from 'browser/renderer/shared/Types';
@@ -149,6 +149,10 @@ export class DomRenderer extends Disposable implements IRenderer {
       ` font-family: ${this._optionsService.rawOptions.fontFamily};` +
       ` font-size: ${this._optionsService.rawOptions.fontSize}px;` +
       `}`;
+    styles +=
+      `${this._terminalSelector} .${ROW_CONTAINER_CLASS} .xterm-dim {` +
+      ` color: ${color.multiplyOpacity(colors.foreground, 0.5).css};` +
+      `}`;
     // Text styles
     styles +=
       `${this._terminalSelector} span:not(.${BOLD_CLASS}) {` +
@@ -224,10 +228,12 @@ export class DomRenderer extends Disposable implements IRenderer {
     for (const [i, c] of colors.ansi.entries()) {
       styles +=
         `${this._terminalSelector} .${FG_CLASS_PREFIX}${i} { color: ${c.css}; }` +
+        `${this._terminalSelector} .${FG_CLASS_PREFIX}${i}.${DIM_CLASS} { color: ${color.multiplyOpacity(c, 0.5).css}; }` +
         `${this._terminalSelector} .${BG_CLASS_PREFIX}${i} { background-color: ${c.css}; }`;
     }
     styles +=
       `${this._terminalSelector} .${FG_CLASS_PREFIX}${INVERTED_DEFAULT_COLOR} { color: ${color.opaque(colors.background).css}; }` +
+      `${this._terminalSelector} .${FG_CLASS_PREFIX}${INVERTED_DEFAULT_COLOR}.${DIM_CLASS} { color: ${color.multiplyOpacity(color.opaque(colors.background), 0.5).css}; }` +
       `${this._terminalSelector} .${BG_CLASS_PREFIX}${INVERTED_DEFAULT_COLOR} { background-color: ${colors.foreground.css}; }`;
 
     this._themeStyleElement.textContent = styles;

src/browser/renderer/dom/DomRendererRowFactory.ts

@@ -9,7 +9,6 @@ import { NULL_CELL_CODE, WHITESPACE_CELL_CHAR, Attributes } from 'common/buffer/
 import { CellData } from 'common/buffer/CellData';
 import { ICoreService, IDecorationService, IOptionsService } from 'common/services/Services';
 import { color, rgba } from 'common/Color';
-import { IColorSet, ReadonlyColorSet } from 'browser/Types';
 import { ICharacterJoinerService, ICoreBrowserService, IThemeService } from 'browser/services/Services';
 import { JoinedCellData } from 'browser/services/CharacterJoinerService';
 import { excludeFromContrastRatioDemands } from 'browser/renderer/shared/RendererUtils';

src/browser/renderer/shared/TextureAtlas.ts

@@ -304,12 +304,6 @@ export class TextureAtlas implements ITextureAtlas {
         break;
     }
 
-    if (dim) {
-      // Blend here instead of using opacity because transparent colors mess with clipping the
-      // glyph's bounding box
-      result = color.blend(this._config.colors.background, color.multiplyOpacity(result, DIM_OPACITY));
-    }
-
     return result;
   }