Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support 24bit true color #356

Closed
aeosynth opened this issue Jul 3, 2016 · 16 comments
Closed

support 24bit true color #356

aeosynth opened this issue Jul 3, 2016 · 16 comments

Comments

@aeosynth
Copy link
Contributor

aeosynth commented Jul 3, 2016

vis currently only supports 256 colors

@pickfire
Copy link

pickfire commented Aug 6, 2016

Would be nice to bump @XVilka https://gist.github.com/XVilka/8346728

@martanne
Copy link
Owner

martanne commented Aug 6, 2016

As you noted in another comment this is a restriction of curses because the terminfo database doesn't currently handle 24bit color support.

For our use case we don't actually need a lot of different colors. We just want to specify them via RGB values. Interestingly curses provides the init_color(3) function to redefine an existing color to a RGB value. I don't know whether popular terminals support these run-time changes to their color palette.

If you are interested in this feature I suggest to:

  1. Implement a minimal curses application which tries to modify one of the standard colors
  2. Test it in various terminals to see whether it works
  3. If it works, hook it up to the color handling code in ui-curses.c

As for completely ditching curses, a minimal vt100 compatible ui backend might be of interest for some usage scenarios. The re-drawing code of vis is currently intentionally very primitive. The window content is often redrawn from scratch and it is assumed that it can efficiently be "blitted" to the display. For curses this works because it uses double buffering to minimize actual terminal output. I don't know how bad the effects would be for a raw vt100 backend.

@Screwtapello
Copy link
Contributor

I believe palette setting is fairly-widely supported, but frowned on because there's no good way to undo the changes after they're made, so you leave a mess for any later tools that run in the same terminal.

There's a lot of useful functionality that is widely implemented but (for whatever reason) is not advertised in the xterm-256color terminfo entry, like mouse support or setting the title-bar text. At some point Vis might have to do what everybody else seems to do, which is obey terminfo + use some hard-coded terminal sequences if $TERM starts with xterm.

@mawww
Copy link

mawww commented Dec 16, 2016

Stumbled on this issue, I figured I could add some info I learned:

  • Most terminal emulators do support changing palette, at least vte, rxvt-unicode, and xterm do. Tmux does not yet.
  • Some support OSC 104, which resets a color/the whole palette. notably vte and xterm.

I tend to think palette + changing colors is the best way for a text editor, its very unlikely there would be more than 256 colors on screen at the same time, and 24bit colors support is not free in a terminal emulator either (depending on how it stores it screen data, but seems the common method is an array of char + attribute, where 24bit colors adds a big memory overhead).

@martanne
Copy link
Owner

Hi Maxime, thanks for your insight, I agree. The "problem" is that my preferred terminal st(1) and dvtm do not currently support it. As a result I only have limited interest to use it in vis.

For reference I include my test program:

#include <curses.h>
#include <string.h>
#include <stdio.h>

int main(int argc, char *argv[]) {
	const char *color = argc > 1 ? argv[1] : "abcdef";

	short r, g, b;
	if (strlen(color) != 6 || sscanf(color, "%2hx%2hx%2hx", &r, &g, &b) != 3) {
		fprintf(stderr, "Invalid color, `RRGGBB' expected\n");
		return 1;
	}

	if (!initscr()) {
		fprintf(stderr, "Failed to initalize curses\n");
		return 1;
	}
	
	if (start_color() != OK)
		printw("Failed to start color mode\n");

	if (!can_change_color())
		printw("Dynamic color changes not supported by terminal\n");
	
	int color_index = COLORS-1;
	short oldr, oldg, oldb;
	if (color_content(color_index, &oldr, &oldg, &oldb) == OK) {
		printw("Old RGB value for color %d: #%02hx%02hx%02hx\n", color_index,
		       (short)(oldr*255/1000), (short)(oldg*255/1000), (short)(oldb*255/1000));
	} else {
		printw("Failed to read out value of color: %d\n", color_index);
	}

	if (init_color(color_index, r*1000/255, g*1000/255, b*1000/255) != OK)
		printw("Failed to initialize color\n");

	int colorpair_index = COLOR_PAIRS-2;
	if (init_pair(colorpair_index, color_index, 0) != OK)
		printw("Failed to initialize color pair\n");
	
	attrset(COLOR_PAIR(colorpair_index));
        printw("#%02hx%02hx%02hx\n", r, g, b);

	getch();

	if (can_change_color()) {
		/* Reset palette, or should we manually change back all affected colors?
		 * Move after endwin()? */
		puts("\033]104;\007");
		fflush(stdout);
	}

        endwin();
	return 0;
}

Usage:

cc color-change.c `pkg-config --cflags --libs ncurses` && ./a.out aabbcc

@XVilka
Copy link

XVilka commented Dec 16, 2016

@mawww well, speaking about memory consumption - it's largely exaggerated. It wont eat more than a couple of megabytes. Considering modern operating systems (and computers, including embedded ones), it's a very small number.

@pickfire
Copy link

