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

Undercurl support #11

Closed
bellini666 opened this issue Dec 9, 2021 · 14 comments
Closed

Undercurl support #11

bellini666 opened this issue Dec 9, 2021 · 14 comments
Labels
bug Something isn't working

Comments

@bellini666
Copy link

Hi,

Trying to set some LSP/Spell highlights to use undercurl instead of underline but it doesn't seem to work with neovim-gtk. For example, with:

highlight! SpellBad guibg=NONE cterm=undercurl gui=undercurl guisp=#ef2929

I get this on terminal (using gnome-terminal btw):

Screenshot from 2021-12-09 19-17-37

And this on neovim-gtk:

Screenshot from 2021-12-09 19-17-50

Even the color seems wrong in that example

@Lyude Lyude added the bug Something isn't working label Dec 11, 2021
@Lyude
Copy link
Owner

Lyude commented Dec 11, 2021

I think there might be more that's required to trigger this, on the latest git main undercurl appears to work fine for me:

image

I tried some of the highlighting categories with my local LSP setup + rust as well just to make sure:

image

What font are you using? On that note, I wonder if it might be good to just get the rest of your .vimrc

@jacobmischka
Copy link

I admit I'm not yet using this fork as my daily driver and haven't tested it in a few weeks, but undercurl also worked fine for me last I did.

@Lyude Lyude added the question Further information is requested label Dec 11, 2021
@bellini666
Copy link
Author

Here is my whole neovimrc: https://github.com/bellini666/dotfiles/tree/master/vim

This is the specific function that gets loaded from an autocmd that runs when the GUI itself loads: https://github.com/bellini666/dotfiles/blob/master/vim/lua/options.lua#L153

For font I'm using Inconsolata patched from Nerd Fonts, specifically this one: https://github.com/ryanoasis/nerd-fonts/blob/master/patched-fonts/Inconsolata/complete/Inconsolata%20Nerd%20Font%20Complete.otf?raw=true

Note that I'm using Debian Unstable, which I update daily. I don't know exactly what system libs neovim-gtk interacts with to do things like undercurl work, but if you tell me I can tell you what is the version that I have installed of those libs

@Lyude
Copy link
Owner

Lyude commented Dec 11, 2021

Note that I'm using Debian Unstable, which I update daily. I don't know exactly what system libs neovim-gtk interacts with to do things like undercurl work, but if you tell me I can tell you what is the version that I have installed of those libs

Yeah this would probably also be useful to know just in case

@Lyude
Copy link
Owner

Lyude commented Dec 12, 2021

Note that I'm using Debian Unstable, which I update daily. I don't know exactly what system libs neovim-gtk interacts with to do things like undercurl work, but if you tell me I can tell you what is the version that I have installed of those libs

Yeah this would probably also be useful to know just in case

Whoops I actually forgot to mention the libraries :P. It would be cairo, pango, and gtk+ - but I'm not sure that's causing the problem though because I managed to get neovim-gtk running in a Debian Unstable container in podman and I didn't see any issues there either. Could you try changing your font to something like "Source Code Pro 11" to see if that changes anything? (Also JFYI - haven't had a chance to go through your .vimrc yet)

@bellini666
Copy link
Author

Hey @Lyude ,

Testing "Source Code Pro 11" did make the undercurl somewhat "work". But it is rendering it really different than from my terminal:

Terminal using source code pro:
Screenshot from 2021-12-12 10-00-09

neovim-gtk using source code pro:
Screenshot from 2021-12-12 10-01-04

Also note that the terminal from my previous example was also using Inconsolata, and my terminal emulator is gnome-terminal.

Btw, here are the relevant libs from my system:

ii  libpango-1.0-0:amd64                  1.48.10+ds1-1
ii  libcairo2:amd64                       1.16.0-5
ii  libgtk-3-0:amd64                      3.24.30-4
ii  libgtk-4-1:amd64                      4.4.1+ds1-3 

Please tell me if you need any more info

@jacobmischka
Copy link

That second image is what is to be expected, the undercurl in gnome-shell is different. The kerning or line height of your original font must be causing it to render mostly below the line, cutting it off except for the very top.

@bellini666
Copy link
Author

Hey @jacobmischka ,

Just a quick note, it is not actually gnome-shell but gnome-terminal. I mentioned it because, theoretically, both use the same base libs (pangpo/cairo/gtk) for rendering stuff.

Now, related to the issue that you mentioned... What can be done to solve it? The issue happened to that font in specific, but it can in theory happen to others that have the same issue.

Can it be worked around at neovim-gtk's side?

@jacobmischka
Copy link

Er sorry, yes, I meant gnome-terminal. Misspoke.

@Lyude
Copy link
Owner

Lyude commented Dec 12, 2021

Hey @jacobmischka ,

Just a quick note, it is not actually gnome-shell but gnome-terminal. I mentioned it because, theoretically, both use the same base libs (pangpo/cairo/gtk) for rendering stuff.

Now, related to the issue that you mentioned... What can be done to solve it? The issue happened to that font in specific, but it can in theory happen to others that have the same issue.

Can it be worked around at neovim-gtk's side?

Potentially - I like to think that if VTE (the underlying terminal library that gnome-terminal uses, which is the one that employs a combination of pango/cairo/gtk in order to provide a terminal emulation widget) can do it, there's also probably a way we can implement it in neovim-gtk.

…However, next time I sit down to fix this issue I think I've already noticed a good hint as to what might be happening here. If you zoom in and look at the line that we draw in @bellini666's screenshot very closely, you'll notice it's not actually a straight line of red pixels with the same color - the color varies as the line goes on, which might mean we are actually drawing the undercurl - but the damage box ("damage" as in the area within the neovim-gtk drawing area that has been modified as a result of the screen changing, and must be redrawn with cairo. neovim-gtk typically calls these "dirty" regions) we're calculating for each glyph is too small and ends up cutting off part of the undercurl. So - I think that's likely going to end up being the actual issue that needs to be fixed here

