-
Notifications
You must be signed in to change notification settings - Fork 167
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
stemHist finds different stem lists on Windows than on Mac. #210
Comments
It’s even better than that. Lists for the same source UFO differ across Macs. |
@frankrolf do you have any test fonts to reproduce this? |
font.ufo.zip In general, the resulting report files slightly differ from machine to machine. |
@frankrolf in #552 I made changes that stabilize the sort order, and remove duplicate entries, but none of those changes fix the inconsistencies across environments you've reported, because the bug is really in autohintexe. stemhist basically just reformats the data provided by the reports ( Mac
Win
It seems that this problem may be mostly happening with sans-serif glyphs. My guess is that it has something to do with the creation of ghost hints; these don't get generated if the edge of a vertical stem is not within an alignment zone. As you know, autohint is getting replaced by psautohint. The latter doesn't have the report options yet, but there's this open issue to address it https://github.com/adobe-type-tools/psautohint/issues/71 /cc @khaledhosny |
@khaledhosny I've built |
Tracking in adobe-type-tools/psautohint#71 is fine. |
Can you try with the autohintexe binary from https://ci.appveyor.com/project/adobe-type-tools/psautohint/build/1.0.627 (check the artifacts of the relevant build)? |
@khaledhosny with that build the Win output is now the same as the Mac. But I think the extra lines in the Win output were correct. |
The changes I did in this branch are fixing undefined behavior issues related to bit shifting and integer overflow, so I don’t know what output is correct, it is just now that the compiled code does not rely on any undefined behavior (which with some compilers and aggressive optimizations, can result in code being optimized away in unexpected ways). We should now debug this and see why these extra lines are missing. |
@khaledhosny is this fixed? |
I still need to find out whether the old Windows output was the correct one, but the inconsistency between platforms should be fixed. |
OK. The reason why I think the extra lines in the Windows output were correct is because |
I’m rather lost here. So far, this seems to be caused by |
@khaledhosny can you add a unit test for |
We don’t currently have the infrastructure to add unit tests for functions deep inside the C library like this one. |
This issue has been eluding you for weeks. Is that not a strong enough motivation to get C unit testing off the ground? |
It is not that I don’t like having unit tests, it is that I’m not sure it is as easy as it sounds. The code uses global variables a lot and most of the active state; the parsed paths, the hints, and what not are all in global variables, so I’m not sure how this all will be possible to cleanly test single functions. Based on experience, it is going to be a multi-week small project if not more. |
Got it, thanks. I'll work with the latest version and log any issues I find. Is version 1.9.0 stable? |
1.9.0b2 should be stable, if you can give it a try and if everything is OK I can cut a final release. |
Added a test case using the font and command provided and the CI is green. Closing as fixed. |
No description provided.
The text was updated successfully, but these errors were encountered: