Sort out the userData/lib stuff + other fixes

Lib/glyphsLib/builder/__init__.py

@@ -25,8 +25,12 @@
 logger = logging.getLogger(__name__)
 
 
-def to_ufos(font, include_instances=False, family_name=None,
-            propagate_anchors=True, ufo_module=defcon):
+def to_ufos(font,
+            include_instances=False,
+            family_name=None,
+            propagate_anchors=True,
+            ufo_module=defcon,
+            minimize_glyphs_diffs=False):
     # TODO: (jany) Update documentation
     """Take .glyphs file data and load it into UFOs.
 
@@ -42,7 +46,8 @@ def to_ufos(font, include_instances=False, family_name=None,
         font,
         ufo_module=ufo_module,
         family_name=family_name,
-        propagate_anchors=propagate_anchors)
+        propagate_anchors=propagate_anchors,
+        minimize_glyphs_diffs=minimize_glyphs_diffs)
 
     result = list(builder.masters)
 
@@ -51,8 +56,11 @@ def to_ufos(font, include_instances=False, family_name=None,
     return result
 
 
-def to_designspace(font, family_name=None, propagate_anchors=True,
-                   ufo_module=defcon):
+def to_designspace(font,
+                   family_name=None,
+                   propagate_anchors=True,
+                   ufo_module=defcon,
+                   minimize_glyphs_diffs=False):
     # TODO: (jany) Update documentation
     """Take .glyphs file data and load it into a Designspace Document + UFOS.
 
@@ -68,11 +76,15 @@ def to_designspace(font, family_name=None, propagate_anchors=True,
         font,
         ufo_module=ufo_module,
         family_name=family_name,
-        propagate_anchors=propagate_anchors)
+        propagate_anchors=propagate_anchors,
+        use_designspace=True,
+        minimize_glyphs_diffs=minimize_glyphs_diffs)
     return builder.designspace
 
 
-def to_glyphs(ufos_or_designspace, glyphs_module=classes):
+def to_glyphs(ufos_or_designspace,
+              glyphs_module=classes,
+              minimize_ufo_diffs=False):
     """
     Take a list of UFOs and combine them into a single .glyphs file.
 
@@ -83,8 +95,10 @@ def to_glyphs(ufos_or_designspace, glyphs_module=classes):
     # FIXME: (jany) duck-type instead of isinstance
     if isinstance(ufos_or_designspace, DesignSpaceDocument):
         builder = GlyphsBuilder(designspace=ufos_or_designspace,
-                                glyphs_module=glyphs_module)
+                                glyphs_module=glyphs_module,
+                                minimize_ufo_diffs=minimize_ufo_diffs)
     else:
         builder = GlyphsBuilder(ufos=ufos_or_designspace,
-                                glyphs_module=glyphs_module)
+                                glyphs_module=glyphs_module,
+                                minimize_ufo_diffs=minimize_ufo_diffs)
     return builder.font

Lib/glyphsLib/builder/builders.py

@@ -51,7 +51,9 @@ def __init__(self,
                  ufo_module=defcon,
                  designspace_module=designSpaceDocument,
                  family_name=None,
-                 propagate_anchors=True):
+                 propagate_anchors=True,
+                 use_designspace=False,
+                 minimize_glyphs_diffs=False):
         """Create a builder that goes from Glyphs to UFO + designspace.
 
         Keyword arguments:
