foliojs / fontkit

An advanced font engine for Node and the browser
1.45k stars 213 forks source link

font.namedVariations fails on some fonts #272

Open guidoferreyra opened 2 years ago

guidoferreyra commented 2 years ago

When parsing some fonts and trying to get namedVariation. it throws the following error:

TypeError: Cannot read property 'en' of undefined

When printing the instance where the undefined is raised it shows the following, name is missing.
{ nameID: 2, flags: 0, coord: [ 0, 400, 100, 0 ], postscriptNameID: 6 }

I dug a bit and the issue is cause because when getting the instance name is not fnding id 2 since name.records.fontFeatures starts at ID 256. When instance name matches with font ID 2 will use that, it seems this is very common and allowed by the spec:

The subfamilyNameID field provides a name ID that can be used to obtain strings from the 'name' table that can be treated as equivalent to name ID 17 (typographic subfamily) strings for the given instance. Values of 2 or 17 can be used; otherwise, values must be greater than 255 and less than 32768. The values 2 or 17 should only be used if the named instance corresponds to the font’s default instance.

guidoferreyra commented 2 years ago

By changing
https://github.com/foliojs/fontkit/blob/417af0c79c5664271a07a783574ec7fac7ebad0c/src/tables/name.js#L79 and https://github.com/foliojs/fontkit/blob/417af0c79c5664271a07a783574ec7fac7ebad0c/src/tables/name.js#L85 to: record.nameID >= 256 || record.nameID == 2 || record.nameID == 17 will solve the issue. Is there a reason to create the name array fontFeatures? Why when looking for a name just dont search on the full list?

ptoussai commented 1 year ago

I have the same problem. Glyphs uses nameID: 2 in some cases. @guidoferreyra do you want to create a PR? Otherwise I can do it.

quitequinn commented 1 year ago

Can we get this merged?

quitequinn commented 1 year ago

Font Subfamily name is Name ID Code 2. This means you could search and replace any named instances with nameID: 2 with the Subfamily name.

freder commented 1 year ago

also getting this error. — if there is a fix, please merge 🙏

Typogram commented 10 months ago

I also run into this issue, it is actually quite common. The fix PR above seem to have an error at least in the PR description, so please double check before merge: https://github.com/foliojs/fontkit/pull/317#issuecomment-1726714841

quitequinn commented 2 months ago

This also happens with other references (eg. ID 17 was known as “Preferred Subfamily”)

quitequinn commented 2 months ago

Have you found a work around @Typogram ?

Even just a way to lookup by name id without having to create a dictionary via https://learn.microsoft.com/en-us/typography/opentype/spec/name