Add support for 256 color indexed palette#9
Conversation
Signed-off-by: Pavel Butsykin <p.butsykin@gmail.com>
The cast to size_t is safe here since res == -1 is checked first. Signed-off-by: Pavel Butsykin <p.butsykin@gmail.com>
Comparison is always true due to limited range of data type [-Werror=type-limits]. Signed-off-by: Pavel Butsykin <p.butsykin@gmail.com>
Signed-off-by: Pavel Butsykin <p.butsykin@gmail.com>
Signed-off-by: Pavel Butsykin <p.butsykin@gmail.com>
Signed-off-by: Pavel Butsykin <p.butsykin@gmail.com>
the-eugen
left a comment
the-eugen
left a comment
There was a problem hiding this comment.
Awesome! Just a couple minor questions.
vtrenderlib.c
Outdated
| if (fgc < VTR_COLOR_SGR8_TOTAL) { | ||
| seq[3] = (fgc == VTR_COLOR_DEFAULT ? '9' : '0' + fgc - 1); | ||
| seq[4] = 'm'; | ||
| return 5; |
There was a problem hiding this comment.
If we do this we substitute 8-bit 0-8 color values with the default terminal palette. Is that fine because the 8-bit space is backwards-compatible or are we losing some 8-bit colors that way?
There was a problem hiding this comment.
Yep, that's fine, and we are not losing any colors because the first 8 entries in the 256 color palette are shared with ANSI colors 0-7.
https://github.com/ThomasDickey/xterm-snapshots/blob/5afc76e089167da6d07c49334d0bf350377f6c0a/terminfo#L2182
In the updated patch I removed the fallback to legacy ANSI colors to simplify set_foreground_color_s().
vtrenderlib.c
Outdated
| seq[4] = 'm'; | ||
| return 5; | ||
| } | ||
| assert(seqcap >= 11); |
There was a problem hiding this comment.
We will now consume more space in the preallocated sequence buffer. Which is correct, we will reallocate it if needed. However, to avoid reallocation during frame rendering we guess what is going to be a reasonable-enough preallocated seq buffer size (see VT_SEQLIST_BUFFER_SIZE). You could fiddle with it to account for extra space for the 8-bit color.
Use the 256-color escape sequence ESC[38;5;INDEXm for all foreground colors, replacing the legacy SGR 30-37 path. The existing enum vtr_color values remain valid because the standard ANSI colors 0-7 share the same palette indices in the 256-color mode. VTR_COLOR_DEFAULT still emits ESC[39m to reset to the terminal default color. Palette indices are stored as idx+1 in vtr_fgcolor_t since value 0 is reserved for VTR_COLOR_DEFAULT. This means index 255 would overflow uint8_t, so it's mapped to VTR_COLOR_WHITE (the closest color match). Signed-off-by: Pavel Butsykin <p.butsykin@gmail.com>
pbutsykin
force-pushed
the
256_color_palette
branch
from
9c40690 to
4a90910
Compare
2 participants
No description provided.