render: defer conversion of TextColor into COLORREF until actual rendering time (avoid ConPTY narrowing bugs) #2661
Description
Right now, every time Renderer needs to switch the brush colors, it does the following:
- get the
TextColorfrom the attribute run - get the RGB value of the color from the color table
- pass that to
Engine::UpdateDrawingBrushes
Step 2 is lossy, and is the source of a class of bugs I'll call "ConPTY narrowing" bugs. In addition, there's a step where VtEngine needs to know the color table to do a translation from RGB back into indexed color.
"narrowing" bugs
| application output | conpty sends | why |
|---|---|---|
\e[38;2;22;198;12m |
\e[92m |
RGB(22, 198, 12) matches color index 10 in Campbell |
\e[38;5;127m |
\e[38;2;175;0;175m |
the xterm-256color color palette gets translated into RGB because Campbell only covers 0-16 |
\e[38;2;12;12;12m |
\e[30m |
RGB(12, 12, 12) matches index 0 in Campbell and index 0 is the default color (#293) |
If Renderer passed the TextColor directly to the Engine, Xterm256Engine could pass through all indexed colors unchanged, and XtermEngine and WinTelnetEngine could flatten to the 16-color space.
- Underline not rendered after some SGR 22/39/49 #3076 is caused by us doing early unpacking of colors and boldness; deferring until later will yield an improvement in preservation of underline status.
Metadata
Metadata
Assignees
Labels
Text rendering, emoji, complex glyph & font-fallback issuesIt's a feature request, but it doesn't really need a major design.A description (P1)For issues in the Console codebaseFor console issues specifically related to conptyThe new Windows Terminal.Fix is checked in, but it might be 3-4 weeks until a release.