Better colour approximation #202
Conversation
mina86
force-pushed
the
master
branch
2 times, most recently
from
db35958
to
d2bc9e0
Compare
Thank you for putting that much work into it. Are there any noticeable differences?
That would be great! |
|
The differences can be observed though they are subtle. To be fair, some are arguably worse (left is original, right is with change):
This could be avoided by not trying greyscale ramp for non-grey colours. Below is a full list of colours used in each theme. Lines with two columns indicate old and new methods give the same result, three columns show colour in the theme, approximation using old code and as last column approximation using new code: |
|
I’ve updated the code so greyscale ramp is no longer used for non-grey colours. |
Add measure of error introduced when mapping 24-bit colours into 8-bit ANSI palette. Having such measure makes it possible to benchmark any further changes meant to improve the approximation.
Improve the way 24-bit colours are mapped to 8-bit ANSI palette by observing that neither grayscale ramp nor 6×6×6 colour cube have linear staps throughout. For example, sequence of values in the cube is: 0, 95, 135, 175, 215 and 255. The first step is 95 colours while the rest are only 40 colours. Similarly, grayscale ramp (plus black and white) create a sequence 0, 8, 18, 28, …, 228, 238, 255. Here, the first step is 8, the next ones are 10 until final one which is 17. Assuming the mapping is linear causes wrong results. For example, colour #1C1C1C is at index 234 but the code incorrectly mapped it to index 233 (which is 0x121212). To further improve the approximation, the new code shifts values to use rounding during division rather than truncating them. This leads #070707 to be approximated by #080808 rather than #000000.
The 6×6×6 colour cube includes gray colours. Black and white are already used but the cube also includes other greys such as #5f5f5f or #d7d7d7. For those colours it’s better to use colour from the cube rather than from the grayscale ramp. Change rgb2ansi_grey to try approximation using coulours in either section in the pallette and choose the best one. This only affects 20 colours in total.
|
I have compared your version with (1) the truecolor version and (2) the old 8-bit approximation and I have to say that I definitely like your version better! One thing we should look at before integrating this is performance. The |
|
(obviously, this would be rather easy to optimize by calling |
|
I ran some simple benchmarks and I cannot measure any significant difference in performance (Benchmark |
The most obvious way to do it would be with a hash map but I wonder if dealing with hash map would actually be slower than recalculating the colour each time. Best if But either way, the new calculation method doesn’t add that many operations, so I’m not that surprised that performance effects are negligible. If I was maintaining |
👍
Exactly, that's why I think it would be great if this would be part of
I don't quite understand the reason behind this change. Wouldn't it be "optimal" to just always choose the best match in terms of perceived color difference? |
I suppose that depends on which terminal in my second screenshot does better job at highlighting syntax, since that’s bat’s ultimate goal. I think colours do it better than shades of grey.
I can certainly do that, but it will take me potentially a few weeks. |
I think this is a problem with the standard "Monokai Extended" color scheme which doesn't really look great for JSON files. I think the question should rather be which one resembles the actual color scheme better (in terms of perceived color difference) and the right screenshot is closer to the true color (which the color scheme defins) than the left. The biggest difference seems to be in the color for comments (original: d( So yours is clearly better. My only question was why you "updated the code so greyscale ramp is no longer used for non-grey colours" because I would always just choose the nearest neighbor with respect to d(·, ·) whether or not is is on the greyscale ramp. |
So old school. ;) There’s now But anyway, let’s then close this PR and I’ll prepare the code as a PR against |
Didn't know that, thanks! 😄
Sounds great. |
I’ll let you decide whether this is worth it. It almost feels like it should be separate crate or part of ansi_term.