googlefonts/glyphsLib

default "gasp Table" custom parameter (empty "") crashes conversion

anthrotype opened this issue · 4 comments

There are a few issues with the "gasp Table" custom parameter.

First of all, the table name is lowecase, at least in the Glyphs.app version I'm currently looking at which is 3.2 (3217).
Was it ever called "GASP Table" all uppercase in the past? If so we might have to support both. /cc @schriftgestalt

Or are the custom parameter names treated as case-insensitive?

Secondly, if the parameter is set from Glyphs.app with default values, it is serialised to .glyphs file as an empty "" string, not a dict as glyphsLib expects it... So we get an AttributeError crash by the time we attempt to convert to UFO:

  File "/Users/clupo/.local/pipx/venvs/fontmake/lib/python3.10/site-packages/glyphsLib/builder/custom_params.py", line 528, in to_ufo_gasp_table
    value = {int(k): int(v) for k, v in value.items()}
AttributeError: 'str' object has no attribute 'items'

Finally, if the "gasp Table" is set to non-default values, it is written out by Glyphs.app as a dict in the .glyphs source, e.g.:

{
name = "gasp Table";
value = {
9 = 10;
21 = 7;
65535 = 15;
};

however glyphsLib seems to completely ignore it, I cannot find any trace in the generated UFOs. (CORRECTION: it does go to fontinfo.plist)

We have some unit-tests in custom_params_test.py that deal with "GASP Table" but they do not run end-to-end and may be missing something.

Oh and one more thing, which definitely looks suspiciously wrong is this line:

_CUSTOM_DICT_PARAMS = frozenset("GASP Table")

should it not be frozenset(("GASP Table",)) (plus perhaps the lowercased name)? otherwise one gets frozenset({' ', 'A', 'G', 'P', 'S', 'T', 'a', 'b', 'e', 'l'})

however glyphsLib seems to completely ignore it, I cannot find any trace in the generated UFOs.

ok I was looking at the wrong place, it goes to fontinfo.plist, not lib.plist. Somehow glyphsLib manages to convert "gasp Table" (lowercase) to UFO's openTypeGaspRangeRecords, despite I can't find where exactly the case is being ignored...

oh sorry for the noise, we do have already two duplicate ParamHandlers one for "GASP Table" (uppercase) and one for "gasp Table" (lowercase)...

Found another one.. when doing ufo2glyphs from a UFO whose fontinfo.plist contains openTypeGaspRangeRecords, the generated .glyphs file contains both GASP Table and gasp Table with the same content..

customParameters = (
{
name = "GASP Table";
value = {
"9" = 10;
"21" = 7;
"65535" = 15;
};
},
{
name = "gasp Table";
value = {
"9" = 10;
"21" = 7;
"65535" = 15;
};
},

I made another find (and I am now about to give up altogether)

When you set a new "gasp Table" custom parameter from the GUI inside Glyphs.app, and do not modify the default settings, not only (as I noted above), the .glyphs source will contain an empty value = ""; for the gasp Table (instead of an actual dictionary value containing the values shown in the UI), but when you export the font, no gasp table is generated!
I would have expected that a gasp table with the following ranges would get built:

Glyphs.font.customParameters["gasp Table"] = {
    8: 10,
    20: 7,
    65535: 15,
}

.. or they do work if I set them with python in the Macro panel, but as soon as I interact with the gasp Table GUI, it goes back to the empty default:

Screenshot 2023-09-07 at 16 47 35