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

CFF2: interpret empty glyphs from non-default sparse master as missing, like with gvar #3233

Closed
anthrotype opened this issue Aug 1, 2023 · 5 comments · Fixed by #3234
Closed

Comments

@anthrotype
Copy link
Member

in #2082, @justvanrossum added support in varLib for interpreting empty glyphs as missing (thus not participating) if they occur in non-default sparse masters, but this only works for TrueType-flavored VFs:

if allData[defaultMasterIndex][1].numberOfContours != 0:
# If the default master is not empty, interpret empty non-default masters
# as missing glyphs from a sparse master
allData = [
d if d is not None and d[1].numberOfContours != 0 else None
for d in allData
]

Ideally we would do the same thing for variable CFF2 fonts, but I am not familiar enough with the varLib.cff module to know exactly how/where this change should best be placed.

If one attempts to use this pattern (default master has a non-empty glyph, non-default master has an empty glyph with the same name) one would get the following error:

fontTools.varLib.errors.VarLibCFFPointTypeMergeError: Glyph '.notdef': 'rlineto' at point index 4 in master index 1 ...

Maybe @behdad or @justvanrossum can help?

anthrotype added a commit to googlefonts/ufo2ft that referenced this issue Aug 1, 2023
previously we were copying one from the default master, but if this contains cubic curves our own check fails because the .notdef we inserted in the sparse masters OutlineCompiler was not passed through cu2qu...
Our intention is to have sparse masters not participate in the interpolation of that particular glyph so we make it empty (gid0=.notdef still needs to be there in a valid TTF so we must have one).
Currently the same trick does not work for CFF2 variable fonts (fonttools/fonttools#3233) but those are fine with cubics in .notdef glyph any way.. For now at least
@behdad
Copy link
Member

behdad commented Aug 1, 2023

Good news is that it's supposed to work from glancing at the code:

def merge_charstrings(glyphOrder, num_masters, top_dicts, masterModel): 
 
    vsindex_dict = {} 
    vsindex_by_key = {} 
    varDataList = [] 
    masterSupports = [] 
    default_charstrings = top_dicts[0].CharStrings 
    for gid, gname in enumerate(glyphOrder): 
        all_cs = [_get_cs(td.CharStrings, gname) for td in top_dicts] 
        if len([gs for gs in all_cs if gs is not None]) == 1: 
            continue 
        model, model_cs = masterModel.getSubModel(all_cs) 

@behdad
Copy link
Member

behdad commented Aug 1, 2023

We just need to change it to filter out emtpy glyphs as well. I think I can do that.

@anthrotype
Copy link
Member Author

well.. it does not as I have shown above. I get an VarLibCFFPointTypeMergeError if I try to pass such a sparse empty glyph setup. That code you linked, _get_cs, returns None if the glyph is not present in a master; here it is present, but empty

@behdad
Copy link
Member

behdad commented Aug 1, 2023

well.. it does not as I have shown above. I get an VarLibCFFPointTypeMergeError if I try to pass such a sparse empty glyph setup. That code you linked, _get_cs, returns None if the glyph is not present in a master; here it is present, but empty

Yeah. We just need to change _get_cs to filter out empty (for CFF2) or just endchar (for CFF1) and return None.

@anthrotype
Copy link
Member Author

right.. detecting when a charstring is empty is what I couldn't figure it out. Checking its program length doesn't work because it has to be decompiled or something

behdad added a commit that referenced this issue Aug 1, 2023
@behdad behdad mentioned this issue Aug 1, 2023
behdad added a commit that referenced this issue Aug 1, 2023
behdad added a commit that referenced this issue Aug 1, 2023
behdad added a commit that referenced this issue Aug 1, 2023
behdad added a commit that referenced this issue Aug 1, 2023
behdad added a commit that referenced this issue Aug 1, 2023
behdad added a commit that referenced this issue Aug 1, 2023
behdad added a commit that referenced this issue Aug 1, 2023
behdad added a commit that referenced this issue Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants