render: defer conversion of TextColor into COLORREF until actual rendering time (avoid ConPTY narrowing bugs) #3747

@Undercat commented on GitHub (Apr 2, 2020):

I cannot find any example where they are disjoint in practice.

The scheme I describe avoids disambiguating a union on the color data—the 24-bit color is always available. The extra data can just be stripped out or ignored by anyone that doesn't need it.

@Undercat commented on GitHub (Apr 2, 2020): I cannot find any example where they are disjoint in practice. The scheme I describe avoids disambiguating a union on the color data—the 24-bit color is always available. The extra data can just be stripped out or ignored by anyone that doesn't need it.

claunia commented

@egmontkob commented on GitHub (Apr 2, 2020):

Unless the issue is that the first 16 colors in the 256-color palette and the traditional 16-color palette are disjunct.

The first 8 used to be different in xterm; I mean the colors were the same (i.e. a single 256-color palette, along with OSC 4 for setting it), but the legacy notation (e.g. 31) did convert it to bright counterpart when asked to be bold (1); whereas the 256-color notation (e.g. 38:5:1) did not, it kept the dark color and painted that with bold typeface. VTE copied this behavior.

Xterm stopped doing this in version 331 (I'm not aware of any place where 31 works differently than 38:5:1 (which doesn't imply there isn't any)). In VTE we kept treating them differently.

@egmontkob commented on GitHub (Apr 2, 2020): > Unless the issue is that the first 16 colors in the 256-color palette and the traditional 16-color palette are disjunct. The first 8 used to be different in xterm; I mean the colors were the same (i.e. a single 256-color palette, along with OSC 4 for setting it), but the legacy notation (e.g. `31`) did convert it to bright counterpart when asked to be bold (`1`); whereas the 256-color notation (e.g. `38:5:1`) did not, it kept the dark color and painted that with bold typeface. VTE copied this behavior. Xterm stopped doing this in version 331 (I'm not aware of any place where `31` works differently than `38:5:1` (which doesn't imply there isn't any)). In VTE we [kept treating them differently](https://gitlab.gnome.org/GNOME/vte/-/issues/149).

claunia commented

@zadjii-msft commented on GitHub (Apr 2, 2020):

@DHowett-MSFT It is capable of storing 8bit in the index there, but doesn't currently. This commit 608c660b from the aforementioned branch changes conhost to actually use the index as a 8bit value, not just a 4bit one. We're currently storing two tables, but ignoring the first 16 of the 256 color table. That's dumb, obviously, but an artifact of the organic growth of RGB color in conhost.

@zadjii-msft commented on GitHub (Apr 2, 2020): @DHowett-MSFT It is capable of storing 8bit in the index there, but doesn't currently. This commit 608c660b from the aforementioned branch changes conhost to actually use the index as a 8bit value, not just a 4bit one. We're currently storing two tables, but ignoring the first 16 of the 256 color table. That's dumb, obviously, but an artifact of the organic growth of RGB color in conhost.

claunia commented

2026-01-30 23:29:13 +00:00

@Undercat commented on GitHub (Apr 2, 2020):

Mmmm. Interesting.

Well, the “ideal” solution would be to push a UINT64 to the renderers that has complete color information in it, without overlap. I doubt this would cause any overflow to the stack for the relatively modest function calls involved, even on 32-bit platforms. That way, a renderer could immediately access whatever encoding it needed and only one forward look-up would need to be done to decode the color at the time the escape sequence is parsed.

@Undercat commented on GitHub (Apr 2, 2020): Mmmm. Interesting. Well, the “ideal” solution would be to push a `UINT64` to the renderers that has complete color information in it, without overlap. I doubt this would cause any overflow to the stack for the relatively modest function calls involved, even on 32-bit platforms. That way, a renderer could immediately access whatever encoding it needed and only one forward look-up would need to be done to decode the color at the time the escape sequence is parsed.

claunia commented

@zadjii-msft commented on GitHub (Apr 2, 2020):

@Undercat or perhaps 2 32bit TextColor's? 😆

@zadjii-msft commented on GitHub (Apr 2, 2020): @Undercat or perhaps 2 [32bit `TextColor`](https://github.com/microsoft/terminal/blob/92830abfefe87438fe393a7e227c71c8a2ad86ea/src/buffer/out/TextColor.h#L170)'s? 😆

claunia commented

2026-01-30 23:29:13 +00:00

@Undercat commented on GitHub (Apr 2, 2020):

@zadjii-msft lol.

You could, I suppose, push overlapped (union'd) data down and let the renderers figure out what to do with it for their own purposes. You could then unambiguously record 4-bit, 8-bit and 24-bit SGR labelings just by extending TextColor's metadata to add the 8-bit case and have three encoding cases instead of two. You would have color index look-ups all over the place that way, but then again, who cares? It would, at least, keep the 32-bit DWORD parameter....

@Undercat commented on GitHub (Apr 2, 2020): @zadjii-msft lol. You could, I suppose, push overlapped (union'd) data down and let the renderers figure out what to do with it for their own purposes. You could then unambiguously record 4-bit, 8-bit and 24-bit SGR labelings just by extending `TextColor`'s metadata to add the 8-bit case and have three encoding cases instead of two. You would have color index look-ups all over the place that way, but then again, who cares? It would, at least, keep the 32-bit `DWORD` parameter....

claunia commented

2026-01-30 23:29:13 +00:00

@egmontkob commented on GitHub (Apr 2, 2020):

You would have color index look-ups all over the place that way, but then again, who cares?

If this is not a problem then encoding the union of 256-color index and 16M truecolor plus some special values is trivial in 25 bits. Sounds to me like the best approach. (Also what VTE does by the way.)

Somehow I have the impression that WT devs don't like it, or it's not sufficient for WT's architecture; the resolved color (24 bits) and the origin (256 indices or direct true color = 257 or so possible different values) have to be carried together. And for this you would need just a tiny little more than 32 bits.

If indeed the resolved RGB and the origin of the color needs to be both carried, and squeezed into 32 bits, there are still some tricks to consider.

One idea is to drop the color precision of the R and/or B channel to 7 bits. No one will notice with their bare eyes, only when using some color picker software to check the exact color.

Another idea is not to allow to alter the extra 240 colors, only the basic 16 ones. (All terminals I've checked have the exact same default RGB for these 240 additional entries.) For the additional ones, the RGB value wouldn't be directly stored, but could be derived using a fixed hardwired table.

But you can also relax on this limitation: allow the extra 240 colors to be freely modified, with just one limitation: the last 24 entries (the grayscale ones) must be grayscale. With this limitation all the information can be squeezed into 32 bits. E.g. the high 8 bits are either the index (0..231) or maybe 232 for "default", 233 for "grayscale", in the latter case you have 24 more bits to store the actual grayscale index (4 bits), plus the R=G=B component (8 bits) and still have 12 bits free... which you could use to further loosen the restriction, and allow almost-grayscale colors, where the color components aren't too far away from each other :-)

@egmontkob commented on GitHub (Apr 2, 2020): > You would have color index look-ups all over the place that way, but then again, who cares? If this is not a problem then encoding the union of 256-color index and 16M truecolor plus some special values is trivial in 25 bits. Sounds to me like the best approach. (Also what VTE does by the way.) Somehow I have the impression that WT devs don't like it, or it's not sufficient for WT's architecture; the resolved color (24 bits) and the origin (256 indices or direct true color = 257 or so possible different values) have to be carried together. And for this you would need just a tiny little more than 32 bits. If indeed the resolved RGB and the origin of the color needs to be both carried, and squeezed into 32 bits, there are still some tricks to consider. One idea is to drop the color precision of the R and/or B channel to 7 bits. No one will notice with their bare eyes, only when using some color picker software to check the exact color. Another idea is not to allow to alter the extra 240 colors, only the basic 16 ones. (All terminals I've checked have the exact same default RGB for these 240 additional entries.) For the additional ones, the RGB value wouldn't be directly stored, but could be derived using a fixed hardwired table. But you can also relax on this limitation: allow the extra 240 colors to be freely modified, with just one limitation: the last 24 entries (the grayscale ones) must be grayscale. With this limitation all the information can be squeezed into 32 bits. E.g. the high 8 bits are either the index (0..231) or maybe 232 for "default", 233 for "grayscale", in the latter case you have 24 more bits to store the actual grayscale index (4 bits), plus the R=G=B component (8 bits) and still have 12 bits free... which you could use to further loosen the restriction, and allow almost-grayscale colors, where the color components aren't too far away from each other :-)

claunia commented

2026-01-30 23:29:14 +00:00

@Undercat commented on GitHub (Apr 2, 2020):

@egmontkob That was rather like what I was thinking about originally. There appears to be one additional difficulty, however. There is an extra flag in TextColor used for “default color.”

Really, as long as you’re okay with multiplexing color information (by creating unions that have to be decoded by the consumers), you can directly encode the 4-bit, 8-bit and 24-bit cases and then reconstruct the original SGR input for VT sinks with 100% fidelity.

I'm doodling a test case right now by creating a class (really just a structure with methods) that has assignment overloads for WORD and DWORD that automatically reconstruct the 4-bit legacy attribute color or 24-bit COLORREF data from a union...with a gaggle of methods to write SGR4|8|24 color sequences from the same data. This would minimize changes to the consumers of color data—basically, you would just have to change the color parameter’s formal type from COLORREF to something like Color.

Since the new type would behave like a COLORREF in most situations, fewer changes would need to be made to all the hungry consumers of color data, hopefully making this parameter enhancement (to support differentiable SGR labelings) less apocalyptic, in terms of code-change footprint.

@Undercat commented on GitHub (Apr 2, 2020): @egmontkob That was rather like what I was thinking about originally. There appears to be one additional difficulty, however. There is an extra flag in `TextColor` used for “default color.” Really, as long as you’re okay with multiplexing color information (by creating unions that have to be decoded by the consumers), you can directly encode the 4-bit, 8-bit and 24-bit cases and then reconstruct the original SGR input for VT sinks with 100% fidelity. I'm doodling a test case right now by creating a class (really just a structure with methods) that has assignment overloads for `WORD` and `DWORD` that automatically reconstruct the 4-bit legacy attribute color or 24-bit `COLORREF` data from a union...with a gaggle of methods to write SGR4|8|24 color sequences from the same data. This would minimize changes to the consumers of color data—basically, you would just have to change the color parameter’s formal type from `COLORREF` to something like `Color`. Since the new type would behave like a `COLORREF` in most situations, fewer changes would need to be made to all the hungry consumers of color data, hopefully making this parameter enhancement (to support differentiable SGR labelings) less apocalyptic, in terms of code-change footprint.

claunia commented

2026-01-30 23:29:14 +00:00

@Undercat commented on GitHub (Apr 3, 2020):

Well, this is a skeleton of what I propose morphing TextColor into (I prototyped it in GNU using a much more compact style because it's more comfortable for me to design and test things that way, but just imagine it expanded out into Microsoft syntax for the moment).

#include <unistd.h>
#include <stdlib.h>
#include <stdio.h>

unsigned table[256];
unsigned *getDefaultColorTable(int x) { return table; }
unsigned getDefaultColor() { return 0; } // CONSOLE_INFORMATION::GetDefault[Foreground|Background]()

enum ColorType { Default = 0, SGR4 = 1, SGR8 = 2, SGR24 = 3, Legacy = 4, RGB = 3 };
struct Color {
  union {
    unsigned u32;
    unsigned char u8;
    struct {
      unsigned R:8;
      unsigned G:8;
      unsigned B:8;
      unsigned type:3;
      unsigned background:1;
    };
    struct {
      unsigned legacy:4;
    };
  };
  Color() : type(ColorType::Default) { }
  Color(unsigned char red, unsigned char green, unsigned char blue, bool bg = false) { set(red, green, blue, bg); }
  Color(unsigned char attr, bool bg = false) { set(attr, bg); }
  Color(unsigned colorref, bool bg = false) { set(colorref, bg); }
  Color(unsigned color, ColorType ct, bool bg = false) { set(color, ct, bg); }
  void set(unsigned char red, unsigned char green, unsigned char blue, bool bg = false) { type = ColorType::SGR24; background = bg; R = red; G = green; B = blue; }
  void set(unsigned char attr, bool bg = false) { type = ColorType::Legacy; background = bg; legacy = bg ? attr >> 4 : attr; }
  void set(unsigned colorref, bool bg = false) { type = ColorType::RGB; background = bg; u32 = colorref; }
  void set(unsigned color, ColorType ct, bool bg = false) { type = ct; background = bg;
    switch (type) {
      case ColorType::Default: return;
      case ColorType::SGR4:
        if ((color >= 30 && color <= 37) || (color >= 90 && color <= 97)) { u8 = color; background = 0; }
        if ((color >= 40 && color <= 47) || (color >= 100 && color <= 107)) { u8 = color - 10; background = 1; }
        return;
      case ColorType::SGR8: u8 = color; return;
      case ColorType::SGR24: u32 &= 0xff000000; u32 |= color & 0xffffff; return;
      case ColorType::Legacy: u8 = background ? color >> 4 : color; return;
  } }
  unsigned asRGB(unsigned *table4, unsigned *table8) {
    switch (type) {
      case ColorType::Default: return getDefaultColor();
      case ColorType::SGR4: return table4[u8 > 37 ? u8 - 90 : u8 - 30];
      case ColorType::SGR8: return table8[u8];
      case ColorType::SGR24: return u32 & 0xffffff;
      case ColorType::Legacy: return table4[legacy];
  } }
  operator unsigned() { return asRGB(getDefaultColorTable(4), getDefaultColorTable(8)); }
  int emitSGR(FILE *target = stdout) {
    switch (type) {
      case ColorType::Default: return fprintf(target, "\x1b[%hhum", background ? 49 : 39);
      case ColorType::SGR4: return fprintf(target, "\x1b[%hhum", background ? u8 + 10 : u8);
      case ColorType::SGR8: return background ? fprintf(target, "\x1b[48;5;%hhum", u8) : fprintf(target, "\x1b[38;5;%hhum", u8);
      case ColorType::SGR24: return background ? fprintf(target, "\x1b[48;2;%hhu;%hhu;%hhum", R, G, B) : fprintf(target, "\x1b[38;2;%hhu;%hhu;%hhum", R, G, B);
      case ColorType::Legacy: return fprintf(target, "\x1b[%hhum", background ? (u8 < 8 ? u8 + 30 : u8 + 90) : (u8 < 8 ? u8 + 40 : u8 + 100));
  } }
};

int main() {
  for (int i = 0; i < 256; ++i) table[i] = i;
  Color color(128, 128, 128);
  printf("%x\n", color.asRGB(0, 0));
  color.emitSGR();
  printf("half-intensity\n");
  Color normal;
  normal.emitSGR();
  printf("full-intensity\n");
  Color red(31, ColorType::SGR4);
  red.emitSGR();
  printf("red text\n");
  Color greenish(40, ColorType::SGR8);
  greenish.emitSGR();
  printf("greenish text\n");
  normal.emitSGR();
  printf("normal again\n");
  Color bluebg(0, 0, 80, true);
  bluebg.emitSGR(); red.emitSGR();
  printf("nightmare\x1b[m\n");
  printf("%x %x %x\n", (unsigned)red, (unsigned)greenish, (unsigned)bluebg);
}

Basically:

The components are all placed into anonymous bitfield/unions so the data layout is more obvious.
That data layout is then rearranged to match COLORREF for easy interconversion (simple masking).
The type metadata enum has been extended to completely support all the various color regimes, which are supported as natively as possible for maximal performance and minimum potential for loss/bugs.
Type conversion and color look-up is done in one place instead of a dozen.
It could “easily” be extended to also convert RGB to legacy attribute-color, if there really is a need for that somewhere (???)

It would be necessary to re-plumb TerminalApi.cpp and TerminalDispatchGraphics.cpp in order to record the source type accurately enough (right now, there is a complete reliance on constructor type overloading in order to determine the color regime used by the source, but that is inappropriate and dangerous when you start getting multiple types that are built on single octet data words, in addition to being confusing...better to make it explicit).

Before I even start to shoe-horn this monster into the existing code (which will obviously take more than a day, not including validation), I'd be interested in some feedback. Comments? Suggestions? Flames?

@Undercat commented on GitHub (Apr 3, 2020): Well, this is a skeleton of what I propose morphing `TextColor` into (I prototyped it in GNU using a much more compact style because it's more comfortable for me to design and test things that way, but just imagine it expanded out into Microsoft syntax for the moment). ```c #include <unistd.h> #include <stdlib.h> #include <stdio.h> unsigned table[256]; unsigned *getDefaultColorTable(int x) { return table; } unsigned getDefaultColor() { return 0; } // CONSOLE_INFORMATION::GetDefault[Foreground|Background]() enum ColorType { Default = 0, SGR4 = 1, SGR8 = 2, SGR24 = 3, Legacy = 4, RGB = 3 }; struct Color { union { unsigned u32; unsigned char u8; struct { unsigned R:8; unsigned G:8; unsigned B:8; unsigned type:3; unsigned background:1; }; struct { unsigned legacy:4; }; }; Color() : type(ColorType::Default) { } Color(unsigned char red, unsigned char green, unsigned char blue, bool bg = false) { set(red, green, blue, bg); } Color(unsigned char attr, bool bg = false) { set(attr, bg); } Color(unsigned colorref, bool bg = false) { set(colorref, bg); } Color(unsigned color, ColorType ct, bool bg = false) { set(color, ct, bg); } void set(unsigned char red, unsigned char green, unsigned char blue, bool bg = false) { type = ColorType::SGR24; background = bg; R = red; G = green; B = blue; } void set(unsigned char attr, bool bg = false) { type = ColorType::Legacy; background = bg; legacy = bg ? attr >> 4 : attr; } void set(unsigned colorref, bool bg = false) { type = ColorType::RGB; background = bg; u32 = colorref; } void set(unsigned color, ColorType ct, bool bg = false) { type = ct; background = bg; switch (type) { case ColorType::Default: return; case ColorType::SGR4: if ((color >= 30 && color <= 37) || (color >= 90 && color <= 97)) { u8 = color; background = 0; } if ((color >= 40 && color <= 47) || (color >= 100 && color <= 107)) { u8 = color - 10; background = 1; } return; case ColorType::SGR8: u8 = color; return; case ColorType::SGR24: u32 &= 0xff000000; u32 |= color & 0xffffff; return; case ColorType::Legacy: u8 = background ? color >> 4 : color; return; } } unsigned asRGB(unsigned *table4, unsigned *table8) { switch (type) { case ColorType::Default: return getDefaultColor(); case ColorType::SGR4: return table4[u8 > 37 ? u8 - 90 : u8 - 30]; case ColorType::SGR8: return table8[u8]; case ColorType::SGR24: return u32 & 0xffffff; case ColorType::Legacy: return table4[legacy]; } } operator unsigned() { return asRGB(getDefaultColorTable(4), getDefaultColorTable(8)); } int emitSGR(FILE *target = stdout) { switch (type) { case ColorType::Default: return fprintf(target, "\x1b[%hhum", background ? 49 : 39); case ColorType::SGR4: return fprintf(target, "\x1b[%hhum", background ? u8 + 10 : u8); case ColorType::SGR8: return background ? fprintf(target, "\x1b[48;5;%hhum", u8) : fprintf(target, "\x1b[38;5;%hhum", u8); case ColorType::SGR24: return background ? fprintf(target, "\x1b[48;2;%hhu;%hhu;%hhum", R, G, B) : fprintf(target, "\x1b[38;2;%hhu;%hhu;%hhum", R, G, B); case ColorType::Legacy: return fprintf(target, "\x1b[%hhum", background ? (u8 < 8 ? u8 + 30 : u8 + 90) : (u8 < 8 ? u8 + 40 : u8 + 100)); } } }; int main() { for (int i = 0; i < 256; ++i) table[i] = i; Color color(128, 128, 128); printf("%x\n", color.asRGB(0, 0)); color.emitSGR(); printf("half-intensity\n"); Color normal; normal.emitSGR(); printf("full-intensity\n"); Color red(31, ColorType::SGR4); red.emitSGR(); printf("red text\n"); Color greenish(40, ColorType::SGR8); greenish.emitSGR(); printf("greenish text\n"); normal.emitSGR(); printf("normal again\n"); Color bluebg(0, 0, 80, true); bluebg.emitSGR(); red.emitSGR(); printf("nightmare\x1b[m\n"); printf("%x %x %x\n", (unsigned)red, (unsigned)greenish, (unsigned)bluebg); } ``` Basically: * The components are all placed into anonymous bitfield/unions so the data layout is more obvious. * That data layout is then rearranged to match `COLORREF` for easy interconversion (simple masking). * The type metadata enum has been extended to completely support all the various color regimes, which are supported as natively as possible for maximal performance and minimum potential for loss/bugs. * Type conversion and color look-up is done in one place instead of a dozen. * It could “easily” be extended to also convert RGB to legacy attribute-color, if there really is a need for that somewhere (???) It would be necessary to re-plumb `TerminalApi.cpp` and `TerminalDispatchGraphics.cpp` in order to record the source type accurately enough (right now, there is a complete reliance on constructor type overloading in order to determine the color regime used by the source, but that is inappropriate and dangerous when you start getting multiple types that are built on single octet data words, in addition to being confusing...better to make it explicit). Before I even start to shoe-horn this monster into the existing code (which will obviously take more than a day, not including validation), I'd be interested in some feedback. Comments? Suggestions? Flames?

claunia commented

2026-01-30 23:29:14 +00:00

@j4james commented on GitHub (Apr 21, 2020):

Before I even start to shoe-horn this monster into the existing code (which will obviously take more than a day, not including validation), I'd be interested in some feedback. Comments? Suggestions? Flames?

@Undercat Personally, I'd recommend leaving the TextColor class alone. Fixing this issue requires changes to a number of areas of the code base, including the renderer, the SGR dispatcher, and color table management (I'm working on some of these things at the moment). The TextColor class is the least of the problems, and rewriting that now is just going to cause a lot of pain without actually fixing anything. At most I think it might need an additional ColorType option.

@j4james commented on GitHub (Apr 21, 2020): > Before I even start to shoe-horn this monster into the existing code (which will obviously take more than a day, not including validation), I'd be interested in some feedback. Comments? Suggestions? Flames? @Undercat Personally, I'd recommend leaving the `TextColor` class alone. Fixing this issue requires changes to a number of areas of the code base, including the renderer, the SGR dispatcher, and color table management (I'm working on some of these things at the moment). The `TextColor` class is the least of the problems, and rewriting that now is just going to cause a lot of pain without actually fixing anything. At most I think it might need an additional `ColorType` option.

claunia commented

2026-01-30 23:29:15 +00:00

@zadjii-msft commented on GitHub (Apr 24, 2020):

Holy crap I had this whole big long message typed up and I never sent it to this thread D: That's my bad. Lemme think if I can try and recover some of my thoughts here

Woah, thanks for the contribution here. There's certainly a lot to look at, so I might be reading some of the details wrong here.

I'm still not sure how this is better than TextColor+"change _index to be a 8bit index, instead of just a 4bit one". I'm not sure there's a lot of value in tracking 4bit VT indices separately from legacy ones - since legacy colors were always a 4bit index, we're trying to treat them as effectively the same thing as a VT 4bit index.

It could “easily” be extended to also convert RGB to legacy attribute-color, if there really is a need for that somewhere (???)

There unfortunately is - because the Win32 console API provides a mechanism for reading back the contents of the buffer, including text attributes, but only as legacy attributes, we need to be able to convert the non-legacy attrs back into legacy ones.

Type conversion and color look-up is done in one place instead of a dozen

I'm not sure that Color::asRGB is going to be called in any fewer places than TextColor::GetColor already is

That data layout is then rearranged to match COLORREF for easy interconversion (simple masking).

This is nice, and I’d be interested to see if it could be integrated with TextColor rather than replace it entirely

Sorry again to ghost this thread accidentally 😬

@zadjii-msft commented on GitHub (Apr 24, 2020): Holy crap I had this whole big long message typed up and I never sent it to this thread D: That's my bad. Lemme think if I can try and recover some of my thoughts here Woah, thanks for the contribution here. There's certainly a lot to look at, so I might be reading some of the details wrong here. I'm still not sure how this is better than TextColor+"change `_index` to be a 8bit index, instead of just a 4bit one". I'm not sure there's a lot of value in tracking 4bit VT indices separately from legacy ones - since legacy colors were always a 4bit index, we're trying to treat them as effectively the same thing as a VT 4bit index. > It could “easily” be extended to also convert RGB to legacy attribute-color, if there really is a need for that somewhere (???) There unfortunately is - because the Win32 console API provides a mechanism for reading back the contents of the buffer, including text attributes, _but only as legacy attributes_, we need to be able to convert the non-legacy attrs back into legacy ones. > Type conversion and color look-up is done in one place instead of a dozen I'm not sure that `Color::asRGB` is going to be called in any fewer places than `TextColor::GetColor` already is > That data layout is then rearranged to match `COLORREF` for easy interconversion (simple masking). This is nice, and I’d be interested to see if it could be integrated with `TextColor` rather than replace it entirely Sorry again to ghost this thread accidentally 😬

claunia commented

2026-01-30 23:29:15 +00:00

@Undercat commented on GitHub (Apr 25, 2020):

I changed the "class" name to Color only to make debugging easier. Changing the name would cause all consumers of color data to fail catastrophically when TextColor gets deleted, insuring that all downstream dependencies would be found and updated to conform to the new, extended, API.

This would prevent subtle bugs from being masked by incomplete functional similarities between the old and new color representations, but in the abstract, it really doesn't matter what you call the class/structure—the feature extension that I consider to be sine qua non is adding explicit support for distinguishing 4- and 8-bit SGR labelings from legacy 4-bit color and truecolor.

I distinguish legacy 4-bit color from 4-bit SGR color to avoid very subtle, and fortunately also very obscure, color bugs.

Different screen writers can have different expectations about what color mapping is used for 4-bit color, and TextColor is unable to satisfy both those expectations at the same time. It cannot distinguish between text that is written using the Windows API and text that should be passed-through using SGR. Most of the time, this will not cause a problem, and for those situations where it is a problem, there frankly is no good fix. Converting the Windows 4-bit legacy color values to 24-bit color values would eliminate any color infidelities caused by mismatched color tables, but it assumes that the downstream renderer is 24-bit capable. Still, I'd rather have the option of making the distinction than not having that option...and later needing it. Just my bias.

Of course, this does not seem to be an issue for 8-bit SGR because, as near as I can tell, the Windows console has never supported 8-bit native color. Besides, the upper part of the VGA color table is almost never changed from its canonical values, so it's likely to have the same values everywhere.

Now, as far as color tables are concerned: the new structure does not care how color mappings are represented—such mappings are an altogether different branch of code I haven't dealt with here at all, except as a dependency. The only thing Color attempts to do is encapsulate the color transformations in one place, but you could just as easily generate SGR externally from 4/8/24-bit values and an index key.

Leaving TextColor largely intact would be difficult, but then, in all fairness, shoe-horning a new representation in to replace it would be non-trivial, too.

To minimize changes to TextColor, you'd basically have to modify the input state machine to explicitly record 4- and 8-bit SGR codes; extend TextColor to support those codes in some way, at least by eliminating TextColor's existing dependence on constructor overloading to implicitly differentiate between 4-bit and 24-bit color values (which presently is the only way of initializing color data); then extend the existing color mapping system to provide support for 8-bit color indices, in addition to the 4-bit indices; and finally, modify the SGR output routines (in VtEngine) to explicitly reconstruct 4-bit and 8-bit SGR codes, when appropriate, for VTs.

And, of course, you can then also change whatever readback function @zadjii-msft referred to, above, to reconstruct values from the new color representations (4-bit and 8-bit SGR values). The goal is essentially to avoid using truecolor as a "universal" intermediate for extended colors...a use which presently makes it impossible to regenerate SGR codes with fidelity, and is what got me started on this whole thing in the first place.

I really would try hard to encapsulate all these color representations to avoid leaking different color encodings all over the place, even if it requires separating the SGR parser/generator for colors values from the rest of the SGR parser/renderer. That's just my personal paranoia, though.

Somehow, I managed to spent over an hour looking again for the exact code that parses the CSIs out of the input stream...to no avail. Should have bookmarked it, I suppose, but I didn't think this was going anywhere, since I got no feedback for weeks.... Anyway, I really don't have the time to spend on this right now—especially given the high potential for long and painful iterations raised by the highly-non-local nature of the changes required, and the fact that @j4james is already modifying some of the code involved. The fact that I don't actually have a personal use-case for any of the code involved doesn't help.

Still, I did just outline (in very broad strokes) some of the changes that I feel would be required to support SGR color with any fidelity. Some of you Microsoft natives can probably agree amongst yourselves on how to make the required changes in the course of your other work that already touches upon the relevant code. That would probably be 10x easier than having me attempt to make such changes in relative isolation.

Sorry I couldn't be of more help.

Cheers!

@Undercat commented on GitHub (Apr 25, 2020): I changed the "class" name to `Color` only to make debugging easier. Changing the name would cause all consumers of color data to *fail* catastrophically when `TextColor` gets deleted, insuring that all downstream dependencies would be found and updated to conform to the new, extended, API. This would prevent subtle bugs from being masked by incomplete functional similarities between the old and new color representations, but in the abstract, it really doesn't matter what you call the class/structure—the feature extension that I consider to be *sine qua non* is adding explicit support for distinguishing 4- and 8-bit SGR labelings from legacy 4-bit color and truecolor. I distinguish legacy 4-bit color from 4-bit SGR color to avoid very subtle, and fortunately also very obscure, color bugs. Different screen writers can have different expectations about what color mapping is used for 4-bit color, and `TextColor` is unable to satisfy both those expectations at the same time. It cannot distinguish between text that is written using the Windows API and text that should be passed-through using SGR. Most of the time, this will not cause a problem, and for those situations where it *is* a problem, there frankly is no good fix. Converting the Windows 4-bit legacy color values to 24-bit color values would eliminate any color infidelities caused by mismatched color tables, but it assumes that the downstream renderer is 24-bit capable. Still, I'd rather have the option of making the distinction than not having that option...and later needing it. Just my bias. Of course, this does not seem to be an issue for 8-bit SGR because, as near as I can tell, the Windows console has never supported 8-bit native color. Besides, the upper part of the VGA color table is almost never changed from its canonical values, so it's likely to have the same values everywhere. Now, as far as color tables are concerned: the new structure does not care how color mappings are represented—such mappings are an altogether different branch of code I haven't dealt with here at all, except as a dependency. The only thing `Color` attempts to do is *encapsulate* the color transformations in one place, but you could just as easily generate SGR externally from 4/8/24-bit values and an index key. Leaving `TextColor` largely intact would be difficult, but then, in all fairness, shoe-horning a new representation in to replace it would be non-trivial, too. To minimize changes to `TextColor`, you'd basically have to modify the input state machine to explicitly record 4- and 8-bit SGR codes; extend `TextColor` to support those codes in some way, at least by eliminating `TextColor`'s existing dependence on constructor overloading to implicitly differentiate between 4-bit and 24-bit color values (which presently is the only way of initializing color data); then extend the existing color mapping system to provide support for 8-bit color indices, in addition to the 4-bit indices; and finally, modify the SGR output routines (in `VtEngine`) to explicitly reconstruct 4-bit and 8-bit SGR codes, when appropriate, for VTs. And, of course, you can then also change whatever readback function @zadjii-msft referred to, above, to reconstruct values from the new color representations (4-bit and 8-bit SGR values). The goal is essentially to avoid using truecolor as a "universal" intermediate for extended colors...a use which presently makes it impossible to regenerate SGR codes with fidelity, and is what got me started on this whole thing in the first place. I really would try hard to encapsulate all these color representations to avoid leaking different color encodings all over the place, even if it requires separating the SGR parser/generator for colors values from the rest of the SGR parser/renderer. That's just my personal paranoia, though. Somehow, I managed to spent over an hour looking *again* for the exact code that parses the CSIs out of the input stream...to no avail. Should have bookmarked it, I suppose, but I didn't think this was going anywhere, since I got no feedback for weeks.... Anyway, I really don't have the time to spend on this right now—especially given the high potential for long and painful iterations raised by the highly-non-local nature of the changes required, and the fact that @j4james is *already* modifying some of the code involved. The fact that I don't actually have a personal use-case for any of the code involved doesn't help. Still, I did just outline (in very broad strokes) some of the changes that I feel would be required to support SGR color with any fidelity. Some of you Microsoft natives can probably agree amongst yourselves on how to make the required changes in the course of your other work that already touches upon the relevant code. That would probably be 10x easier than having me attempt to make such changes in relative isolation. Sorry I couldn't be of more help. Cheers!

claunia commented

2026-01-30 23:29:15 +00:00

@DHowett-MSFT commented on GitHub (Apr 25, 2020):

@undercat (sorry again for the delay, and we absolutely appreciate your input on this -- it's one of the ways we're hoping to be "better together" 😄)

@DHowett-MSFT commented on GitHub (Apr 25, 2020): @undercat (sorry again for the delay, and we absolutely appreciate your input on this -- it's one of the ways we're hoping to be "better together" :smile:)

claunia commented

@j4james commented on GitHub (Apr 25, 2020):

Just to clarify my position, I'm working on some areas of the code that relate to this issue (like the SGR dispatcher), and as part of that I've been experimenting with a partial fix for this issue as well. But that's mostly been to validate the changes I'm making to the API. Fixing this issue is not the focus of my work at the moment, so I don't want to discourage anyone else from taking it on in the meantime (although I think that'll be tricky without other fixes being in place first).

The main point of my previous comment, though, was that I didn't think a TextColor rewrite was necessary, and would entail a lot of work with no real benefit. I don't think there's any value in trying to preserve the exact format of every SGR sequence in the screen buffer, because realistically that's never going to be a perfect representation. And in the long term, clients that need that level of fidelity will be using something like the pass-through mode anyway.

So if we stick to what is actually needed to generate the correct output, rather than trying to reproduce the original input, I think there are only four color types we need to worry about.

Default colors - SGR 0, 39, and 49.
Indexed colors that can be brightened by SGR 1 - SGR 30 to 37 and 40 to 47.
Indexed colors that shouldn't be brightened - SGR 38:5, 48:5, 90 to 97, and 100 to 107.
Direct colors - SGR 38:2, 48:2, and at some point potentially the CMY(K) variants as well.

I would probably expect colors set via the console APIs (e.g. SetConsoleTextAttribute) to map to case 3. They're indexed colors, which should be affected by the color table, but they probably shouldn't be brightened by SGR 1.

So as I see it, the only thing missing from the existing TextColor implementation is the ability to distinguish been cases 2 and 3. And for that, all we really need is one more ColorType, and maybe something on the index constructor designating which index variant is being set.

@j4james commented on GitHub (Apr 25, 2020): Just to clarify my position, I'm working on some areas of the code that relate to this issue (like the SGR dispatcher), and as part of that I've been experimenting with a partial fix for this issue as well. But that's mostly been to validate the changes I'm making to the API. Fixing this issue is not the focus of my work at the moment, so I don't want to discourage anyone else from taking it on in the meantime (although I think that'll be tricky without other fixes being in place first). The main point of my previous comment, though, was that I didn't think a `TextColor` rewrite was necessary, and would entail a lot of work with no real benefit. I don't think there's any value in trying to preserve the exact format of every SGR sequence in the screen buffer, because realistically that's never going to be a perfect representation. And in the long term, clients that need that level of fidelity will be using something like the pass-through mode anyway. So if we stick to what is actually needed to generate the correct output, rather than trying to reproduce the original input, I think there are only four color types we need to worry about. 1. Default colors - SGR 0, 39, and 49. 2. Indexed colors that _can_ be brightened by SGR 1 - SGR 30 to 37 and 40 to 47. 3. Indexed colors that _shouldn't_ be brightened - SGR 38:5, 48:5, 90 to 97, and 100 to 107. 4. Direct colors - SGR 38:2, 48:2, and at some point potentially the CMY(K) variants as well. I would probably expect colors set via the console APIs (e.g. `SetConsoleTextAttribute`) to map to case 3. They're indexed colors, which should be affected by the color table, but they probably shouldn't be brightened by SGR 1. So as I see it, the only thing missing from the existing `TextColor` implementation is the ability to distinguish been cases 2 and 3. And for that, all we really need is one more `ColorType`, and maybe something on the index constructor designating which index variant is being set.

claunia commented

@Undercat commented on GitHub (Apr 25, 2020):

You absolutely need to store information about the original SGR labeling that led to a color being in the buffer. I cannot emphasize this enough: if you do not know the labeling that produced a color value in the buffer, you cannot guarantee its correct reproduction.

@j4james you need to remember that the terminal "wrapper" may have a different view of the indexed colors than any downstream VT used to actually render the output. They each can have different color tables. When they do, if you continue to generate SGR codes based on the actual color value in your buffer, without knowing what SGR labeling produced it, you have no way of distinguishing between labelings that map to the same color in terminal, but different colors on the VT.

You wind up with a non-invertible many-to-one mapping in terminal that forces you to arbitrarily choose an SGR code to output. Right now, it chooses the shortest possible SGR code. That produces the visual error in mintty/mintty#981 and is why I ran down this rabbit hole to begin with.

You cannot fix this problem without extending TextColor to distinguish the original SGR code that produced a color in the buffer. It's not as simple as differentiating colors based on how other attributes, like brightening, might apply to them.

@Undercat commented on GitHub (Apr 25, 2020): You absolutely need to store information about the original SGR labeling that led to a color being in the buffer. I cannot emphasize this enough: if you do not know the labeling that produced a color value in the buffer, you cannot guarantee its correct reproduction. @j4james you need to remember that the terminal "wrapper" may have a different view of the indexed colors than any downstream VT used to actually render the output. *They each can have different color tables.* When they do, if you continue to generate SGR codes based on the actual color value in your buffer, without knowing what SGR labeling produced it, you have no way of distinguishing between labelings that map to the same color *in terminal,* but *different* colors on the VT. You wind up with a non-invertible many-to-one mapping in terminal that forces you to arbitrarily choose an SGR code to output. Right now, it chooses the shortest possible SGR code. That produces the visual error in mintty/mintty#981 and is why I ran down this rabbit hole to begin with. You cannot fix this problem without extending `TextColor` to distinguish the original SGR code that produced a color in the buffer. It's not as simple as differentiating colors based on how other attributes, like brightening, might apply to them.

claunia commented

@j4james commented on GitHub (Apr 25, 2020):

if you continue to generate SGR codes based on the actual color value in your buffer, without knowing what SGR labeling produced it, you have no way of distinguishing between labelings that map to the same color in terminal, but different colors on the VT.

We definitely won't be generating the SGR codes based on the RGB value if that's what you mean. As I said above, the are four different color types I think we need to retain, and we would generate VT sequences that best represent those types. I just don't think we need to differentiate between something like SGR 95 and SGR 38:5:13 if they're going to produce the exact same color on every terminal. Mintty certainly doesn't seem to interpret them differently.

It would help if you could provide an example of a specific color sequence that you think we'd be getting wrong without the additional types you're proposing, and also what terminal you'd expect it to fail on.

@j4james commented on GitHub (Apr 25, 2020): > if you continue to generate SGR codes based on the actual color value in your buffer, without knowing what SGR labeling produced it, you have no way of distinguishing between labelings that map to the same color in terminal, but different colors on the VT. We definitely won't be generating the SGR codes based on the RGB value if that's what you mean. As I said above, the are four different color types I think we need to retain, and we would generate VT sequences that best represent those types. I just don't think we need to differentiate between something like SGR 95 and SGR 38:5:13 if they're going to produce the exact same color on every terminal. Mintty certainly doesn't seem to interpret them differently. It would help if you could provide an example of a specific color sequence that you think we'd be getting wrong without the additional types you're proposing, and also what terminal you'd expect it to fail on.

claunia commented

@egmontkob commented on GitHub (Apr 25, 2020):

I just don't think we need to differentiate between something like SGR 95 and SGR 38:5:13 if they're going to produce the exact same color on every terminal.

Are you sure that you won't ever need it, and that they look exactly the same on every terminal?

For the second eight colors (SGR 90 .. 97 vs. 38:5:8 .. 38:5:15, and the same with hundreds and forties for background) this might easily be correct, probably it is correct, although I'm not 100% sure. And no one can guarantee that xterm or some other popular terminal will never introduce some difference, e.g. SGR 2 faint getting applied to the first format only.

For the first eight colors (SGR 30 .. 37 vs. 38:5:0 .. 38:5:7, and the same with forties for the background) you know it's not true, we've just recently talked about it in #5384. Unless you want to hardcode the xterm 331 .. 347 behavior forever.

Which means you can perhaps squash the two representation of the second 8 colors, but not the first 8. Where does it take you? A more complex, harder to understand and harder to maintain codebase, with some squeezing logic where it's not required at all. Different codepath for the two possible representations of the first 8 colors versus the two possible representations of the second 8 colors, not just where they need to differ, but elsewhere too. Harder to debug, as you'll run into unexpected inconsistencies wherever you peek into the data. Saving 8 possible values out of the little-bit-more-than-2^24 values, still resulting in little-bit-more-than-2^24, that is, not solving anything here. And in some presumably very rare cases, where the two representations of the same color reside next to each other, you might save a few bytes getting transferred. I find it absolutely pointless.

The experience of having already fixed quite a few color handling bugs in VTE tells me that @Undercat is on the right track here. Treat 95 and 38:5:13 differently all the way through, it's pointless to "optimize" solely for the sake of 8 colors, based on the current knowledge / future assumption that they'll end up being the same RGB. Such "optimization" is counterproductive, you lose a lot by having more complex architecture and more complex code, while practically win nothing.

IMO squeezing them only made sense if the two different notations of the 16 colors would be synonyms, and would be guaranteed that they'll always remain so (as in, let's say, it's okay to forget whether it was 38:5:13 with colons, or 38;5;13 with semicolons). This is clearly not the case here.

@egmontkob commented on GitHub (Apr 25, 2020): > I just don't think we need to differentiate between something like SGR 95 and SGR 38:5:13 if they're going to produce the exact same color on every terminal. Are you sure that you won't ever need it, and that they look exactly the same on every terminal? For the second eight colors (SGR 90 .. 97 vs. 38:5:8 .. 38:5:15, and the same with hundreds and forties for background) this might easily be correct, probably it is correct, although I'm not 100% sure. And no one can guarantee that xterm or some other popular terminal will never introduce some difference, e.g. SGR 2 faint getting applied to the first format only. For the first eight colors (SGR 30 .. 37 vs. 38:5:0 .. 38:5:7, and the same with forties for the background) you know it's _not_ true, we've just recently talked about it in #5384. Unless you want to hardcode the xterm 331 .. 347 behavior forever. Which means you can _perhaps_ squash the two representation of the second 8 colors, but not the first 8. Where does it take you? A more complex, harder to understand and harder to maintain codebase, with some squeezing logic where it's not required at all. Different codepath for the two possible representations of the first 8 colors versus the two possible representations of the second 8 colors, not just where they need to differ, but elsewhere too. Harder to debug, as you'll run into unexpected inconsistencies wherever you peek into the data. Saving 8 possible values out of the little-bit-more-than-2^24 values, still resulting in little-bit-more-than-2^24, that is, not solving anything here. And in some presumably very rare cases, where the two representations of the same color reside next to each other, you might save a few bytes getting transferred. I find it absolutely pointless. The experience of having already fixed quite a few color handling bugs in VTE tells me that @Undercat is on the right track here. Treat 95 and 38:5:13 differently all the way through, it's pointless to "optimize" solely for the sake of 8 colors, based on the current knowledge / future assumption that they'll end up being the same RGB. Such "optimization" is counterproductive, you lose a lot by having more complex architecture and more complex code, while practically win nothing. IMO squeezing them only made sense if the two different notations of the 16 colors would be synonyms, and would be guaranteed that they'll always remain so (as in, let's say, it's okay to forget whether it was `38:5:13` with colons, or `38;5;13` with semicolons). This is clearly not the case here.

claunia commented

@j4james commented on GitHub (Apr 25, 2020):

Are you sure that you won't ever need it, and that they look exactly the same on every terminal?

Sure enough that I'm not going to encourage a whole lot of unnecessary changes to the code base on the off chance that it might be an issue 10 years from now on some obscure terminal that nobody has heard of. However, if you can come up with a genuine use case that is going to impact users in a meaningful way, please do let me know.

For the first eight colors (SGR 30 .. 37 vs. 38:5:0 .. 38:5:7, and the same with forties for the background) you know it's not true, we've just recently talked about it in #5384.

Which is why I'm not suggesting we treat SGR 30 the same as 38:5:0. See my message above. I proposed two index types to distinguish between these two cases. SGR 30 is case 2 (indexed colors that can be brightened), and SGR 38:5:0 would be case 3 (indexed colors that shouldn't be brightened).

Anyway, I can't stress enough how little any of this matters. The difficult part of getting this issue fixed is in places like the dispatcher and the renderers, and then figuring out all the little issues that are going to break once the existing behaviour changes. Adding more color types in the future, if we really need to, is trivial in comparison.

@j4james commented on GitHub (Apr 25, 2020): > Are you sure that you won't ever need it, and that they look exactly the same on every terminal? Sure enough that I'm not going to encourage a whole lot of unnecessary changes to the code base on the off chance that it might be an issue 10 years from now on some obscure terminal that nobody has heard of. However, if you can come up with a genuine use case that is going to impact users in a meaningful way, please do let me know. > For the first eight colors (SGR 30 .. 37 vs. 38:5:0 .. 38:5:7, and the same with forties for the background) you know it's not true, we've just recently talked about it in #5384. Which is why I'm _not_ suggesting we treat SGR 30 the same as 38:5:0. See my message above. I proposed two index types to distinguish between these two cases. SGR 30 is case 2 (indexed colors that can be brightened), and SGR 38:5:0 would be case 3 (indexed colors that shouldn't be brightened). Anyway, I can't stress enough how little any of this matters. The difficult part of getting this issue fixed is in places like the dispatcher and the renderers, and then figuring out all the little issues that are going to break once the existing behaviour changes. Adding more color types in the future, if we really need to, is trivial in comparison.

claunia commented

@DHowett-MSFT commented on GitHub (Apr 25, 2020):

Right- regardless of which particular representation we use and its fidelity, the first step towards closing #2661 is us having a framework for piping attributes around instead of just RGB color values. Once we nail that, we can work on improving the fidelity wherever it is practical.

@DHowett-MSFT commented on GitHub (Apr 25, 2020): Right- regardless of which particular representation we use and its fidelity, the first step towards closing #2661 is us having a framework for piping _attributes_ around instead of just _RGB color values_. Once we nail that, we can work on improving the fidelity wherever it is practical.

claunia commented

@egmontkob commented on GitHub (Apr 25, 2020):

@j4james I admit I slipped through some of the details in your earlier posts, apologies for that. Indeed if you're certain that the status quo won't change, the solution you're proposing is technically correct.

I'm still not a great fan of it architecturally, since some of the decisions (within case 3 mapping 90..97 into the same slots as 38:5:8..15) happen before the colors are stored wherever they are stored (and you lose info that might be useful for debugging etc.) and some other decisions (e.g. whether to actually brighten case 2) when it comes to hitting the screen pixels; that is, the whole logic is split into two vastly different places – whereas all of these could be done at this latter step, which I'd find a much cleaner, simpler architecture.

Anyway, as you say, it indeed really doesn't matter.

@egmontkob commented on GitHub (Apr 25, 2020): @j4james I admit I slipped through some of the details in your earlier posts, apologies for that. Indeed if you're certain that the status quo won't change, the solution you're proposing is _technically_ correct. I'm still not a great fan of it _architecturally_, since _some_ of the decisions (within case 3 mapping 90..97 into the same slots as 38:5:8..15) happen before the colors are stored wherever they are stored (and you lose info that might be useful for debugging etc.) and _some_ other decisions (e.g. whether to actually brighten case 2) when it comes to hitting the screen pixels; that is, the whole logic is split into two vastly different places – whereas all of these could be done at this latter step, which I'd find a much cleaner, simpler architecture. Anyway, as you say, it indeed really doesn't matter.

claunia commented

@Undercat commented on GitHub (Apr 25, 2020):

I just don't think we need to differentiate between something like SGR 95 and SGR 38:5:13 if they're going to produce the exact same color on every terminal. Mintty certainly doesn't seem to interpret them differently. [emphasis mine]

They absolutely are not guaranteed to be the same color intensity values on every terminal. They will only be the same color if terminal's internal color table for the primary colors + intensity (i.e., the first 16 colors in 4-bit and 8-bit color) exactly matches the corresponding color table of the VT renderer, whatever it may be.

You need to tag the source of the color so you can follow it all the way through terminal—at least given the way you convert all colors (and every other text property, for that matter) to a non-portable "common ground" internal format right now, and then render everything from that internal format. If you do not tag the origin of the colors, you will be unable to distinguish between \e[90m and \e[38;5;8m and \e[38;5;243m and \e[38;2;118;118;118m because they will all have the same color value in the default Microsoft mapping.

If you distinguish only based on whether a color can be brightened, you separate the first two cases. You do nothing to distinguish them from the last two cases, which should be absolute colors, but in your scheme will wind up being rendered as Microsoft default colors.

Classifying the colors based on the ancillary properties that might apply to them is insufficient. You need enough information to distinguish between the four alternatives I just listed, and the only approach that guarantees a given SGR input stream will produce the same SGR output stream is to tag the colors by their origin.

Now, you could get around this problem by only generating 24-bit SGR colors, but this assumes that the VT renders truecolor (likely) and it also eliminates emitting primary SGR codes (30/90 series) entirely. If you restrict the 24-bit up-conversion to VT SGR renderings only, then all will be well from a color fidelity standpoint...assuming the downstream renderer can handle 24-bit SGR codes. But you will never be able to generate a primary SGR color this way, and since those are the colors that everyone remaps, you will effectively be forcing all renderers to use terminal's colormap.

I already had that discussion with @zadjii-msft a long time ago, but I don't think the implications are being appreciated here.

@Undercat commented on GitHub (Apr 25, 2020): > I just don't think we need to differentiate between something like SGR 95 and SGR 38:5:13 if they're going to produce the exact same color on every terminal. Mintty certainly doesn't seem to interpret them differently. [emphasis mine] They absolutely are *not* guaranteed to be the same color intensity values on every terminal. They will only be the same color if terminal's internal color table for the primary colors + intensity (i.e., the first 16 colors in 4-bit and 8-bit color) *exactly matches* the corresponding color table of the VT renderer, whatever it may be. You need to tag the source of the color so you can follow it all the way through terminal—at least given the way you convert all colors (and every other text property, for that matter) to a non-portable "common ground" internal format right now, and then render everything from that internal format. If you do not tag the origin of the colors, you will be unable to distinguish between `\e[90m` and `\e[38;5;8m` and `\e[38;5;243m` and `\e[38;2;118;118;118m` *because they will all have the same color value in the default Microsoft mapping.* If you distinguish only based on whether a color can be brightened, you separate the first two cases. You do nothing to distinguish them from the last two cases, which should be absolute colors, but in your scheme will wind up being rendered *as Microsoft default colors.* Classifying the colors based on the ancillary properties that might apply to them is insufficient. You need enough information to distinguish between the four alternatives I just listed, and the only approach that guarantees a given SGR input stream will produce the same SGR output stream is to tag the colors *by their origin.* Now, you *could* get around this problem by only generating 24-bit SGR colors, but this assumes that the VT renders truecolor (likely) and it also eliminates emitting primary SGR codes (30/90 series) entirely. If you restrict the 24-bit up-conversion to VT SGR renderings *only*, then all will be well from a color fidelity standpoint...assuming the downstream renderer can handle 24-bit SGR codes. But you will *never* be able to generate a primary SGR color this way, and since those are the colors that everyone remaps, you will effectively be *forcing* all renderers to use terminal's colormap. I already had that discussion with @zadjii-msft a long time ago, but I don't think the implications are being appreciated here.

claunia commented

@DHowett-MSFT commented on GitHub (Apr 25, 2020):

You need to tag the source of the color so you can follow it all the way through terminal—at least given the way you convert all colors (and every other text property, for that matter) to a non-portable "common ground" internal format right now, and then render everything from that internal format. If you do not tag the origin of the colors, you will be unable to distinguish between \e[90m and \e[38;5;8m and \e[38;5;243m and \e[38;2;118;118;118m because they will all have the same color value in the default Microsoft mapping.

@Undercat given that this entire workitem was filed to capture "stop converting all colors into their 24-bit RGB representations and therefore losing fidelity", I think that most of your concerns are already noted.

What we have, today:

INPUT: All colors are converted into 24-bit color representations
OUTPUT: All 24-bit color representations are palettized into 0-16 or sent as 24-bit

This is incorrect. This is what this issue seeks to solve.

What we have, with this issue closed:

INPUT: All colors are stored as palette-index or 24-bit color with an enumerator indicating what they are.
OUTPUT: All palette-index or 24-bit color representations are passed through in a format that most closely represents what they were on input.

What we can have in the future, with the architecture this issue lays down:

INPUT: All colors are stored as palette-index (16 normal), palette-index (8 bright), palette-index (256 xterm), or 24-bit color
OUTPUT: All colors are emitted as close to INPUT as possible.

All that to say:

When this issue is closed, there will be no dependency on the console host's internal color table and no assumption that a connected terminal will share the same color table.

@DHowett-MSFT commented on GitHub (Apr 25, 2020): > You need to tag the source of the color so you can follow it all the way through terminal—at least given the way you convert all colors (and every other text property, for that matter) to a non-portable "common ground" internal format right now, and then render everything from that internal format. If you do not tag the origin of the colors, you will be unable to distinguish between `\e[90m` and `\e[38;5;8m` and `\e[38;5;243m` and `\e[38;2;118;118;118m` *because they will all have the same color value in the default Microsoft mapping.* @Undercat given that this entire workitem was filed to capture "stop converting all colors into their 24-bit RGB representations and therefore losing fidelity", I think that most of your concerns are already noted. What we have, today: * INPUT: All colors are converted into 24-bit color representations * OUTPUT: All 24-bit color representations are palettized into 0-16 or sent as 24-bit This is incorrect. This is what this issue seeks to solve. What we have, with this issue closed: * INPUT: All colors are stored as palette-index _or_ 24-bit color with an enumerator indicating what they are. * OUTPUT: All palette-index _or_ 24-bit color representations are passed through in a format that most closely represents what they were on input. What we can have in the future, _with the architecture this issue lays down_: * INPUT: All colors are stored as palette-index (16 normal), palette-index (8 bright), palette-index (256 xterm), or 24-bit color * OUTPUT: All colors are emitted as close to INPUT as possible. All that to say: **When this issue is closed,** there will be no dependency on the console host's internal color table and no assumption that a connected terminal will share the same color table.

claunia commented

@Undercat commented on GitHub (Apr 25, 2020):

OUTPUT: All colors are emitted as close to INPUT as possible.

I can't seem to get through, unfortunately. You have absolutely no way of knowing if a color you output is "close" to its "input value." You don't know what the input color is; you only know what SGR encoding the application asked for.

You convert that SGR code using your internal color tables. Those color tables may not match what is used by an external renderer.

Specifically, if the renderer thinks that \e[90m is rgb(85,85,85) and you put rgb(118,118,118) into the buffer because that's what you think \e[90m's color should be, according to your internal color table, you have already irretrievably lost information that will foreverafter prevent you from recovering and reproducing the VT's notion of what that color should be.

On output, you will generate an SGR value that corresponds "closely" to rgb(118, 118, 118) because "that's what's in the buffer," but its not what the SGR color you received was calling for.

If you do not make the architectural change, you will either do it later or you will ever-after force users to choose their primary palettes to be "close enough" to the Microsoft palette not to introduce visual artifacts.

@Undercat commented on GitHub (Apr 25, 2020): > OUTPUT: All colors are emitted as close to INPUT as possible. I can't seem to get through, unfortunately. You have absolutely no way of knowing if a color you output is "close" to its "input value." You don't know what the input *color* is; you only know what SGR encoding the application asked for. You *convert* that SGR code using your *internal* color tables. Those color tables may *not* match what is used by an external renderer. Specifically, if the renderer thinks that `\e[90m` is `rgb(85,85,85) ` and you put `rgb(118,118,118) ` into the buffer *because that's what you **think** `\e[90m`'s color should be,* according to your internal color table, you have already irretrievably lost information that will foreverafter prevent you from recovering and reproducing the **VT's** notion of what that color should be. On output, you will generate an SGR value that corresponds "closely" to `rgb(118, 118, 118) ` because "that's what's in the buffer," but its *not* what the SGR color you received was calling for. If you do not make the architectural change, you will either do it later or you will ever-after force users to choose their primary palettes to be "close enough" to the Microsoft palette not to introduce visual artifacts.

claunia commented

@DHowett-MSFT commented on GitHub (Apr 26, 2020):

I think we are both talking past eachother.

What I mean to say is “this issue, #2661, represents the work required to make sure we can forward the SGR values without using the color as an intermediary.”

When this issue is closed, there will be no dependency on the console host's internal color table and no assumption that a connected terminal will share the same color table.

I really do mean this. When 2661 is closed, there will be no dependency on the color values in the console host’s color table.

I’d like some feedback about how I can communicate that point more effectively 😄

@DHowett-MSFT commented on GitHub (Apr 26, 2020): I think we are both talking past eachother. What I mean to say is “this issue, #2661, represents the work required to make sure we can forward the SGR values without using the color as an intermediary.” > When this issue is closed, there will be no dependency on the console host's internal color table and no assumption that a connected terminal will share the same color table. I really do mean this. When 2661 is closed, _there will be no dependency on the color values in the console host’s color table._ I’d like some feedback about how I can communicate that point more effectively 😄

claunia commented

2026-01-30 23:29:19 +00:00

@Undercat commented on GitHub (Apr 26, 2020):

Oh, I see. The operative notion here is "when the issue is closed", not "when the modifications thus far suggested have been implemented."

My bad. It certainly isn't a pressing issue for me, or anything. I just wanted to make sure you understood that attaching non-SGR-specific attributes to the buffer wasn't going to fix the underlying problem.

Cheers!

@Undercat commented on GitHub (Apr 26, 2020): Oh, I see. The operative notion here is "when the issue is **closed**", not "when the modifications thus far suggested have been implemented." My bad. It certainly isn't a pressing issue for me, or anything. I just wanted to make sure you understood that attaching non-SGR-specific attributes to the buffer wasn't going to fix the underlying problem. Cheers!

claunia commented

@DHowett-MSFT commented on GitHub (Apr 26, 2020):

(The reason I say “as close to input as possible” is that \e[4m\e[4m would likely be “optimized” to, you know, not be repeated. Same with something like “4;24” or anything that overrides or contradicts itself or stuff like that.)

@DHowett-MSFT commented on GitHub (Apr 26, 2020): (The reason I say “as close to input as possible” is that `\e[4m\e[4m` would likely be “optimized” to, you know, not be repeated. Same with something like “4;24” or anything that overrides or contradicts itself or stuff like that.)

claunia commented

2026-01-30 23:29:19 +00:00

@Undercat commented on GitHub (Apr 26, 2020):

Gotcha. Visually-idempotent transformations: no problem. 😸

@Undercat commented on GitHub (Apr 26, 2020): Gotcha. Visually-idempotent transformations: no problem. 😸

claunia commented

2026-01-30 23:29:19 +00:00

@j4james commented on GitHub (Jun 13, 2020):

I've been working around the edges of this issue for a while now, but I've got to the point where I'd like to try and finish off my POC and turn it into a PR. Does anyone have any objections to me doing that? Technically this issue is still assigned to @zadjii-msft so I don't want step on any toes.

Also, any suggestions for testing the 16-color xterm and telnet engines? So far I've just been patching the conhost command line in winconpty.cpp to startup with a different mode, but is there a better way? In the case of telnet, it would be nice to test via the telnet client itself if that were possible.

@j4james commented on GitHub (Jun 13, 2020): I've been working around the edges of this issue for a while now, but I've got to the point where I'd like to try and finish off my POC and turn it into a PR. Does anyone have any objections to me doing that? Technically this issue is still assigned to @zadjii-msft so I don't want step on any toes. Also, any suggestions for testing the 16-color xterm and telnet engines? So far I've just been patching the conhost command line in winconpty.cpp to startup with a different mode, but is there a better way? In the case of telnet, it would be nice to test via the telnet client itself if that were possible.

claunia commented

@DHowett commented on GitHub (Jun 13, 2020):

@j4james you’re more than welcome to. I view my assignments as some hybrid between “would be the person on my team tasked with doing the work eventually” and “is the subject matter expert, and can answer community questions for the implementor.” In this case, I think you might be the current subject matter expert 😉 so conversely, is it okay if I assign you?

The “best” way to test those engines right now is, unfortunately, exactly what you’re doing. If you’d like to test with the telnet engine and the actual telnet client, it’ll be a bit difficult because the telnet server no longer ships with desktop Windows ... hm.

Our coverage story for these engines is very, very shaky. I don’t even know of a use of the non-24bit color xterm engine anywhere in Windows.

If you replace your system’s conhost.exe with OpenConsole (no warranty!) running Windows executables through WSL will launch ConPTY, but even that just uses the Xterm256Engine.

There might be something fun to be done with VTPipeTerm?

@DHowett commented on GitHub (Jun 13, 2020): @j4james you’re more than welcome to. I view my assignments as some hybrid between “would be the person on my team tasked with doing the work eventually” and “is the subject matter expert, and can answer community questions for the implementor.” In this case, I think you might be the current subject matter expert 😉 so conversely, is it okay if I assign you? The “best” way to test those engines right now is, unfortunately, exactly what you’re doing. If you’d like to test with the telnet engine and the actual telnet client, it’ll be a bit difficult because the telnet server no longer ships with desktop Windows ... hm. Our coverage story for these engines is very, very shaky. I don’t even know of a use of the non-24bit color xterm engine anywhere in Windows. If you replace your system’s `conhost.exe` with OpenConsole (no warranty!) running Windows executables through WSL will launch ConPTY, but even that just uses the Xterm256Engine. There might be something fun to be done with VTPipeTerm?

claunia commented

@j4james commented on GitHub (Jun 13, 2020):

In this case, I think you might be the current subject matter expert 😉 so conversely, is it okay if I assign you?

This part of the code was actually a little outside my comfort zone to start with, but I'm far enough along now that I'm reasonably confident I can get it done, so you're welcome to assign to me. With any luck I should have something ready this weekend. Although I should point out that we'll probably still need to address #5952 before we'll want to merge these fixes.

Our coverage story for these engines is very, very shaky. I don’t even know of a use of the non-24bit color xterm engine anywhere in Windows.

Yeah, I was wondering about that too.

If you replace your system’s conhost.exe with OpenConsole (no warranty!) running Windows executables through WSL will launch ConPTY, but even that just uses the Xterm256Engine.

OK. I might try that with a patched version of OpenConsole with one of the other engines hard coded.

There might be something fun to be done with VTPipeTerm?

I did briefly try that, but couldn't get it to work. It kept randomly dropping escape characters (or something to that effect) so half the VT sequences would get printed to the screen instead of being interpreted.

@j4james commented on GitHub (Jun 13, 2020): > In this case, I think you might be the current subject matter expert 😉 so conversely, is it okay if I assign you? This part of the code was actually a little outside my comfort zone to start with, but I'm far enough along now that I'm reasonably confident I can get it done, so you're welcome to assign to me. With any luck I should have something ready this weekend. Although I should point out that we'll probably still need to address #5952 before we'll want to merge these fixes. > Our coverage story for these engines is very, very shaky. I don’t even know of a use of the non-24bit color xterm engine anywhere in Windows. Yeah, I was wondering about that too. > If you replace your system’s `conhost.exe` with OpenConsole (no warranty!) running Windows executables through WSL will launch ConPTY, but even that just uses the Xterm256Engine. OK. I might try that with a patched version of OpenConsole with one of the other engines hard coded. > There might be something fun to be done with VTPipeTerm? I did briefly try that, but couldn't get it to work. It kept randomly dropping escape characters (or something to that effect) so half the VT sequences would get printed to the screen instead of being interpreted.

claunia commented

@zadjii-msft commented on GitHub (Jun 15, 2020):

(just for more info - the inbox telnetd that we ship internally actually uses xterm-ascii, not the WinTelnetEngine. After building the WinTelnetEngine, I discovered that the inbox telnet was capable enough of doing everything the xterm engine was doing, aside from 256 color and utf-8 character encoding. We probably should just delete it at some point.)

@zadjii-msft commented on GitHub (Jun 15, 2020): (just for more info - the inbox `telnetd` that we ship internally actually uses `xterm-ascii`, not the `WinTelnetEngine`. After building the WinTelnetEngine, I discovered that the inbox `telnet` was capable enough of doing everything the xterm engine was doing, aside from 256 color and utf-8 character encoding. We probably should just delete it at some point.)

claunia commented