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

[varLib.merger] Limitation in _PairPosFormat2_merge #888

Closed
miguelsousa opened this issue Mar 15, 2017 · 11 comments
Closed

[varLib.merger] Limitation in _PairPosFormat2_merge #888

miguelsousa opened this issue Mar 15, 2017 · 11 comments

Comments

@miguelsousa
Copy link
Collaborator

miguelsousa commented Mar 15, 2017

While trying to build a variable version of Source Sans I ran into this documented limitation,

# Currently, if the coverage of PairPosFormat2 subtables are different,
# we do NOT bother walking down the subtable list when filling in new
# rows for alignment.  As such, this is only correct if current subtable
# is the last subtable in the lookup.  Ensure that.
# TODO: Remove this requirement

I've put together a test case example in the Source Sans repository in a branch named _PairPosFormat2_merge to help addressing this limitation.

@behdad
Copy link
Member

behdad commented Mar 16, 2017

Thanks for the concise test case!! Looking into it. I think I know of a simple fix...

@behdad behdad closed this as completed in 49d3115 Mar 17, 2017
@behdad
Copy link
Member

behdad commented Mar 17, 2017

Would be great if you can add a test case for it. Thanks.

@miguelsousa
Copy link
Collaborator Author

Will do. Thanks!

@miguelsousa
Copy link
Collaborator Author

Something isn't right. I've spot-checked a few kern pairs and they're not working in the variable font, whereas they're fine in the master sources. See the FontView screenshots below. The result in CoreText is the same (tested via axis-praxis).

vf-master0

vf-master1

master0

master1

@miguelsousa miguelsousa reopened this Mar 17, 2017
@behdad
Copy link
Member

behdad commented Mar 18, 2017

Fix coming soon.

Note that CoreText as far as I know doesn't support GSUB / GPOS variations yet.

@behdad behdad closed this as completed in 9798c30 Mar 18, 2017
@miguelsousa
Copy link
Collaborator Author

Thanks Behdad. The kerning now seems to be working in the Black (master1) but it's still not in the ExtraLight (master0). Do you think that this is now a FontView/FreeType/HarfBuzz issue?

vf-master0

master0

vf-master1

master1

@miguelsousa
Copy link
Collaborator Author

miguelsousa commented Mar 18, 2017

I've made new master fonts that don't have Greek and Cyrillic class kerning and put them in a branch named no-greek-cyrillic-kerning. While building these fonts there were no Attempting to fix OTLOffsetOverflowError messages.

The variable font made with these new masters works fine in FontView, in terms of kerning. While building the VF there were two Attempting to fix OTLOffsetOverflowError messages.

I think that based on this we can rule out a bug in FontView.

@miguelsousa miguelsousa reopened this Mar 18, 2017
@behdad
Copy link
Member

behdad commented Mar 19, 2017

Thanks Miguel. I'll debug.

Sorry for not properly testing my fixes. I'm of very limited time right now :(.

@miguelsousa
Copy link
Collaborator Author

miguelsousa commented Mar 19, 2017

No worries. I appreciate any time you can dedicate to this. I'd fix it if I knew how.

@behdad behdad closed this as completed in 8654931 Apr 6, 2017
anthrotype pushed a commit to googlefonts/fontmake that referenced this issue Apr 8, 2017
Fixes issue in fontTools.varLib.merger with recombining multiple
PairPosFormat2 subtables (fonttools/fonttools#888)
@miguelsousa
Copy link
Collaborator Author

Thank you very much for the fix Behdad. AFAICT the GPOS merging of that test variable font is working as expected now 🎉 🏆 🎉 🏆 🎉 🏆

However I'm sorry to say that I have another example v-font that ends up with non-functioning kern pairs 😢 The difference between the set of source TTFs referenced in this issue and my other TTFs
is that the GPOS tables of the former were made via fontmake/feaLib, whereas in the latter they were made via makeotf.

I'm going to add test cases to fontTools for the examples that are working, and then provide test files that will show what's not working. I'll open a new issue instead of reopening this one, since I don't know if it's the same exact problem.

@behdad
Copy link
Member

behdad commented Apr 11, 2017

Thanks. I'll take a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants