Qix- / color-convert

Plain color conversion functions in JavaScript
MIT License
744 stars 96 forks source link

inaccurate ansi256 conversion #74

Closed spenserblack closed 3 years ago

spenserblack commented 4 years ago

Thanks for this awesome library!

It seems like converting #282629 returns 59 (#5f5f5f).

console.log(convert.hex.ansi256('#282629'));

However, I think 235 (#262626) is a better 256-color approximation of #282629.

I'm using 2.0.1 of color-convert.

spenserblack commented 4 years ago

Looks like this might be related.

console.log(convert.rgb.ansi256(40,38,41)); // 59
console.log(convert.ansi256.rgb(59)); // [51, 51, 51]
console.log(convert.ansi256.rgb(235)); // [38, 38, 38], which is much closer to 40, 38, 41
Qix- commented 4 years ago

That could be. When I ported over an rgb->ansi256 implementation originally I also found that grey handling was inadequate. I wrote my own grey matcher for it that matches greys better, but it uses a naive grey matching conditional (all channels are the same = grey) instead of using a saturation threshold.

I'm curious, does #262626 give you a better result when run through the rgb->ansi256 function? I would check myself but I'm not at a machine at the moment.

spenserblack commented 4 years ago

I'm curious, does #262626 give you a better result when run through the rgb->ansi256 function? I would check myself but I'm not at a machine at the moment.

Yes, that seems to work perfectly :smiley: Unfortunately, the PR I'm working on (mjswensen/themer#35) can't take hard-coded hex values (or else I would just hard-code the 256-colors :rofl:).

Perhaps you could check if only the most significant bits of the numbers are equal?

Example:

const r = 40 >> 4; // 2
const g = 38 >> 4; // 2
const b = 41 >> 4; // 2

console.log(r === g && r === b); // true

I could make a PR for this if that makes sense.