ansi256 => ansi16 squashes a bunch of grayscale values and basic colors to black

[jwalton]

This prints out all the grayscale values at the end of ansi256:

for (let i = 232; i < 256; i++) {
  console.log(colorConvert.ansi256.ansi16(i));
}

Which results in:

30
30
30
30
30
30
30
30
30
30
30
30
37
37
37
37
37
37
37
97
97
97
97
97

Note that the first half of the scale all gets crushed to 30 black, but I'd expect values from ~236-243 to be converted to 90 bright black, just like in the second half values are converted to white and bright white.

[jwalton]

Oh, also, this same function crushes the first 16 colors to black and bright black:

for (let i = 0; i < 15; i++) {
  console.log(colorConvert.ansi256.ansi16(i));
}

This should print out 0-15, since these colors are the same in 256 and 16 color ANSI, but instead it prints out 30 for everything.

[jwalton]

Going even deeper (and possibly more subjective, but maybe this is related)... here's the full color table generated by color-convert:

Aside from the issues already discussed above, I notice a lot of "30"s in there in the top half where maybe they shouldn't be; like 102 in ansi256 is rgb(102,102,102), which should probably be bright black instead of black - 102 is a long ways from 0. There are a few "grey" colors in the middle of ansi256, so I'd expect to see a few 37s and 90s scattered about, but actually there's no 90s in this table at all, and 37 only shows up once (for 145), which is suspicious.

[Qix-]

Yep, this is weird indeed. Not sure how I never noticed this before.

Semi-jokingly, in my defense I'm not entirely sure I wrote this code ;) This package has a long lineage. Don't blame me just yet! :D

https://github.com/Qix-/color-convert/blob/master/conversions.js#L518-L538

That's the offending code. Since there is no clear conversion from ansi256 to ansi16, we use RGB+HSV as intermediary models. The V channel is what's used for grayscale derivation.

The problem with the missing 90 code is that I'm erroneously biparting the value domain into 2 using round, instead of simply doing the division and checking if it's less than 0.5 (which would correctly split it into a domain of 4 - black, light black, white, light white).

I think I have a fix in mind. I'm going to try to fix this in a bit.

[Qix-]

Wow, this stuff was seriously broken.

Firstly, let me point out a much larger problem I'm facing: a VERY LARGE portion of websites I'm using to verify color codes (to make sure everyone agrees) uses this library. While they haven't been using it for ANSI codes in particular, it's just something for myself to keep in mind if I'm ever doing some "dumb"/"naive" checks against other converters to make sure values are reasonably sane at a quick-glance - there's a high enough chance that I'd be checking potentially fixed values against a buggy version of this very library.

Secondly, the ANSI256 to RGB code is straight-up broken. How it hasn't been caught before this is baffling (perhaps it has I was just a Bad Maintainer - that's very possible).

Third, my color grid code had a subtle bug with the escapes; I'm just writing it here for future reference. I was doing two blocks similar to yours and realized that the code to switch to the second color just prepended the reset code (0) to the next color's escape. However, since the next escape was yielding negative RGB values (which aren't valid in escapes), the code was being silently swallowed and no render mode changes were occurring. This led me to believe that the code was actually working when it, in fact, was not. The solution was to do a separate 0m between the two colors, since it's guaranteed to reset the mode properly.

Here's what it looked like before I realized this:

The thing that made me look hard at it was the fact that the hex codes made absolutely no sense.

---

The reason why the basic colors (< 16) are being completely ignored was not obvious at first.

In your screenshots, they appear black. This is because you have a black background color. The confusion started when they appeared grey to me (whereas 16 and 232 looked more black than my background). They matched my background.

In fact, they're not generating correct values at all.

color-convert/conversions.js

Line 606 in 3808cc7

args -= 16;

Between this line and the greyscale check should have been a check to see if the value is < 16, to do a 1:1 conversion between the legacy colors. Instead, it just blindly subtracts 16 and spits back out a set of negative RGB values. This is why this bug is spectacularly bad, lol.

I have some fixes coming in, so stay tuned.

[Qix-]

Hmm okay, having issues finding a good hueristic. Maybe a LUT really is the better option here.

Let me think about this overnight. For now, I've created the fix-ansi branch which fixes some of the more glaring issues. The eager snap to black/white still isn't fixed yet, so I'll have to think about how that's addressed next. Feel free to check out the changes made there.

[jwalton]

I'll point out that if you generate a LUT at runtime (similar to what I did in chalk/ansi-styles#68), then generating the LUT is quite fast - there's only 256 values in the array, so even if you did something expensive like a perceptual-color-distance, it will generate very fast. If you lazy-initialize the LUT the first time it's used, then you only incur that cost once. And, since the LUT is only an array of 256 numbers, it's only going to take up something like 2K of memory (you could make it even less by storing the contents of the LUT as characters instead of as numbers, but that's probably more complexity than is warranted).

[Qix-]

I'm leaning toward generating a LUT at build time at this point. I also noticed that no other terminal I can find uses the ANSI256 encoding, but instead uses the XTerm lookup table (that has, from what I can tell, slightly lighter/more saturated colors).

I'm inclined to keep the ansi encoding as part of the library but under a separate function.