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

How to have mark/m2mk features from Glyphs source? #682

Open
yanone opened this issue Jul 13, 2020 · 19 comments
Open

How to have mark/m2mk features from Glyphs source? #682

yanone opened this issue Jul 13, 2020 · 19 comments

Comments

@yanone
Copy link
Contributor

yanone commented Jul 13, 2020

I have seen mentioned in the issues that mark/m2mk feature can be auto-generated by fontmake, but I couldn't figure out how. It’s for Arabic.
I already confirmed that GDEF table correctly defines the marks in question, so the font seems to have all prerequisites.

Could you please point me in the right direction? Sorry if the question seems trivial. Thanks

@typoman
Copy link

typoman commented Jul 13, 2020

This is for kerning, same also works for mark feature:
googlefonts/ufo2ft#353 (comment)

@yanone
Copy link
Contributor Author

yanone commented Jul 13, 2020

Thank you. I'd prefer to use it from the command line. Would this be the correct way to write both kern as well as mark/mkmk features: fontmake -g "../sources/Kufam_Arabic_Latin_Roman_Master.glyphs" -o ttf -i --output-dir $TTFDIR -a --feature-writer MarkFeatureWriter --feature-writer KernFeatureWriter?

Another question, as you seem to have done this before ;):

In the feature code produced by MarkFeatureWriter, the single marks such as shadda-ar are correctly defined with their anchor positions like so: markClass shadda-ar <anchor 170 521> @MC_top;, but the combined marks like shadda_fatha-ar are missing from these definitions. I read somewhere that the glyph info needs to be set manually in Glyphs.app (script, category, sub category), but no change.
Do you know how to include them?

@typoman
Copy link

typoman commented Jul 13, 2020

I can only answer the second one. It works for me without any special setup except I have a script that I use to propagate anchors in composites from thei base glyphs. Have you checked that in your feature file, the glyphs are being decomposed in ccmp or there are anchors inside the glyph? Glyphs app propagates composite anchors during compile and I'm not sure if fontmake can reproduce the exact same result all the time.

@anthrotype
Copy link
Member

@yanone the feature is on automatically, you don't have to pass any --feature-writer option.

the combined marks like shadda_fatha-ar are missing from these definitions

you said your "GDEF table correctly defines the marks in question" -- is that shadda_fatha-ar glyph also classified as a mark in your GDEF? If a GDEF table definition is present in the UFO features.fea, ufo2ft markFeatureWriter will use that to classify bases/marks/ligatures. When compiling from .glyphs source, the GDEF table classes of the master UFOs are autogenerated using the GlyphInfo.xml database that glyphsLib comes with. Sometimes (e.g. if using custom glyph names not present in GlyphInfo.xml) you may have to set category and sub-category yourself for specific glyphs from within Glyphs.app (CMD+CTRL+I shortcut to view Glyph Info panel for selected glyph).

@yanone
Copy link
Contributor Author

yanone commented Jul 14, 2020

Yes, shadda_fatha-ar is decomposed in ccmp, although this feature is generated by Glyphs.app.
And yes, it shows up as a mark glyph in GDEF, which is created by or through fontmake.

So my only guess is that because it doesn’t have its own anchors, but is composed of two components only is is custom in Glyphs.app, that MarkFeatureWriter doesn’t create the entry.

Writing a script that propagates anchors will work, but is a crutch. This feels worth looking into. I'll look into the code and report what I find.

@anthrotype
Copy link
Member

Yes, it might be the different way glyphsLib and Glyhps.app do the anchor propagation. Georg sent me a snippet of code regartding that but have to find time to integrate that into glyphsLib.

I found out that Glyphs.app Python scripting API has a method to traverse all the anchors for a composite glyph, you can use that to propagate the anchors:

for glyph in Glyphs.font.glyphs:
	for layer in layers:
		if not layer.components:
			continue
		layer.anchors = list(layer.anchorsTraversingComponents())

Let me know if that fixes the problem, thanks.

@yanone
Copy link
Contributor Author

yanone commented Jul 14, 2020

I found out that Glyphs.app Python scripting API has a method to traverse all the anchors for a composite glyph, you can use that to propagate the anchors:

The snippet works fine, but I really don't want to do that. The whole point is to not have to have anchors in component glyphs. It kind of breaks the source imo, as anchors can move and then knowledge of these propagated anchors needs to exist to not make mistakes.

Yes, it might be the different way glyphsLib and Glyhps.app do the anchor propagation. Georg sent me a snippet of code regartding that but have to find time to integrate that into glyphsLib.

If you feel like it, I could do it; I already found MarkFeatureWriter._getAnchorLists().
It's as simple as looking at components’ anchors, too, but knowing Georg’s code would help.
My guess is that it just takes the first of the components to look at the _bottom/_top anchor; the last for the bottom/top one.

So I would be happy to implement it.

@anthrotype
Copy link
Member

@anthrotype
Copy link
Member

