Tetralet / LilyTerm

A lightweight, but functional terminal emulator
http://lilyterm.luna.com.tw/
GNU General Public License v3.0
199 stars 31 forks source link

Solarized dark color 10 is off. #50

Open Profpatsch opened 10 years ago

Profpatsch commented 10 years ago

It’s green but should be grey-ish.

Comparison lilyterm/urxvt:

screenshot

Tetralet commented 10 years ago

The Solarized theme in LilyTerm is a forked custom color theme. It is different with the original version of Solarized theme.

Profpatsch commented 10 years ago

Solarized consists of a set of fixed color values that are the same everywhere and should be displayed the same everywhere, defined at http://ethanschoonover.com/solarized

If you change one of them it’s not Solarized anymore, simple as that.

Tetralet commented 10 years ago

OK, I'll rename it. Sorry...

Profpatsch commented 10 years ago

No problem at all.

How about including the original Solarized as solarized and the current version as solarized-alternative?

Tetralet commented 10 years ago

OK. I'll fix it. Thanks!

Profpatsch commented 10 years ago

Hm, I don’t get that:

{5, "solarized",
      {{ 0, 0x1d1d, 0x1c1c, 0x1a1a },
       { 0, 0xb0b0, 0x2828, 0x2525 },
       { 0, 0x4f4f, 0x7a7a, 0x0000 },
       { 0, 0xa8a8, 0x6d6d, 0x0000 },
       { 0, 0x1e1e, 0x6f6f, 0xa8a8 },
       { 0, 0xa8a8, 0x2b2b, 0x6868 },
       { 0, 0x2121, 0x8080, 0x7979 },
       { 0, 0xbebe, 0xb9b9, 0xaaaa },
       { 0, 0x5050, 0x4d4d, 0x4747 },
       { 0, 0xe3e3, 0x3333, 0x3030 },
       { 0, 0x6f6f, 0xadad, 0x0000 },
       { 0, 0xdbdb, 0x8e8e, 0x0000 },
       { 0, 0x2727, 0x9090, 0xdbdb },
       { 0, 0xdbdb, 0x3838, 0x8686 },
       { 0, 0x2e2e, 0xb3b3, 0xa8a8 },
       { 0, 0xf1f1, 0xeaea, 0xd7d7 }}},

This is the original solarized color table:

$base03:    #002b36;
$base02:    #073642;
$base01:    #586e75;
$base00:    #657b83;
$base0:     #839496;
$base1:     #93a1a1;
$base2:     #eee8d5;
$base3:     #fdf6e3;
$yellow:    #b58900;
$orange:    #cb4b16;
$red:       #dc322f;
$magenta:   #d33682;
$violet:    #6c71c4;
$blue:      #268bd2;
$cyan:      #2aa198;
$green:     #859900;

Aren’t 4-value hex colors only crude approximations to the intended color scheme? Why do you use only 4 hex aka 2 bytes?

blueyed commented 10 years ago

@Tetralet I am interested in the answer to @Profpatsch question, too.

With a default lilyterm config file (moved my own away), and the solarized theme, it looks as follows for me: image

This is quite different from using the default lilyterm color theme and then the base16 script to setup solarized: image

Profpatsch commented 10 years ago

I guess it’s because of the two bit representation, but most of it is certainly an optical illusion, because the background color in the second picture is darker.

Oh, and for it to be true solarized you need to fix your background color to #002b36. That’s a dark green-blue hue.

blueyed commented 10 years ago

@Profpatsch Yes, the background is different (LilyTerms' solarized and default respectively).

It isn't just an optical illusion. Here's the base16-setup after using "Solarized" in LilyTerm:

- lilyterm_021

Thanks for the hint at the solarized bg color. This is certainly something else, which the "Solarized" profile in LilyTerm should provide, shouldn't it?

Using this bg color, I end up with black and grey being not really visible: - lilyterm_022 (LilyTerm's solarized theme + custom bg and the base16 setup for solarized) Is this expected?

Profpatsch commented 10 years ago

The Images here are the reference.

I can’t code C good enough to fix this bug myself and @Tetralet seems to be inactive atm.

blueyed commented 10 years ago

@Profpatsch The map you listed in https://github.com/Tetralet/LilyTerm/issues/50#issuecomment-27487462 has 2_3 bytes for each color definition (2 bytes for each of red, green, blue; see (GdkColor)[https://developer.gnome.org/gdk/unstable/gdk-Colormaps-and-Colors.html#GdkColor]), whereas the HTML notation only has 1_3 bytes (00-FF for red, green, blue).

LilyTerm's definition are rgba colors, where one byte is used for alpha transparency. This gets converted via convert_color_to_rgba (https://github.com/Tetralet/LilyTerm/blob/master/src/misc.c#L731), where the alpha value is shifted off. I.e. 0x1d1d becomes 0x1d.

The colors appear to be way off from the original values. When the light/dark themes where added initially, they matched (at least I can find matches there ;): https://github.com/Tetralet/LilyTerm/commit/d5d766c400e434c0da007b0b19bc4581ea398d4f

But then they were changed multiple times:

git log --oneline src/profile.c|grep solarized cf09a06 Better solarized ansi color theme. 225ecae Better solarized ansi color theme. e3383ff Better solarized ansi color themes. 53e0da6 Better solarized ansi color themes. d5d766c Add two new color themes: solarized dark and light. 3bd3c24 Add a new theme: "solarized". Thanks medicalwei for this idea.

Commit 225ecae also removed the light/dark variants, and only left a single one.

I will try to build LilyTerm now with the definitions from d5d766c, and see what that gives.

Profpatsch commented 10 years ago

Cool!

Now I see, it makes perfect sense. I hope you are successful.

blueyed commented 10 years ago

Ok, for one, the background of the light variant appears to be the same as with the dark one.

It looks as follows: -tmp-lilyterm_ -tmp-lilyterm - lilyterm_023

I am not sure, if the colors are correct otherwise, given that I do not know in which order they should appear. If you are interested and would like to investigate, I can push my changes to a branch.

Profpatsch commented 10 years ago

Yes, please do.

blueyed commented 10 years ago

@Profpatsch see above. The branch is at https://github.com/blueyed/LilyTerm/tree/solarized-themes.

blueyed commented 10 years ago

The solarized theme in LilyTerm has been fixed, this issue should be fixed.

Profpatsch commented 10 years ago

It would be nice if there were an official new version sometime soon. Atm one has to build it to have anything newer than 0.9.4.