From 9f321e60303d76c40eeff1bea931fbbead6434ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 12 Dec 2020 20:50:43 +0100 Subject: [PATCH] csi: fix sub-parameter versions of 38/48 SGR escapes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Well this is embarrassing; the sub-parameter versions of the 38/48 SGR escapes all required an extra ‘:2’ that wasn’t supposed to be there, causing all the other sub-parameters to be shifted one step to the right. That is, foot expected e.g. 38:2:2:r:g:b, or 38:2:5:idx when the correct sequences are 38:2:cs:r:g:b and 38:5:idx. I.e. I mixed up the color-space ID (cs) of 38:2 with *type* of color: RGB or indexed. In addition to fixing this, this patch also adds support for a “bastard” version of the sub-parameter based RGB escapes, where the color-space identifier has been left out: e.g. 38:2:r:g:b. This sequence is invalid, but applications tend to “forget” the color-space ID... --- CHANGELOG.md | 6 +++ csi.c | 148 +++++++++++++++++++++++++-------------------------- foot.info | 8 +-- 3 files changed, 82 insertions(+), 80 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index caf1a73d..c11a5189 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -107,6 +107,10 @@ means foot can be PGO:d in e.g. sandboxed build scripts. See has been used to enable blinking. **cursor.blink** in `foot.ini` controls the default state of `CSI Ps SP q` (https://codeberg.org/dnkl/foot/issues/218). +* The sub-parameter versions of the SGR RGB color escapes (e.g + `\E[38:2...m`) can now be used _without_ the color space ID + parameter. + ### Deprecated ### Removed @@ -125,6 +129,8 @@ means foot can be PGO:d in e.g. sandboxed build scripts. See attributes and charset configuration, just like `\E7`+`\E8`. * Report mouse motion events to the client application also while dragging the cursor outside the grid. +* Parsing of the sub-parameter versions of indexed SGR color escapes + (e.g. `\E[38:5...m`) ### Security diff --git a/csi.c b/csi.c index 7893926a..fba044bc 100644 --- a/csi.c +++ b/csi.c @@ -141,53 +141,51 @@ csi_sgr(struct terminal *term) i += 4; } - /* Sub-parameter style: 38:2:... */ + /* Indexed: 38:5: */ else if (term->vt.params.v[i].sub.idx >= 2 && + term->vt.params.v[i].sub.value[0] == 5) + { + const struct vt_param *param = &term->vt.params.v[i]; + + uint8_t idx = param->sub.value[1]; + term->vt.attrs.have_fg = 1; + term->vt.attrs.fg = term->colors.table[idx]; + } + + /* + * RGB: 38:2::r:g:b[:ignored:tolerance:tolerance-color-space] + * RGB: 38:2:r:g:b + * + * The second version is a "bastard" version - many + * programs "forget" the color space ID + * parameter... *sigh* + */ + else if (term->vt.params.v[i].sub.idx >= 4 && term->vt.params.v[i].sub.value[0] == 2) { const struct vt_param *param = &term->vt.params.v[i]; - const int color_space_id = param->sub.value[1]; + bool have_color_space_id = param->sub.idx >= 5; - switch (color_space_id) { - case 0: /* Implementation defined - we map it to '2' */ - case 2: { /* RGB - 38:2:2::: */ - if (param->sub.idx < 5) { - UNHANDLED_SGR(i); - break; - } + /* 0 - color space (ignored) */ + int r_idx = 2 - !have_color_space_id; + int g_idx = 3 - !have_color_space_id; + int b_idx = 4 - !have_color_space_id; + /* 5 - unused */ + /* 6 - CS tolerance */ + /* 7 - color space associated with tolerance */ - uint8_t r = param->sub.value[2]; - uint8_t g = param->sub.value[3]; - uint8_t b = param->sub.value[4]; - /* 5 - unused */ - /* 6 - CS tolerance */ - /* 7 - color space associated with tolerance */ + uint8_t r = param->sub.value[r_idx]; + uint8_t g = param->sub.value[g_idx]; + uint8_t b = param->sub.value[b_idx]; - term->vt.attrs.have_fg = 1; - term->vt.attrs.fg = r << 16 | g << 8 | b; - break; - } - - case 5: { /* Indexed - 38:2:5: */ - if (param->sub.idx < 3) { - UNHANDLED_SGR(i); - break; - } - - uint8_t idx = param->sub.value[2]; - term->vt.attrs.have_fg = 1; - term->vt.attrs.fg = term->colors.table[idx]; - break; - } - - case 1: /* Transparent */ - case 3: /* CMY */ - case 4: /* CMYK */ - UNHANDLED_SGR(i); - break; - } + term->vt.attrs.have_fg = 1; + term->vt.attrs.fg = r << 16 | g << 8 | b; } + /* Transparent: 38:1 */ + /* CMY: 38:3::c:m:y[:tolerance:tolerance-color-space] */ + /* CMYK: 38:4::c:m:y:k[:tolerance:tolerance-color-space] */ + /* Unrecognized */ else UNHANDLED_SGR(i); @@ -236,53 +234,51 @@ csi_sgr(struct terminal *term) i += 4; } - /* Sub-parameter style: 48:2:... */ + /* Indexed: 48:5: */ else if (term->vt.params.v[i].sub.idx >= 2 && + term->vt.params.v[i].sub.value[0] == 5) + { + const struct vt_param *param = &term->vt.params.v[i]; + + uint8_t idx = param->sub.value[1]; + term->vt.attrs.have_bg = 1; + term->vt.attrs.bg = term->colors.table[idx]; + } + + /* + * RGB: 48:2::r:g:b[:ignored:tolerance:tolerance-color-space] + * RGB: 48:2:r:g:b + * + * The second version is a "bastard" version - many + * programs "forget" the color space ID + * parameter... *sigh* + */ + else if (term->vt.params.v[i].sub.idx >= 4 && term->vt.params.v[i].sub.value[0] == 2) { const struct vt_param *param = &term->vt.params.v[i]; - const int color_space_id = param->sub.value[1]; + bool have_color_space_id = param->sub.idx >= 5; - switch (color_space_id) { - case 0: /* Implementation defined - we map it to '2' */ - case 2: { /* RGB - 48:2:2::: */ - if (param->sub.idx < 5) { - UNHANDLED_SGR(i); - break; - } + /* 0 - color space (ignored) */ + int r_idx = 2 - !have_color_space_id; + int g_idx = 3 - !have_color_space_id; + int b_idx = 4 - !have_color_space_id; + /* 5 - unused */ + /* 6 - CS tolerance */ + /* 7 - color space associated with tolerance */ - uint8_t r = param->sub.value[2]; - uint8_t g = param->sub.value[3]; - uint8_t b = param->sub.value[4]; - /* 5 - unused */ - /* 6 - CS tolerance */ - /* 7 - color space associated with tolerance */ + uint8_t r = param->sub.value[r_idx]; + uint8_t g = param->sub.value[g_idx]; + uint8_t b = param->sub.value[b_idx]; - term->vt.attrs.have_bg = 1; - term->vt.attrs.bg = r << 16 | g << 8 | b; - break; - } - - case 5: { /* Indexed - 48:2:5: */ - if (param->sub.idx < 3) { - UNHANDLED_SGR(i); - break; - } - - uint8_t idx = param->sub.value[2]; - term->vt.attrs.have_bg = 1; - term->vt.attrs.bg = term->colors.table[idx]; - break; - } - - case 1: /* Transparent */ - case 3: /* CMY */ - case 4: /* CMYK */ - UNHANDLED_SGR(i); - break; - } + term->vt.attrs.have_bg = 1; + term->vt.attrs.bg = r << 16 | g << 8 | b; } + /* Transparent: 48:1 */ + /* CMY: 48:3::c:m:y[:tolerance:tolerance-color-space] */ + /* CMYK: 48:4::c:m:y:k[:tolerance:tolerance-color-space] */ + else UNHANDLED_SGR(i); diff --git a/foot.info b/foot.info index c7a05653..df824023 100644 --- a/foot.info +++ b/foot.info @@ -1,15 +1,15 @@ foot|foot terminal emulator, use=foot+base, colors#256, - setab=\E[%?%p1%{8}%<%t4%p1%d%e%p1%{16}%<%t10%p1%{8}%-%d%e48\:2\:5\:%p1%d%;m, - setaf=\E[%?%p1%{8}%<%t3%p1%d%e%p1%{16}%<%t9%p1%{8}%-%d%e38\:2\:5\:%p1%d%;m, + setab=\E[%?%p1%{8}%<%t4%p1%d%e%p1%{16}%<%t10%p1%{8}%-%d%e48\:5\:%p1%d%;m, + setaf=\E[%?%p1%{8}%<%t3%p1%d%e%p1%{16}%<%t9%p1%{8}%-%d%e38\:5\:%p1%d%;m, foot-direct|foot with direct color indexing, use=foot+base, colors#16777216, RGB, - setab=\E[%?%p1%{8}%<%t4%p1%d%e48\:2\:2\:%p1%{65536}%/%d\:%p1%{256}%/%{255}%&%d\:%p1%{255}%&%d%;m, - setaf=\E[%?%p1%{8}%<%t3%p1%d%e38\:2\:2\:%p1%{65536}%/%d\:%p1%{256}%/%{255}%&%d\:%p1%{255}%&%d%;m, + setab=\E[%?%p1%{8}%<%t4%p1%d%e48\:2\:\:%p1%{65536}%/%d\:%p1%{256}%/%{255}%&%d\:%p1%{255}%&%d%;m, + setaf=\E[%?%p1%{8}%<%t3%p1%d%e38\:2\:\:%p1%{65536}%/%d\:%p1%{256}%/%{255}%&%d\:%p1%{255}%&%d%;m, foot+base|foot base fragment, am,