-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Request: Improve squiggly underline rendering #853
Comments
This is not worth the effort to me -- but it should be any easy thing for someone else to implement. The relevant code is simply one function of ~ 25 lines: add_curl() in kitty/fonts/render.py The problem in your case is likely that kitty simply needs to use more space for the underline rendering.You can test that by increasing font size, which should make the rendering look similar to that of MacVim. I assume MacVim is using some algorithm to modify the underline size/position metrics that come from the font at small font sizes, kitty could presumably do the same. Feel free to ask if any details need clarification. |
I took a quick look at improving that function for small font sizes and made a fix but you'd still need to improve the antialiasing logic in that function at small sizes for perfect rendering. |
Thanks. A small improvement will probably go a long ways. |
I dont get rendering remotely like that at that font size. Hard for me to fix something I cannot reproduce. But feel free to submit a patch. Relevant code is in the add_curl() function in fonts/render.py That function is basically responsible for rendering the underline in a buffer the size of a single cell. |
Returning max thickness to 1 works for me: diff --git a/kitty/fonts/render.py b/kitty/fonts/render.py
index d5e33a8b..bfb98ea2 100644
--- a/kitty/fonts/render.py
+++ b/kitty/fonts/render.py
@@ -116,7 +116,7 @@ def add_dline(buf, cell_width, position, thickness, cell_height):
def add_curl(buf, cell_width, position, thickness, cell_height):
xfactor = 2.0 * pi / cell_width
- yfactor = max(thickness, 2)
+ yfactor = max(thickness, 1)
def clamp_y(y):
return max(0, min(int(y), cell_height - 1))
@@ -131,7 +131,7 @@ def add_curl(buf, cell_width, position, thickness, cell_height):
)
for x_exact in range(cell_width):
- y_exact = yfactor * cos(x_exact * xfactor) + position
+ y_exact = yfactor * cos(x_exact * xfactor) + position + 1
y = clamp_y(ceil(y_exact))
x_before, x_after = map(clamp_x, (x_exact - 1, x_exact + 1))
for x in {x_before, x_exact, x_after}: I also increased y by 1 because undercurl overlaps text a bit, so this moves it down. |
position comes from the font, making arbitrary changes to it doesn't seem like a good idea. It might look better for some fonts and worse for others. As for changing minimum thickness from 2 to 1, that seems reasonable, though I need to test it with a few more fonts/font sizes to see if it has bad effects in other cases. |
On second thoughts, there is a robust way to change position. I have committed it. Hwever, reducing minimum thickness to 1 gives me much worse rendering at small font sizes. The problem is that at thickness 1 the underline has to be rendered in basically a 2 pixel height which means the wave does not look well defined. |
Do you have idea how could i improve rendering without messing around with the source code? |
you basically need to improve the antialiasing and drawing logic to prevent the central blank line in your case. As for reproducing it should be purely deterministic, so if you provide the cell_height, cell_width position and tihickness values for your font, it should be trivial to reproduce. |
These are the values:
Hope it helps. |
are you sure about those values? position should never be larger than cell_height. Have you interchanged some numbers? |
I'm using |
I think it would be great if Kitty made the wavy underline rendering a little smoother and maybe more prominent. Here's how MacVim/Mac render it on the right compared to Kitty on the left:
It looks less like a line and more like a wave, and less "aliased"
The text was updated successfully, but these errors were encountered: