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

Hack hints are being removed in Nerd Fonts patched versions, here's the fix #70

Closed
chrissimpkins opened this issue Mar 15, 2016 · 38 comments
Assignees
Labels
Milestone

Comments

@chrissimpkins
Copy link
Collaborator

Great project Ryan. Thanks for including Hack with your patched releases.

It looks like your patching approach is removing the hints from the Hack ttf variants. This could lead to issues for some Linux distros and all Windows users. They are simple to restore if you would like to modify your releases (or for anyone who wants to do this on their own).

Use the following approach prior to the font installs:

These hints may/likely will change with new Hack releases so it is necessary to use Control Instruction Files that are part of the same Hack release that you patch.

@ryanoasis
Copy link
Owner

@chrissimpkins Awesome thanks! Thanks for the compliments 😊 . I love Hack 😉

Your support means a lot, I will try to get this fixed when I can (might not be next 'release' because I have already pushed to much into it 😆) but at the very least your instructions are provided for help 👍

@chrissimpkins
Copy link
Collaborator Author

👍

@dingmaotu
Copy link

@ryanoasis Hi, the Hack font patch is awesome but it is unusable on Windows because the hinting (in 'post' table where isFixedPitch is set to 1) is removed. In gVim and mintty, only Monospace font is allowed. Without the hinting Windows does not recognize the font as Monospace.

I know little about font design and fontforge scripting, so I can not help with the code. I can only manually edit the isFixedPitch and regenerate the font you provided. This is very frustrating since I downloaded the "Windows Compatible" font... I spent almost one day to dig the Internet and the True Type font specification to locate the problem (though it is a great learning experience, haha). I wanted to submit an issue and I found this.

Please fix this issue and thank you 😊

@chrissimpkins
Copy link
Collaborator Author

chrissimpkins commented Nov 25, 2016

@dingmaotu

This script will update any font isFixedPitch post table setting to a value of 1. It will work for any of the fonts included in Ryan's collection.

Add the following script on the path fpfix.py and execute with:

$ python fpfix.py [path to font file]

Dependencies:

  1. Python install
  2. fonttools library (Python) install (pip install fonttools)

Gist link

https://gist.github.com/chrissimpkins/d0898e9580b9aa8f2ae692afe724bf93

fpfix.py Script

#!/usr/bin/env python
# -*- coding: utf-8 -*-

### DEPENDENCY:
#     fontTools Python library
#       ==> https://github.com/fonttools/fonttools
#       ==> Install: pip install fonttools

### USAGE:
#     python fpfix.py [filepath to font]

import sys
from fontTools import ttLib

if len(sys.argv) < 2:
    sys.stderr.write("[fpfix.py] ERROR: Please enter a path to the font file")
    sys.exit(1)

try:
    tt = ttLib.TTFont(sys.argv[1])
    post = tt["post"]__dict___
    post["isFixedPitch"] = 1
    tt.save(outfile)
    print("[fpfix.py]: '" + sys.argv[1] + "' fixed pitch setting was changed to 1 in the post table")
except Exception as e:
    sys.stderr.write("[fpfix.py] : ERROR: Unable to update the font isFixedPitch setting. " + str(e))
    sys.exit(1)

@ryanoasis
Copy link
Owner

ryanoasis commented Nov 25, 2016

Been busy on 0.9.0 so still haven't gotten around to this


@dingmaotu Thanks for the info. Sorry for your troubles. Yeah I was just going to say to try the instructions @chrissimpkins has provided

@chrissimpkins thanks again for the details on the fix. I should probably add some process post patching to avoid having these missing hints


Edit: Actually this doesn't seem overly difficult. I'll try to resolve this in 0.9.0 with some variation on what @chrissimpkins provided 😄

@chrissimpkins
Copy link
Collaborator Author

👍 let me know if there is anything I can do

@dingmaotu
Copy link

@chrissimpkins Thanks for the script. Actually I used the fonttools provided ttx command to fix individual fonts. Having a script is a lot better

@chrissimpkins
Copy link
Collaborator Author

@dingmaotu great! You can modify it to process multiple fonts at one go. Enjoy!

@ryanoasis Ryan let me know if there are any open type issues that need work. Happy to pitch in to help fix these.

@ryanoasis
Copy link
Owner

@chrissimpkins Thanks appreciate it. I can't think of anything specific off the top of my head but if I do I'll let you know. Also of course if you see anything you can/want to fix or improve that would be great.

A lot of the open issues should be fixed soon (with 0.9.0) so the picture of what is actually still open should clear up drastically

@ryanoasis ryanoasis self-assigned this Dec 3, 2016
@ryanoasis ryanoasis added this to the v0.9.0 milestone Dec 3, 2016
@ryanoasis
Copy link
Owner

I had some trouble with building from source (dependencies, etc) but I did finally manage to get it working.

I modified the provided scripts (a few issues on the python one) and made the necessary changes on the shell one to work. Called both of these after patching (with a simple wrapper shell file).

I got to the point of the new font generating and was about to test if the hints and glyphs were there ... but now my primary OS is messed up. Trying to restore it but first I am trying to backup my changes that I had not yet pushed 😟

@ryanoasis
Copy link
Owner

I can't seem to get the hinting to work. Any ideas?

ttfautohint -l 4 -r 80 -G 350 -x 0 -H 181 -D latn -f latn -w G -W -t -X "" -m "bin/scripts/Hack/Hack-Regular-TA.txt" Hack-Regular.ttf posthinted/Hack-Regular.ttf
ttfautohint --version
ttfautohint 1.6

@chrissimpkins
Copy link
Collaborator Author

What error are you getting?

@ryanoasis
Copy link
Owner

@chrissimpkins No errors but I don't see the hints (at least in FontForge). Is there another/better way to verify it worked as expected? Excuse my ignorance 😄

@chrissimpkins
Copy link
Collaborator Author

@ryanoasis If you include the -t flag (which you do in the above command) a TTFA sfnt table is added to the font. You can check for the presence of the table which does not otherwise exist in a font.

We built this tool that reports the TTFA Open Type table that ttfautohint produces when it applies the hints. Would require a manual check that the hints are present though I can modify it to raise a standard error exit code when the table is not present if that is helpful.

https://github.com/source-foundry/font-ttfa

@ryanoasis
Copy link
Owner

@chrissimpkins Hey thanks a lot... I think it is working just fine then.

  1. FontForge ignores the TTFA table... so no wonder I was having trouble seeing the differences via FontForge 😊
The following table(s) in the font have been ignored by FontForge
 Ignoring 'TTFA'
  1. I found that using the -T (capital t) would print that table
--ttfa-info, -T   (not in ttfautohintGUI)
Print TTFA table of the input font on standard output if present, then exit. 

https://www.freetype.org/ttfautohint/doc/ttfautohint.html

So given this I was able to verify the presence or lack of:

