vt: underline colors and style from kitty #1099

Closes #828

Thanks for contributing! From a quick look, this looks good. Nice and clean.

I'm trying to decide whether I want to merge this as-is, or try to come up with an alternative way of storing extra data. I have a budding idea but haven't thought it through, and don't have the time to test it out yet.

I'm going to leave this open for the time being.

Hey all! I gave these changes a try, as I've been looking into a low latency/high performance terminal to use for rendering NeoVim. What was missing was support for displaying underlines/undercurls with a color, which this PR aims to add.

Functionality wise it's working, though I had to set VTE_VERSION=6800 (like gnome-terminal, not sure what the exact value should be for foot) to get coloring to work. This is a neovim issue though, as it seems to use this variable to detect terminal support.

One thing I noticed is that underlines are quite thin. I ended up doubling the thickenss with this patch, and it looks a bit better (see the screenshots attached):

diff --git a/render.c b/render.c
index 233e9edb..622027f9 100644
--- a/render.c
+++ b/render.c
@@ -395,7 +395,7 @@ draw_ext_underline(const struct terminal *term, pixman_image_t *pix,
 {
 if (style == UNDERLINE_NONE)
 return;
- const int thickness = font->underline.thickness;
+ const int thickness = font->underline.thickness * 2;
 
 int y_ofs;
 /* Make sure the line isn't positioned below the cell */

Would it be possible to allow customising the thickness (or perhaps increasing the thickness if no option is desired)? Thanks!

Would it be possible to allow customising the thickness (or perhaps increasing the thickness if no option is desired)? Thanks!

It's currently not possible. Normally, the font itself defines the underline thickness. When the font doesn't, fcft (the font library used by foot) estimates a thickness.

Since we already allow customizing a lot of other font metrics (offsets, line height etc), adding an option for overriding the underline thickness is not out of scope for foot. So feel free to open a feature request for this. Or even better - a PR that implements it ;)

Would it be possible to allow customising the thickness (or perhaps increasing the thickness if no option is desired)? Thanks!

It's currently not possible. Normally, the font itself defines the underline thickness. When the font doesn't, fcft (the font library used by foot) estimates a thickness.

Since we already allow customizing a lot of other font metrics (offsets, line height etc), adding an option for overriding the underline thickness is not out of scope for foot. So feel free to open a feature request for this. Or even better - a PR that implements it ;)

That makes sense, I created an issue for it in #1136 :)

Just as an FYI: should #1140 be merged, its changes would have to be applied to draw_ext_underline as well.

@dnkl, done w/ changes you requested, although I feel that https://codeberg.org/dnkl/foot/src/branch/master/doc/foot-ctlseqs.7.scd#L198 should be more documented on the meaning of each specifier (i.e. 48 - background, 58 - underline etc)

Here are some screenshots of how the new underlines look like

image

image

done w/ changes you requested

Thanks! Still debating with myself whether to merge this now, or wait until I have implemented a better solution for "extra" cell data. I have an idea that should cover both this, as well as OSC-8 URLs, but there will be some time before I can start working on it.

although I feel that https://codeberg.org/dnkl/foot/src/branch/master/doc/foot-ctlseqs.7.scd#L198 should be more documented on the meaning of each specifier (i.e. 48 - background, 58 - underline etc)

Possibly, but I don't think that's necessary. foot-ctlseqs isn't meant to be a reference manual on how the escapes work. Just a list of escapes that foot implements.

