googlefonts / glyphsLib

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

Supports Rename Glyphs parameter #735

Open RosaWagner opened 3 years ago

RosaWagner commented 3 years ago

Some issues are talking about it #498 #126 and this one aims to list all the useful parameters that could be handled from source:

We are trying to unroll an update of cormorant which is creating different families by swapping stylistic sets, it looks like that in sources:

Capture d’écran 2021-10-21 à 14 43 38

I see how I could reproduce the remove glyphs and remove feature with fonttools subsetter, but not the Rename Glyphs thing though (although making additional post-export script doesn't help the designer to use fontmake). We made a little test case for this particular parameter in case you want to implement it 😬

test-swap.zip Exported result should be a circle.

cc @emmamarichal

anthrotype commented 3 years ago

while it'd be nice to support all the myriad of custom parameters eventually, in the meantime I believe you should be able to achieve what you need by using the fonttools subsetter (to remove features and glyphs), and perhaps also @twardoch's pyftfeatfreeze script from https://github.com/twardoch/fonttools-opentype-feature-freezer to get what the "Rename Glyphs" does (set .ss02 glyphs as the default ones).

RosaWagner commented 3 years ago

yeah I was expecting that answer. Thanks for the tips with pyftfeatfreeze.

anthrotype commented 3 years ago

at least some of the custom params you mention should be supported already. IIRC, we do support Keep Glyphs and Replace feature. Doesn't Remove Glyphs achieve the same thing as Keep Glyphs (only reversed)? Same for Remove Feature, you can replace it with an empty one.. I'm trying to understand which one is really needed, and which one kind of isn't

anthrotype commented 3 years ago

as for "Remove class", I think we do support "Replace Prefix"

RosaWagner commented 3 years ago

Ah interesting, I'll look into the replace feature tricks, thanks. Reversing the Keep Glyphs is a tiny bit more laborious when only 3 glyphs need to be removed but yes I can do that ;)

So my biggest issue atm is then the Rename Glyphs parameter. Also because I see it more and more in font projects.

MariannaPaszkowska commented 2 years ago

➕1 to add this functionality. I tried pyftfeatfreeze script, and it seems to change a bit too much in the font. I have a lot of unrelated diffs.

anthrotype commented 2 years ago

which one you mean? Rename Glyphs?

anthrotype commented 2 years ago

How is Rename Glyphs supposed to work exactly? I found this thread which may be relevant: https://forum.glyphsapp.com/t/swapping-two-characters-on-generate-using-a-custom-parameter/3582/4

RosaWagner commented 2 years ago

Rename glyphs swap variant glyphs with base glyphs, or else. For exemple, in one version of the font you want a.ss01 in base glyph and former /a to become a.ss01.

anthrotype commented 2 years ago

this blog post (esp. section "2. Alternate glyphs") deals with Rename/Remove Glyphs: https://glyphsapp.com/learn/switching-shapes

anthrotype commented 2 years ago

Thanks Rosalie. I also found this in the Handbook: https://glyphsapp.com/media/pages/learn/3ec528a11c-1634835554/glyphs-3-handbook.pdf

13.7.2 Replacing Glyphs at Export Glyphs can be replaced by other glyphs at export for both static instances and variable font settings. For this, two custom parameters must be added in File → Font Info… → Exports. Firstly, add the ‘Rename Glyphs’ custom parameter, click its value to edit it, and write one glyph swap per line. A glyph swap is written as someglyph=otherglyph. This switches the two glyphs in the exported font. For instance, switch the dollar glyph for a simplified dollar.alt by adding the line dollar=dollar.alt. List all glyphs with their replacement and confirm with OK. Add the ‘Remove Glyphs’ custom parameter to not just swap, but replace glyphs. Click its value to edit it and list all alternative glyphs that now, after the swap, contain the normal glyph outlines.

simoncozens commented 2 years ago

Keep Glyphs works; it is implemented in fontmake, not in glyphsLib or ufo2ft. I feel like jamming Glyphs-specific custom parameter stuff into fontmake is not the best architecture, and perhaps that should have been implemented as a custom parameter handler on the GSInstance, as we have done with the other similar custom parameters. At any rate, it's funny that Keep Glyphs is the odd one out in that respect.

For Remove Glyphs, it would be much easier to support it as a custom parameter handler on the instance:

class RemoveGlyphsParamHandler(AbstractParamHandler):
    def to_ufo(self, builder, glyphs, ufo):
        remove_list = glyphs.get_custom_value("Remove Glyphs")
        if not remove_list:
            return
        ufo = ufo._owner
        for entry in remove_list:
            del ufo[entry]
            # and we're done

    def to_glyphs(self, glyphs, ufo):
        pass

But it seems like we decided not to support them: https://github.com/googlefonts/glyphsLib/issues/295#issuecomment-397267190

simoncozens commented 2 years ago

Remove Features is also easily implementable as a custom parameter handler. Popping that here until we work out which ones we're actually planning on supporting:

class RemoveFeaturesParamHandler(AbstractParamHandler):
    def to_ufo(self, builder, glyphs, ufo):
        tags = glyphs.get_custom_values("Remove Features")
        # List of lists, for some reason
        if not tags:
            return
        for tag in tags[0]:
            ufo._owner.features.text = replace_feature(
                tag, "", ufo._owner.features.text or ""
            )

    def to_glyphs(self, glyphs, ufo):
        pass

register(RemoveFeaturesParamHandler())

The class-related custom parameters are:

anthrotype commented 2 years ago

[Keep Glyphs] should have been implemented as a custom parameter handler on the GSInstance, as we have done with the other similar custom parameters

yes, I agree that's odd. I think it's because it uses the fonttools subsetter and requires an ttf/otf binary font to work with. Doing it at the source level would require us to be able to also subset feature files, which at least at that time wasn't available (not sure now, but Simon probably can).

but we decided not to support them

If it's that easy to add Remove Glyphs, feel free to add it. Same for Remove Features, as you proposed in that snippet above.

I'm ok if we eventually add them all, it's just that there are so many of them and I don't have the time to implement them all. But if others will contribute them, sure!

simoncozens commented 2 years ago

Doing it at the source level would require us to be able to also subset feature files

The Glyphs app does not handle this so intelligently. If you have a manually created feature file which mentions glyphs which are removed using Remove Glyphs, it just fails to export. We can easily achieve parity with that.

anthrotype commented 2 years ago

hm ok, though I wonder if somebody is actually relying on the fontmake subset feature via Keep Glyphs parameter actually subsetting the layout tables as well, if we implement the Glyphs.app way they may lose that. So I'd just keep that one as is for the time being. Remove Glyphs we can implement the easy way as you proposed.

khaledhosny commented 2 years ago

Doing it at the source level would require us to be able to also subset feature files

The Glyphs app does not handle this so intelligently. If you have a manually created feature file which mentions glyphs which are removed using Remove Glyphs, it just fails to export. We can easily achieve parity with that.

But it handles the auto features, and I don’t think glyphsLib does anything special for auto features (not suggesting we have to use the FontTools subsetter, but some kind of subsetting will be needed).

simoncozens commented 2 years ago

~We handle auto features by constructing the feature code dynamically in our feature writers. If the glyphs aren't in the UFO at that point, the feature writers won't see them, so no problem.~ Ah, you mean stuff like auto ss01. Yes, that will need thinking through.

anthrotype commented 2 years ago

I think Khaled meant Glyphs.app's own auto-features for glyph name suffixed GSUB stuff like ss01, smcp, etc.

simoncozens commented 2 years ago

Well, these have a well-defined format, so we can just scrub the appropriate lines out of them. I'll work on it.

khaledhosny commented 2 years ago

There is also stuff like this:

image
simoncozens commented 2 years ago

Oh urgh. Yeah, easier just to do this with a subsetter.

kalapi commented 2 months ago

Hello folks, this is probably the best place to discuss this. I'm having an issue with the 'Replace Feature' custom parameter. I remember that in an older version of fontmake if we added the replaceFeature custom parameter under each master in the Masters tab, glyphsLib would interpolate the values when generating a VF.

I have two master-specific dist features in a file using the above method. This is what they look like:

Light Master:

# Automatic Code End
script dev2;

lookup DEV_dlft_half_singleAdjustment {
# Ka
    pos ka-deva.half.a ka-deva.half.b' <0 16 -40 0> pa-deva;
    pos ka-deva.half.a ka-deva.half.b' <0 16 -40 0> pha-deva;
    pos ka-deva.half.a ka-deva.half.b' <0 22 -30 0> nga-deva;
    pos ka-deva.half.a ka-deva.half.b' <0 22 -32 0> jha-deva;
    pos ka-deva.half.a ka-deva.half.b' <0 22 -30 0> dda-deva;
} DEV_dlft_half_singleAdjustment;

ExtraBold master:

dist;
# Automatic Code End
script dev2;

lookup DEV_dlft_half_singleAdjustment {
# Ka
    pos ka-deva.half.a ka-deva.half.b' <0 26 -40 0> pa-deva;
    pos ka-deva.half.a ka-deva.half.b' <0 26 -40 0> pha-deva;
    pos ka-deva.half.a ka-deva.half.b' <0 28 -20 0> nga-deva;
    pos ka-deva.half.a ka-deva.half.b' <0 28 -20 0> jha-deva;
    pos ka-deva.half.a ka-deva.half.b' <0 28 -20 0> dda-deva;
} DEV_dlft_half_singleAdjustment;

The only difference in the two snippets is the the valueRecord.

I also have an empty dist feature under the Features tab to 'trigger' the inclusion of the feature and without which the font generates without including the dist data.

I'm also getting this error which I cannot decode:

fontmake.errors.FontmakeError: In 'Documents/[...]/TestFont.glyphs' -> 'master_ufo/TestFont.designspace': Generating fonts from Designspace failed: 

Couldn't merge the fonts, because some values were different, but should have
been the same. This happened while performing the following operation:
GPOS.table.FeatureList.FeatureRecord[0].Feature.LookupListIndex[0]

The problem is likely to be in Test Font Bold:
Expected to see [0]==4, instead saw 3

I've tried running the file through Simon's debug-merge.py script and didn't get any errors. This was the output:

Converting to UFO
Checking compatibility of blwm/mkmk/abvm/mark features
Checking compatibility of LigatureCarets/GlyphClassDefs features

Is there a new way to implement this? Thanks in advance!

I'm on:

kalapi commented 2 months ago

@simoncozens @anthrotype Any insights on this? Thanks in advance!