-
Notifications
You must be signed in to change notification settings - Fork 14
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
Use other tables generated by fea-rs #406
Comments
I'm inclined to say we should start by erroring if fea gave us back any table we don't deal with |
We'll want to check which things fontmake accepts to be defined by fea and how it handles multiple definitions |
Fontmake accepts anything that fonttools feaLib returns with no questions, those are not multiple definitions but rather "overrides" |
That's how Adobe FEA spec describes these, and also how makeotf interprets them |
Is there any case where you have to merge the results or can you always take teh whole table from fea? - for example, I think Colin said you can set modified time in fea and that made me think you'd need to take just that field and copy it into the table you built from non-fea sources. If we can just always take the whole table that would be nice. |
You'd take the whole thing, the feature compiler is responsible for applying the overrides |
If the fea compiler is to apply overrides it would have to have access to the thing it is to apply them to? And that makes for all sorts of execution order limitations, e.g. if fea can override modified in |
I think that having the fea compiler be responsible for merging is a bit of a layering violation; you should be able to compile FEA without knowing much about the final font. I would propose a design where |
feature files can end up creating or modifying a bunch of different tables; see https://github.com/cmyr/fea-rs/blob/main/fea-rs/src/compile/output.rs#L22 for the list. It is likely that many of these are rarely used, but at the very least we should be checking if they're present.
The text was updated successfully, but these errors were encountered: