-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Bugfix vertical lines in powerline prompts #780
Conversation
Issue not completely fixed, needs probably 4% of overlap. Okay... let's pretend to be scientific... Left negative bearing is 100, total width is 1200, this is well... 8% overlap horizontally. But vertically, the glyph extends from 2226 to -480 while we have these metrics: So there is virtually no overlap. I will increase to 4% and limit vertical overlap to 1% as this will become ugly if stretched too much vertically. |
Maybe I should check out Cascadia... I do like Liberation a lot but some stuff are not OK. |
Just as that is kind of related... Keep it here for reference if Issues turn up ;) |
@musjj I guess you used the non-Mono version, i.e. Something seems to be wrong with the scaling in the non-Mono fonts. |
Hmm, It allows the upper D shape thing to extend into the right cell, but cuts off the stuff that extrudes into the left cell for the inverse D thing on the lower line. Hmm, and this is Both D shape things extend on both sided, as encoded in the font. (That is a bug in the font, but anyhow) So I can not reproduce @musjj My version is freshly downloaded from https://github.com/neovide/neovide/releases/latest/download/neovide.tar.gz (reports |
[why] The vertical overlap has never been a problem (as far as I know). It is maybe good to have some overlap for the terminal emulators that support vertical overlap. On terminals that truncate at the nominal cell border too much overlap looks bad, i.e. the glyphs 'distorted'. If we ever increase the overlap it is most likely be meant to be the left-right overlap. [note] Originally this has been part of commit fecda6a of #780. Signed-off-by: Fini Jastrow <[email protected]>
What's the status here friends? Should this be merged or at least rebased? It aged a bit... |
I'd still love to see this fixed. 🙂 |
I'll rebase later, not sure if this should go into 2.3.0 or not 🤔 |
A rebase would be enough for now as I can patch manually :} Thanks I really appreciate it because it allows me to continue using Neovim-qt. |
[why] The vertical overlap has never been a problem (as far as I know). It is maybe good to have some overlap for the terminal emulators that support vertical overlap. On terminals that truncate at the nominal cell border too much overlap looks bad, i.e. the glyphs 'distorted'. If we ever increase the overlap it is most likely be meant to be the left-right overlap. [note] Originally this has been part of commit fecda6a of #780. [note2] Originally this has been part of PR #967. Although that had a bug 😬 It used max() instead of min() (T_T) Signed-off-by: Fini Jastrow <[email protected]>
[why] The vertical overlap has never been a problem (as far as I know). It is maybe good to have some overlap for the terminal emulators that support vertical overlap. On terminals that truncate at the nominal cell border too much overlap looks bad, i.e. the glyphs 'distorted'. If we ever increase the overlap it is most likely be meant to be the left-right overlap. [note] Originally this has been part of commit fecda6a of #780. [note2] Originally this has been part of PR #967. Although that had a bug 😬 It used max() instead of min() (T_T) Signed-off-by: Fini Jastrow <[email protected]>
Rebase on Force push. |
Hmm, I fixed some mathematical bug in the |
On it! Thank you @Finii |
[why] The vertical overlap has never been a problem (as far as I know). It is maybe good to have some overlap for the terminal emulators that support vertical overlap. On terminals that truncate at the nominal cell border too much overlap looks bad, i.e. the glyphs 'distorted'. If we ever increase the overlap it is most likely be meant to be the left-right overlap. Note that the glyphs are usually valign='c' and the overlap is distributed half top and half bottom. There are no other valign values implemented (just 'not align' which is ... most likely bad). [note] Originally this has been part of commit fecda6a of #780. [note2] Originally this has been part of PR #967. Although that had a bug 😬 It used max() instead of min() (T_T) Signed-off-by: Fini Jastrow <[email protected]> f
Merged the other PR, rebase on master, force push. All the changes (apart from increasing the overlap percentages) are merged now, so the changes here are trivial. |
I have no idea as well, still testing :) Will be back shortly! |
Thanks for all your hard free work! |
I'm having some issues testing because the patcher sets weight to Book vs Regular. Is there a way to change this @Finii |
Which font do you patch? |
I used LiberationMono Regular from https://github.com/liberationfonts/liberation-fonts/files/7261482/liberation-fonts-ttf-2.1.5.tar.gz, will test --makegroups in a bit :) |
Hmm,
Indeed what would be
And it is not from the sourcefont. Investigating |
At least the 'new' method does is as expected:
I would advise anyone to always use |
Lets separate the issues and discuss here only the vertical lines. Created new Issue #1046 |
Possible to get another rebase to account for the naming issues? :)) |
[why] With the previous commit the gap between powerline glyphs became much better, but a faint line is still visible for LCD antialiasing (on my machine). Having seen how big the overlap is for Cascadia Code the overlap here can be increase maybe a bit. [how] Increase the overlap to 3 % (from mostly 2 %) for the glyphs that have a 'full colored edge' on one side. Fixes: #29 Signed-off-by: Fini Jastrow <[email protected]>
[why] On some machine the gap is still visible, but with 4% overlap the result looks good. [how] Increase the overlap from 3% to 4%. Cascadia Code has 8% of overlap, but they have a special extension to the overlap side, we will have a visual glitch if the overlap becomes too big. So lets stay with 4% or we should change the basis for the powerline glyphs. Signed-off-by: Fini Jastrow <[email protected]>
Here you are. Grump, it runs the PackSVG workflow ... sigh |
Thanks in case you're wondering why I don't rebase myself, there's a slight issue with the repo size ;) |
So all of my issues are resolved currently, I see no glitching. |
Increasing the overlap introduces issues with some terminal emulators. Anyhow, as terminals behave differently, if we improve the situation for one it will get worse for another. Maybe we can live with what we have now. Moving this to the Parallel Universe. |
[why] The vertical overlap has never been a problem (as far as I know). It is maybe good to have some overlap for the terminal emulators that support vertical overlap. On terminals that truncate at the nominal cell border too much overlap looks bad, i.e. the glyphs 'distorted'. If we ever increase the overlap it is most likely be meant to be the left-right overlap. Note that the glyphs are usually valign='c' and the overlap is distributed half top and half bottom. There are no other valign values implemented (just 'not align' which is ... most likely bad). [note] Originally this has been part of commit fecda6a of ryanoasis#780. [note2] Originally this has been part of PR ryanoasis#967. Although that had a bug 😬 It used max() instead of min() (T_T) Signed-off-by: Fini Jastrow <[email protected]> f
Description
First part of solution:
[why]
For some powerline symbols we add a certain amount of overlap into the
previous or next character to cover up a small gap between the symbols
that otherwise can show up as ugly thin (usually colored) line.
But after we carefully design that glyph with a bit overlap (over-sized
and having negative bearings) we remove all bearings. That breaks of
course the glyph and no actual overlap on the left side happens.
[how]
Just do not remove negative bearings on overlap-enabled glyphs. As they
are rescaled in both directions anyhow all bearings are wanted and must
be kept.
Second part of solution:
[why]
With the previous commit the gap between powerline glyphs became much
better, but a faint line is still visible for LCD antialiasing (on my
machine).
Having seen how big the overlap is for Cascadia Code the overlap here
can be increase maybe a bit.
[how]
Increase the overlap to 3 % (from mostly 2 %) for the glyphs that have a
'full colored edge' on one side.
Fixes: #29
Requirements / Checklist
What does this Pull Request (PR) do?
Increase the
overlap
for some glyphs slightly and correct the needed negative left side bearings for glyphs that should have had an overlap on the left side but had not.How should this be manually tested?
Any background context you can provide?
The issue comes up relatively often, I think. I was never bothered by the lines, so I dod not really follow that. Maybe I'm wrong. I have real list of Issues that are related.
What are the relevant tickets (if any)?
#29
Also see #779 for
params
future as dict.Screenshots (if appropriate or helpful)
@MIvanchev used
LiterationMono Nerd Font Mono
and neovim-Qt, so I use that here as examples.Note: Hinting full, Subpixel antialiasing, X11
Current behavior (before the PR):
After fixing the code: (first part)
Note more pronounced line for round things, as that has only 1%, the other have 2% overlap
After increasing the overlap: (second part)
I guess 3% is enough, it is still very very faintly visible. Can be increased if users still complain