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

wip markFeatureWriter: only allow Nonspacing Marks to be classified as GDEF marks #392

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

Conversation

m4rc1e
Copy link
Contributor

@m4rc1e m4rc1e commented Jul 6, 2020

The current method to classify marks in the GDEF table is too greedy. If a glyph contains an anchor whose name starts with an "_", it will be classed as a mark glyph. Recently, this implementation has forced us to rollback some fonts we recently pushed

google/fonts#2453

I spoke with Cosimo last week and he had the following idea for a less greedy implementation which this PR implements.

  1. Implement the following heuristic only if the source files haven't specified a GDEF table
  2. Get all the encoded Nonspacing Marks in the font
  3. Use fontTools subset to find the unencoded Nonspacing Marks using a subset closure
  4. Iterate through every glyph and every anchor in each glyph. If the anchor starts with an "_" and the anchor's glyph isn't a Nonspacing Mark, output a warning and skip it.

I'm not too familiar with the ufo2ft lib so my implementation may be incorrect/suboptimal. My idea was to simply add this functionality to the MarkFeatureWriter._getAnchorLists function since it's already filtering anchors.

Once we're happy that I'm adding this functionality in the correct place, I still need to do the following:

  • Add tests
  • Add doc strings
  • tidyup variable names etc

@m4rc1e
Copy link
Contributor Author

m4rc1e commented Jul 6, 2020

Also on the todo, fix the existing failing tests which appears to be a dumpster fire.

@madig
Copy link
Collaborator

madig commented Jul 6, 2020

Random thought: make ufo2ft or whatever print a warning when a Unicode value suggest a non-mark but there is an underscored anchor in the glyph.

Base automatically changed from master to main March 1, 2021 09:01
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.

2 participants