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

Optionally keep empty lookups for interpolatability #3073

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

madig
Copy link
Contributor

@madig madig commented Apr 5, 2023

WIP.

For dealing with googlefonts/fontmake#894 (comment), we may want to inject empty lookups into fonts, to give varLib something to grab onto. We could do this at the feature level in ufo2ft, before passing the font to varLib to merge. This is some hacking to make feaLib not drop empty lookups.

lookup = ot.Lookup()
lookup.LookupType = 2
lookup.LookupFlag = 0
lookup.SubTable = []
Copy link
Member

@anthrotype anthrotype Apr 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure that we don't actually want tp have lookup.SubTable = None here instead of the empty list?
We need to check what varLib makes of these, whether it disinguishes one with SubTable=[] vs one with SubTable=None. In general what we want in these cases is that this particular lookup "does not participate" in variations basically.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be really interesting indeed, to allow sparse models at the lookup level.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but it's impossible with FEA because truly empty lookups have no type, it's their rules that give them their type. Unless you want to hack this by adding dummy pos .notdef .notdef 0 ...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's okay to give the lookup type 0. Except that both fonttools compiler and varLib would choke on it then. :(

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we extend the format?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could...

@@ -1069,6 +1089,8 @@ def add_lookup_call(self, lookup_name):
assert lookup_name in self.named_lookups_, lookup_name
self.cur_lookup_ = None
lookup = self.named_lookups_[lookup_name]
if lookup is None and self.cur_feature_name_ in ("kern", "dist"):
Copy link
Member

@anthrotype anthrotype Apr 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why just kern/dist? I'd leave that out unspecified.
Maybe we can have a global setting in the Builder that controls whether to emit empty lookups or not, and activate that selectively from ufo2ft when building VFs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you need to specify the subtype of the lookup and I'm not sure how to make them universal.

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

Successfully merging this pull request may close these issues.

None yet

3 participants