ttfautohint -T patched-fonts/Hack/Regular/complete/Knack\ Regular\ Nerd\ Font\ Complete.ttf
No `TTFA' table in font.
ttfautohint -T posthinted/Knack\ Regular\ Nerd\ Font\ Complete.ttf

adjust-subglyphs = 0
default-script = latn
dw-cleartype-strong-stem-width = 0
fallback-scaling = 0
fallback-script = latn
fallback-stem-width = 181
gdi-cleartype-strong-stem-width = 1
gray-strong-stem-width = 0
hinting-limit = 350
hinting-range-max = 80
hinting-range-min = 4
hint-composites = 0
ignore-restrictions = 0
increase-x-height = 0
reference = 
reference-index = 0
symbol = 0
TTFA-info = 1
windows-compatibility = 1
x-height-snapping-exceptions = 
control-instructions = \
   0 zero left 40; \
   0 zero touch 39-40, 71 xshift 0 yshift -0.5 @ 7; \
   0 zero touch 39-40, 71 xshift 0 yshift -1 @ 8, 12-18; \
   0 zero touch 55-57 xshift 0 yshift -1 @ 19-25

@chrissimpkins
Copy link
Collaborator Author

👍

@ryanoasis
Copy link
Owner

I am updating Hack to v3.003

@chrissimpkins Don't want to bug you but a couple questions came to mind:

I am wondering do we need to execute the Python scripts in the fixes folder post-patching ?

The current post processing script that gets run for Hack is bin/scripts/Hack/postprocess.sh.

Do we still need fpfix.py or should we also use the Python scripts in the postbuild folder?

There were no changes to the Control Instructions Files since we lasted grabbed them

@chrissimpkins
Copy link
Collaborator Author

chrissimpkins commented Mar 16, 2018

I am wondering do we need to execute the Python scripts in the fixes folder post-patching ?

The fix-dsig.py script is not critical and likely unnecessary for the needs here. It adds an empty DSIG table. Very edge case issue for some software that I have never come across...

The fstype.py fix is relevant and probably warranted. This sets the fstype field (https://docs.microsoft.com/en-us/typography/opentype/spec/os2#fstype) to "Installable embedding" defined as follows:

Fonts with this setting indicate that they may be embedded and permanently installed on the remote system by an application. The user of the remote system acquires the identical rights, obligations and licenses for that font as the original purchaser of the font, and is subject to the same end-user license agreement, copyright, design patent, and/or trademark as was the original purchaser.


Do we still need fpfix.py or should we also use the Python scripts in the postbuild folder?

Don't believe so. Here is a SS of the post table from a build of the regular variant in current development source with our current build tooling:

k1hcv-image

That fix should not be necessary.


There were no changes to the Control Instructions Files since we lasted grabbed them

We updated the ttfautohint settings at the v3.000 release and, as I recall we did modify the CIF's at that release vs. the last v2.x build. If you are based on < v3 you might need to update both. If you are currently based on v3.x, you should be ok. We tinkered with a few ttfautohint changes but reverted back to where we were because we had a bizarre variable height lowercase glyph issue that developed with several moving parts in the hints. Still not completely clear what led to it. We stuck with ttfautohint version 1.7 with current settings and Control Instructions Files. These settings have been heavily tested and are functioning as intended. If you have the capacity to build derivatives of our upstream source on those pinned versions and settings, you shouldn't have problems. If you have moved to ttfautohint v1.8.1, have a look at the renders before you push them. There is something funky going on in that version with our shapes. Werner is aware and intends to look into it.

Let me know if there is anything else that I can do. Happy to help! The patches here are fantastic Ryan.

@ryanoasis
Copy link
Owner

Looks like we have the latest ttfautohint settings since v3.000

If you are currently based on v3.x

Yep that looks to be the case, I seem to no longer have ttfautohint installed on my system so not sure what happened there. I'll have to make sure to get the 1.7 version you mentioned and try to rebuild the patched fonts and see what happens.

Working on updating our postprocess script

Let me know if there is anything else that I can do. Happy to help! The patches here are fantastic Ryan.

Thanks! I take any help offered! I will reach out if there is something specific though :)

@chrissimpkins
Copy link
Collaborator Author

chrissimpkins commented Mar 17, 2018

This script will build a local development copy of ttfautohint at v1.7 off of your system PATH if you want/need current ttfautohint on PATH:

https://github.com/source-foundry/Hack/blob/master/tools/scripts/install/ttfautohint-build.sh

You can find the executable on the path $HOME/ttfautohint-build/local/bin/ttfautohint.

@ryanoasis
Copy link
Owner

Thanks, that helps greatly! I've got ttfautohint setup again and ran through our Sanity check without error.

I have updated the Dev steps with the Hack script option: https://github.com/ryanoasis/nerd-fonts/wiki/Contributor-Developer-Setup#hack-font-specific-development

The way I was doing it before definitely was a lot more painful on Linux 😆


Seem to be close on this (the Python scripts in post processing work just fine). However,
ttautohint with the settings from the Hack build work well pre-patching but of course not working with post-patched version 🙈 . Looking into it but I can definitely see the glyphs still have the same names and code point. Need to keep digging. Found reference in sections like 'eight.subs'.

Errors for anything in the control files:

Hack-Regular-TA.txt:3:1: invalid glyph name (0x204)
  uni0023 touch 0,1,2,3,18,19,20,21,22,23,24,25,26,27,28,31 x 0.25  @ 13

It seems FontForge (or at least how we are using it) is destroying some data when patching that references these glyphs

@chrissimpkins
Copy link
Collaborator Author

Mind pushing one of the post-patched font files so that I can have a look at it? FontForge is likely remapping the glyph names when you patch. Should be a simple solution once we know the glyph names. Will just need to replace uniXXXX in CIF's with the proper post-patched name.

@ryanoasis
Copy link
Owner

Thanks! That's a brilliant idea. I wondered about that but when I opened the pre and post patched fonts in FontForge I was comparing the glyphs names, etc and I didn't realize what you are saying (FontForge already changed the name once I opened it in FF).

I did a quick test with the 0023 glyph and it seemed to work!

numbersign touch 0,1,2,3,18,19,20,21,22,23,24,25,26,27,28,31 x 0.25  @ 13

I am going to upload a patched version (without the hinting fixes) just in case you would like to confirm or check it out!

@ryanoasis
Copy link
Owner

Here is a patched version of Hack after running through the patcher (normal settings, with the Python script post process fixes but WITHOUT the autohint fixes):

Hack Regular Nerd Font Complete.ttf.zip

@chrissimpkins
Copy link
Collaborator Author

chrissimpkins commented Mar 17, 2018

yep! switch all of those names to whatever FF is using and you should be gtg!

let me know where I can find the font. will definitely have a look.

@chrissimpkins
Copy link
Collaborator Author

chrissimpkins commented Mar 17, 2018

can you give me a list of all glyphs that are raising exceptions? is it all of them in the CIF's?

@ryanoasis
Copy link
Owner

@chrissimpkins Thanks a lot for the help! Really appreciate it 😃 I probably would've wasted at least several hours.

Going to take a nice break. Will be pushing fixes to the postprocess Nerd Fonts uses. Very happy it's not some serious issue that would cause me to start looking at something different from FontForge for building at the moment!

can you give me a list of all glyphs that are raising exceptions? is it all of them on the CIF's?

I believe it was all of them. When I first saw the error with 0023 I just commented it out but then it errored out on 0025 and etc. I only tested Hack-Regular-TA but I would assume it will be all of them..

@chrissimpkins
Copy link
Collaborator Author

Will have a look tonight. Will let you know if I have any other thoughts. I think swapping those names should do the trick.

@ryanoasis
Copy link
Owner

Thanks again, no worries. No rush 😄

Got Regular working, this is what I had to change it to (only zero it put as uni0030 - go figure):

# U+0023 numbersign glyph ID 582
numbersign touch 0,1,2,3,18,19,20,21,22,23,24,25,26,27,28,31 x 0.25  @ 13

# U+0025 percent glyph 761
percent touch 0,1,21,22,23,39    y 0.5    @10
percent touch 40                 y 0.75   @10
percent touch 41,42,43           y 0.5    @10
percent touch 51,52,53,70,71,72  y 0.5    @10

percent touch 40,43              y -0.75  @11
percent touch 41,42              y 0.75   @11

percent touch 0,1,21,22,23,39    y -0.25  @14
percent touch 8,9,10,30,31,32    y 0.25   @14
percent touch 51,52,53,70,71,72  y -0.5   @14
percent touch 40,41,42,43        y -0.25  @14

# U+002B plus glyph ID 764
plus touch 4,5,10,11          y 0.5    @12
plus touch 4,5                y 1.0    @13

# U+0030 zero glyph ID 548
uni0030 touch 35,36,45,46,47,56  y -0.5   @8
uni0030 touch 35,36,56           y -1.0   @12,13,14

@chrissimpkins
Copy link
Collaborator Author

For italic CIF:

  • uni0025 --> percent

For bold CIF:

  • uni0025 --> percent
  • uni002B --> plus
  • uni0038 (do not change)

For bold italic CIF:

  • uni002B --> plus

Hopefully, that will take care of it!

@ryanoasis
Copy link
Owner

@chrissimpkins Thanks again again. I think we are good. Confirmed those CIF changes (already pushed).

@chrissimpkins
Copy link
Collaborator Author

Fantastic!!

@chrissimpkins
Copy link
Collaborator Author

chrissimpkins commented Mar 19, 2018

BTW, here are a handful of other tools for font management that we've developed in case they happen to be useful for maintainer duties here:

  • font-v : font version string mods, supports git SHA1 commit writes for current HEAD commit at build time. includes library with public functions/methods to support nearly anything you would want to do with version strings (writes to binaries)
  • font-name.py : font family name changes (writes to binaries)
  • font-line : vertical metrics reporting / line spacing adjustments (writes to binaries)
  • hack-linux-installer : shell script example of how to automate pull + install fonts + update font caches on Linux. Our approach to support automated installs from repo releases for Linux users (though can be used with any public GET request accessible end point).

Hope they help!

@ryanoasis
Copy link
Owner

Thanks @chrissimpkins those look like they could be very helpful!

We'll probably have to consider some of those or at least items like them 😄

@mrzealot
Copy link

mrzealot commented May 2, 2019

Hey @chrissimpkins & @ryanoasis !

Trying to make my "perfect terminal font" and after a lot of A/B testing and an embarrassing amount of Googling about, I've arrived at forward-slash-zero'd, nerdfont'ed Hack. Getting it to work is a little bumpy, tho.

Building Hack from source leads to the same error that kicked this issue off (that's how I got here), i.e., uni0023 is an invalid glyph name. But even if it were to build, a subsequent nerdfont patching is still in order. So I guess my questions are:

  1. should the Hack build work as-is? because I'm encountering even in the vanilla build (mostly targeted @chrissimpkins, I think :) )
  2. is it okay, if I use this repo's alternate hints, or maybe even completely omit them as I'll have to re-patch after nerd-ification anyway?
  3. does the same alt disclaimer apply here that I should modify even this repo's hint fixing to omit the new alternate slashed zero glyph?

Also, feel free to set me straight if my questions suggest that I don't understand something fundamental...
Thanks to both of you for the great work you're doing!

@chrissimpkins
Copy link
Collaborator Author

chrissimpkins commented May 2, 2019

uni0023 is an invalid glyph name

should the Hack build work as-is? because I'm encountering even in the vanilla build (mostly targeted @chrissimpkins, I think :) )

I believe that in more recent versions of the fontmake compiler (c/w what we were using when v3.003 = current master branch) was released, the default glyph naming changed which ttfautohint doesn't like... The control instructions files that we defined to manually help ttfautohint with the Hack hints are defined using the old production names in the master branch. This is fixed in the dev branch as of source-foundry/Hack@ac48920 however I am woefully behind on maintainer + development work on the project. Sorry to cause these troubles. I have been meaning to prioritize the new v4.000 release but seem to always have something else that rises to the top of my stack...

is it okay, if I use this repo's alternate hints, or maybe even completely omit them as I'll have to re-patch after nerd-ification anyway?

For personal use, this question can be answered by what platform you are using. If you are on macOS you can use unhinted builds. If you are on Win or Linux then you likely need the instruction sets. If you intend to release to others you can safely assume that you need the hints. If Ryan modified the hints here I'll defer to him to respond to this.

does the same alt disclaimer apply here that I should modify even this repo's hint fixing to omit the new alternate slashed zero glyph?

I suspect that the answer here is yes. Zero will always be hinted by ttfautohint, but we are currently manually modifying the automated default hints in the upstream Hack project with scripts that ttfautohint calls Control Instructions Files. Short answer is that these define a glyph by name (where problem 1 arose) and then indicate how the automated hinting defaults should be changed at specific point definitions in the glyph design. They tell ttfa to nudge point #n up/down/left/right by x units at point size y and this gets repeated across all point sizes of concern (for our project across the size range zone of 8 - 14 ppem that we target for typical use case). This provides a reasonably granular way of adjusting the size-specific instruction sets based on manual, visual review of the fonts.

Let me know how I can help with your builds Bán. Happy to help however I can.

@mrzealot
Copy link

mrzealot commented May 2, 2019

Whoa, thanks a lot for the quick answer! I seem to have managed the vanilla Hack build in the meantime (slashed zeroes ftw). Also good to know that dev already has this (will probably rebuild with that, just to have the peace of mind that it was produced with a clean repo checkout...)

However, the nerd-ification fails for some reason (the font produced is proportional), so I'll need to continue debugging there (or, alternatively, config kitty with the extra symbols from a different, release nerd font... will see which is easier.)

On linux btw, and this is just for personal "ricing" purposes. Will report back if/when I completely solve this. Thanks again!

@github-actions
Copy link
Contributor

This issue has been automatically locked since there has not been any recent activity (i.e. last half year) after it was closed. It helps our maintainers focus on the active issues. If you have found a problem that seems similar, please open a new issue, complete the issue template with all the details necessary to reproduce, and mention this issue as reference.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 13, 2023
LNKLEO pushed a commit to LNKLEO/Nerd that referenced this issue Nov 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants