Monofixes #107
Monofixes #107
Conversation
|
First of all I saw this PR just after you submitted it so sorry for the delay. I haven't had time to deeply go through your changes but did look them over. I must say this is pretty impressive and might fix quite a few lingering issues as well. It looks like you have spent a good amount of time and effort on these fixes so I really appreciate that.
It looks good to me. I'm all for these changes (as long as it doesn't create new problems) |
|
I did a bit more refactoring. Can you check it out and see if the change looks good? If so, do you typically merge to master, or would you like me to do so? |
|
Cool thanks. Yes I will check it out. I typically do the merging (at least for now). The soonest this would be merged would probably be this weekend. Trivial changes I do try to merge to master asap. 😄 |
|
So I just quickly tested this out on a few different fonts (em 2048 and 1000) with and without mono option. Do you realize what you have done??? This is going to single-handedly fix a handful of issues and problems! 👍 🎉 😱 This is really going to make a huge difference. Much thanks indeed! |
|
No problem. I'm currently working on adding support for adding "standard" Unicode symbols from the Google Noto font as another option. It's probably a good idea to use careful mode for this, which seemed to be broken. See commit 7e1ff32 above. |
|
I really want to see this, as right now none of the fonts I tried actually worked on macOS. |
|
@santagada If you checkout the branch or just download the font-patcher from sharkusk's branch you could test it out 😄 If you don't want to do that let me know and I can send you an updated patched version. Just tell me which font you use. |
|
@ryanoasis - I'm fairly happy with the code at this point. Feel free to merge at your leisure and let me know if you have any questions or comments. -Marcus |
|
BTW, the open/close was caused by me accidentally hitting the "close and comment" button. Not sure why those two buttons are so close together. :) |
|
Please do tell people when the fonts have been rebuilt :) |
|
@ryanoasis I actually want to test most fonts (at least 3270, anonymous, pro font and Iosevka) |
|
@sharkusk I have merged into 0.9.0 thanks. No problem on the accidental close I think we've all done it 😄 @santagada Sure. I was trying to get this done yesterday but I was fixing a few things. I am trying to improve the rebuild all script to make generating the combinations easier going forward. I'll make sure to ping here when I'm done. |
|
@ryanoasis I made another slight tweak. Using one common glyph to define a scaling factor caused many glyphs to be smaller than otherwise needed. So, I now use a list of glyphs that need to be scaled using a common factor, and maximize the rest. |
|
One other thing I noticed... When not using 'exact' mode, any unicode gaps between the symbol font glpyhs are removed when copying the to the source font. For example, if the symbol font had glyphs at 0xE001 and 0xE004, these two glphys would be inserted into the source font as: 0xX001 and 0xX002. This means if the symbol font changes and a new glyph is inserted in between, the resulting source font will have different glyph values. Unfortunately, fixing this will also change a few of glphy's unicode values in 0.9.0 if introduced... I did a patch last night when i was goofing around, but wasn't sure if this is expected behavior so I didn't push it to my branch. -Marcus |
|
@sharkusk Sorry but I have been busy... that includes doing some work in the 0.9.0 branch though 😉 Yeah you are correct those gaps aren't maintained.. I am not sure that that's necessarily a bad thing though since at some point theoretically we would run out of codepoints 😛 I would say at least for now we keep that functioning as is at least for 0.9.0 , maybe I will change my mind if I think about it some more or if it causes confusion in the future 👍 |
|
@santagada Feel free to test out fonts in 0.9.0 branch 😈 |
|
testing only on 3270, it is still wrong on macOS. 3270-Medium Nerd Font Complete Mono Windows Compatible.otf looks almost good on size 11, but any other size there is a gap below (and all other variations give the same results) size11: size 14: and on fontbook you kinda can see that it was not going to work (squashed characters): |
|
it happens with other fonts as well: The streching is bad with powerline extended symbols mainly maybe it is a particular problem of them (but look that the other symbols also look smaller than they should). |
|
@santagada Well I can definitely see what you mean by the scaling issues but the vertical alignment looks good for me (though I am not on macOS): 3270 Mono (size 14):3270 Non Mono (size 14):Also I forgot to actually build the NON-mono fonts ... whoops! Those might work better for you, at least the scaling won't be so small. Believe it or not the issues you are showing are a vast improvement to how a lot the glyphs looked before.. so I am actually encouraged so far 😄 |
|
Two things, the scaling still looks wrong but might be better than before, and you can improve a lot your tests actually making powerline lines like I have in my prompt. what is the script you use? I can try to improve it. |
|
@ryanoasis Did you pull 96eafa9 and 7cd183b? Those should maximize the glyph size, while still keeping the relative glyphs (i.e. arrows, etc) sized correctly. |
|
Okay, 9770856 may be a fix for the gaps @santagada is seeing. I haven't done a ton of testing, so I'm not sure if it may have other side effects. Some fonts have these values, many do not. The OS2 parameters are really inconsistently used. |
@sharkusk Hey good catch. I did not merge those latest changes in the 0.9.0 branch. I was hoping to do that just now.. let me see if I can get things up-to-date without conflicts... else I will do it tomorrow |
|
I fixed some errors as a result of the merge. Please take the latest 0.9.0 Did some quick tests and yes scale isssues seem to be fixed 👍 I will rebuild the fonts on 0.9.0 again today |
|
Nice people if I get time on the weekend I will improve the font testing script and try most fonts out. |
Fixes issues patching octicons and/or fontlinux due to changes from m…
|
@santagada Sure thanks |
|
just tested font 3270, and although some powerline symbols don't look perfect they seem to be working (I think they have the correct height but not correct aspect). |
|
@santagada Thanks for testing again.
|
|
I'm using the mono otf version, now that it worked (more or less) I will try the others to see if there is any change and I will post the screenshots |
|
@santagada Thanks I appreciate it. 'More or less' is actually a huge improvement compared to before so I will take it! 😄 ... and I don't expect it to be perfect at least for this next release |
|
Keep in mind that the aspect ratio of the powerline symbols is going to be The other symbols' aspect ratios are preserved, however this makes them On Thu, Nov 10, 2016 at 4:36 PM, Ryan L McIntyre notifications@github.com
|
|
maybe the final solution will be to have different versions of the symbols for different width. two or three variations should be enough to most fonts. |
Description
TL;DR - See "What does this Pull Request (PR) do?" below
I was having problems getting many of the monospace fonts to work under windows. Specifically, the fonts were not being detected as monospace, so they wouldn't work under MobaXterm or GVim.
Unfortunately, getting things to work properly with both Windows and MacOS turned out to require a lot of trial and error.
Some of the fun I discovered:
OTF formatted monospace fonts are detected as such by Windows. So, initially I added the option to modify the extension, forced the fonts I wanted to be OTF, and called it a day. Unfortunately, I discovered that TTF fonts converted to OTF had a strange problem with MacOS. Specifically glyph number 0xE0C8 would be somehow skipped, and all subsequent glyphs would be mis-numbered.
To get TTF fonts detected properly as monospace under Windows, I found that EVERY glyph must be assigned the same width. This includes empty/blank glyphs, and often requires the source font glyphs to be modified in this way.
When using the windows-compatible named fonts on MacOS, I found that some font families where not being handled properly. Different weights of the same font would conflict with each other. This was caused by the font name being the identical across the font weights. I solved this by tweaking how the windows name is generated, and ensure it is unique for each font weight. IMHO, it would be better to have only one naming convention that works everywhere. This would eliminate half of the binary fonts and generation time.
I also wanted to preserve the aspect ratio of the glyphs when generating a monospace font, but wanted the powerline separators to remain line-height. After trial and error, I found that setting the ymax/ymin/height based upon the os2 parameters to be more consistent than scanning the line-draw characters.
I ended up doing some minor refactoring (normalizing indent amounts, removing unused variables, etc) in the process, so I completely understand if my changes are too drastic for a pull.
Requirements / Checklist
./font-patcher Inconsolata.otf --fontawesome --octicons --pomicons./gotta-patch-em-all-font-patcher\!.sh HermitWhat does this Pull Request (PR) do?
Improves generation of monospace fonts so they properly work under windows
How should this be manually tested?
Generate some fonts.
Any background context you can provide?
See description.
What are the relevant tickets (if any)?
Screenshots (if appropriate or helpful)