Merge pull request #955 from googlefonts/virtual-masters

Add support for 'Virtual Master' custom parameters

.github/workflows/ci.yml

@@ -15,7 +15,7 @@ jobs:
     - name: Set up Python
       uses: actions/setup-python@v4
       with:
-        python-version: "3.x"
+        python-version: "3.11"
     - name: Install dependencies
       run: pip install tox
     - name: Run style and typing checks

Lib/glyphsLib/builder/axes.py

@@ -152,13 +152,23 @@ def update_mapping_from_instances(
             mapping[userLoc] = designLoc
 
 
+def is_identity(mapping):
+    """Return whether the mapping is an identity mapping."""
+    return all(userLoc == designLoc for userLoc, designLoc in mapping.items())
+
+
 def to_designspace_axes(self):
     if not self.font.masters:
         return
     regular_master = get_regular_master(self.font)
     assert isinstance(regular_master, classes.GSFontMaster)
 
     custom_mapping = self.font.customParameters["Axis Mappings"]
+    virtual_masters = [
+        {v["Axis"]: v["Location"] for v in cp.value}
+        for cp in self.font.customParameters
+        if cp.name == "Virtual Master"
+    ]
 
     for axis_def in get_axis_definitions(self.font):
         axis = self.designspace.newAxisDescriptor()
@@ -194,9 +204,7 @@ def to_designspace_axes(self):
         elif font_uses_axis_locations(self.font):
             # If all masters have an "Axis Location" custom parameter, only the values
             # from this parameter will be used to build the mapping of the masters and
-            # instances
-            # TODO: (jany) use Virtual Masters as well?
-            #       (jenskutilek) virtual masters can't have an Axis Location parameter.
+            # instances.
             mapping = {}
             for master in self.font.masters:
                 designLoc = axis_def.get_design_loc(master)
@@ -238,8 +246,12 @@ def to_designspace_axes(self):
                 userLoc = designLoc = axis_def.get_design_loc(master)
                 master_mapping[userLoc] = designLoc
 
-            # Prefer the instance-based mapping
-            mapping = instance_mapping or master_mapping
+            # Prefer the instance-based mapping (but only if interesting)
+            mapping = (
+                instance_mapping
+                if (instance_mapping and not is_identity(instance_mapping))
+                else master_mapping
+            )
 
             regularDesignLoc = axis_def.get_design_loc(regular_master)
             # Glyphs masters don't have a user location, so we compute it by
@@ -248,11 +260,28 @@ def to_designspace_axes(self):
             regularUserLoc = piecewiseLinearMap(regularDesignLoc, reverse_mapping)
             # TODO make sure that the default is in mapping?
 
+        is_identity_map = is_identity(mapping)
+
+        # Virtual Masters can't have an Axis Location parameter; their coordinates
+        # can either be mapped via Axis Mappings, or implicitly by neighbouring
+        # non-virtual masters' Axis Location params at least for existing axes; for
+        # newly defined axes the virtual master coordinates are assumed to be un-mapped
+        # (user==design).
+        # Only if the {user:design} mapping so far is an identity map (because it
+        # has not been 'bent' by one of the above mechanisms), the virtual masters
+        # contribute to extend the current axis' min/max range.
+        # https://github.com/googlefonts/glyphsLib/issues/859
+        if is_identity_map:
+            for vm in virtual_masters:
+                for axis_name, axis_coord in vm.items():
+                    if axis_name != axis.name:
+                        continue
+                    mapping[axis_coord] = axis_coord
+
         minimum = min(mapping)
         maximum = max(mapping)
         default = min(maximum, max(minimum, regularUserLoc))  # clamp
 
-        is_identity_map = all(uloc == dloc for uloc, dloc in mapping.items())
         if (
             minimum < maximum
             or minimum != axis_def.default_user_loc

Lib/glyphsLib/builder/custom_params.py

@@ -1153,3 +1153,25 @@ def _unset_default_params(glyphs):
             and glyphs.customParameters[glyphs_name] == default_value
         ):
             del glyphs.customParameters[glyphs_name]
+
+
+class GSFontParamHandler(ParamHandler):
+    def to_glyphs(self, glyphs, ufo):
+        if not glyphs.is_font():
+            return
+        super().to_glyphs(glyphs, ufo)
+
+    def to_ufo(self, builder, glyphs, ufo):
+        if not glyphs.is_font():
+            return
+        super().to_ufo(builder, glyphs, ufo)
+
+
+# 'Virtual Master' params are GSFont-only and multi-valued (i.e. there can be multiple
+# custom parameters named 'Virtual Master'); we know we want them stored in lib.plist
+# hence ufo_info=False
+register(
+    GSFontParamHandler(
+        "Virtual Master", ufo_info=False, ufo_default=[], glyphs_multivalued=True
+    )
+)

tests/builder/axes_test.py

@@ -680,3 +680,48 @@ def test_variable_instance(ufo_module):
     assert doc.axes[0].map[2] == (400, 80)
     assert doc.axes[0].default == 400
     assert len(doc.instances) == 27  # The VF setting should not be in the DS
+
+
+def test_virtual_masters_extend_min_max_for_unmapped_axis(ufo_module, datadir):
+    # https://github.com/googlefonts/glyphsLib/issues/859
+    font = GSFont(datadir.join("IntermediateLayer.glyphs"))
+    assert ["Cap Height", "Weight"] == [a.name for a in font.axes]
+
+    assert "Axis Mappings" not in font.customParameters
+    for master in font.masters:
+        assert "Axis Location" not in master.customParameters
+        # all non-virtual masters are at the default Cap Height location
+        assert master.axes[0] == 700
+
+    virtual_masters = [
+        cp.value for cp in font.customParameters if cp.name == "Virtual Master"
+    ]
+    assert virtual_masters[0] == [
+        {"Axis": "Cap Height", "Location": 600},
+        {"Axis": "Weight", "Location": 400},
+    ]
+    assert virtual_masters[1] == [
+        {"Axis": "Cap Height", "Location": 800},
+        {"Axis": "Weight", "Location": 400},
+    ]
+
+    ds = to_designspace(font, ufo_module=ufo_module)
+
+    # the min/max for this axis are taken from the virtual masters
+    assert ds.axes[0].name == "Cap Height"
+    assert ds.axes[0].minimum == 600
+    assert ds.axes[0].default == 700
+    assert ds.axes[0].maximum == 800
+    assert not ds.axes[0].map
+
+    assert ds.axes[1].name == "Weight"
+    assert ds.axes[1].minimum == 400
+    assert ds.axes[1].default == 400
+    assert ds.axes[1].maximum == 900
+    assert not ds.axes[1].map
+
+    font2 = to_glyphs(ds)
+
+    assert [
+        cp.value for cp in font2.customParameters if cp.name == "Virtual Master"
+    ] == virtual_masters