csi: store color index, not actual color, in cell’s fg/bg attributes

When using indexed colors (i.e. SGR 30/40/90/100), store the index
into the cell’s fg/bg attributes, not the actual color value.

This has a couple of consequences:

Color table lookup is now done when rendering. This means a rendered
cell will always reflect the *current* color table, not the color
table that was in use when the cell was printed to.

This simplifies the OSC-4/104 logic, since we no longer need to update
the grid - we just have to damage it to trigger rendering.

Furthermore, this change simplifies the VT parsing, since we no longer
need to do any memory loads (except loading the SGR parameter values),
only writes.
This commit is contained in:
Daniel Eklöf 2021-12-25 17:13:50 +01:00
parent 8bf757f466
commit c2bf2d3650
No known key found for this signature in database
GPG key ID: 5BBD4992C116573F
3 changed files with 43 additions and 74 deletions

20
csi.c
View file

@ -114,7 +114,7 @@ csi_sgr(struct terminal *term)
case 36:
case 37:
term->vt.attrs.fg_src = COLOR_BASE16;
term->vt.attrs.fg = term->colors.table[param - 30];
term->vt.attrs.fg = param - 30;
break;
case 38: {
@ -122,9 +122,8 @@ csi_sgr(struct terminal *term)
if (term->vt.params.idx - i - 1 >= 2 &&
term->vt.params.v[i + 1].value == 5)
{
uint8_t idx = term->vt.params.v[i + 2].value;
term->vt.attrs.fg_src = COLOR_BASE256;
term->vt.attrs.fg = term->colors.table[idx];
term->vt.attrs.fg = term->vt.params.v[i + 2].value;
i += 2;
}
@ -147,9 +146,8 @@ csi_sgr(struct terminal *term)
{
const struct vt_param *param = &term->vt.params.v[i];
uint8_t idx = param->sub.value[1];
term->vt.attrs.fg_src = COLOR_BASE256;
term->vt.attrs.fg = term->colors.table[idx];
term->vt.attrs.fg = param->sub.value[1];
}
/*
@ -207,7 +205,7 @@ csi_sgr(struct terminal *term)
case 46:
case 47:
term->vt.attrs.bg_src = COLOR_BASE16;
term->vt.attrs.bg = term->colors.table[param - 40];
term->vt.attrs.bg = param - 40;
break;
case 48: {
@ -215,9 +213,8 @@ csi_sgr(struct terminal *term)
if (term->vt.params.idx - i - 1 >= 2 &&
term->vt.params.v[i + 1].value == 5)
{
uint8_t idx = term->vt.params.v[i + 2].value;
term->vt.attrs.bg_src = COLOR_BASE256;
term->vt.attrs.bg = term->colors.table[idx];
term->vt.attrs.bg = term->vt.params.v[i + 2].value;
i += 2;
}
@ -240,9 +237,8 @@ csi_sgr(struct terminal *term)
{
const struct vt_param *param = &term->vt.params.v[i];
uint8_t idx = param->sub.value[1];
term->vt.attrs.bg_src = COLOR_BASE256;
term->vt.attrs.bg = term->colors.table[idx];
term->vt.attrs.bg = param->sub.value[1];
}
/*
@ -298,7 +294,7 @@ csi_sgr(struct terminal *term)
case 96:
case 97:
term->vt.attrs.fg_src = COLOR_BASE16;
term->vt.attrs.fg = term->colors.table[param - 90 + 8];
term->vt.attrs.fg = param - 90 + 8;
break;
/* Bright background colors */
@ -311,7 +307,7 @@ csi_sgr(struct terminal *term)
case 106:
case 107:
term->vt.attrs.bg_src = COLOR_BASE16;
term->vt.attrs.bg = term->colors.table[param - 100 + 8];
term->vt.attrs.bg = param - 100 + 8;
break;
default:

64
osc.c
View file

@ -527,61 +527,6 @@ osc_notify(struct terminal *term, char *string)
notify_notify(term, title, msg != NULL ? msg : "");
}
static void
update_color_in_grids(struct terminal *term, int palette_idx, uint32_t new_color)
{
/*
* Update color of already rendered cells.
*
* Note that we do *not* store the original palette
* index. Therefore, the best we can do is compare colors - if
* they match, assume "our" palette index was the one used to
* render the cell.
*
* There are a couple of cases where this isn't necessarily true:
* - user has configured the 16 base colors with non-unique
* colors. - the client has used 24-bit escapes for colors
*
* In general though, if the client configures the palette, it is
* very likely only using index:ed coloring (i.e. not 24-bit
* direct colors), and I hope that it is unusual with palettes
* where all the colors aren't unique.
*
* TODO(?): for performance reasons, we only update the current
* screen rows (of both grids). I.e. scrollback is *not* updated.
*/
for (size_t i = 0; i < 2; i++) {
struct grid *grid = i == 0 ? &term->normal : &term->alt;
for (size_t r = 0; r < term->rows; r++) {
struct row *row = grid_row(grid, r);
xassert(row != NULL);
for (size_t c = 0; c < term->grid->num_cols; c++) {
struct cell *cell = &row->cells[c];
enum color_source fg_src = cell->attrs.fg_src;
enum color_source bg_src = cell->attrs.bg_src;
if ((fg_src == COLOR_BASE16 || fg_src == COLOR_BASE256) &&
cell->attrs.fg == term->colors.table[palette_idx])
{
cell->attrs.fg = new_color;
cell->attrs.clean = 0;
row->dirty = true;
}
if ((bg_src == COLOR_BASE16 || bg_src == COLOR_BASE256) &&
cell->attrs.bg == term->colors.table[palette_idx])
{
cell->attrs.bg = new_color;
cell->attrs.clean = 0;
row->dirty = true;
}
}
}
}
}
void
osc_dispatch(struct terminal *term)
{
@ -668,8 +613,8 @@ osc_dispatch(struct terminal *term)
LOG_DBG("change color definition for #%u from %06x to %06x",
idx, term->colors.table[idx], color);
update_color_in_grids(term, idx, color);
term->colors.table[idx] = color;
term_damage_view(term);
}
}
@ -811,11 +756,9 @@ osc_dispatch(struct terminal *term)
if (strlen(string) == 0) {
LOG_DBG("resetting all colors");
for (size_t i = 0; i < ALEN(term->colors.table); i++) {
update_color_in_grids(term, i, term->conf->colors.table[i]);
for (size_t i = 0; i < ALEN(term->colors.table); i++)
term->colors.table[i] = term->conf->colors.table[i];
}
}
else {
for (const char *s_idx = strtok(string, ";");
@ -835,11 +778,12 @@ osc_dispatch(struct terminal *term)
}
LOG_DBG("resetting color #%u", idx);
update_color_in_grids(term, idx, term->conf->colors.table[idx]);
term->colors.table[idx] = term->conf->colors.table[idx];
}
}
term_damage_view(term);
break;
}

View file

@ -494,8 +494,37 @@ render_cell(struct terminal *term, pixman_image_t *pix,
_bg = term->colors.selection_bg;
} else {
/* Use cell specific color, if set, otherwise the default colors (possible reversed) */
_fg = cell->attrs.fg_src != COLOR_DEFAULT ? cell->attrs.fg : term->reverse ? term->colors.bg : term->colors.fg;
_bg = cell->attrs.bg_src != COLOR_DEFAULT ? cell->attrs.bg : term->reverse ? term->colors.fg : term->colors.bg;
switch (cell->attrs.fg_src) {
case COLOR_RGB:
_fg = cell->attrs.fg;
break;
case COLOR_BASE16:
case COLOR_BASE256:
xassert(cell->attrs.fg < ALEN(term->colors.table));
_fg = term->colors.table[cell->attrs.fg];
break;
case COLOR_DEFAULT:
_fg = term->reverse ? term->colors.bg : term->colors.fg;
break;
}
switch (cell->attrs.bg_src) {
case COLOR_RGB:
_bg = cell->attrs.bg;
break;
case COLOR_BASE16:
case COLOR_BASE256:
xassert(cell->attrs.bg < ALEN(term->colors.table));
_bg = term->colors.table[cell->attrs.bg];
break;
case COLOR_DEFAULT:
_bg = term->reverse ? term->colors.fg : term->colors.bg;
break;
}
if (cell->attrs.reverse ^ is_selected) {
uint32_t swap = _fg;