googlefonts / ufo2ft

A bridge from UFOs to FontTools objects (and therefore, OTFs and TTFs).
MIT License
151 stars 43 forks source link

FeatureWriter: How to add to an existing feature from another FeatureWriter? #596

Closed yanone closed 2 years ago

yanone commented 2 years ago

I’m currently fixing my own situation as noted in a previous issue, and for that I'm implementing my own FeatureWriter.

I'm almost there, but need one little nudge: How do I add a lookup to the mkmk feature that the MarkFeatureWriter has previously written?

Currently, it's creating a second mkmk feature with my code, which obviously won't work:

feature mkmk {
    # Previous mkmk feature
} mkmk;

# My mkmk feature:

lookup post_mkmk {
    pos behDotless-ar twodotshorizontalabove-ar fatha-ar' <-300 0 -300 0>;
} post_mkmk;

feature mkmk {
    script DFLT;
    language dflt;
    lookup post_mkmk;
} mkmk;

This is my code so far:

class MarkPosFeatureWriter(BaseFeatureWriter):
    tableTag = "GPOS"
    features = frozenset(["mkmk"])
    mode = "append"

    def setContext(self, font, feaFile, compiler=None):
        ctx = super().setContext(font, feaFile, compiler=compiler)
        print(font)
        return ctx

    def _makeFeatureBlocks(self, lookups):
        features = {}
        mkmk = ast.FeatureBlock("mkmk")
        ast.addLookupReferences(mkmk, lookups, script="DFLT")
        features["mkmk"] = mkmk
        return features

    def _makeLookups(self):
        lookups = []

        rules = []

        valuerecord = ast.ValueRecord(
            xPlacement=-300,
            yPlacement=0,
            xAdvance=-300,
            yAdvance=0,
        )
        rules.append(
            ast.SinglePosStatement(
                [[ast.GlyphName("fatha-ar"), valuerecord]],
                [ast.GlyphName("behDotless-ar"), ast.GlyphName("twodotshorizontalabove-ar")],
                [],
                False,
            )
        )

        if rules:
            lookup = ast.LookupBlock("post_mkmk")
            lookup.statements.extend(rules)
            lookups.append(lookup)

        return lookups

    def _write(self):
        lookups = self._makeLookups()
        features = self._makeFeatureBlocks(lookups)

        # extend feature file with the new generated statements
        feaFile = self.context.feaFile

        # lookupGroups = []
        # for _, lookupGroup in sorted(lookups.items()):
        #     lookupGroups.extend(lookupGroup)

        self._insert(
            feaFile=feaFile,
            lookups=lookups,
            features=[features[tag] for tag in ["mkmk"] if tag in features],
        )
        return True
yanone commented 2 years ago

After much digging into the code, I figured out how to manipulate existing features:

class MarkPosFeatureWriter(BaseFeatureWriter):
    tableTag = "GPOS"
    features = frozenset(["mkmk"])
    mode = "append"

    def setContext(self, font, feaFile, compiler=None):
        ctx = super().setContext(font, feaFile, compiler=compiler)
        print(font)
        return ctx

    def _makeLookups(self):
        lookups = []

        rules = []

        valuerecord = ast.ValueRecord(
            xPlacement=-200,
            yPlacement=0,
            xAdvance=-200,
            yAdvance=0,
        )
        rules.append(
            ast.SinglePosStatement(
                [[ast.GlyphName("fatha-ar"), valuerecord]],
                [ast.GlyphName("behDotless-ar"), ast.GlyphName("twodotshorizontalabove-ar")],
                [],
                False,
            )
        )

        if rules:
            lookup = ast.LookupBlock("post_mkmk")
            lookup.statements.extend(rules)
            lookups.append(lookup)

        return lookups

    def _findFeatureBlock(self, name):
        feaFile = self.context.feaFile
        for statement in feaFile.statements:
            if type(statement) == ast.FeatureBlock and statement.name == name:
                return statement

    def _write(self):
        lookups = self._makeLookups()

        mkmk = self._findFeatureBlock("mkmk")
        mkmk.statements.extend(lookups)

        return True

Closing this. Thank you.

twardoch commented 2 years ago

The fact that you cannot do:

feature smcp { 
  SOME CODE
} smcp; 

feature smcp { 
  SOME OTHER CODE
} smcp; 

is actually very stupid and limiting.

It works if you do

lookup smcp1 { 
  SOME CODE
} smcp1; 

lookup smcp2 { 
  SOME OTHER CODE
} smcp2;

feature smcp { 
  lookup smcp1;
  lookup smcp2;
} smcp;  

and just repeating feature blocks should ideally work as well, without the need to employ programmatic code.

The syntax spec does not explicitly disallow repeating the feature keyword with the same feature tag, but does not define any behavior either. The AFDKO implementation does not allow such a thing, and feaLab follows that.

But life would be easier if it was possible to use the same feature block repeatedly, for example when defining features for different scripts.

twardoch commented 2 years ago

https://github.com/adobe-type-tools/afdko/issues/1477

khaledhosny commented 2 years ago

and just repeating feature blocks should ideally work as well, without the need to employ programmatic code.

It does. makeotf will warn but make it work anyway, feaLib does not even warn.