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

Improve anchor propagation #617

Merged
merged 23 commits into from
Jul 30, 2020

Conversation

yanone
Copy link
Contributor

@yanone yanone commented Jul 16, 2020

This PR is addressing two separate issues of anchor propagation:

A) marks that are in turn composed of other marks (e.g. shadda_fatha-ar) didn’t receive propagated anchors at all
B) ligature anchors (top_1, top_2 etc) weren’t correctly propagated when composed of base glyphs and a mark (e.g. lam_alefHamzaabove-ar).

The solution to A) was already developed in ufo2ft’s PropagateAnchorsFilter, so I simply copied the code over.

The solution to B) is to check for marks attached to named anchors (stored in Glyph.lib['com.schriftgestaltung.Glyphs.ComponentInfo']) and override the named anchor’s position with the transformation result.

Copy link
Member

@anthrotype anthrotype left a comment

Choose a reason for hiding this comment

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

Thanks @yanone for working on this.

Ideally I would like to avoid keeping duplicate pieces of code in glyphsLib and ufo2ft (the latter is copied from the former), and instead have ufo2ft only do the anchor propagation at build time, instead of glyphsLib doing this at ufo-export time.
The PropagateAnchorsFilter is disabled by default in ufo2ft. One can enable it by setting/appending the desired filter in the UFO lib (like Cantarell fonts do.), e.g.

ufo.lib["com.github.googlei18n.ufo2ft.filters"] = [{"name": "propagateAnchors", "pre": True}]

(the pre boolean flag means to run this filter before the default component decomposition step).

The reason we haven't done this yet (and kept the current duplication) is because we haven't found the time to fully understand and integrate the anchor propagation code that Georg shared, and besides there are Glyphs-specific features and hacks for which there are no ready-made solutions in a UFO-only workflow, such as the named anchors for auto-aligned components that are currently dumped to the private glyph.lib.

At some point someone should document how this composite anchor propagation is supposed to work and we could generalize it such that it works the same for either a Glyphs- or UFO-based workflow.

However, I may be tempted to live with the current code duplication and merge in a fix like yours for the time being until we figure that out.

I left some comments below, pls take a look thanks.

Lib/glyphsLib/builder/anchors.py Show resolved Hide resolved
Lib/glyphsLib/builder/anchors.py Outdated Show resolved Hide resolved
Lib/glyphsLib/builder/anchors.py Outdated Show resolved Hide resolved
@anthrotype anthrotype requested a review from madig July 22, 2020 13:09
@yanone
Copy link
Contributor Author

yanone commented Jul 29, 2020

@anthrotype
With the exception of the bounds calculation I've addressed all your remarks. Please have another look.

@anthrotype
Copy link
Member

i'll take a look soon, thanks

@madig
Copy link
Collaborator

madig commented Jul 30, 2020

Promote the mark that is positioned closest to the origin to a base.

Random thought: I have written at least two different algorithms for determining the base glyph. It's almost as if there should be some form of semantics for this, somewhere.

as suggested by madig
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.

4 participants