@martanne st supports 24-bit a long time ago. That's why I use st, about dvtm, I think they are planning to support it but it isn't being done yet.

@XVilka
Copy link

XVilka commented Dec 16, 2016

@pickfire, @martanne is the author of dvtm

@vvug
Copy link

vvug commented Dec 20, 2016

@martanne Do you think it would be difficult to implement init_color() and similar functions in st? I could try to submit a patch for that, but I can't find a reference implementation from another terminal. Do you have any idea about where I could look?

@s-gilles
Copy link
Contributor

s-gilles commented Jan 2, 2017

I have implemented 24-bit color support via init_color() at https://github.com/s-gilles/vis/tree/24bit-via-init-color . I've tested it (with latest st) and it seems to work fine. A quick check is to use #501b2a as a background: it's a dark red that is interpreted as gray when converted to 256color. I've not submitted this as a pull request for two reasons.

  • It doesn't work with tmux (it degrades to current behavior) and there's no easy fix. Under TERM=tmux-256color, can_change_color() is false, since they've chosen[0] to support 24-bit color by directly specifying RGB values in escape codes instead of adjusting the palette (if I understand the discussion[1] correctly), and therefore have no desire to support palette adjustment.

    Vis could try to detect the unofficial Tc variable of terminfo, and then use RGB colors via escape codes, bypassing ncurses' color support. This would, I believe, work with just about everything in use today[2]. That, however, sounds like an extremely intrusive change and there's currently no way to do it portably. As far as I know, programs[3][4] which currently support this style of color setting do so by hardcoding the escape sequence, which the terminfo/ncurses folks have no intent of standardizing[5][6].

    That puts ncurses-based programs which want portable, arbitrary, 24-bit color (palette or no) with tmux in a bit of a bind. Tmux won't implement initc, and ncurses won't implement an alternate color-setting method (please, somebody, tell me I'm wrong).

  • Since init_color() modifies the 256 color palette, you can get some pretty interesting bugs by loading a theme that sets colors by RGB value, then switching to a theme that sets colors by 0--255 and which happens to use colors that were clobbered by the previous theme. The solution is not difficult (factor out the palette resetting bit of ui_curses_free(), call it from vis_theme_load() or such), but that may be an unacceptable conceptual intertwining between two separate components, all for a pretty minor feature. I've not done it because WORKSFORME.

[0] tmux/tmux@96d0e8a
[1] tmux/tmux#34
[2] https://gist.github.com/XVilka/8346728
[3] radareorg/radare2@be46b9d
[4] neovim/neovim@8dd415e
[5] http://lists.schmorp.de/pipermail/rxvt-unicode/2013q3/001829.html
[6] https://lists.gnu.org/archive/html/bug-ncurses/2013-10/msg00007.html

@mawww
Copy link

mawww commented Jan 2, 2017

According to tmux/tmux#552 it looks like they are not against palette changing, they seem ready to accept a PR that would implement it.

@s-gilles
Copy link
Contributor

s-gilles commented Jan 2, 2017

Oh, that's a much better situation than I'd thought - thanks for the correction. I'll look into how difficult it would be to do properly.

@martanne
Copy link
Owner

martanne commented Jan 2, 2017

@s-gilles thanks, I will take a more detailed look at the code once I have a bit more time.

Working in tmux is not a prerequisite for a merge (as I understand the behavior would be the same as today?).

Allowing themes to use terminal color indices was a mistake. In theory themes should be user interface agnostic. I'm fine with removing that again and forcing them to specify colors in RGB format. This should fix the theme changing bugs.

We probably also need to save/restore the color palette in ui_terminal_{save,restore}. Otherwise something like :!mc will look funny.

Also the existing color code uses two mostly redundant lookup tables color_{to,from}_256. At the time I copied it from tmux but wondered why they are doing it that way. It has since been changed in tmux, we should probably also clean it up (although we need the reverse lookup to restore the palette changes).

@rofl0r
Copy link

rofl0r commented Jan 3, 2017

you might want to check out this 50 line C program that paints an image using true-color RGB values via init_color() into an xterm-256color terminal. https://gist.github.com/rofl0r/12c766fc7e72f90cb56daf5d15652fc9 (screenshot: https://0x0.st/pYT.png ), or my more elaborate program conpix that can display any image in the terminal (screenshot: https://0x0.st/pgA.png )

@s-gilles
Copy link
Contributor

s-gilles commented Jan 3, 2017

@martanne Sounds good, I'll try and update my branch to deal with the other save/restore cases, then PR it.

I'm not positive, but I think we might be able to bypass reverse lookup. The ubiquitous 256colors.pl script actually sets the entire palette before displaying, so its RGB <-> index mapping could probably be used. It might be that this is even what tmux is doing with colour_to_6cube already.

@martanne
Copy link
Owner

Thanks to @s-gilles this should now work (except for broken terminals like Apple's Terminal.app #456).

We will have to revisit this if we ever decide to use something different than curses.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants