Chain autohinting parameters from Glyphs source to fontmake

[yanone]

I wouldn't know how to write a test for this, so unless you insist, I'm skipping

[anthrotype]

I wouldn't know how to write a test for this, so unless you insist, I'm skipping

I would insist, if you guys don't mind :)
You can make a simple test .glyphs file with one master and one instace, add the custom parameter to the instance, check that in the tests directory along side other test files, then add a new test test_main.py where you call fontmake to create the instance ttf, and then load the built TTFont and make sure that it's hinted (check for some key hinting table like fpgm or whatever).

[anthrotype]

to make the test run on the CI, you would have to first install ttfautohint. The easiest would be to use homebrew on the mac CI runner.
This reminds me we have ttfautohint-py (which is pip installable) but fontmake isn't making use of it.. #562

[yanone]

to make the test run on the CI, you would have to first install ttfautohint. The easiest would be to use homebrew on the mac CI runner.

Please advise on how to do this. I don't get the CI setup here.

[anthrotype]

Please advise on how to do this. I don't get the CI setup here.

i'm working on hooking up fontmake to ttfautohint-py, after which it'll be easier to pip install ttfautohint-py like a regular python module

[yanone]

I added a test. Please review it. I'm now quietly assuming that TTFAutohint is installed via brew.

[yanone]

I'm now quietly assuming that TTFAutohint is installed via brew.

Of course it fails now. Waiting for further instructions

[anthrotype]

After #851 is merged on main, you should be able to merge main into your PR branch (or rebase on main) and your test should just work

[anthrotype]

@yanone I merged #851, so when you can please git pull, then git merge main and push.

[yanone]

At the expense of exposing my ignorance, I can't get this to run. Neither of your proposed commands do anything in my fork, be it main branch or the PR's branch. I'm not good with working with branches. I normally update my fork with git merge upstream/main, but that also doesn't work: Already up to date.

[anthrotype]

yes, that's what I mean git merge upstream/main. But first you need to git fetch upstream otherwise you don't see the newly added commits

[yanone]

So that line with the lib key is too long for flake8, which is why I originally broke it.

[anthrotype]

Add # noqa: B950 next to it to mute flake8

[yanone]

And the pypy test fails (to install ttfautohint-py): https://github.com/googlefonts/fontmake/runs/4982698831?check_suite_focus=true#step:5:510

[anthrotype]

And the pypy test fails

pls ignore that for now, I'll investigate and fix that separately.

The PR looks good to me, I'll merge and take over from here. Thank you!

[yanone]

Thanks

[anthrotype]

one last thing, actually. That test .glyphs file you added, what's its license? If you don't add any special license file for it, it'll automatically get the license of the rest of the codebase.

Also, it's quite big and long, can you make a subset that contains only a couple glyphs? We don't need the full font just to exercise the ttfautohint code path.

[yanone]

It's OFL, so I didn't care. I already subsetted it to just the basic Latin A-Z and a-z. First I had just the A in it, but ttfautohint required a bunch of different characters to take measurements from, so I re-added the basics. I would have to dive into ttfautohint to make the minimum subset.

[anthrotype]

I filed fonttools/ttfautohint-py#11 for ttfautohint-py failing on pypy3. I'll just skip the test on pypy for now.