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

Clarification of logic in _build_public_opentype_categories #1024

Open
cmyr opened this issue Aug 21, 2024 · 6 comments
Open

Clarification of logic in _build_public_opentype_categories #1024

cmyr opened this issue Aug 21, 2024 · 6 comments

Comments

@cmyr
Copy link
Member

cmyr commented Aug 21, 2024

Investigating another minor difference between fontc and fontmake. When assigning the 'ligature' class, the stated logic involves two checks:

  • that the glyph has the 'Ligature' subcategory,
  • and the glyph has an "attaching anchor".

However the logic for deciding if something is an attaching anchor is very simple and considers any anchor that does not start with an underscore to be an "attaching anchor":

has_attaching_anchor = False
for anchor in glyph.anchors:
name = anchor.name
if name and not name.startswith("_"):
has_attaching_anchor = True

This has the concrete consequence that caret anchors ('caret_1', etc) are considered 'attaching anchors'. As these anchors are common in ligature glyphs, this happens to mean that various ligature glyphs will end up in the ligature category, even if they do not have any attaching marks.

Is this behaviour desired? If it is, then I think this can be addressed with a comment, or by renaming the has_attaching_anchors variable. If it is not desired, then the logic should be reworked somewhat.

@anthrotype
Copy link
Member

caret anchors ('caret_1', etc) are considered 'attaching anchors'.

yeah I noticed that too, the logic is arguably too liberal.
On the other hand, the fact that glyphs which contain caret_ sort of anchors (and which have subcategory 'Ligature') are classified in GDEF as ligatures is not that bad.. It certainly would not be un-desirable.

@khaledhosny
Copy link
Collaborator

It should be enough to consider a glyph a ligature when it has a Ligature subcategory, why do we check for anchors at all?

@anthrotype
Copy link
Member

we could even go further and ignore the attaching-anchor check, and just take any glyph whose subCateogory is 'Ligature' to be in a ligature class for GDEF.

honestly I'm not entirely sure why the code currently restricts it to only ligatures with (non-underscore-prefixed) anchors; the GDEF ligature class can potentially be of use e.g. setting lookupflag IgnoreLigatures independently of them having anchors or not

@anthrotype
Copy link
Member

ofc khaled beat me to it

@khaledhosny
Copy link
Collaborator

"""Returns a dictionary mapping glyph names to GDEF categories.
Does not handle ligature carets. A GDEF table with both categories and
ligature carets is generated by ufo2ft's GdefFeatureWriter at compile time,
using the categories dict and glyph data.
Determining the categories requires anchor propagation or user care to work
as expected, as Glyphs.app also looks at anchors for classification:
* Base: any glyph that has an attaching anchor (such as "top"; "_top" does
not count) and is neither classified as Ligature nor Mark using the
definitions below;
* Ligature: if subCategory is "Ligature" and the glyph has at least one
attaching anchor;
* Mark: if category is "Mark" and subCategory is either "Nonspacing" or
"Spacing Combining";
* Compound: never assigned by Glyphs.app.
See:
* https://github.com/googlefonts/glyphsLib/issues/85
* https://github.com/googlefonts/glyphsLib/pull/100#issuecomment-275430289
"""

Someone needs to re-read the linked old discussions, though I feel the check for the anchors is superfluous, or at least should be a fallback when no explicit subcategory is set.

@cmyr
Copy link
Member Author

cmyr commented Aug 21, 2024

okay so for the time being I am going to match the implementation here, and treat any anchor that doesn't begin with '_' as 'an attaching anchor'.

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

No branches or pull requests

3 participants