-
-
Notifications
You must be signed in to change notification settings - Fork 615
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
Powerline: Improve rendering at small size/hidpi #234
Powerline: Improve rendering at small size/hidpi #234
Conversation
I noticed that by scaling the triangles 126% vertically (to support more variable line heights), at normal line heights, the triangle appears more narrow and squashed to the right, making text between two separators appear uncentered. I produced an alternate option in 3253e1a that only scales the triangles by 113% vertically (and modifies the arrow separators to match the new angle). This makes them appear to fill the bounding box more fully and makes text between the separators appear more centered. The difference is very subtle, but noticeable in practice. I prefer this second option, but it comes at the cost of supporting less variable line heights. The choice may come down to which option works better for more people. |
As hopefully one last minor improvement on top of 3253e1a, this last commit 0daa7f6 improves the vertical continuity of the arrow separators when they appear on top of each other: This is barely noticeable at normal sizes and it sort of depends on having the default line height, but it's only an optimization anyway. It won't make things look any worse on larger line heights. I've tested this on Windows and it works there too. I've included these changes in hopefully this last set of test fonts for your consideration: |
I doubt you can ever create glyphs that will render well everywhere. The correct solution is to actually have the terminal override the rendering of those glyphs so that it renders them directly rather than from the font (this is how most terminal render the box ddrawing characters, for example). I just implemted that in kitty: kovidgoyal/kitty@77a161b |
Until source-foundry/Hack#234 is accepted, I am using a hacked version of Hack.
I finally got a hold of a macOS system to try this change out on, and, while I'm afraid I can't say it's as good on macOS (in Terminal and iTerm2) as depicted in my screenshots from Linux and Windows, I can at least say it's not any worse than the version being distributed now.
The points on the triangle separators are visible above and below the line, but that was also the case before, and I would argue that they more vertically centered with my changes, and the arrow (line) separator glyphs look better too. I agree with what kovidgoyal said about the right solution being to implement the powerline separators in the terminal, but since these are not standardized, I don't see that happening anytime soon, and I think you would still want a high quality set of powerline glyphs in this font for non-terminal applications and as a fallback for unsupported terminals. Given that this change dramatically improves the rendering on both Linux and Windows, and at the very least doesn't negatively affect macOS, I would humbly ask that you (@chrissimpkins) please accept this pull request. I spent a fair amount of time on this, I think the quality of work is good, and I think it will make a difference for a lot of people. If I had better access to macOS, I could try further tweaking it to improve the situation there, but (a) I don't, and (b) it can be done in a separate pull request. |
This is fantastic @iamjamestl! I really apologize for the delay and greatly appreciate all of your efforts. We went round and round on this issue and you appear to have a significantly improved solution, even if it does not completely address the issue on OS X. It sounds like you explored this issue across a range of font sizes in the first post in the thread and that the images are being shown at a size of 9. Can you confirm the alignment on Win/Linux with your current changes across sizes 8 - 13? I can look into the OS X renders across the same sizes on my system if it looks like you are solid across that range on those platforms. Be happy to merge this PR into the next release if it looks like we are heading in the right direction, even if we are not there quite yet. We could go so far as to build separate fonts for Win/Linux & OS X Powerline users if necessary to make it as good as it can get on all platforms. Will also invite users in the original thread to load these fonts up and confirm that we are looking good across different Linux distros / Win versions. |
Thanks, Chris. I did test these changes against all sizes from 6 and up on both my standard DPI monitor and sorta high DPI monitor (160 dpi). Here are some sample screenshots I have just taken showing a variety of Powerline separators on three different renderers on my standard DPI display. They look just as good at higher DPIs. And just to be clear, this is with all three commits from this PR applied. I've been using this patched font for six months now and I've been very happy with it. FreeType 2.7 FreeType 2.8 Despite the extra space between the lines, the triangle separators still look correct. A small gap is introduced between lines of continuous arrow separators, but I don't think it looks bad. As I said in 0daa7f6, making these continuous was only an optimization anyway, and I don't think it's worth trying to keep them continuous when the renderer is such a moving target. ClearType (Windows 10) |
What hinting settings are you using for Freetype? |
We use ttfautohint to hint the ttf font files and can take care of extraneous points here and there for individuals who allow the instructions in the fonts to override the autohinting on their platform. To date, we have not added instructions for the Powerline glyphs and this is something that we can look into. Won't apply to the OS X issues that you found here but it will nudge things in the proper direction in a size dependent fashion on Win/Linux platforms which is where your improvements have occurred in this PR. @burodepeper and I just began to discuss new work on the fonts and will begin to work through plans for the upcoming release schedule. My bias would be to release another set of fonts in the v2.x line with some relatively minor OpenType table changes and hinting modifications to address issues in some of the issue reports. This should be fairly simple and will permit us time to organize and prioritize other time intensive design issues. I think that the changes here would be an excellent addition to our v3.0 release which would include other design changes that we have kicked around for some time now. Will see if David has any thoughts about other changes that he feels are important before we dig in fully to v3.x with some important design changes, possibly some forks with variant glyph options, a greatly improved and automated build chain, and more. These improvements would be a great way to lead off 3.0 given how broadly used Powerline is among our target user population. There is a great deal that I would like to do and I'm sure that David has ideas as well. Let's see how things emerge. All further work on new v2.x release(s) will be in the development branch before we merge to master on release and I will keep the v3.0 branch current with changes that occur in development until we are ready to make 3.0 our next release. We can (and should) push out pre-release builds of the v3.0 fonts as we make changes so that we can obtain feedback from testers - or simply use them if you like to live on the font cutting edge ;) |
Sounds good to me. I'm very grateful for Hack and the work you've put into it. In my opinion, there is simply no better terminal/code typeface than this one. My previous favorite was DejaVu, and Hack fixes all the little annoyances I had with it, and I love that they're metrically compatible so that I can continue to use DejaVu as a fallback. I spent some time today trying to get a little more consistency out of the Powerline separator glyphs after spotting some glitches under Konsole and FreeType 2.7. I've just pushed in 51972cd what I believe is the best I can do. After not getting anywhere with the scaling approach from the patched DejaVu fonts, I took another look at the Fantasque Sans approach of forcing some overlap with a flag along the vertical edge. As I mentioned in the original post, straight up copying the Fantasque Sans approach did not yield very good results, especially on my HiDPI display; but after some massaging and a lot of trial and error, I found a solution that seems to work much better across all of the terminals, operating systems, renderers, and display densities that I have access to. FreeType 2.8 FreeType 2.7 The only remaining sore spots I can see are some stray points under Konsole, FreeType 2.7, HiDPI (a rather unusual combination); and macOS. I really don't know what to do about macOS because nothing I tried seemed to produce satisfactory results. At least this latest change looks a little better, I think. Windows and rxvt-unicode look straight up perfect to me at all sizes. For your convenience, I've wrapped these changes up into a patched font called "Hack Test 4": |
@iamjamestl What editor did you use? And upon saving, did you intentionally chose to remove all the lib data? Just asking, no harm done ;) |
@burodepeper I used FontForge and it removed the lib data. I committed it that way because it looked like hinting data that I figured would need to be regenerated for the reshaped glyphs anyway. I'm not a font guy so I may have been mistaken--if so, I can edit the commit and add it back in. |
@iamjamestl Thanks! It is probably okay like this. @chrissimpkins and I were having a discussion about how various editors change the UFO data, and whether all of that data was actually needed, or could be safely stripped from the source. |
This will push us to make the move to a fully automated build process and create some proofing/testing tools. This is a good thing. No worries. In the works. |
When you have a chance, would you mind merging the current master branch with your fork and then execute the following tool over the commits that you would like to introduce? https://github.com/source-foundry/ufodiff It looks like command (as of v0.2.0 ufodiff release) will be:
for this PR. Interested to see if this works as intended to point out the files (including the individual glyphs) that PR's modify in a succint, simple to understand way. We are going to add more diff capabilities to the tool and would greatly appreciate ongoing testing with this PR if you would be willing! Thanks! |
51972cd
to
06e3b73
Compare
I rebased on top of current master. The
Should I move my changes out of the deprecated directory? Also, so far I've preserved all four of my commits to the |
No this is perfect where it is right now. All of your changes are in UFO and preserved at the moment. The current master branch UFO that we will use for v3.0 likely will change as we modify the OpenType tables in preparation for the conversion to the scripted build. Once we are ready, we will have you push each of those four *.glif files into the appropriate master branch UFO source in this PR (just an overwrite of each existing .glif text file that specifies the four glyph shapes that you modified), then will merge it in. Will be very simple. Goal with this test was simply to prove that the only files modified are the ones that specify the glyph shapes, not the opentype tables themselves. This is good news! |
For future reference (for us), we may need to convert the curves in this PR to quadratic with the new source. Something that David noticed on the new UFO source. Will examine this when we merge. |
Chris, we don't need to convert them. The shapes of the curves are the same, whether you use quadratics or cubic beziers. I only noticed that they had changed, and that my visual-diff tool didn't support quadratic curves yet. |
David- Shape will be same but question is whether the ttf font build compiler accepts different curve specs in the same source and corrects this before the build so that they are all consistent, or if it leads to a build exception. My understanding is that the curves differ for ttf and otf builds (believe quadratic for the former and cubic for the latter?). These curves were modified in the vfb source prior to builds for ttf and otf files (reason for the two different sets of source) for all of v2.x builds. This is a simple automated process (menu item generally) in font editors so I am sure that he / we can change it if necessary. With editor based builds, I suspect that this is automated at compile time when you request one or the other (or at least user is prompted during the build process to make the modification if they try to build the wrong file type from a specific curve type). At the moment an unknown and something that we will need to keep in the back of our mind if we are seeing build issues when we try to import into UFO source that now has quadratic curve definitions. |
If it leads to problems, we have a new testing tool to create! :) |
Next version of ufodiff will support GH flavored markdown btw. Will permit formatted direct copy/paste into PR/IR threads. Implemented in |
Can pull https://github.com/source-foundry/Hack/tree/dev-hints branch to get the current manual hinting adjustments and build again to see where we are with these adjustments. This is based on As you build and see anything that appears to be a problem, feel free to open an issue report on it. I have manually adjusted hints for all identified problems in ASCII glyphs at sizes 8 - 14. If you are working outside of this range, we aren't currently trying to optimize there. There were no planned changes in glyph height (and to date no intended change in design of any glyph shapes that are included in your downstream source) short of complete conversion to UFO source for builds (from FontLab Studio source). Some minor autohinting adjustments that could possibly be opening up the spacing in the glyphs in a vertical direction somewhat. David looked at the shapes in the UFO vs what we had in the past and across ASCII glyphs there did not appear to be any significant A/B differences in the curves. |
Don't worry about the test failure here. I have to merge Travis CI settings changes to this branch that will lead to pass. Travis just changed their Linux images and created havoc for us (and a large number of others)... Fixed in |
For what it's worth, after the rebase, I can confirm that the powerline glyphs render exactly the same as in #234 (comment) on all of the platforms I have access to, including Windows 10, which somehow always surprises me: |
Fantastic! Let's try to include this as of the v3.0 release. Let me get the build scripts merged over to the dev branch then we will pull this in and build some fonts for others out there to play with. @pikeas pointed out an issue above with the renders on OS X. We can take a more detailed look at this too if available for testing? |
The macOS rendering is, sadly, no better than I commented before, but at least the underline issue that @pikeas pointed out seems to be gone with the new builds: Granted, I have no idea how representative this is of a real Mac with a retina display--this is just from a cloud instance over RDP. If anyone with a Mac wants to give it a better test, the TTF files are linked in #234 (comment). |
Not terrible. Given the improvements on Linux and Windows seems acceptable to me. @pikeas thoughts? |
@pikeas 👍 excellent! Do you mind examining all sizes between 8 - 14 (no need to take the time to share further screenshots unless you want to share issues that you find) and verify that we are ok at all of those sizes? These are the sizes that we have defined as our primary objectives with the typeface. If we seem to largely be ok across those, then I think that we are ok to include this as is at this point. Seems that we have significant Win/Linux improvements without significant OS X damage and that seems to be a reasonably good compromise for these glyphs (which have been very problematic! fonts aren't designed to be UI tools...). |
I've tested down to 9 pt, the smallest size supported by iTerm 2 (3.2, OS X 10.12). The arrow looks fine-to-good through out the range, but is consistently slightly out of alignment where the arrow meets the bottom edge (visible in my earlier screen shot). Overall, this is a nice improvement to a wonderful font, which is now my daily driver. This is so close to pixel perfect - is there any way to improve alignment under OS X? |
@pikeas Thanks for looking into this Aris. OS X is a bit tricky because it ignores our manual instruction set adjustments so it would require someone to do what James did here but on the OS X platform. There is likely to be a shape for the glyphs that nearly perfectly matches the adjacent "UI" that the combination of glyphs and background coloring are being used to accomplish. The next step would be a platform specific compilation process. If you have the time to look into it, grab a free font editor and try moving the shapes to address the discrepancy on OS X. We are looking into ways to support a customized compile process and should be able to come up with an approach that will work. Until that is in place, it would simply require a drag/drop of the four .glif files from the UFO source into the glyphs directory of the source (with overwrite of the existing files there) followed by a re-build to have a brand new set of fonts that you can use. Let us know if you come up with shapes that look better on OS X. |
@burodepeper David, any concerns here? This is a significant improvement to the rendering of the Powerline glyphs on both linux and windows. OS X is very slightly off alignment and I think that we can safely say that we will not reach x-platform perfection in a single design. If we end up converting at some stage to a configure based build approach, it is possible that we could modify the glyph design to get to "perfection" on OS X platforms. I am prepared to merge this to dev branch once |
@chrissimpkins It looks good enough to me. It's still a bit wonky, but I see no artifacts to be concerned about, or anything that prevents them from being functional. |
@iamjamestl it will be simpler to wait on the merge of |
Is indeed unrelated to powerline. Could you submit a new issue for this? |
@pikeas I've never seen a render of the combination of an underscore and underline before, but you will want to be able to distinguish those things (i.e. you need to know that an underscore exists and that it isn't a space). Unclear why the renderer eliminates the underline below, but probably good that it does or it may appear as an emboldened line there. Suspect that underline only shows for you when glyph does not extend to within certain vertical metric. You can see this with the |
@iamjamestl Will be merging all of the build tooling to dev this weekend and will merge this into the sets over the upcoming week. Still intend to make this part of v3.0 release which should be available in a few weeks. |
sorry, this was based on |
Merged and new development build files with these changes are available in
|
These changes to the Powerline separator glyphs is the result of studying the ones from DejaVu Sans Mono and Fantasque Sans Mono which appear seamless, without gaps or points extending above and below the line. We make the outlines slightly larger than the bounding box to cause the renderer to overlap the character with the surrounding ones, eliminating the gaps. Experimentation with different sizes and shapes was performed in source-foundry#234 under several renderers and applications to try to get the most consistency across platforms. Closes source-foundry#33
In alacritty/alacritty#2693, we see two problems: powerline separator glyphs are offset both vertically and horizontally. Having meticulously redesigned the powerline separator glyphs for Hack in source-foundry/Hack#234, I can say with some degree of confidence that the horizontal misalignment is due to the separator glyphs not being designed to be larger than the bounding box. That is to say, it's a font problem. The vertical misaligment, however, comes down to how this library is calculating the font height. FreeType provides a height metric, but it represents "the vertical distance between two consecutive baselines" [1] which doesn't necessarily correspond to the height of the glyphs. Thus we get a situation where glyphs appear slightly above and below than the line on which their drawn. You might want that for typesetting proportional fonts, but it doesn't make much sense in a monospace, terminal-like application. For calculating the global glyph height, the FreeType documentation says to use the difference between the ascender and descender metrics. Indeed, that is what urxvt [2] and xterm [3] do, and that is what is implemented in this patch. This change may cause alacritty to render a pixel or two more space between lines depending on the font. The spacing can be tweaked with the `font.offset` setting at the expense of powerline separator seamlessness. [1] https://www.freetype.org/freetype2/docs/reference/ft2-base_interface.html#ft_facerec [2] https://github.com/exg/rxvt-unicode/blob/rxvt-unicode-rel-9.22/src/rxvtfont.C#L1244 [3] https://github.com/Maximus5/xterm/blob/73ac00b39ec6f4af3b43908996a4bc73410a47df/src/fontutils.c#L833
In alacritty/alacritty#2693, we see two problems: powerline separator glyphs are offset both vertically and horizontally. Having meticulously redesigned the powerline separator glyphs for Hack in source-foundry/Hack#234, I can say with some degree of confidence that the horizontal misalignment is due to the separator glyphs not being designed to be larger than the bounding box. That is to say, it's a font problem. The vertical misaligment, however, comes down to how this library is calculating the font height. FreeType provides a height metric, but it represents "the vertical distance between two consecutive baselines" [1] which doesn't necessarily correspond to the height of the glyphs. Thus we get a situation where glyphs appear slightly above and below than the line on which their drawn. You might want that for typesetting proportional fonts, but it doesn't make much sense in a monospace, terminal-like application. For calculating the global glyph height, the FreeType documentation says to use the difference between the ascender and descender metrics. Indeed, that is what urxvt [2] and xterm [3] do, and that is what is implemented in this patch. This change may cause alacritty to render a pixel or two more space between lines depending on the font. The spacing can be tweaked with the `font.offset` setting at the expense of powerline separator seamlessness. [1] https://www.freetype.org/freetype2/docs/reference/ft2-base_interface.html#ft_facerec [2] https://github.com/exg/rxvt-unicode/blob/rxvt-unicode-rel-9.22/src/rxvtfont.C#L1244 [3] https://github.com/Maximus5/xterm/blob/73ac00b39ec6f4af3b43908996a4bc73410a47df/src/fontutils.c#L833
After trying what must be every monospace font in existence, I have only found three so far that work well with the Powerline separator characters: DejaVu (and Menlo), Fantasque, and Terminus. Terminus works because it is a bitmap font--no room for renderer interpretation, but also not very helpful. So I spent some time trying to figure out how the characters work in DejaVu and Fantasque:
In DejaVu, the separators are simply about 10% bigger than the bounding box, letting them overlap with the character before and after it:
In Fantasque, the designer added a "flag" on the side of the separator triangle to force some overlap:
which is an interesting solution, but introduced some rendering glitches in my experience.
I think the problem with Hack has been well documented over in issue #33, but here are some additional examples using Hack 2.020 on my systems:
Notice in all of the cases there appear to be gaps or triangles that are too short or too tall. This is all shown at size 9, but occurs at all sizes up to about 26, when the renderer doesn't have to do as much fudging to fit the characters on the pixel grid.
To solve the gap problem, I chose to take the DejaVu approach. I started by redrawing the triangles to perfectly fill the bounding box (and recentered the point of the triangle), then scaled it horizontally by 113%. This lets them overlap with the surrounding characters at the cost of some sharpness on the point of the triangle. I chose a 13% increase instead of the ~10% increase I found in DejaVu because I prefered the way it rendered on my HiDPI screen at more font sizes.
To solve the height problem, I scaled the triangles vertically by 126%. This allows the triangle to appear to perfectly mate with the preceding/following character on a wider range of line heights at the cost of appearing more narrow (a side effect of this is that the triangle matches the Hack style better, imo). Freetype and ClearType seem smart enough to chop off the top and bottom of the triangle to fit the displayed line height. I do not have access to macOS to see how it renders there.
To improve the separator arrow characters, I redrew them based on the exact angle formed by the scaled triangles. I used a stroke size of 160, matching the other line drawing characters in the font. The result is a separator arrow that completely fills the line vertically, matches the angle formed by the separator triangles, and has a thickness more in line with the Hack style.
(Naturally, I made these changes for both the right and left pointing arrows.)
The results are, I believe, as good or better than the patched DejaVu font on my Freetype and ClearType terminals, at all sizes, and on two different DPI displays that I have access to. Again, I have not been able to test this on macOS.
I worked with extreme precision. The bottom of every triangle lines up with the top of the one below it, and the point on the arrows is perfectly vertically centered:
Here is Hack 2.020 with the four changed characters patched in and with a different name so that you can give it a try:
HackTest-powerline-fixes.zip
Thanks for your consideration.