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

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

Closed
frankrolf opened this issue Sep 26, 2023 · 0 comments · Fixed by #1703
Closed

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

frankrolf opened this issue Sep 26, 2023 · 0 comments · Fixed by #1703

Comments

@frankrolf
Copy link
Member

frankrolf commented Sep 26, 2023

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.

skef added a commit that referenced this issue Sep 27, 2023
   #1697 barfs w/o any arguments
   #1699 cannot specify glyphs by GID
   #1700 cannot specify glyphs via external glyph list
   #1701 -h reports wrong tool name
@skef skef mentioned this issue Sep 27, 2023
6 tasks
skef added a commit that referenced this issue Sep 29, 2023
* Fix otfstemhist bugs:
   #1697 barfs w/o any arguments
   #1699 cannot specify glyphs by GID
   #1700 cannot specify glyphs via external glyph list
   #1701 -h reports wrong tool name
skef added a commit that referenced this issue Jul 8, 2024
* Fix otfstemhist bugs:
   #1697 barfs w/o any arguments
   #1699 cannot specify glyphs by GID
   #1700 cannot specify glyphs via external glyph list
   #1701 -h reports wrong tool name
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

Successfully merging a pull request may close this issue.

1 participant