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

fontmake fails compiling FiraCode #445

Closed
jamesgk opened this issue Jun 14, 2018 · 6 comments
Closed

fontmake fails compiling FiraCode #445

jamesgk opened this issue Jun 14, 2018 · 6 comments

Comments

@jamesgk
Copy link
Contributor

jamesgk commented Jun 14, 2018

From https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=900777.

When compiling https://github.com/tonsky/FiraCode, the following error occurs:

$ fontmake -g FiraCode.glyphs
INFO:fontmake.font_project:Building master UFOs and designspace from Glyphs source
INFO:glyphsLib.parser:Parsing .glyphs file
INFO:glyphsLib:Loading to UFOs
INFO:glyphsLib.util:Writing master_ufo/FiraCode-Regular.ufo
INFO:glyphsLib.util:Writing master_ufo/FiraCode-Bold.ufo
INFO:fontmake.font_project:Building OTFs
INFO:fontmake.font_project:Decomposing glyphs for FiraCode-Regular
INFO:fontmake.font_project:Decomposing glyphs for FiraCode-Bold
INFO:fontmake.font_project:Removing overlaps for FiraCode-Regular
INFO:fontmake.font_project:Removing overlaps for FiraCode-Bold
INFO:fontmake.font_project:Saving OTF for FiraCode-Regular
WARNING:ufo2ft.outlineCompiler:The copyright was normalized for storage in the CFF table and consequently some characters were dropped: 'Copyright Copyright 2015 by Nikita Prokopov'
Traceback (most recent call last):
  File "/usr/bin/fontmake", line 11, in <module>
    load_entry_point('fontmake==1.4.0', 'console_scripts', 'fontmake')()
...
  File "/usr/lib/python3/dist-packages/fontTools/feaLib/builder.py", line 458, in <listcomp>
    return [l.build() for l in lookups]
  File "/usr/lib/python3/dist-packages/fontTools/feaLib/builder.py", line 1168, in build
    self.setBacktrackCoverage_(prefix, st)
  File "/usr/lib/python3/dist-packages/fontTools/feaLib/builder.py", line 1088, in setBacktrackCoverage_
    coverage = otl.buildCoverage(p, self.glyphMap)
  File "/usr/lib/python3/dist-packages/fontTools/otlLib/builder.py", line 11, in buildCoverage
    self.glyphs = sorted(glyphs, key=glyphMap.__getitem__)
KeyError: ''

I dug into this a bit and I believe fontmake is tripping up on this feature source:

lookup brace_period {
  ignore sub braceleft \\period' \\period;
  ignore sub \\period \\period' braceright;
  sub braceleft \\period' by \\bullet;
  sub \\period' braceright by \\bullet;
} brace_period;

Which is found in the Glyphs source. Is this valid OT feature syntax? Should these double backslashes be ignored?

@anthrotype
Copy link
Member

Interesting, i can take a look tomorrow.
Glad to hear back from you!

@fabiangreffrath
Copy link
Contributor

Is there a chance this gets fixed in the near future?

@anthrotype
Copy link
Member

I just had a look and got the same exception traceback as above.

here's the relevant quote from the Adobe Feature File spec:

An initial backslash serves to differentiate a glyph name from an identical keyword in the feature file language. (See §2.c for a list of keywords.) For example, a glyph named "table" must be specified in the feature file as: \table

https://www.adobe.com/devnet/opentype/afdko/topic_feature_file_syntax.html#2.f.i

So, only one backslash should be required for escaping a glyph name and distinguish it from a same-named keyword. But looking at the text of FiraCode.glyphs I see the features contains glyph names preceded by two backslashes.

I think this is a glyphsLib bug. Within quoted strings in an ascii plist file (usde by the .glyphs format), the backslash is used as an escape character to write non-ascii characters. The backslash literal is also escaped with an additional backslash. The glyphsLib writer correctly dumps these \period glyph names as \\period in the written .glyphs file. However, the parser doesn't seem to be un-escaping them, and leaves them as double \\, which produces that error in the feaLib.

I'll file a bug report over there. I can't guarantee I will be able to fix it immediately as I'm attending a type design conference this week.

anthrotype added a commit to anthrotype/glyphsLib that referenced this issue Sep 14, 2018
reproduces this fontmake bug:
googlefonts/fontmake#445

the writer correctly escapes literal backslashes (doubling them up), but the parser
does not un-escape them.
anthrotype added a commit to anthrotype/glyphsLib that referenced this issue Sep 14, 2018
this fixes googlefonts/fontmake#445 when backslashes
occur within normal quoted string values. For the GSNode.userData we need
further escaping/unescaping (fixed in subsequent commits).

NOTE: the _trim_value method returns a string unmodified if it is not
within quotes, because escape sequences can only occur inside quoted strings
anthrotype added a commit to anthrotype/glyphsLib that referenced this issue Sep 14, 2018
reproduces this fontmake bug:
googlefonts/fontmake#445

the writer correctly escapes literal backslashes (doubling them up), but the parser
does not un-escape them.
@anthrotype
Copy link
Member

@fabiangreffrath we now have a fix in glyphsLib master branch, I will update fontmake as soon as we tag a new glyphsLib release.

@anthrotype anthrotype reopened this Sep 16, 2018
@fabiangreffrath
Copy link
Contributor

Great, thanks!

@anthrotype
Copy link
Member

this should be fixed as of glyphsLib 3.1.1
https://github.com/googlei18n/glyphsLib/releases/tag/v3.1.1

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

No branches or pull requests

4 participants