adobe-type-tools / afdko

Adobe Font Development Kit for OpenType
https://adobe-type-tools.github.io/afdko/
Other
1.04k stars 169 forks source link

[otfstemhist] cannot specify glyphs via external glyph list #1700

Closed frankrolf closed 1 year ago

frankrolf commented 1 year ago

I have a Thai font for which I’d like to analyze stem information. The Thai glyph names look like this:

koKai-thai
khoKhai-thai
khoKhuat-thai
khoKhwai-thai
khoKhon-thai
khoRakhang-thai

etc.

Since I cannot specify those dashed glyph names with the -g option (#1698), and GIDs are not an option either (#1699), I tried an external text file.

According to otfstemhist -h:

  --glyphs-file PATH    file containing a list of glyphs to process
                        The file must contain a comma-separated sequence of glyph identifiers.

My text file looks like this:

koKai-thai,khoKhai-thai,khoKhuat-thai,khoKhwai-thai,khoKhon-thai

otfstemhist --glyphs-file text_file.txt my_font.ufo

Traceback (most recent call last):
  File "/Users/fg/.pyenv/versions/3.10.13/bin/otfstemhist", line 8, in <module>
    sys.exit(stemhist())
  File "/Users/fg/.pyenv/versions/3.10.13/lib/python3.10/site-packages/afdko/otfautohint/__main__.py", line 941, in stemhist
    options, pargs = get_stemhist_options(args)
  File "/Users/fg/.pyenv/versions/3.10.13/lib/python3.10/site-packages/afdko/otfautohint/__main__.py", line 932, in get_stemhist_options
    handle_glyph_lists(options, parsed_args)
  File "/Users/fg/.pyenv/versions/3.10.13/lib/python3.10/site-packages/afdko/otfautohint/__main__.py", line 626, in handle_glyph_lists
    options.glyphList = _process_glyph_list_arg(
  File "/Users/fg/.pyenv/versions/3.10.13/lib/python3.10/site-packages/afdko/otfautohint/__main__.py", line 322, in _process_glyph_list_arg
    return [_expand_cid_name(n, name_aliases) for n in glyph_list]
  File "/Users/fg/.pyenv/versions/3.10.13/lib/python3.10/site-packages/afdko/otfautohint/__main__.py", line 322, in <listcomp>
    return [_expand_cid_name(n, name_aliases) for n in glyph_list]
  File "/Users/fg/.pyenv/versions/3.10.13/lib/python3.10/site-packages/afdko/otfautohint/__main__.py", line 302, in _expand_cid_name
    g1 = _expand_cid_name(glyphRange[0], name_aliases)
  File "/Users/fg/.pyenv/versions/3.10.13/lib/python3.10/site-packages/afdko/otfautohint/__main__.py", line 306, in _expand_cid_name
    elif glyph_name[0] == "/":
IndexError: string index out of range

This has to do with the _process_glyph_list_arg method (https://github.com/adobe-type-tools/afdko/blob/develop/python/afdko/otfautohint/__main__.py#L321-L322) – it seems to just go through every character of the supplied glyph list, instead of splitting by the expected comma.

When I change that to

def _process_glyph_list_arg(glyph_list, name_aliases):
    return [_expand_cid_name(n, name_aliases) for n in glyph_list.split(',')]

my output is:

WARNING: glyph ID <koKai> in range koKai-thai from glyph selection list option is not in font. <xxx Traditional-Regular>.
WARNING: glyph ID <khoKhai> in range khoKhai-thai from glyph selection list option is not in font. <xxx Traditional-Regular>.
WARNING: glyph ID <khoKhuat> in range khoKhuat-thai from glyph selection list option is not in font. <xxx Traditional-Regular>.
WARNING: glyph ID <khoKhwai> in range khoKhwai-thai from glyph selection list option is not in font. <xxx Traditional-Regular>.
WARNING: glyph ID <khoKhon> in range khoKhon-thai from glyph selection list option is not in font. <xxx Traditional-Regular>.
ERROR: Selected glyph list is empty for font <xxx Traditional-Regular>.

This means I also need to change getGlyphNames in autohint.py, I attempted this:

def getGlyphNames(glyphSpec, fontGlyphList, fDesc):
    glyphNameList = []
    rangeList = glyphSpec.split("-")
    try:
        if rangeList[0] in fontGlyphList:
            is_range = True
            prevGID = fontGlyphList.index(rangeList[0])
        else:
            is_range = False
            prevGID = fontGlyphList.index(glyphSpec)
    except ValueError:
        if len(rangeList) > 1:
            log.warning("glyph ID <%s> in range %s from glyph selection "
                        "list option is not in font. <%s>.",
                        rangeList[0], glyphSpec, fDesc)
        else:
            log.warning("glyph ID <%s> from glyph selection list option "
                        "is not in font. <%s>.", rangeList[0], fDesc)
        return None
    glyphNameList.append(fontGlyphList[prevGID])

    if is_range:
        for glyphName2 in rangeList[1:]:
            try:
                gid = fontGlyphList.index(glyphName2)
            except ValueError:
                log.warning("glyph ID <%s> in range %s from glyph selection "
                            "list option is not in font. <%s>.",
                            glyphName2, glyphSpec, fDesc)
                return None
            for i in range(prevGID + 1, gid + 1):
                glyphNameList.append(fontGlyphList[i])
            prevGID = gid

    return glyphNameList

This finally made it work. However, I think the range parsing in getGlyphNames is brittle, and maybe we should just say goodbye to specifying glyph ranges altogether.