Ideallly this should not be done in the markFeatureWriter itself, but in glyphsLib -- or perhpas in the propagateAnchors filter in ufo2ft so UFO-based workflow could take advantate of it as well (if we deem it not too Glyphs-app specific)

@anthrotype
Copy link
Member

@yanone
Copy link
Contributor Author

yanone commented Jul 14, 2020

I know this is not what we discussed, but I've gotten the intended results by having _getAnchorLists() use the existing unchanged PropagateAnchorsFilter propagate them when a glyph has components but no anchors:

    def _getAnchorLists(self):
        gdefClasses = self.context.gdefClasses
        if gdefClasses.base is not None:
            # only include the glyphs listed in the GDEF.GlyphClassDef groups
            include = gdefClasses.base | gdefClasses.ligature | gdefClasses.mark
        else:
            # no GDEF table defined in feature file, include all glyphs
            include = None
        result = OrderedDict()
        _orderedGlyphSet = self.getOrderedGlyphSet()
        propagateAnchors = PropagateAnchorsFilter()
        propagateAnchors.set_context(self.context.font, _orderedGlyphSet)
        for glyphName, glyph in _orderedGlyphSet.items():
            if include is not None and glyphName not in include:
                continue
            anchorDict = OrderedDict()
            if glyph.components and not glyph.anchors:
                propagateAnchors.filter(glyph)
            for anchor in glyph.anchors:
                anchorName = anchor.name
                if not anchorName:
                    self.log.warning(
                        "unnamed anchor discarded in glyph '%s'", glyphName
                    )
                    continue
                if anchorName in anchorDict:
                    self.log.warning(
                        "duplicate anchor '%s' in glyph '%s'", anchorName, glyphName
                    )
                a = self.NamedAnchor(name=anchorName, x=anchor.x, y=anchor.y)
                anchorDict[anchorName] = a
            if anchorDict:
                result[glyphName] = list(anchorDict.values())
        return result

I know that the way I create the filter, or rather set the context, is quirky, because I don't fully understand ufo2ft yet.
What do you generally think about that? It seems like PropagateAnchorsFilter is already capable of doing the right thing.
There might be a better place for it instead of the MarkFeatureWriter as you said. Where could it be?

@yanone
Copy link
Contributor Author

yanone commented Jul 15, 2020

Okay, it's not that simple: I found a case where the anchors are propagated incorrectly by fontmake, in a ligature glyph with a mark. The attached screenshot is how Glyphs.app propagates the anchors correctly. fontmake instead chooses the top_2 anchor of the base glyph, just above the alef, so an additional floating mark ends up layered over the hamza.

I’ll look into it further now by actually trying to make Georg’s code work.

Bildschirmfoto 2020-07-15 um 10 49 33

@yanone
Copy link
Contributor Author

yanone commented Jul 16, 2020

Done: googlefonts/glyphsLib#617

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.

Please note that while the solution to A) is now in sync between glyphsLib and ufo2ft, the solution to B) exists now only in my glyphsLib PR, and not in ufo2ft. I find this okay since these named anchors are to my knowledge a Glyphs.app-specific solution, and their data is stored in Glyph.lib['com.schriftgestaltung.Glyphs.ComponentInfo'] to prove it.
Let me know if you'd like to see the same change in ufo2ft as well.

@madig
Copy link
Collaborator

madig commented Jul 16, 2020

(Note that we are sitting on the original Glyphs anchor propagation code Georg gave us and I think @anthrotype even implemented something in Python, but there was something I can't remember stopping us from integrating it)

@yanone
Copy link
Contributor Author

yanone commented Jul 16, 2020

Yes, I forgot to mention. Cosimo posted the code above in this thread, and I got it running, but it didn’t yield the expected result. Instead of adjusting top_2’s position, another anchor top was added to the list with the correctly adjusted position. So a rewrite of that would have been needed along the same lines as my change now in glyphsLib. I believe that Georg’s code must have changed since.

Instead I looked at the code already present in glyphsLib and improved it. Supplementary code changes of the respective commit aside, it’s essentially a two-line change, which I find a very clean solution.

So this is an incremental improvement that produces correct mark placement for Arabic, which I find critical, and doesn’t implement the entry/exit anchor propagation, which I have no experience with.

@anthrotype
Copy link
Member

I believe that Georg’s code must have changed since.

@schriftgestalt do you confirm?

@schriftgestalt
Copy link

I’m not sure what version of the code you have. I have worked on this some time ago.

@simoncozens
Copy link
Contributor

Another related problem: in Glyphs you can say

# Automatic Code End

* your feature code here *

to use both Glyphs-generated code and also your own custom code for a feature. fontmake's mark/mkmk generation doesn't support this case.

@anthrotype
Copy link
Member

anthrotype commented Aug 20, 2020

# Automatic Code End

@simoncozens see googlefonts/ufo2ft#351 and googlefonts/ufo2ft#352

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

6 participants