@@ -64,10 +66,18 @@ def __init__(self,
         family_name -- if provided, the master UFOs will be given this name and
                        only instances with this name will be returned.
         propagate_anchors -- set to False to prevent anchor propagation
+        use_designspace -- set to True to make optimal use of the designspace:
+                           data that is common to all ufos will go there.
+        minimize_glyphs_diffs -- set to True to store extra info in UFOs
+                                 in order to get smaller diffs between .glyphs
+                                 .glyphs files when going glyphs->ufo->glyphs.
         """
         self.font = font
         self.ufo_module = ufo_module
         self.designspace_module = designspace_module
+        self.propagate_anchors = propagate_anchors
+        self.use_designspace = use_designspace
+        self.minimize_glyphs_diffs = minimize_glyphs_diffs
 
         # The set of UFOs (= defcon.Font objects) that will be built,
         # indexed by master ID, the same order as masters in the source GSFont.
@@ -100,8 +110,6 @@ def __init__(self,
             else:
                 self._instance_family_name = family_name
 
-        self.propagate_anchors = propagate_anchors
-
     @property
     def masters(self):
         """Get an iterator over master UFOs that match the given family_name.
@@ -137,8 +145,8 @@ def masters(self):
                 if assoc_id != layer.layerId:
                     # Store all layers, even the invalid ones, and just skip
                     # them and print a warning below.
-                    supplementary_layer_data.append(
-                        (assoc_id, glyph_name, layer_name, layer))
+                    supplementary_layer_data.append((assoc_id, glyph_name,
+                                                     layer_name, layer))
                     continue
 
                 ufo = self._ufos[layer_id]
@@ -150,14 +158,14 @@ def masters(self):
 
         for master_id, glyph_name, layer_name, layer \
                 in supplementary_layer_data:
-            if (layer.layerId not in master_layer_ids and
-                    layer.associatedMasterId not in master_layer_ids):
+            if (layer.layerId not in master_layer_ids
+                    and layer.associatedMasterId not in master_layer_ids):
                 self.logger.warn(
                     '{}, glyph "{}": Layer "{}" is dangling and will be '
                     'skipped. Did you copy a glyph from a different font? If '
                     'so, you should clean up any phantom layers not associated '
-                    'with an actual master.'.format(
-                        self.font.familyName, glyph_name, layer.layerId))
+                    'with an actual master.'.format(self.font.familyName,
+                                                    glyph_name, layer.layerId))
                 continue
 
             if not layer_name:
@@ -184,6 +192,8 @@ def masters(self):
                 self.to_ufo_propagate_font_anchors(ufo)
             self.to_ufo_features(ufo)  # This depends on the glyphOrder key
             self.to_ufo_kerning_groups(ufo, kerning_groups)
+            for layer in ufo.layers:
+                self.to_ufo_layer_lib(layer)
 
         for master_id, kerning in self.font.kerning.items():
             self.to_ufo_kerning(self._ufos[master_id], kerning)
@@ -215,7 +225,8 @@ def designspace(self):
         self._designspace = self.designspace_module.DesignSpaceDocument(
             writerClass=designSpaceDocument.InMemoryDocWriter,
             fontClass=self.ufo_module.Font)
-        self.to_ufo_instances()
+        self.to_designspace_instances()
+        self.to_designspace_family_user_data()
         return self._designspace
 
     # DEPRECATED
@@ -252,15 +263,16 @@ def instance_data(self):
     from .glyph import to_ufo_glyph, to_ufo_glyph_background
     from .guidelines import to_ufo_guidelines
     from .hints import to_ufo_hints
-    from .instances import to_ufo_instances
+    from .instances import to_designspace_instances
     from .kerning import (to_ufo_kerning, to_ufo_glyph_groups,
                           to_ufo_kerning_groups)
     from .masters import to_ufo_master_attributes
     from .names import to_ufo_names
     from .paths import to_ufo_paths
-    from .user_data import (to_ufo_family_user_data, to_ufo_master_user_data,
-                            to_ufo_glyph_user_data, to_ufo_layer_user_data,
-                            to_ufo_node_user_data)
+    from .user_data import (to_designspace_family_user_data,
+                            to_ufo_family_user_data, to_ufo_master_user_data,
+                            to_ufo_glyph_user_data, to_ufo_layer_lib,
+                            to_ufo_layer_user_data, to_ufo_node_user_data)
 
 
 def filter_instances_by_family(instances, family_name=None):
@@ -280,7 +292,11 @@ def filter_instances_by_family(instances, family_name=None):
 class GlyphsBuilder(_LoggerMixin):
     """Builder for UFO + designspace to Glyphs."""
 
