googlefonts / glyphsLib

A bridge from Glyphs source files (.glyphs) to UFOs
Apache License 2.0
178 stars 51 forks source link

Add mark filtering to context anchor #910

Open MariannaPaszkowska opened 1 year ago

anthrotype commented 1 year ago

are you going to provide a bit more context to understand what this is about? thanks :)

schriftgestalt commented 1 year ago

The anchor context can now have a lookup flag prefix. When you could do C * before, now you can do:

lookupflag UseMarkFilteringSet @CombiningTopAccents;C *

Everything before the semicolon is supposed to be at the beginning of the lookup. I do a dict lookup with the prefix to collect all rules with the same prefix.

If there are any questions, we can have a meeting.

simoncozens commented 1 year ago

Even without the mark filtering, we don't currently support contextual anchors at all - but I'm pretty sure we will by the end of today...

schriftgestalt commented 1 year ago

I thought @khaledhosny had worked on it?

khaledhosny commented 1 year ago

No, I didn’t do any work related to contextual anchors.

simoncozens commented 1 year ago

OK, I have this working now, at least as a custom MarkFeatureWriter subclass. The question is what is best to do with it:

1) Make it the default MarkFeatureWriter. This involves embedding Glyphs-specific conventions (anchor names, context in object lib) in ufo2ft, which is a bit non-standard but doesn't do any harm to anyone who doesn't use anchor names starting with an asterisk, and is cleaner in terms of the pipeline. 2) Make a glyphsLib-specific feature writer, and set the UFO feature writer lib key to use it.

(2) is probably the right choice now I come to look at it.

simoncozens commented 1 year ago

I've done 2 and chucked it into a branch (https://github.com/googlefonts/glyphsLib/tree/contextual-mark-writer) so I can use it. It's not ready for PR yet as it doesn't handle the mkmk case.

khaledhosny commented 1 year ago

My vote for 2) as well, and it can be also extended to ignore anchors that Glyphs ignores (IIRC any anchors starting with non-letter, other than * and _ of course).

anthrotype commented 1 year ago

set the UFO feature writer lib key to use it

note that setting that lib key overwrites the default list of writers, which means if you still want the default ones to be run you'll have to list them explicitly in the list. We added an ... placeholder to mean the default writers, but that only works when you use the fontmake CLI option --feature-writers or when you call ufo2ft compile methods and pass the featureWriters parameter.

simoncozens commented 1 year ago

We have a DEFAULT_FEATURE_WRITERS constant in glyphsLib constants, which we check when round-tripping. So I did this: https://github.com/googlefonts/glyphsLib/commit/7dacee459a3a099c8a4ce31327c9bc04111d4817

anthrotype commented 1 year ago

yeah but we stopped using that DEFAULT_FEATURE_WRITERS at some point because it was outdated, ufo2ft gained another default writer I belive (gdefFeatureWriter?) and if we hard-coded the list we wouldn't get that

anthrotype commented 1 year ago

maybe we need to extend support for the ellipsis ... placeholder mechanism to the featureWriters lib key in addition to the parameter override

simoncozens commented 1 year ago

Or we can grab it from ufo2ft.featureCompiler.FeatureCompiler.defaultFeatureWriters.

anthrotype commented 1 year ago

we decided not to do that as it would involve adding ufo2ft as a dependency of glyphsLib...

simoncozens commented 1 year ago

Ack, I was confused by the fact that the existing glyphsLib filters import ufo2ft modules, so I thought it was already a dependency; but of course they are running within a ufo2ft context.

The problem with the ... prefix is that we want to use our own MarkFeatureWriter instead of ufo2ft's version, not as well as it. So we can't just use ... to call the default.

anthrotype commented 1 year ago

can we not change the ufo2ft feature writer instead so both glyphs and ufo will gain from this?

simoncozens commented 1 year ago

We could but it's kind of ugly, and the conventions involved aren't a UFO standard (or likely to become one).

Counter-thought: If ufo2ft grows more feature writers in the future, would we actually want them? Maybe, maybe not, but we want control over that. Having a list of feature writers that are specific to a Glyphs compile makes a lot of sense; we should define that list ourselves in glyphsLib.

anthrotype commented 1 year ago

we should define that list ourselves in glyphsLib.

ok but at least we should try to keep it updated

simoncozens commented 1 year ago

My point is we don't want to keep it updated with ufo2ft, in the sense that we don't want to update our list of feature writers just because ufo2ft has. If ufo2ft does something new, we only want to implement that if it's something that Glyphs also does.