ANSI/VT transparent and black background mismatch

[PhMajerus]

Tested on Windows Terminal 0.7.3382.0 from Store on Windows 10 1909 build 18363.476.
CUI App is my ActiveScript Shell, but only used because it makes it easy to send full strings to the Console API and JavaScript is easy for most to read, the problem seems to be with the rendering, or with the way conhost stores and retrieves cells colors.

ANSI VT has codes to set foreground and background colors, and codes to use default foreground and background colors (39, 49).
Windows Terminal has a nice transparency effect when the default background is used that makes it use Acrylic translucency.
The problem is that the terminal seems to lose track of what has been output as black (30 or 40) when it is the cell's background color and what has been output as default background color (49).

Writing something with a black background, using color 40 (or 30 and reversed using 7), the expectation is to have that color show as black, transparency should be achieved only by color 49.
This is important when doing ANSI-art and for consistency.
However, it seems the current implementation doesn't keep track of default colors 39 and 49, and instead assumes all cells with color 40 as their background to be the default background color or transparent, basically converting it to 49.
It also converts the other way around, using transparent (49) reversed should show the text as transparent, but instead converts it to black (30).

Let me know if my explanations aren't clear enough or if you really need a Win32 API repro.

[j4james]

This sounds like #2661.

[PhMajerus]

This sounds like #2661.

Indeed, if colors are converted too early.
Important to note for this bug is that default background and default foreground should have their own indexes, and not get converted to their current color indexes either. This means storing the 16 base colors as indexes in the 256 colors palette and keeping it as index, converting them to RGB only for rendering still wouldn't be enough. we need special index values for DEFAULT_BACKGROUND and DEFAULT_FOREGROUND and map colors reset codes 39 and 49 to those.

Acrylic transparency should only happen when DEFAULT_BACKGROUND color is used, not when another color happens to have the same RGB value as the one selected as the background color, or the index selected as the background in the old console.
Also, when reverse is used (7), we can have DEFAULT_FOREGROUND used as a background color and DEFAULT_BACKGROUND used as a foreground color, those should be handled properly, and transparency should be supported on text foreground in that case.

[j4james]

Important to note for this bug is that default background and default foreground should have their own indexes, and not get converted to their current color indexes either.

The way it works is that default colors are indicated with a distinct type. So a color can either be default, an index, or an rgb value. You can see the ColorType enum here:

terminal/src/buffer/out/TextColor.h

Lines 42 to 47 in 429af0e

	enum class ColorType : BYTE
	{
	IsIndex = 0x0,
	IsDefault = 0x1,
	IsRgb = 0x2
	};

The narrowing problem, as I understand it, is that these color types are all first converted to a COLORREF, and then that COLORREF is sometimes "optimised" to an index value. But that can obviously result in the conpty connection receiving an index color when the original attribute was not.

[PhMajerus]

@j4james
Thanks for the details, that structure should indeed take care of the original color in all cases.

I'm concerned about the conversion to a COLORREF and optimization back to an index when the COLORRED matches a color of the palette though.
Any of the 256 colors of the palette can be redefined at any time, using the SetConsoleScreenBufferInfoEx API function for the base 16 colors, or "ESC]4;[index];rgb:[r]/[g]/[b]BEL" VT sequence for any of the 256 colors.
Because of this, any RGB color that did match a palette index earlier might now reference a color that isn't the same anymore.

This means we need to keep the original intent of the VT sequences, an index should never be converted to an RGB and an RGB should never be simplified into an index. Converting to COLORREF should be done just before rendering, as the palette might have been changed after the color codes have been output, and palette colors should always render using the current palette, not the palette from when they were received.

[j4james]

This means we need to keep the original intent of the VT sequences, an index should never be converted to an RGB and an RGB should never be simplified into an index.

Yes! That's exactly what issue #2661 is about!

[DHowett-MSFT]

This specific case is actually tracked by /dupe #293. #2661 will help make sure we don’t narrow arbitrary colors into indexed ones, but 293 tracks the compatibility behavior of crushing the “default win32 console background” index (0) to “default VT background” (49). That’ll apply even in reverse video mode.

[ghost]

Hi! We've identified this issue as a duplicate of another one that already exists on this Issue Tracker. This specific instance is being closed in favor of tracking the concern over on the referenced thread. Thanks for your report!