From 7bc46f9b81b541216bc25c6bd1fadbfba207412e Mon Sep 17 00:00:00 2001 From: Just van Rossum Date: Mon, 4 Sep 2023 11:01:21 +0200 Subject: [PATCH 1/4] getGlobalPortionOfLocation() must return a full location, not sparse --- src/fontra/backends/designspace.py | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/fontra/backends/designspace.py b/src/fontra/backends/designspace.py index 02e6a3e7a..474d2d81f 100644 --- a/src/fontra/backends/designspace.py +++ b/src/fontra/backends/designspace.py @@ -268,7 +268,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) ) @@ -385,7 +387,9 @@ async def putGlyph(self, glyphName, glyph, unicodes): def _prepareUFOSourceLayer(self, source, localAxisNames, revLayerNameMapping): sourceLocation = {**self.defaultLocation, **source.location} - globalLocation = getGlobalPortionOfLocation(sourceLocation, localAxisNames) + globalLocation = self._getGlobalPortionOfLocation( + sourceLocation, localAxisNames + ) dsSource = self.dsSources.findItem( locationTuple=tuplifyLocation(globalLocation) @@ -503,6 +507,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 @@ -936,9 +948,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 - } From 40c96c9855e4e3212491c6d14c89d8de0a479245 Mon Sep 17 00:00:00 2001 From: Just van Rossum Date: Mon, 4 Sep 2023 17:34:03 +0200 Subject: [PATCH 2/4] Compute the correct source location when a local axis exists with the same name as a global axis --- src/fontra/backends/designspace.py | 45 ++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/src/fontra/backends/designspace.py b/src/fontra/backends/designspace.py index 474d2d81f..345a787af 100644 --- 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,24 @@ 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) + 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( @@ -301,7 +309,7 @@ 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 = {} @@ -309,7 +317,7 @@ async def putGlyph(self, glyphName, glyph, unicodes): 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 @@ -385,10 +393,15 @@ 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 = self._getGlobalPortionOfLocation( - sourceLocation, localAxisNames + sourceLocation, localDefaultLocation ) dsSource = self.dsSources.findItem( @@ -397,7 +410,7 @@ def _prepareUFOSourceLayer(self, source, localAxisNames, revLayerNameMapping): 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 @@ -717,10 +730,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, ) From 68a3f8cc7aceb52ff230f47bebafde37d1984747 Mon Sep 17 00:00:00 2001 From: Just van Rossum Date: Mon, 4 Sep 2023 18:07:28 +0200 Subject: [PATCH 3/4] Add a comment --- src/fontra/backends/designspace.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/fontra/backends/designspace.py b/src/fontra/backends/designspace.py index 345a787af..2e1906912 100644 --- a/src/fontra/backends/designspace.py +++ b/src/fontra/backends/designspace.py @@ -225,6 +225,10 @@ async def getGlyph(self, glyphName): 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 From 9b899e73848f1ca4435f245a413e4cf0e2f0c23a Mon Sep 17 00:00:00 2001 From: Just van Rossum Date: Mon, 4 Sep 2023 20:13:21 +0200 Subject: [PATCH 4/4] Add R.alt test glyph that redefines global axes as local axes --- .../glyphs.weight=1/R_.alt.glif | 41 ++++++ .../glyphs.weight=1/contents.plist | 8 ++ .../glyphs.width=1,weight=1/R_.alt.glif | 41 ++++++ .../glyphs.width=1,weight=1/contents.plist | 8 ++ .../glyphs.width=1/R_.alt.glif | 41 ++++++ .../glyphs.width=1/contents.plist | 8 ++ .../glyphs/R_.alt.glif | 119 ++++++++++++++++++ .../glyphs/contents.plist | 2 + .../layercontents.plist | 12 ++ test-py/test_backend_designspace.py | 4 +- test-py/test_font.py | 8 +- 11 files changed, 287 insertions(+), 5 deletions(-) create mode 100644 test-py/data/mutatorsans/MutatorSansLightCondensed.ufo/glyphs.weight=1/R_.alt.glif create mode 100644 test-py/data/mutatorsans/MutatorSansLightCondensed.ufo/glyphs.weight=1/contents.plist create mode 100644 test-py/data/mutatorsans/MutatorSansLightCondensed.ufo/glyphs.width=1,weight=1/R_.alt.glif create mode 100644 test-py/data/mutatorsans/MutatorSansLightCondensed.ufo/glyphs.width=1,weight=1/contents.plist create mode 100644 test-py/data/mutatorsans/MutatorSansLightCondensed.ufo/glyphs.width=1/R_.alt.glif create mode 100644 test-py/data/mutatorsans/MutatorSansLightCondensed.ufo/glyphs.width=1/contents.plist create mode 100644 test-py/data/mutatorsans/MutatorSansLightCondensed.ufo/glyphs/R_.alt.glif 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 new file mode 100644 index 000000000..c79129550 --- /dev/null +++ b/test-py/data/mutatorsans/MutatorSansLightCondensed.ufo/glyphs.weight=1/R_.alt.glif @@ -0,0 +1,41 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + 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 new file mode 100644 index 000000000..b07300116 --- /dev/null +++ b/test-py/data/mutatorsans/MutatorSansLightCondensed.ufo/glyphs.weight=1/contents.plist @@ -0,0 +1,8 @@ + + + + + R.alt + R_.alt.glif + + 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 new file mode 100644 index 000000000..f9728ed21 --- /dev/null +++ b/test-py/data/mutatorsans/MutatorSansLightCondensed.ufo/glyphs.width=1,weight=1/R_.alt.glif @@ -0,0 +1,41 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test-py/data/mutatorsans/MutatorSansLightCondensed.ufo/glyphs.width=1,weight=1/contents.plist b/test-py/data/mutatorsans/MutatorSansLightCondensed.ufo/glyphs.width=1,weight=1/contents.plist new file mode 100644 index 000000000..b07300116 --- /dev/null +++ b/test-py/data/mutatorsans/MutatorSansLightCondensed.ufo/glyphs.width=1,weight=1/contents.plist @@ -0,0 +1,8 @@ + + + + + R.alt + R_.alt.glif + + 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 new file mode 100644 index 000000000..7f015db57 --- /dev/null +++ b/test-py/data/mutatorsans/MutatorSansLightCondensed.ufo/glyphs.width=1/R_.alt.glif @@ -0,0 +1,41 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + 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 new file mode 100644 index 000000000..b07300116 --- /dev/null +++ b/test-py/data/mutatorsans/MutatorSansLightCondensed.ufo/glyphs.width=1/contents.plist @@ -0,0 +1,8 @@ + + + + + R.alt + R_.alt.glif + + diff --git a/test-py/data/mutatorsans/MutatorSansLightCondensed.ufo/glyphs/R_.alt.glif b/test-py/data/mutatorsans/MutatorSansLightCondensed.ufo/glyphs/R_.alt.glif new file mode 100644 index 000000000..8b69e2967 --- /dev/null +++ b/test-py/data/mutatorsans/MutatorSansLightCondensed.ufo/glyphs/R_.alt.glif @@ -0,0 +1,119 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + com.black-foundry.glyph-designspace + + axes + + + default + 0 + maximum + 1 + minimum + 0 + name + width + + + default + 0 + maximum + 1 + minimum + 0 + name + weight + + + sources + + + layername + weight=1 + location + + weight + 1 + + + + layername + width=1 + location + + width + 1 + + + + layername + width=1,weight=1 + location + + weight + 1 + width + 1 + + + + + xyz.fontra.layer-names + + MutatorSansLightCondensed/foreground + <default> + MutatorSansLightCondensed/weight=1 + weight=1 + MutatorSansLightCondensed/width=1 + width=1 + MutatorSansLightCondensed/width=1,weight=1 + width=1,weight=1 + + xyz.fontra.source-names + + LightCondensed + <default> + + + + diff --git a/test-py/data/mutatorsans/MutatorSansLightCondensed.ufo/glyphs/contents.plist b/test-py/data/mutatorsans/MutatorSansLightCondensed.ufo/glyphs/contents.plist index 0cd6bdf19..cd85400b8 100644 --- a/test-py/data/mutatorsans/MutatorSansLightCondensed.ufo/glyphs/contents.plist +++ b/test-py/data/mutatorsans/MutatorSansLightCondensed.ufo/glyphs/contents.plist @@ -48,6 +48,8 @@ Q_.glif R R_.glif + R.alt + R_.alt.glif S S_.glif S.closed diff --git a/test-py/data/mutatorsans/MutatorSansLightCondensed.ufo/layercontents.plist b/test-py/data/mutatorsans/MutatorSansLightCondensed.ufo/layercontents.plist index 03bc248b4..363065214 100644 --- a/test-py/data/mutatorsans/MutatorSansLightCondensed.ufo/layercontents.plist +++ b/test-py/data/mutatorsans/MutatorSansLightCondensed.ufo/layercontents.plist @@ -34,5 +34,17 @@ varco_flop glyphs.varco_flop + + weight=1 + glyphs.weight=1 + + + width=1 + glyphs.width=1 + + + width=1,weight=1 + glyphs.width=1,weight=1 + diff --git a/test-py/test_backend_designspace.py b/test-py/test_backend_designspace.py index ff43712ca..05ee0539d 100644 --- a/test-py/test_backend_designspace.py +++ b/test-py/test_backend_designspace.py @@ -50,7 +50,9 @@ def readGLIFData(glyphName, ufoLayers): } -@pytest.mark.parametrize("glyphName", ["A", "B", "Q", "varcotest1", "varcotest2"]) +@pytest.mark.parametrize( + "glyphName", ["A", "B", "Q", "R.alt", "varcotest1", "varcotest2"] +) async def test_roundTripGlyph(writableTestFont, glyphName): existingData = readGLIFData(glyphName, writableTestFont.ufoLayers) glyphMap = await writableTestFont.getGlyphMap() diff --git a/test-py/test_font.py b/test-py/test_font.py index dff767aeb..1a67aa4df 100644 --- a/test-py/test_font.py +++ b/test-py/test_font.py @@ -849,8 +849,8 @@ def getTestFont(testFontName): getGlyphNamesTestData = [ - ("designspace", 51, ["A", "Aacute", "Adieresis", "B"]), - ("ufo", 51, ["A", "Aacute", "Adieresis", "B"]), + ("designspace", 52, ["A", "Aacute", "Adieresis", "B"]), + ("ufo", 52, ["A", "Aacute", "Adieresis", "B"]), ] @@ -869,10 +869,10 @@ async def test_getGlyphNames(testFontName, numGlyphs, firstFourGlyphNames): getGlyphMapTestData = [ ( "designspace", - 51, + 52, {"A": [ord("A"), ord("a")], "B": [ord("B"), ord("b")], "I.narrow": []}, ), - ("ufo", 51, {"A": [ord("A"), ord("a")], "B": [ord("B"), ord("b")], "I.narrow": []}), + ("ufo", 52, {"A": [ord("A"), ord("a")], "B": [ord("B"), ord("b")], "I.narrow": []}), ]