Users are expected to lookup the documentation from the source (typically XTerm's ctlseq document) for details on a specific escape.

Thank you for working on this! This, along with vi mode for the scrollback, are the only two features that I'm really missing in foot.

I compiled this branch and tested it out, and for the most part, it works great! However, I ran into an address boundary error when attempting to use one of the ANSI colors in the facespec for the underline:

fish: Job 3, 'foot' terminated by signal SIGSEGV (Address boundary error)

This does not happen in kitty, so I think this is an issue with foot.

EDIT: I was able to confirm with certainty that the issue is with foot. If you run a foot server and connect one client to kak, then perform the steps, the entire server crashes and all clients close. A kitty window connected to the same kak session displays the red just fine.

To reproduce

• Compile this PR
• Install kakoune
• Add the following to your kakrc:

declare-optionrange-specsface_test_rangesdefine-command-params1test-face%{add-highlighter-overridewindow/face-test-rangesrangesface_test_rangesset-facewindowtest_face%arg{1}set-optionwindowface_test_ranges%val{timestamp}"%val{selection_desc}|test_face"}

• Open a file in kak, select some text with w (or anything you want)
• Run :test-face ",,rgb:ff0000+c", and see how the text is curly underlined in red
• Run :test-face ",,red+c", and see crash

image

One other question: why is this locked behind a compile time flag? Does it really add that much overhead compared to not having it, that it shouldn't work by default? I would like to be able to use the distribution package manager for foot, since it is one of the most essential programs on my system.

I believe this misses setal in the terminfo description (+ maybe a new setrgbl using the format of setrgbf / setrgbb with their own values for R, G and B instead of the combined one from setal).

In libvte (TERM=vte-direct) one can successfully do this for example:

$ printf "%sfoo %s bar\n" "$(tput setal 0xff0000)" "$(tput Smulx 3)underlined$(tput Smulx 0)"
foo underlined bar
$ tput setal 0xff0000 | cat -v
^[[58:2::255:0:0m

Edit:
Added setrgbl suggestion.

The crash I reported earlier with using ANSI facespecs for underlines in kakoune no longer occurs, but instead the underlines don't show up at all. It continues to work fine in kitty.

@raiguard, the issue you mentioned should be gone in b9c8d68845, am being a victim of blind copy-pasting. Let me know if you find any more bugs with it.

image

why is this locked behind a compile time flag

Because of #828 (comment), I initially planned to make this feature optional so users can choose between more-features and less-bloat. And because this feature affects how structure (of a cell) is laying in memory, it is static.

P.S. I'm sorry for making you wait for it that long.

Underlines in the IME preedit area disappear in the current code.

This patch can fix it.

diff --git a/render.c b/render.c
index e92247b4..d619d7d5 100644
--- a/render.c
+++ b/render.c
@@ -926,12 +926,13 @@ render_cell(struct terminal *term, pixman_image_t *pix,
 
 /* Underline */
 #if FOOT_EXT_UNDERLINE
- draw_ext_underline(term, pix, font, &ul, cell->attrs.ul_style,
- x, y, cell_cols);
-#else
+ if (cell->attrs.ul_style != UNDERLINE_NONE) {
+ draw_ext_underline(term, pix, font, &ul, cell->attrs.ul_style,
+ x, y, cell_cols);
+ } else
+#endif
 if (cell->attrs.underline)
 draw_underline(term, pix, font, &fg, x, y, cell_cols);
-#endif
 
 if (cell->attrs.strikethrough)
 draw_strikeout(term, pix, font, &fg, x, y, cell_cols);

Are these features supported in latest foot now (version 1.15.3)?

Are these features supported in latest foot now (version 1.15.3)?

This PR has not been merged into the master branch yet, for now the only thing you could do is clone the fork of the PR and build from source.

It does not seem like the built in terminfo reflects the extended underline capabilities? Am i missing something?

For now i was able to get curly underlines to work in neovim by manually adding the line Smulx=\E[4:3m, to foot.info before building, or by building normally and running foot --term=xterm-kitty.

I rebased this PR onto the last foot release (1.16.1). For anyone who is interested: queezle/foot@85d680151c

@kraftwerk28 Thanks for the feature, it is essential when working with a language server and I would not be using foot without it.

For now i was able to get curly underlines to work in neovim by manually adding the line Smulx=\E[4:3m, to foot.info before building, or by building normally and running foot --term=xterm-kitty.

Where do I have to put the line Smulx=\E[4:3m, for it to work? I pasted it at the end of the foot.info file and it didn't work.

Where do I have to put the line Smulx=\E[4:3m, for it to work? I pasted it at the end of the foot.info file and it didn't work.

That should probably be Smulx=\E[4:%p1%dm, (copied from kittys terminfo). Setting it to \E[4:3m, only allows for curly underlines, but this PR supports different styles as well.

@gmr458 At the end of the foot.info is correct (see queezle/foot@70a3c2f505), but this is a terminfo file which has to be compiled and installed in your system. It's needed by other programs to select the right escape codes to talk to foot.

@queezle you're right, added such termcap into foot.info.

For those who are using arch and it's derivatives: I've made an AUR package containing this patch. It also installs additional terminfos into /usr/share/terminfo/f (to avoid clash with ncurses's files) and changes the default $TERM to foot-ext-underline. Enjoy and let me know if I made any AUR-related mistakes.

Please care underlines in IME pre-edit area.

diff --git a/ime.c b/ime.c
index f3a3ec18..7cadc0f2 100644
--- a/ime.c
+++ b/ime.c
@@ -350,7 +350,11 @@ done(void *data, struct zwp_text_input_v3 *zwp_text_input_v3,
 for (size_t i = 0, cell_idx = 0; i < wchars; cell_idx += widths[i], i++) {
 if (hidden || start == end || cell_idx < start || cell_idx >= end) {
 struct cell *cell = &seat->ime.preedit.cells[cell_idx];
+#if FOOT_EXT_UNDERLINE
+ cell->attrs.ul_style = UNDERLINE_STRAIGHT;
+#else
 cell->attrs.underline = true;
+#endif
 }
 }
 

This patch serves the same purpose as the patch in #1099 (comment) in a more appropriate way.

For those who are using arch and it's derivatives: I've made an AUR package containing this patch. ...

Thank you, it works wonderfully, just a question: Is it expected that underline thickness cannot be changed? I tried changing the offset according to the foot.ini manpage which worked correctly, but underline-thickness=5px or any other value hadn't changed anything about the appearance of the underlines. I am unsure if this is related to foot itself or to this PR, but so far I haven't encountered any other issues.

I can confirm that underline-thickness doesn't appear to do anything. Additionally, the thickness doesn't appear to account for display scale. Is that a known issue?

Additionally, URL selection appears to be entirely broken.

Font size 10.5, display scale 1.25:

image

Font size 13.0, display scale 1.0:

image

I'm currently using the changes from this PR rebased unto the master branch. (削除) The undercurl peaks/valleys look a bit sharp , but (削除ここまで) I'm pleased and will be staying with this setup.

Edit: I believe the undercurl "sharpness" is defined by the font.

image

image

If anyone is using this branch and would like the characters to sit within the dips of the undercurl rather than upon the peaks, this patch will work:

diff --git a/render.c b/render.c
index f88d9ba3..294e0dd6 100644
--- a/render.c
+++ b/render.c
@@ -456,13 +456,13 @@ draw_ext_underline(const struct terminal *term, pixman_image_t *pix,
 const pixman_trapezoid_t traps[] = {
 {
 I(top), I(bot),
- {{I(x), I(bot - th)}, {I(half_x), I(top - th)}},
- {{I(x), I(bot + th)}, {I(half_x), I(top + th)}},
+ {{I(x), I(top + th)}, {I(half_x), I(bot + th)}},
+ {{I(x), I(top - th)}, {I(half_x), I(bot - th)}},
 },
 {
 I(top), I(bot),
- {{I(half_x), I(top + th)}, {I(full_x), I(bot + th)}},
- {{I(half_x), I(top - th)}, {I(full_x), I(bot - th)}},
+ {{I(half_x), I(bot - th)}, {I(full_x), I(top - th)}},
+ {{I(half_x), I(bot + th)}, {I(full_x), I(top + th)}},
 }};
 pixman_image_t *fill = pixman_image_create_solid_fill(color);
 pixman_composite_trapezoids(

image

Replaced by #1747