Skip to content

[feaLib] Upgrade single substitutions mixed with ligature ones #3805

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

Merged
merged 2 commits into from
Apr 23, 2025

Conversation

khaledhosny
Copy link
Collaborator

If a lookup has only single and ligature substitutions, we now upgrade single substitutions to ligature ones. Previously this would cause an error in named lookup, and would result in splitting lookups in anonymous ones.

This is similar to what we have done with multiple substitutions in #1133, but back then I incorrectly convinced myself that ligature substitutions can’t have a single source glyph, which is obviously not true.

If a lookup has only single and ligature substitutions, we now upgrade
single substitutions to ligature ones. Previously this would cause an
error in named lookup, and would result in splitting lookups in
anonymous ones.

This is similar to what we have done with multiple substitutions in
#1133, but back then I
incorrectly convinced myself that ligature substitutions can’t have a
single source glyph, which is obviously not true.
Doing this is Block.build() is less of a hack than modifying the parsed
ast tree during parsing.
@behdad
Copy link
Member

behdad commented Apr 19, 2025

I incorrectly convinced myself that ligature substitutions can’t have a single source glyph, which is obviously not true.

We even have code in HB to treat them as single substitutions not ligatures, for further processing purposes.

@khaledhosny khaledhosny merged commit 85ff960 into main Apr 23, 2025
11 checks passed
@khaledhosny khaledhosny deleted the upgrade-single-sub-to-ligature branch April 23, 2025 09:44
cmyr added a commit to googlefonts/fontc that referenced this pull request May 21, 2025
This was added to fonttools a few months back (fonttools/fonttools#3805);  this patch gets us back to parity.
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.

3 participants