Merge pull request #776 from googlefonts/fix-local-global-axis-name-c…

…onflict Fix local global axis name conflict
googlefonts · Sep 4, 2023 · 694e496 · 694e496
2 parents 8ecddfc + 9b899e7
commit 694e496
Show file tree

Hide file tree

Showing 12 changed files with 334 additions and 27 deletions.
diff --git a/src/fontra/backends/designspace.py b/src/fontra/backends/designspace.py
@@ -5,7 +5,7 @@
 import os
 import pathlib
 from collections import defaultdict
-from copy import copy, deepcopy
+from copy import deepcopy
 from dataclasses import asdict, dataclass
 from datetime import datetime
 from functools import cache, cached_property
@@ -205,16 +205,11 @@ async def getGlyph(self, glyphName):
 
         axes = []
         sources = []
+        localSources = []
         layers = {}
         sourceNameMapping = {}
         layerNameMapping = {}
 
-        for dsSource in self.dsSources:
-            glyphSet = dsSource.layer.glyphSet
-            if glyphName not in glyphSet:
-                continue
-            sources.append(dsSource.newFontraSource())
-
         for ufoLayer in self.ufoLayers:
             if glyphName not in ufoLayer.glyphSet:
                 continue
@@ -226,11 +221,28 @@ async def getGlyph(self, glyphName):
                     axes, localSources = self._unpackLocalDesignSpace(
                         localDS, ufoLayer.name
                     )
-                    sources.extend(localSources)
                 sourceNameMapping = ufoGlyph.lib.get(SOURCE_NAME_MAPPING_LIB_KEY, {})
                 layerNameMapping = ufoGlyph.lib.get(LAYER_NAME_MAPPING_LIB_KEY, {})
             layers[ufoLayer.fontraLayerName] = Layer(staticGlyph)
 
+        # When a glyph has axes with names that also exist as global axes, we need
+        # to make sure our source locations use the *local* default values. We do
+        # that with a location dict that only contains local values for such "shadow"
+        # axes.
+        localDefaultOverride = {
+            axis.name: axis.defaultValue
+            for axis in axes
+            if axis.name in self.defaultLocation
+        }
+
+        for dsSource in self.dsSources:
+            glyphSet = dsSource.layer.glyphSet
+            if glyphName not in glyphSet:
+                continue
+            sources.append(dsSource.newFontraSource(localDefaultOverride))
+
+        sources.extend(localSources)
+
         if layerNameMapping:
             for source in sources:
                 source.layerName = layerNameMapping.get(
@@ -268,7 +280,9 @@ def _unpackLocalDesignSpace(self, dsDict, defaultLayerName):
             )
 
             sourceLocation = {**self.defaultLocation, **source["location"]}
-            globalLocation = getGlobalPortionOfLocation(sourceLocation, localAxisNames)
+            globalLocation = self._getGlobalPortionOfLocation(
+                sourceLocation, localAxisNames
+            )
             dsSource = self.dsSources.findItem(
                 locationTuple=tuplifyLocation(globalLocation)
             )
@@ -299,15 +313,15 @@ async def putGlyph(self, glyphName, glyph, unicodes):
         )
 
         localAxes = packLocalAxes(glyph.axes)
-        localAxisNames = {axis.name for axis in glyph.axes}
+        localDefaultLocation = {axis.name: axis.defaultValue for axis in glyph.axes}
 
         # Prepare UFO source layers and local sources
         sourceNameMapping = {}
         layerNameMapping = {}
         localSources = []
         for source in glyph.sources:
             sourceInfo = self._prepareUFOSourceLayer(
-                source, localAxisNames, revLayerNameMapping
+                source, localDefaultLocation, revLayerNameMapping
             )
             if sourceInfo.sourceName != source.name:
                 sourceNameMapping[sourceInfo.sourceName] = source.name
@@ -383,17 +397,24 @@ async def putGlyph(self, glyphName, glyph, unicodes):
 
         self.savedGlyphModificationTimes[glyphName] = modTimes
 
-    def _prepareUFOSourceLayer(self, source, localAxisNames, revLayerNameMapping):
+    def _prepareUFOSourceLayer(self, source, localDefaultLocation, revLayerNameMapping):
+        sparseLocalLocation = {
+            name: source.location[name]
+            for name, value in localDefaultLocation.items()
+            if source.location.get(name, value) != value
+        }
         sourceLocation = {**self.defaultLocation, **source.location}
-        globalLocation = getGlobalPortionOfLocation(sourceLocation, localAxisNames)
+        globalLocation = self._getGlobalPortionOfLocation(
+            sourceLocation, localDefaultLocation
+        )
 
         dsSource = self.dsSources.findItem(
             locationTuple=tuplifyLocation(globalLocation)
         )
         if dsSource is None:
             dsSource = self._createDSSource(source, globalLocation)
 
-        if sourceLocation != globalLocation:
+        if sparseLocalLocation:
             ufoLayer = self.ufoLayers.findItem(
                 fontraLayerName=revLayerNameMapping.get(
                     source.layerName, source.layerName
@@ -503,6 +524,14 @@ def _newUFOLayer(self, ufoPath, suggestedLayerName):
 
         return ufoLayer
 
+    def _getGlobalPortionOfLocation(self, location, localAxisNames):
+        globalLocation = {
+            name: value
+            for name, value in location.items()
+            if name not in localAxisNames
+        }
+        return {**self.defaultLocation, **globalLocation}
+
     async def getGlobalAxes(self):
         return self.axes
 
@@ -705,10 +734,12 @@ class DSSource:
     def locationTuple(self):
         return tuplifyLocation(self.location)
 
-    def newFontraSource(self):
+    def newFontraSource(self, localDefaultOverride=None):
+        if localDefaultOverride is None:
+            localDefaultOverride = {}
         return Source(
             name=self.name,
-            location=copy(self.location),
+            location={**self.location, **localDefaultOverride},
             layerName=self.layer.fontraLayerName,
         )
 
@@ -936,9 +967,3 @@ def glyphHasVariableComponents(glyph):
         for layer in glyph.layers.values()
         for compo in layer.glyph.components
     )
-
-
-def getGlobalPortionOfLocation(location, localAxisNames):
-    return {
-        name: value for name, value in location.items() if name not in localAxisNames
-    }
diff --git a/test-py/data/mutatorsans/MutatorSansLightCondensed.ufo/glyphs.weight=1/R_.alt.glif b/test-py/data/mutatorsans/MutatorSansLightCondensed.ufo/glyphs.weight=1/R_.alt.glif
@@ -0,0 +1,41 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<glyph name="R.alt" format="2">
+  <advance width="720"/>
+  <outline>
+    <contour>
+      <point x="385" y="0" type="line"/>
+      <point x="705" y="0" type="line"/>
+      <point x="672" y="211" type="line" smooth="yes"/>
+      <point x="663" y="266"/>
+      <point x="624" y="304"/>
+      <point x="556" y="304" type="curve"/>
+      <point x="366" y="304" type="line"/>
+    </contour>
+    <contour>
+      <point x="30" y="0" type="line"/>
+      <point x="350" y="0" type="line"/>
+      <point x="350" y="800" type="line"/>
+      <point x="30" y="800" type="line"/>
+    </contour>
+    <contour>
+      <point x="280" y="548" type="line"/>
+      <point x="365" y="548" type="line" smooth="yes"/>
+      <point x="394" y="548"/>
+      <point x="407" y="536"/>
+      <point x="407" y="487" type="curve" smooth="yes"/>
+      <point x="407" y="432"/>
+      <point x="394" y="424"/>
+      <point x="365" y="424" type="curve" smooth="yes"/>
+      <point x="280" y="424" type="line"/>
+      <point x="280" y="232" type="line"/>
+      <point x="398" y="232" type="line" smooth="yes"/>
+      <point x="611" y="232"/>
+      <point x="700" y="325"/>
+      <point x="700" y="517" type="curve" smooth="yes"/>
+      <point x="700" y="708"/>
+      <point x="601" y="800"/>
+      <point x="387" y="800" type="curve" smooth="yes"/>
+      <point x="280" y="800" type="line"/>
+    </contour>
+  </outline>
+</glyph>
diff --git a/test-py/data/mutatorsans/MutatorSansLightCondensed.ufo/glyphs.weight=1/contents.plist b/test-py/data/mutatorsans/MutatorSansLightCondensed.ufo/glyphs.weight=1/contents.plist
@@ -0,0 +1,8 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
+<plist version="1.0">
+  <dict>
+    <key>R.alt</key>
+    <string>R_.alt.glif</string>
+  </dict>
+</plist>
diff --git a/test-py/data/mutatorsans/MutatorSansLightCondensed.ufo/glyphs.width=1,weight=1/R_.alt.glif b/test-py/data/mutatorsans/MutatorSansLightCondensed.ufo/glyphs.width=1,weight=1/R_.alt.glif
@@ -0,0 +1,41 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<glyph name="R.alt" format="2">
+  <advance width="1277"/>
+  <outline>
+    <contour>
+      <point x="800" y="0" type="line"/>
+      <point x="1231" y="0" type="line"/>
+      <point x="1180" y="83" type="line" smooth="yes"/>
+      <point x="1137" y="153"/>
+      <point x="1083" y="210"/>
+      <point x="985" y="210" type="curve"/>
+      <point x="690" y="210" type="line"/>
+    </contour>
+    <contour>
+      <point x="60" y="0" type="line"/>
+      <point x="480" y="0" type="line"/>
+      <point x="480" y="800" type="line"/>
+      <point x="60" y="800" type="line"/>
+    </contour>
+    <contour>
+      <point x="354" y="513" type="line"/>
+      <point x="771" y="513" type="line" smooth="yes"/>
+      <point x="834" y="513"/>
+      <point x="862" y="501"/>
+      <point x="862" y="455" type="curve" smooth="yes"/>
+      <point x="862" y="402"/>
+      <point x="834" y="394"/>
+      <point x="771" y="394" type="curve" smooth="yes"/>
+      <point x="354" y="394" type="line"/>
+      <point x="354" y="126" type="line"/>
+      <point x="737" y="126" type="line" smooth="yes"/>
+      <point x="1114" y="126"/>
+      <point x="1242" y="231"/>
+      <point x="1242" y="445" type="curve" smooth="yes"/>
+      <point x="1242" y="684"/>
+      <point x="1114" y="800"/>
+      <point x="739" y="800" type="curve" smooth="yes"/>
+      <point x="354" y="800" type="line"/>
+    </contour>
+  </outline>
+</glyph>
diff --git a/...-py/data/mutatorsans/MutatorSansLightCondensed.ufo/glyphs.width=1,weight=1/contents.plist b/...-py/data/mutatorsans/MutatorSansLightCondensed.ufo/glyphs.width=1,weight=1/contents.plist
@@ -0,0 +1,8 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
+<plist version="1.0">
+  <dict>
+    <key>R.alt</key>
+    <string>R_.alt.glif</string>
+  </dict>
+</plist>
diff --git a/test-py/data/mutatorsans/MutatorSansLightCondensed.ufo/glyphs.width=1/R_.alt.glif b/test-py/data/mutatorsans/MutatorSansLightCondensed.ufo/glyphs.width=1/R_.alt.glif
@@ -0,0 +1,41 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<glyph name="R.alt" format="2">
+  <advance width="1171"/>
+  <outline>
+    <contour>
+      <point x="980" y="0" type="line"/>
+      <point x="1048" y="0" type="line"/>
+      <point x="913" y="98" type="line" smooth="yes"/>
+      <point x="826" y="161"/>
+      <point x="778" y="187"/>
+      <point x="740" y="185" type="curve"/>
+      <point x="723" y="185" type="line"/>
+    </contour>
+    <contour>
+      <point x="120" y="0" type="line"/>
+      <point x="160" y="0" type="line"/>
+      <point x="160" y="700" type="line"/>
+      <point x="120" y="700" type="line"/>
+    </contour>
+    <contour>
+      <point x="140" y="660" type="line"/>
+      <point x="640" y="660" type="line" smooth="yes"/>
+      <point x="935" y="660"/>
+      <point x="1018" y="575"/>
+      <point x="1018" y="432" type="curve" smooth="yes"/>
+      <point x="1018" y="289"/>
+      <point x="935" y="205"/>
+      <point x="640" y="205" type="curve" smooth="yes"/>
+      <point x="140" y="205" type="line"/>
+      <point x="140" y="165" type="line"/>
+      <point x="620" y="165" type="line" smooth="yes"/>
+      <point x="995" y="165"/>
+      <point x="1061" y="286"/>
+      <point x="1061" y="432" type="curve" smooth="yes"/>
+      <point x="1061" y="580"/>
+      <point x="995" y="700"/>
+      <point x="620" y="700" type="curve" smooth="yes"/>
+      <point x="140" y="700" type="line"/>
+    </contour>
+  </outline>
+</glyph>
diff --git a/test-py/data/mutatorsans/MutatorSansLightCondensed.ufo/glyphs.width=1/contents.plist b/test-py/data/mutatorsans/MutatorSansLightCondensed.ufo/glyphs.width=1/contents.plist
@@ -0,0 +1,8 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
+<plist version="1.0">
+  <dict>
+    <key>R.alt</key>
+    <string>R_.alt.glif</string>
+  </dict>
+</plist>