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

glyphsLib fails to build UFOs from specific .glyphs file #282

Closed
madig opened this issue Nov 11, 2017 · 9 comments
Closed

glyphsLib fails to build UFOs from specific .glyphs file #282

madig opened this issue Nov 11, 2017 · 9 comments

Comments

@madig
Copy link
Collaborator

madig commented Nov 11, 2017

  1. Get https://raw.githubusercontent.com/madig/cantarell-fonts/f17124d041e6ee370a9fcddcc084aa6cbf3d5500/src/Cantarell.glyphs
  2. fontmake -g Cantarell.glyphs -o otf
INFO:fontmake.font_project:Building master UFOs and designspace from Glyphs source
INFO:glyphsLib.parser:Parsing .glyphs file
INFO:glyphsLib:Loading to UFOs
Traceback (most recent call last):
  File "/home/nikolaus/.local/bin/fontmake", line 11, in <module>
    sys.exit(main())
  File "/home/nikolaus/.local/lib/python3.6/site-packages/fontmake/__main__.py", line 183, in main
    project.run_from_glyphs(glyphs_path, **args)
  File "/home/nikolaus/.local/lib/python3.6/site-packages/fontmake/font_project.py", line 376, in run_from_glyphs
    glyphs_path, family_name, mti_source=mti_source)
  File "/home/nikolaus/.local/lib/python3.6/site-packages/fontTools/misc/loggingTools.py", line 372, in wrapper
    return func(*args, **kwds)
  File "/home/nikolaus/.local/lib/python3.6/site-packages/fontmake/font_project.py", line 77, in build_master_ufos
    family_name=family_name)
  File "/home/nikolaus/.local/lib/python3.6/site-packages/glyphsLib/__init__.py", line 81, in build_masters
    propagate_anchors=propagate_anchors)
  File "/home/nikolaus/.local/lib/python3.6/site-packages/glyphsLib/__init__.py", line 59, in load_to_ufos
    propagate_anchors=propagate_anchors)
  File "/home/nikolaus/.local/lib/python3.6/site-packages/glyphsLib/builder/__init__.py", line 43, in to_ufos
    result = list(builder.masters)
  File "/home/nikolaus/.local/lib/python3.6/site-packages/glyphsLib/builder/builders.py", line 137, in masters
    ufo_font = self._ufos[layer_id]
KeyError: ''

The same happens with

import glyphsLib
ufos = glyphsLib.build_instances('Cantarell.glyphs', "masters", "instances")
@moyogo
Copy link
Collaborator

moyogo commented Nov 12, 2017

verticallineabovecomb has some phantom layers.

@madig
Copy link
Collaborator Author

madig commented Nov 12, 2017

Ah! I copied that glyph from a different font; I see it, too, now. I guess testing for phantom layers in the offended method and printing a warning will do the job? Will have a closer look.

@MrBrezina
Copy link

I had the same problem. It is because I copied some glyphs from another font with more masters and the extra layers got preserved. It is not just layers without names, but also others which should not be there.

This Glyphs.app script helps to clean things up.
(Run it multiple times. That’s because I am lazy to figure out how to delete all phantom layer systematically in one run.)

import GlyphsApp

font = Glyphs.font
real_layers = [m.id for m in font.masters]

for g in font.glyphs:
	for l in g.layers:
		if l.layerId not in real_layers:
			del(font.glyphs[g.name].layers[l.layerId])
			print "Deleting layer from", g.name

@madig
Copy link
Collaborator Author

madig commented Nov 13, 2017

@anthrotype I'm thinking about checking for phantom layers somewhere in UFOBuilder, maybe ::masters(), and just log a warning and skip them, since it's ufoLib that chokes. Glyphs seems to just ignore them. I'll send a pull request.

Tell me if this should be handled differently. Not sure if it's wise to keep them for round-trip constness, since I think they're clearly data corruption the user should know about. Should it raise an error instead?

@anthrotype
Copy link
Member

Sounds good, thanks

@anthrotype
Copy link
Member

Drop and warn is ok by me

@schriftgestalt
Copy link
Collaborator

GlyphsLib should only care about layers that have ‘layerIDs’ that are also a ‘Master.id’. Only if you plan to support brace and bracket layers, you need to look in the other layers, too.
And always check for associatedMasterID.

@madig
Copy link
Collaborator Author

madig commented Nov 13, 2017

@schriftgestalt Do you see (associated) layers that do not point to an existing master ID as data corruption? See https://forum.glyphsapp.com/t/glyphs-should-clean-up-or-prune-phantom-layers-when-copying-glyphs-from-other-fonts/7576/1

I assume associated layers are these non-bold layers in Glyphs associated with the master layer.

@anthrotype
Copy link
Member

should be fixed by #291

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

5 participants