@Lyude Lyude removed the question Further information is requested label Dec 12, 2021
@Lyude
Copy link
Owner

Lyude commented Dec 12, 2021

Hey @Lyude ,

Testing "Source Code Pro 11" did make the undercurl somewhat "work". But it is rendering it really different than from my terminal:

Terminal using source code pro: Screenshot from 2021-12-12 10-00-09

neovim-gtk using source code pro: Screenshot from 2021-12-12 10-01-04

Also note that the terminal from my previous example was also using Inconsolata, and my terminal emulator is gnome-terminal.

Btw, here are the relevant libs from my system:

ii  libpango-1.0-0:amd64                  1.48.10+ds1-1
ii  libcairo2:amd64                       1.16.0-5
ii  libgtk-3-0:amd64                      3.24.30-4
ii  libgtk-4-1:amd64                      4.4.1+ds1-3 

Please tell me if you need any more info

Also as for this difference - I wonder if VTE is actually using pango for drawing the undercurl or if it's drawing it manually using cairo - which might explain the difference in how the two look

@Lyude
Copy link
Owner

Lyude commented Dec 12, 2021

That second image is what is to be expected, the undercurl in gnome-shell is different. The kerning or line height of your original font must be causing it to render mostly below the line, cutting it off except for the very top.

Also whoops, looks like someone already figured out the damage region issues! Sorry I missed this comment lol

@bellini666
Copy link
Author

Hey @Lyude ,

Digging around I saw that this is where vte draws the undercurl, which basically calls this function that draws it directly on cairo. That's why it looks so pretty.

I tried to mess around with the neovim-gtk's drawing code and this basically fixed things for me:

diff --git src/render/mod.rs src/render/mod.rs
index 5649691..5e234f0 100644
--- src/render/mod.rs
+++ src/render/mod.rs
@@ -182,10 +182,9 @@ fn draw_underline_strikethrough(
 
         if cell.hl.undercurl {
             let sp = hl.actual_cell_sp(cell).inverse(inverse_level);
-            ctx.set_source_rgba(sp.0, sp.1, sp.2, 0.7);
+            ctx.set_source_rgb(sp.0, sp.1, sp.2);
 
-            let max_undercurl_height = (line_height - underline_position) * 2.0;
-            let undercurl_height = (underline_thickness * 4.0).min(max_undercurl_height);
+            let undercurl_height = underline_thickness * 4.0;
             let undercurl_y = line_y + underline_position - undercurl_height / 2.0;
 
             pangocairo::functions::show_error_underline(

Also, that rgba also explains why the color was different than what I was setting in neovim.

Here are the results:

Using inconsolata (my font, that was having problems before):
Screenshot from 2021-12-13 18-08-38

Using source code pro (which was working before to show that it did not break):
Screenshot from 2021-12-13 18-06-27

Don't know if there could be any drawbacks with that, but if there isn't, applying that diff should fix this issue.

Also, it would be pretty awesome to copy the vte cairo rendering in the future to have its pretty undercurl rendering (but that's for another issue).

@Lyude
Copy link
Owner

Lyude commented Dec 13, 2021

Hey @Lyude ,

Digging around I saw that this is where vte draws the undercurl, which basically calls this function that draws it directly on cairo. That's why it looks so pretty.

I tried to mess around with the neovim-gtk's drawing code and this basically fixed things for me:

diff --git src/render/mod.rs src/render/mod.rs
index 5649691..5e234f0 100644
--- src/render/mod.rs
+++ src/render/mod.rs
@@ -182,10 +182,9 @@ fn draw_underline_strikethrough(
 
         if cell.hl.undercurl {
             let sp = hl.actual_cell_sp(cell).inverse(inverse_level);
-            ctx.set_source_rgba(sp.0, sp.1, sp.2, 0.7);
+            ctx.set_source_rgb(sp.0, sp.1, sp.2);
 
-            let max_undercurl_height = (line_height - underline_position) * 2.0;
-            let undercurl_height = (underline_thickness * 4.0).min(max_undercurl_height);
+            let undercurl_height = underline_thickness * 4.0;
             let undercurl_y = line_y + underline_position - undercurl_height / 2.0;
 
             pangocairo::functions::show_error_underline(

Also, that rgba also explains why the color was different than what I was setting in neovim.

Here are the results:

Using inconsolata (my font, that was having problems before): Screenshot from 2021-12-13 18-08-38

Using source code pro (which was working before to show that it did not break): Screenshot from 2021-12-13 18-06-27

Don't know if there could be any drawbacks with that, but if there isn't, applying that diff should fix this issue.

Lol. I literally wrote up equivalent code on my branch just now before checking back on this issue to find your patch :P. You were close but not quite (although you were right on the rgba bits) - the whole idea we're using for calculating the height of the undercurl here just turned out to be wrong and we need to use the pango descent instead. I've actually managed to reproduce the issue locally and confirmed this fixes it, so I'm going to push :). Thanks for the help!

Also, it would be pretty awesome to copy the vte cairo rendering in the future to have its pretty undercurl rendering (but that's for another issue).

Yes it would be - feel free to file it as a feature request though! No guarantees when I'd get to it though

@Lyude Lyude closed this as completed in 43558cd Dec 13, 2021
Lyude added a commit that referenced this issue Dec 13, 2021
For some reason, we appear to simply be rendering the undercurl color at
70% opacity. I can't think of any good reason for this, besides maybe
maybe thinking it looked nice on some neovim theme, so let's stop doing
this and use 100% opacity like we should be.

Fixes #11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants