-
Notifications
You must be signed in to change notification settings - Fork 169
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
Add python otfautohint
into afdko
#1629
Conversation
I kept the Anyway, we can decide what to do about it. |
A few notes on tests, while they're at the front of my mind: The tests in the Accordingly, the most of the (top-level) contents of the "dummy" directory of the submodule are for unit tests, or at least fairly specific tests. The main exception is the "mm0" directory, which is also the largest (29 out of 33 megs). It would probably make sense to move that stuff, minus mm0, into As for the rest, it mostly seems to involve hinting whole open-source fonts, either individually or as part of multi-master or VF workflow. I think the main thing this buys is diversity of geometry to see if the code crashes. It also arguably involves a correctness test, in that we check to see if the output has changed, but absent actual evaluation of the output it's not clear what is "correct". If someone changes the tool somewhat, would we do anything other than copy the modified output over so that the tests pass again? I did some checking with hintdiff but I know that was broadly my plan with the port. In any case, one thing we could do is make a single font or a modest number of small fonts with "interesting" glyphs pulled from that larger set, and check those into |
I have now ported most of the tests over. Right now there is a largish-subdirectory called "dummy/mm0" which is used for basic multi-master hint testing but the output is never evaluated. The glyphs are from some san-serif font although the UFO metadata says "source serif". Here is one way to get this PR to where it can be integrated:
Later, when we've added the UFO support for FDArray/FDSelect to otfautohint we can also make UFO versions of these sample fonts. |
There's one test that hints a glyph with a huge number of points that now takes an annoyingly long time with the python code. Since we normally skip these I'm not sure it's worth running this test routinely -- it will probably get annoying. |
otfautohint
into afdko (still missing most tests)otfautohint
into afdko (still missing some tests)
Here is a list of tests that have been entirely ported over in terms of the files they're stored in in the psautohint repository:
Partial ports:
I believe these files aren't relevant, mostly because we don't use bez anymore and/or because the C code had hard array limits that the python code doesn't:
We might want to disable test_big_glyph in test_hint.py given how long it takes to run now. |
I backed out the _version.py file to a hard-coded version like the other facilities, bumped to v3.0.0. I also modified the tests to skip big_glyph_test. In addition, I added this branch and draft PR to psautohint-tests: adobe-type-tools/psautohint-testdata#5 . |
otfautohint
into afdko (still missing some tests)otfautohint
into afdko
Topics for messaging:
|
We agreed in discussion that the minimal duplication of code can be cleaned up later. And with that, I think this PR is ready to be evaluated for merging. |
FYI — GitHub is having a hard time loading the diffs even after I filter out the .glif files, so when the page is not responsive I'll directly comment here & include the file & lines I'm referring to. |
Is there a definition of what |
|
Well, I would say that the definition is in the lines you mention in the comment to pt.setAlign. In terms of interpretation "a" is meant to suggest "aligned" and "o" is meant to suggest "opposite", and I guess we could add that somewhere. If you look at the C code of the current psautohint there are separate implementations for vertical and horizontal stem hinting, with many functions that (mostly) differ only in having "x" and "y" swapped. The cleanest way of combining the implementations is to have a property on the |
Remove Python dependency on psautohint package Fix up more references to otfautohint
Port the "non-bulk" tests into the repository
Disable time-consuming big_glyph_test
Correct object path for otfstemhist
OK, I pushed some code-review related changes. This doesn't resolve all of the concerns in hinter.py, but I did add a "conventions" comment to the top of that. We can talk about that stuff. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Skef and I talked about some things offline and I reviewed his latest commit — this looks great!
Description
This PR adds the Python port of
psautohint
back into afdko asotfautohint
. Although the new name is more accurate, the primary motivation is to allow the older C-basedpsautohint
to be installed and used simultaneously.Some of the tests are missing from this PR because they depend on a git submodule, and we need to decide if we're going to retain that convention or do something else. For now these are represented in adobe-type-tools/psautohint-testdata#5 .
There is also some duplicate code, mostly between
python/afdko/convertfonttocid.py
andpython/afdko/otfautohint/fdTools.py
. We should decide what if anything to do about that.There's a bit of documentation but no added messaging about the changes.
Most of the changes to the
makeinstancesufo
tests are just updated hinting, but I did need to change theblueFuzz
for the bold instance because otherwise it had a pair of stems too close together. Not sure why the older hinter wasn't catching that.As we discussed I did not include a command line script for
splitpsdicts
but it can be run asChecklist: