Fix "Axis Mappings" interpretation

Lib/glyphsLib/builder/axes.py

@@ -187,10 +187,22 @@ def to_designspace_axes(self):
         ):
             axis_wanted = True
 
+        # Build the mapping from the instances because they have both
+        # a user location and a design location; we build this here
+        # because we use it to check if a custom mapping needs to be inverted.
+        instance_mapping = {}
+        update_mapping_from_instances(
+            instance_mapping,
+            self.font.instances,
+            axis_def,
+            minimize_glyphs_diffs=self.minimize_glyphs_diffs,
+        )
+
         # See https://github.com/googlefonts/glyphsLib/issues/568
         if custom_mapping:
             if axis.tag in custom_mapping:
                 mapping = {float(k): v for k, v in custom_mapping[axis.tag].items()}
+                mapping = _potentially_invert_mapping(self, mapping, instance_mapping)
                 regularDesignLoc = axis_def.get_design_loc(regular_master)
                 reverse_mapping = {dl: ul for ul, dl in sorted(mapping.items())}
                 regularUserLoc = piecewiseLinearMap(regularDesignLoc, reverse_mapping)
@@ -230,16 +242,6 @@ def to_designspace_axes(self):
             regularDesignLoc = axis_def.get_design_loc(regular_master)
             regularUserLoc = axis_def.get_user_loc(regular_master)
         else:
-            # Build the mapping from the instances because they have both
-            # a user location and a design location.
-            instance_mapping = {}
-            update_mapping_from_instances(
-                instance_mapping,
-                self.font.instances,
-                axis_def,
-                minimize_glyphs_diffs=self.minimize_glyphs_diffs,
-            )
-
             master_mapping = {}
             for master in self.font.masters:
                 # Glyphs masters don't have a user location
@@ -322,6 +324,51 @@ def font_uses_axis_locations(font):
     )
 
 
+def _potentially_invert_mapping(self, mapping, instance_mapping):
+    # Fontmake introduced an Axis Mapping custom parameter before Glyphs did;
+    # then Glyphs introduced it, but with an inverted interpretation. Then
+    # Glyphs changed the interpretation to match fontmake's. Try and figure out
+    # what's going on.
+    # See https://github.com/googlefonts/glyphsLib/issues/745
+
+    # We want to return a userspace->designspace map for our purposes
+    inverted = {v: k for k, v in mapping.items()}
+    if int(self.font.appVersion) >= 3168:
+        # Mapping is *definitely* designspace->userspace, invert
+        return inverted
+    if self.font.format_version != 3:
+        # Glyphs wasn't using this custom parameter, only we were
+        return mapping
+    # Let's get heuristical
+    if min(mapping.keys()) == min(mapping.values()) or max(mapping.keys()) == max(
+        mapping.values()
+    ):
+        # No way of knowing
+        logger.warning(
+            "Axis %s: The Axis Mappings custom parameter is ambiguous, "
+            "assuming it is in userspace->designspace format. Please check the mapping "
+            "and save in a recent version of Glyphs to avoid this warning."
+        )
+        return mapping
+    if min(mapping.keys()) == min(instance_mapping.keys()) and max(
+        mapping.keys()
+    ) == max(instance_mapping.keys()):
+        # Looks like userspace->designspace
+        return mapping
+    if min(mapping.keys()) == min(instance_mapping.values()) and max(
+        mapping.values()
+    ) == max(instance_mapping.keys()):
+        # Looks like designspace->userspace
+        return inverted
+    # No idea
+    logger.warning(
+        "Axis %s: The Axis Mappings custom parameter is ambiguous, "
+        "assuming it is in userspace->designspace format. Please check the mapping "
+        "and save in a recent version of Glyphs to avoid this warning."
+    )
+    return mapping
+
+
 def to_glyphs_axes(self):
     axes_parameter = []
     for axis in self.designspace.axes:

tests/builder/axes_test.py

@@ -740,3 +740,47 @@ def test_virtual_masters_extend_min_max_for_unmapped_axis(ufo_module, datadir):
     assert [
         cp.value for cp in font2.customParameters if cp.name == "Virtual Master"
     ] == virtual_masters
+
+
+def test_axis_mappings_different_interpretations(ufo_module, datadir):
+    # https://github.com/googlefonts/glyphsLib/issues/859
+    font = GSFont(datadir.join("gf", "Rubik.glyphs"))  # Saved under Glyphs 3079
+    expected_map = [
+        (300.0, 60),
+        (400.0, 90),
+        (500.0, 125),
+        (600.0, 142),
+        (700.0, 160),
+        (800.0, 190),
+        (900.0, 220),
+    ]
+    assert "Axis Mappings" in font.customParameters
+    ds = to_designspace(font, ufo_module=ufo_module)
+    assert ds.axes[0].minimum == 300
+    assert ds.axes[0].maximum == 900
+    assert ds.axes[0].map == expected_map
+
+    font.format_version = 3  # Ambiguous, work out interpretation hueristically
+    ds = to_designspace(font, ufo_module=ufo_module)
+    assert ds.axes[0].minimum == 300
+    assert ds.axes[0].maximum == 900
+    assert ds.axes[0].map == expected_map
+
+    # Pretend we saved it under a new version
+    font.appVersion = 3217
+    font.customParameters["Axis Mappings"] = {
+        "wght": {
+            60: 300,
+            90: 400,
+            125: 500,
+            142: 600,
+            160: 700,
+            190: 800,
+            220: 900,
+        }
+    }
+
+    ds = to_designspace(font, ufo_module=ufo_module)
+    assert ds.axes[0].minimum == 300
+    assert ds.axes[0].maximum == 900
+    assert ds.axes[0].map == expected_map