-    def __init__(self, ufos=[], designspace=None, glyphs_module=classes):
+    def __init__(self,
+                 ufos=[],
+                 designspace=None,
+                 glyphs_module=classes,
+                 minimize_ufo_diffs=False):
         """Create a builder that goes from UFOs + designspace to Glyphs.
 
         Keyword arguments:
@@ -292,7 +308,13 @@ def __init__(self, ufos=[], designspace=None, glyphs_module=classes):
                          instances of your own classes, or pass the Glyphs.app
                          module that holds the official classes to import UFOs
                          into Glyphs.app)
+        minimize_ufo_diffs -- set to True to store extra info in .glyphs files
+                              in order to get smaller diffs between UFOs
+                              when going UFOs->glyphs->UFOs
         """
+        self.glyphs_module = glyphs_module
+        self.minimize_ufo_diffs = minimize_ufo_diffs
+
         if designspace is not None:
             self.designspace = designspace
             if ufos:
@@ -301,18 +323,17 @@ def __init__(self, ufos=[], designspace=None, glyphs_module=classes):
             else:
                 self.ufos = []
                 for source in designspace.sources:
-                    try:
-                        # It's an in-memory source descriptor
-                        self.ufos.append(source.font)
-                    except AttributeError:
-                        self.ufos.append(designspace.fontClass(source.path))
+                    # FIXME: (jany) Do something better for the InMemory stuff
+                    # Is it an in-memory source descriptor?
+                    if not hasattr(source, 'font'):
+                        source.font = designspace.fontClass(source.path)
+                    self.ufos.append(source.font)
         elif ufos:
-            self.designspace = None
+            self.designspace = self._fake_designspace(ufos)
             self.ufos = ufos
         else:
             raise RuntimeError(
                 'Please provide a designspace or at least one UFO.')
-        self.glyphs_module = glyphs_module
 
         self._font = None
         """The GSFont that will be built."""
@@ -337,29 +358,74 @@ def font(self):
             self._font.masters.insert(len(self._font.masters), master)
 
             for layer in ufo.layers:
+                self.to_glyphs_layer_lib(layer)
                 for glyph in layer:
                     self.to_glyphs_glyph(glyph, layer, master)
                     self.to_glyphs_glyph_groups(kerning_groups, glyph)
 
             self.to_glyphs_kerning(ufo, master)
 
         # Now that all GSGlyph are built, restore the glyph order
-        first_ufo = next(iter(self.ufos))
-        if GLYPH_ORDER_KEY in first_ufo.lib:
-            glyph_order = first_ufo.lib[GLYPH_ORDER_KEY]
-            lookup = {name: i for i, name in enumerate(glyph_order)}
-            self.font.glyphs = sorted(
-                self.font.glyphs,
-                key=lambda glyph: lookup.get(glyph.name, 1 << 63))
+        for first_ufo in self.ufos:
+            if GLYPH_ORDER_KEY in first_ufo.lib:
+                glyph_order = first_ufo.lib[GLYPH_ORDER_KEY]
+                lookup = {name: i for i, name in enumerate(glyph_order)}
+                self.font.glyphs = sorted(
+                    self.font.glyphs,
+                    key=lambda glyph: lookup.get(glyph.name, 1 << 63))
+
+            # FIXME: (jany) Only do that on the first one. Maybe we should
+            # merge the various `public.glyphorder` values?
+            break
 
         # Restore the layer ordering in each glyph
         for glyph in self._font.glyphs:
             self.to_glyphs_layer_order(glyph)
 
+        self.to_glyphs_family_user_data_from_designspace()
         self.to_glyphs_instances()
 
         return self._font
 
+    def _fake_designspace(self, ufos):
+        """Build a fake designspace with the given UFOs as sources, so that all
+        builder functions can rely on the presence of a designspace.
+        """
+        designspace = designSpaceDocument.DesignSpaceDocument(
+            writerClass=designSpaceDocument.InMemoryDocWriter)
+
+        for ufo in ufos:
+            source = designspace.newSourceDescriptor()
+            source.font = ufo
+            source.familyName = ufo.info.familyName
+            source.styleName = ufo.info.styleName
+            # source.name = '%s %s' % (source.familyName, source.styleName)
+            source.path = ufo.path
+
+            # MutatorMath.DesignSpaceDocumentWriter iterates over the location
+            # dictionary, which is non-deterministic so it can cause test failures.
+            # We therefore use an OrderedDict to which we insert in axis order.
+            # Since glyphsLib will switch to DesignSpaceDocument once that is
+            # integrated into fonttools, it's not worth fixing upstream.
+            # https://github.com/googlei18n/glyphsLib/issues/165
+            # FIXME: (jany) still needed?
+            # location = OrderedDict()
+            # for axis in self.designspace.axes:
+            #     value_key = axis.name + 'Value'
+            #     if axis.name.startswith('custom'):
+            #         # FIXME: (jany) this is getting boring
+            #         value_key = 'customValue' + axis.name[len('custom'):]
+            #     location[axis.name] = ufo.lib.get(
+            #         MASTER_CUSTOM_PARAM_PREFIX + value_key, DEFAULT_LOCS[axis.name])
+            source.location = {}
+            # if font is regular:
+            #     source.copyLib = True
+            #     source.copyInfo = True
+            #     source.copyGroups = True
+            #     source.copyFeatures = True
+            designspace.addSource(source)
+        return designspace
+
     # Implementation is split into one file per feature
     from .anchors import to_glyphs_glyph_anchors
     from .annotations import to_glyphs_annotations
@@ -380,8 +446,10 @@ def font(self):
     from .masters import to_glyphs_master_attributes
     from .names import to_glyphs_family_names, to_glyphs_master_names
     from .paths import to_glyphs_paths
-    from .user_data import (to_glyphs_family_user_data,
+    from .user_data import (to_glyphs_family_user_data_from_designspace,
+                            to_glyphs_family_user_data_from_ufo,
                             to_glyphs_master_user_data,
                             to_glyphs_glyph_user_data,
+                            to_glyphs_layer_lib,
                             to_glyphs_layer_user_data,
                             to_glyphs_node_user_data)

Lib/glyphsLib/builder/font.py

@@ -140,7 +140,7 @@ def _set_glyphs_font_attributes(self, ufo):
         font.manufacturerURL = info.openTypeNameManufacturerURL
 
     self.to_glyphs_family_names(ufo)
-    self.to_glyphs_family_user_data(ufo)
+    self.to_glyphs_family_user_data_from_ufo(ufo)
     self.to_glyphs_custom_params(ufo, font)
     self.to_glyphs_features(ufo)
 
@@ -151,6 +151,8 @@ def to_glyphs_ordered_masters(self):
 
 
 def _original_master_order(ufo):
+    # FIXME: (jany) Here we should rely on order of sources in designspace
+    # if self.use_designspace
     try:
         return ufo.lib[MASTER_ORDER_LIB_KEY]
     except KeyError:

Lib/glyphsLib/builder/glyph.py

@@ -112,7 +112,7 @@ def to_ufo_glyph(self, ufo_glyph, layer, glyph):
     self.to_ufo_glyph_background(ufo_glyph, layer)
     self.to_ufo_annotations(ufo_glyph, layer)
     self.to_ufo_hints(ufo_glyph, layer)
-    self.to_ufo_glyph_user_data(ufo_glyph, glyph)
+    self.to_ufo_glyph_user_data(ufo_glyph.font, glyph)
     self.to_ufo_layer_user_data(ufo_glyph, layer)
     self.to_ufo_smart_component_axes(ufo_glyph, glyph)
 
@@ -211,7 +211,7 @@ def to_glyphs_glyph(self, ufo_glyph, ufo_layer, master):
     self.to_glyphs_guidelines(ufo_glyph, layer)
     self.to_glyphs_annotations(ufo_glyph, layer)
     self.to_glyphs_hints(ufo_glyph, layer)
-    self.to_glyphs_glyph_user_data(ufo_glyph, glyph)
+    self.to_glyphs_glyph_user_data(ufo_glyph.font, glyph)
     self.to_glyphs_layer_user_data(ufo_glyph, layer)
     self.to_glyphs_smart_component_axes(ufo_glyph, glyph)