From de23e837feaefa9d586d331b64145ac705c08579 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Wed, 8 Jan 2025 22:08:19 +0100 Subject: [PATCH 01/88] Add all missing 'put' methods from WritableFontBackend as stubs --- README.md | 2 +- src/fontra_glyphs/backend.py | 54 ++++++++++++++++++++++++++++++++++-- 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 3bec738..c5b1181 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # fontra-glyphs -Fontra file system backend for the Glyphs app file format. **Read-only** for now. +Fontra file system backend for the Glyphs app file format. It supports the following features: diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 05152ca..d42dd79 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -16,6 +16,7 @@ GlyphAxis, GlyphSource, Guideline, + ImageData, Kerning, Layer, LineMetric, @@ -24,7 +25,7 @@ VariableGlyph, ) from fontra.core.path import PackedPathPointPen -from fontra.core.protocols import ReadableFontBackend +from fontra.core.protocols import WritableFontBackend from fontTools.designspaceLib import DesignSpaceDocument from fontTools.misc.transform import DecomposedTransform from glyphsLib.builder.axes import ( @@ -81,7 +82,7 @@ class GlyphsBackend: @classmethod - def fromPath(cls, path: PathLike) -> ReadableFontBackend: + def fromPath(cls, path: PathLike) -> WritableFontBackend: self = cls() self._setupFromPath(path) return self @@ -157,6 +158,14 @@ def _loadFiles(path: PathLike) -> tuple[dict[str, Any], list[Any]]: async def getGlyphMap(self) -> dict[str, list[int]]: return self.glyphMap + async def putGlyphMap(self, value: dict[str, list[int]]) -> None: + print("GlyphsBackend putGlyphMap: ", value) + pass + + async def deleteGlyph(self, glyphName): + print("GlyphsBackend deleteGlyph: ", glyphName) + pass + async def getFontInfo(self) -> FontInfo: infoDict = {} for name in rootInfoNames: @@ -172,15 +181,31 @@ async def getFontInfo(self) -> FontInfo: return FontInfo(**infoDict) + async def putFontInfo(self, fontInfo: FontInfo): + print("GlyphsBackend putFontInfo: ", fontInfo) + pass + async def getSources(self) -> dict[str, FontSource]: return gsMastersToFontraFontSources(self.gsFont, self.locationByMasterID) + async def putSources(self, sources: dict[str, FontSource]) -> None: + print("GlyphsBackend putSources: ", sources) + pass + async def getAxes(self) -> Axes: return Axes(axes=self.axes) + async def putAxes(self, axes: Axes) -> None: + print("GlyphsBackend putAxes: ", axes) + pass + async def getUnitsPerEm(self) -> int: return self.gsFont.upm + async def putUnitsPerEm(self, value: int) -> None: + print("GlyphsBackend putUnitsPerEm: ", value) + pass + async def getKerning(self) -> dict[str, Kerning]: # TODO: RTL kerning: https://docu.glyphsapp.com/#GSFont.kerningRTL kerningLTR = gsKerningToFontraKerning( @@ -200,13 +225,32 @@ async def getKerning(self) -> dict[str, Kerning]: kerning["vkrn"] = kerningVertical return kerning + async def putKerning(self, kerning: dict[str, Kerning]) -> None: + print("GlyphsBackend putKerning: ", kerning) + pass + async def getFeatures(self) -> OpenTypeFeatures: # TODO: extract features return OpenTypeFeatures() + async def putFeatures(self, features: OpenTypeFeatures) -> None: + print("GlyphsBackend putFeatures: ", features) + pass + + async def getBackgroundImage(self, imageIdentifier: str) -> ImageData | None: + return None + + async def putBackgroundImage(self, imageIdentifier: str, data: ImageData) -> None: + print("GlyphsBackend putBackgroundImage: ", imageIdentifier, data) + pass + async def getCustomData(self) -> dict[str, Any]: return {} + async def putCustomData(self, lib): + print("GlyphsBackend putCustomData: ", lib) + pass + async def getGlyph(self, glyphName: str) -> VariableGlyph | None: if glyphName not in self.glyphNameToIndex: return None @@ -330,6 +374,12 @@ def _getSmartLocation(self, gsLayer, localAxesByName): if value != localAxesByName[name].defaultValue } + async def putGlyph( + self, glyphName: str, glyph: VariableGlyph, codePoints: list[int] + ) -> None: + print("GlyphsBackend putGlyph: ", glyphName, glyph, codePoints) + pass + async def aclose(self) -> None: pass From 35015d3759494aaf66fe905ee69f4529d3dcb748 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Thu, 16 Jan 2025 23:53:15 +0100 Subject: [PATCH 02/88] GlyphsBackend work in progress: two options for discussion --- src/fontra_glyphs/backend.py | 108 ++++++++++++++++++++++++++++++++++- tests/test_backend.py | 30 +++++++++- 2 files changed, 134 insertions(+), 4 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index d42dd79..a8252bd 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -1,4 +1,5 @@ import pathlib +import re from collections import defaultdict from os import PathLike from typing import Any @@ -89,6 +90,7 @@ def fromPath(cls, path: PathLike) -> WritableFontBackend: def _setupFromPath(self, path: PathLike) -> None: gsFont = glyphsLib.classes.GSFont() + self.gsFilePath = path rawFontData, rawGlyphsData = self._loadFiles(path) @@ -101,6 +103,7 @@ def _setupFromPath(self, path: PathLike) -> None: self.gsFont.glyphs = [ glyphsLib.classes.GSGlyph() for i in range(len(rawGlyphsData)) ] + self.rawFontData = rawFontData self.rawGlyphsData = rawGlyphsData self.glyphNameToIndex = { @@ -331,7 +334,7 @@ def _ensureGlyphIsParsed(self, glyphName: str) -> None: glyphIndex = self.glyphNameToIndex[glyphName] rawGlyphData = self.rawGlyphsData[glyphIndex] - self.rawGlyphsData[glyphIndex] = None + # self.rawGlyphsData[glyphIndex] = None self.parsedGlyphNames.add(glyphName) gsGlyph = glyphsLib.classes.GSGlyph() @@ -377,8 +380,67 @@ def _getSmartLocation(self, gsLayer, localAxesByName): async def putGlyph( self, glyphName: str, glyph: VariableGlyph, codePoints: list[int] ) -> None: - print("GlyphsBackend putGlyph: ", glyphName, glyph, codePoints) - pass + # NOTE: + # Just said: Compare with the fontra backend and the designspace backend, + # see how they implement writing glyphs. + # 1. option: similar to fontra backend + # 2. option: similar to designspace backend + + # 1. option: + # Convert the fontra glyph and replace the glyph in the rawGlyphsData and + # save it via openstep_plist.dump(). + # 1. issue: How to convert the fontra glyph without losing too much data? + # 2. issue: How do we handle GlyphsApp packages? + # 3. issue: The formatting of openstep_plist.dump() is totally different to how a + # .glyphs file usually looks like. + # 4. issue: Glyphs 3 and 2 may look different. Do we have to support that? And if so, + # how would it look like? + + layerMap = {} + for layerIndex, (layerName, layer) in enumerate(iter(glyph.layers.items())): + # For now only do glyph width and assume the layer order is the same. + # (better would be based on layerId not on index). + layerMap[layerIndex] = { + "anchors": [], + "shapes": [], + "width": layer.glyph.xAdvance, + } + + # # TODO: associatedMasterId, attr, background, layerId, name, width + # for i, contour in enumerate(layer.glyph.path.unpackedContours()): + # # make a rawShape based on our fontra glyph + # shape = {"closed": 1 if contour["isClosed"] else 0, "nodes": []} + + # for j, point in enumerate(contour["points"]): + # pointType = "l" # TODO: fontraPointTypeToGsNodeType(point.get("type")) + # shape["nodes"].append([point["x"], point["y"], pointType]) + + # layerMap[layerIndex]["shapes"].append(shape) + + glyphIndex = self.glyphNameToIndex[glyphName] + rawGlyphData = self.rawGlyphsData[glyphIndex] + + for i, rawLayer in enumerate(rawGlyphData["layers"]): + rawLayer["width"] = layerMap[i]["width"] + + self.rawGlyphsData[glyphIndex] = rawGlyphData + self.rawFontData["glyphs"] = self.rawGlyphsData + + with open(self.gsFilePath, "w", encoding="utf-8") as fp: + openstep_plist.dump(self.rawFontData, fp) + + saveFileWithGsFormatting(self.gsFilePath) + + # # 2. option: + # # Similar to designspace backend: Instead of using fonttools for writing UFO + # # use glyphsLib for writing to a GlyphsApp file + + # gsGlyph = self.gsFont.glyphs[glyphName] + # for layerIndex, (layerName, layer) in enumerate(iter(glyph.layers.items())): + # gsLayer = gsGlyph.layers[layerIndex] + # gsLayer.width = layer.glyph.xAdvance + + # self.gsFont.save(self.gsFilePath) async def aclose(self) -> None: pass @@ -690,3 +752,43 @@ def gsVerticalMetricsToFontraLineMetricsHorizontal(gsFont, gsMaster): # ) return lineMetricsHorizontal + + +def saveFileWithGsFormatting(gsFilePath): + # openstep_plist.dump changes the whole formatting, therefore + # it's very diffucute to see what has changed. + # This function is a very bad try to get close to how the formatting + # looks like for a .glyphs file. + # There must be a better solution, but this is better than nothing. + with open(gsFilePath, "r", encoding="utf-8") as file: + content = file.read() + + content = content.replace(", ", ",") + content = content.replace("{", "{\n") + content = content.replace(";", ";\n") + content = content.replace("(", "(\n") + content = content.replace(",", ",\n") + content = content.replace(")", "\n)") + + content = re.sub(r"\(\s*(\d+),\s*(\d+),\s*([a-zA-Z])\s*\)", r"(\1,\2,\3)", content) + + content = re.sub( + r"\(\s*(\d+),\s*(-?\d+),\s*([a-zA-Z]+)\s*\)", r"(\1,\2,\3)", content + ) + content = re.sub( + r"\{\s*(\d+),\s*(-?\d+),\s*([a-zA-Z]+)\s*\}", r"{\1, \2, \3}", content + ) + + content = re.sub(r"\(\s*([\d.]+),\s*([\d.]+)\s*\)", r"(\1,\2)", content) + content = re.sub(r"\{\s*([\d.]+),\s*([\d.]+)\s*\}", r"{\1,\2}", content) + + content = re.sub( + r"\{\s*([\d.]+),\s*([\d.]+),\s*([\d.]+),\s*([\d.]+)\s*\}", + r"{\1, \2, \3, \4}", + content, + ) + + content = "\n".join(line.strip() for line in content.splitlines()) + + with open(gsFilePath, "w", encoding="utf-8") as file: + file.write(content) diff --git a/tests/test_backend.py b/tests/test_backend.py index 9cd8f4a..4228d1c 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -8,7 +8,9 @@ glyphs2Path = dataDir / "GlyphsUnitTestSans.glyphs" glyphs3Path = dataDir / "GlyphsUnitTestSans3.glyphs" -glyphsPackagePath = dataDir / "GlyphsUnitTestSans3.glyphspackage" +glyphsPackagePath = ( + dataDir / "GlyphsUnitTestSans3.glyphs" +) # "GlyphsUnitTestSans3.glyphspackage" # ignore package for now. referenceFontPath = dataDir / "GlyphsUnitTestSans3.fontra" @@ -116,6 +118,32 @@ async def test_getGlyph(testFont, referenceFont, glyphName): assert referenceGlyph == glyph +@pytest.mark.asyncio +@pytest.mark.parametrize("glyphName", list(expectedGlyphMap)) +async def test_putGlyph(testFont, referenceFont, glyphName): + glyphMap = await testFont.getGlyphMap() + glyph = await testFont.getGlyph(glyphName) + if glyphName == "A" and "com.glyphsapp.glyph-color" not in glyph.customData: + # glyphsLib doesn't read the color attr from Glyphs-2 files, + # so let's monkeypatch the data + glyph.customData = {"com.glyphsapp.glyph-color": [120, 220, 20, 4]} + + # # for testing change every coordinate by 10 units + # for (layerName, layer) in iter(glyph.layers.items()): + # for i, coordinate in enumerate(layer.glyph.path.coordinates): + # layer.glyph.path.coordinates[i] = coordinate + 10 + + # for testing change xAdvance + for layerName, layer in iter(glyph.layers.items()): + for i, coordinate in enumerate(layer.glyph.path.coordinates): + layer.glyph.xAdvance = 500 + + await testFont.putGlyph(glyphName, glyph, glyphMap[glyphName]) + + referenceGlyph = await referenceFont.getGlyph(glyphName) + assert referenceGlyph == glyph + + async def test_getKerning(testFont, referenceFont): assert await testFont.getKerning() == await referenceFont.getKerning() From 417d2fd17a8acbb0c88e869821b1df775fe606cd Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Fri, 17 Jan 2025 16:28:46 +0100 Subject: [PATCH 03/88] Remove option 2, use some better options for openstep_plist.dump, adjust saveFileWithGsFormatting --- src/fontra_glyphs/backend.py | 78 ++++++++++++++++++------------------ 1 file changed, 38 insertions(+), 40 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index a8252bd..5d33d02 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -381,20 +381,21 @@ async def putGlyph( self, glyphName: str, glyph: VariableGlyph, codePoints: list[int] ) -> None: # NOTE: - # Just said: Compare with the fontra backend and the designspace backend, - # see how they implement writing glyphs. - # 1. option: similar to fontra backend - # 2. option: similar to designspace backend - - # 1. option: - # Convert the fontra glyph and replace the glyph in the rawGlyphsData and - # save it via openstep_plist.dump(). - # 1. issue: How to convert the fontra glyph without losing too much data? - # 2. issue: How do we handle GlyphsApp packages? - # 3. issue: The formatting of openstep_plist.dump() is totally different to how a - # .glyphs file usually looks like. - # 4. issue: Glyphs 3 and 2 may look different. Do we have to support that? And if so, - # how would it look like? + # Reading a glyph from .glyphs(package) + + # 1. parse the source text into a "raw" object + # 2. turn the "raw" object into a GSGlyph instance + # 3. convert GSGlyph to VariableGlyph + + # Writing a glyph to .glyphs(package) + + # 1. convert VariableGlyph to GSGlyph (but start with a copy of the original) + # 2. serialize to text with glyphsLib.writer.Writer(), using io.StringIO or io.BytesIO + # 3. parse stream into "raw" object + # 4. replace original "raw" object with new "raw" object + # 5. write whole file with openstep_plist + + # _writeRawGlyph(glyphName, rawGlyph) layerMap = {} for layerIndex, (layerName, layer) in enumerate(iter(glyph.layers.items())): @@ -427,21 +428,17 @@ async def putGlyph( self.rawFontData["glyphs"] = self.rawGlyphsData with open(self.gsFilePath, "w", encoding="utf-8") as fp: - openstep_plist.dump(self.rawFontData, fp) + openstep_plist.dump( + self.rawFontData, + fp, + unicode_escape=False, + indent=0, + single_line_tuples=True, + escape_newlines=False, + ) saveFileWithGsFormatting(self.gsFilePath) - # # 2. option: - # # Similar to designspace backend: Instead of using fonttools for writing UFO - # # use glyphsLib for writing to a GlyphsApp file - - # gsGlyph = self.gsFont.glyphs[glyphName] - # for layerIndex, (layerName, layer) in enumerate(iter(glyph.layers.items())): - # gsLayer = gsGlyph.layers[layerIndex] - # gsLayer.width = layer.glyph.xAdvance - - # self.gsFont.save(self.gsFilePath) - async def aclose(self) -> None: pass @@ -763,32 +760,33 @@ def saveFileWithGsFormatting(gsFilePath): with open(gsFilePath, "r", encoding="utf-8") as file: content = file.read() - content = content.replace(", ", ",") - content = content.replace("{", "{\n") - content = content.replace(";", ";\n") - content = content.replace("(", "(\n") - content = content.replace(",", ",\n") - content = content.replace(")", "\n)") + content = re.sub(r"pos = \(\s*(-?\d+),\s*(-?\d+)\s*\);", r"pos = (\1,\2);", content) - content = re.sub(r"\(\s*(\d+),\s*(\d+),\s*([a-zA-Z])\s*\)", r"(\1,\2,\3)", content) + content = re.sub( + r"pos = \(\s*([\d.]+),\s*([\d.]+)\s*\);", r"pos = (\1,\2);", content + ) content = re.sub( - r"\(\s*(\d+),\s*(-?\d+),\s*([a-zA-Z]+)\s*\)", r"(\1,\2,\3)", content + r"\(\s*([\d.]+),\s*(-?\d+),\s*([a-zA-Z])\s*\)", r"(\1,\2,\3)", content ) + content = re.sub( - r"\{\s*(\d+),\s*(-?\d+),\s*([a-zA-Z]+)\s*\}", r"{\1, \2, \3}", content + r"origin = \(\s*(-?\d+),\s*(-?\d+)\s*\);", r"origin = (\1,\2);", content ) - content = re.sub(r"\(\s*([\d.]+),\s*([\d.]+)\s*\)", r"(\1,\2)", content) - content = re.sub(r"\{\s*([\d.]+),\s*([\d.]+)\s*\}", r"{\1,\2}", content) + content = re.sub( + r"target = \(\s*(-?\d+),\s*(-?\d+)\s*\);", r"target = (\1,\2);", content + ) content = re.sub( - r"\{\s*([\d.]+),\s*([\d.]+),\s*([\d.]+),\s*([\d.]+)\s*\}", - r"{\1, \2, \3, \4}", + r"color = \(\s*(\d+),\s*(\d+),\s*(\d+),\s*(\d+)\s*\);", + r"color = (\1,\2,\3,\4);", content, ) - content = "\n".join(line.strip() for line in content.splitlines()) + content = re.sub( + r"\(\s*(-?\d+),\s*(-?\d+),\s*([a-zA-Z]+)\s*\)", r"(\1,\2,\3)", content + ) with open(gsFilePath, "w", encoding="utf-8") as file: file.write(content) From 1f25bbb2fb58a3df230071244fc1e2b1a4aa26de Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Fri, 17 Jan 2025 17:21:00 +0100 Subject: [PATCH 04/88] Rework putGlyph based on brainstorming together with Just # 1. convert VariableGlyph to GSGlyph (but start with a copy of the original) # 2. serialize to text with glyphsLib.writer.Writer(), using io.StringIO or io.BytesIO # 3. parse stream into "raw" object # 4. replace original "raw" object with new "raw" object # 5. write whole file with openstep_plist --- src/fontra_glyphs/backend.py | 72 +++++++++++++++++------------------- tests/test_backend.py | 7 +--- 2 files changed, 36 insertions(+), 43 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 5d33d02..dd0eb33 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -1,6 +1,8 @@ +import io import pathlib import re from collections import defaultdict +from copy import deepcopy from os import PathLike from typing import Any @@ -380,53 +382,28 @@ def _getSmartLocation(self, gsLayer, localAxesByName): async def putGlyph( self, glyphName: str, glyph: VariableGlyph, codePoints: list[int] ) -> None: - # NOTE: - # Reading a glyph from .glyphs(package) - - # 1. parse the source text into a "raw" object - # 2. turn the "raw" object into a GSGlyph instance - # 3. convert GSGlyph to VariableGlyph - - # Writing a glyph to .glyphs(package) - # 1. convert VariableGlyph to GSGlyph (but start with a copy of the original) - # 2. serialize to text with glyphsLib.writer.Writer(), using io.StringIO or io.BytesIO - # 3. parse stream into "raw" object - # 4. replace original "raw" object with new "raw" object - # 5. write whole file with openstep_plist + gsGlyphNew = deepcopy(self.gsFont.glyphs[glyphName]) - # _writeRawGlyph(glyphName, rawGlyph) - - layerMap = {} - for layerIndex, (layerName, layer) in enumerate(iter(glyph.layers.items())): - # For now only do glyph width and assume the layer order is the same. - # (better would be based on layerId not on index). - layerMap[layerIndex] = { - "anchors": [], - "shapes": [], - "width": layer.glyph.xAdvance, - } - - # # TODO: associatedMasterId, attr, background, layerId, name, width - # for i, contour in enumerate(layer.glyph.path.unpackedContours()): - # # make a rawShape based on our fontra glyph - # shape = {"closed": 1 if contour["isClosed"] else 0, "nodes": []} + # 2. serialize to text with glyphsLib.writer.Writer(), using io.StringIO or io.BytesIO + f = io.StringIO() + writer = glyphsLib.writer.Writer(f) + writer.format_version = self.gsFont.format_version + writer.write(gsGlyphNew) - # for j, point in enumerate(contour["points"]): - # pointType = "l" # TODO: fontraPointTypeToGsNodeType(point.get("type")) - # shape["nodes"].append([point["x"], point["y"], pointType]) + # 3. parse stream into "raw" object + f.seek(0) + rawGlyphData = openstep_plist.load(f, use_numbers=True) - # layerMap[layerIndex]["shapes"].append(shape) + self._writeRawGlyph(glyphName, rawGlyphData) + def _writeRawGlyph(self, glyphName, rawGlyphData): + # 4. replace original "raw" object with new "raw" object glyphIndex = self.glyphNameToIndex[glyphName] - rawGlyphData = self.rawGlyphsData[glyphIndex] - - for i, rawLayer in enumerate(rawGlyphData["layers"]): - rawLayer["width"] = layerMap[i]["width"] - self.rawGlyphsData[glyphIndex] = rawGlyphData self.rawFontData["glyphs"] = self.rawGlyphsData + # 5. write whole file with openstep_plist with open(self.gsFilePath, "w", encoding="utf-8") as fp: openstep_plist.dump( self.rawFontData, @@ -437,6 +414,7 @@ async def putGlyph( escape_newlines=False, ) + # 6. fix formatting saveFileWithGsFormatting(self.gsFilePath) async def aclose(self) -> None: @@ -480,6 +458,24 @@ def sortKey(glyphData): return rawFontData, rawGlyphsData + def _writeRawGlyph(self, glyphName, rawGlyphData): + glyphsPath = pathlib.Path(self.gsFilePath) / "glyphs" + + # 5. write glyh specific file with openstep_plist + glyphPath = f"{glyphsPath}/{glyphName}.glyph" + with open(glyphPath, "w", encoding="utf-8") as fp: + openstep_plist.dump( + rawGlyphData, + fp, + unicode_escape=False, + indent=0, + single_line_tuples=True, + escape_newlines=False, + ) + + # 6. fix formatting + saveFileWithGsFormatting(glyphPath) + def _readGlyphMapAndKerningGroups( rawGlyphsData: list, formatVersion: int diff --git a/tests/test_backend.py b/tests/test_backend.py index 4228d1c..67fd3fc 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -8,9 +8,7 @@ glyphs2Path = dataDir / "GlyphsUnitTestSans.glyphs" glyphs3Path = dataDir / "GlyphsUnitTestSans3.glyphs" -glyphsPackagePath = ( - dataDir / "GlyphsUnitTestSans3.glyphs" -) # "GlyphsUnitTestSans3.glyphspackage" # ignore package for now. +glyphsPackagePath = dataDir / "GlyphsUnitTestSans3.glyphspackage" referenceFontPath = dataDir / "GlyphsUnitTestSans3.fontra" @@ -135,8 +133,7 @@ async def test_putGlyph(testFont, referenceFont, glyphName): # for testing change xAdvance for layerName, layer in iter(glyph.layers.items()): - for i, coordinate in enumerate(layer.glyph.path.coordinates): - layer.glyph.xAdvance = 500 + layer.glyph.xAdvance = 500 await testFont.putGlyph(glyphName, glyph, glyphMap[glyphName]) From e8ef42ae31dcb34cbbfdefd4dc99c8ecbce3d662 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Fri, 17 Jan 2025 17:46:12 +0100 Subject: [PATCH 05/88] Add A-cy for testing --- .../glyphs/A_-cy.glyph | 33 +++++++++++++++++++ .../order.plist | 3 +- tests/test_backend.py | 1 + 3 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 tests/data/GlyphsUnitTestSans3.glyphspackage/glyphs/A_-cy.glyph diff --git a/tests/data/GlyphsUnitTestSans3.glyphspackage/glyphs/A_-cy.glyph b/tests/data/GlyphsUnitTestSans3.glyphspackage/glyphs/A_-cy.glyph new file mode 100644 index 0000000..8da36a6 --- /dev/null +++ b/tests/data/GlyphsUnitTestSans3.glyphspackage/glyphs/A_-cy.glyph @@ -0,0 +1,33 @@ +{ +glyphname = "A-cy"; +layers = ( +{ +layerId = "C4872ECA-A3A9-40AB-960A-1DB2202F16DE"; +shapes = ( +{ +ref = A; +} +); +width = 593; +}, +{ +layerId = "3E7589AA-8194-470F-8E2F-13C1C581BE24"; +shapes = ( +{ +ref = A; +} +); +width = 657; +}, +{ +layerId = "BFFFD157-90D3-4B85-B99D-9A2F366F03CA"; +shapes = ( +{ +ref = A; +} +); +width = 753; +} +); +unicode = 1040; +} diff --git a/tests/data/GlyphsUnitTestSans3.glyphspackage/order.plist b/tests/data/GlyphsUnitTestSans3.glyphspackage/order.plist index 35b0584..5134c32 100644 --- a/tests/data/GlyphsUnitTestSans3.glyphspackage/order.plist +++ b/tests/data/GlyphsUnitTestSans3.glyphspackage/order.plist @@ -10,5 +10,6 @@ a.sc, dieresis, _part.shoulder, _part.stem, -V +V, +"A-cy" ) diff --git a/tests/test_backend.py b/tests/test_backend.py index 67fd3fc..5260044 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -69,6 +69,7 @@ async def test_getAxes(testFont): "m": [109], "n": [110], "V": [86], + "A-cy": [0x0410], } From e2a2626049d8c9f4fa5a6868359302778d631c6b Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Fri, 17 Jan 2025 18:04:22 +0100 Subject: [PATCH 06/88] get the right glyphspackage glyph file name --- src/fontra_glyphs/backend.py | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index dd0eb33..8baf117 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -462,7 +462,10 @@ def _writeRawGlyph(self, glyphName, rawGlyphData): glyphsPath = pathlib.Path(self.gsFilePath) / "glyphs" # 5. write glyh specific file with openstep_plist - glyphPath = f"{glyphsPath}/{glyphName}.glyph" + # TODO: Get the right glyph file name will be challenging, + # because for example the glyph A-cy is stored in the package as A_-cy.glyph + realGlyphName = getGlyphspackageGlyphFileName(glyphName) + glyphPath = f"{glyphsPath}/{realGlyphName}.glyph" with open(glyphPath, "w", encoding="utf-8") as fp: openstep_plist.dump( rawGlyphData, @@ -477,6 +480,18 @@ def _writeRawGlyph(self, glyphName, rawGlyphData): saveFileWithGsFormatting(glyphPath) +def getGlyphspackageGlyphFileName(glyphName): + nameParts = glyphName.split("-") + firstPart = ( + f"{nameParts[0]}_" + if len(nameParts[0]) == 1 and nameParts[0].isupper() + else nameParts[0] + ) + nameParts[0] = firstPart + + return "-".join(nameParts) + + def _readGlyphMapAndKerningGroups( rawGlyphsData: list, formatVersion: int ) -> tuple[dict[str, list[int]], dict[str, tuple[str, str]]]: @@ -784,5 +799,17 @@ def saveFileWithGsFormatting(gsFilePath): r"\(\s*(-?\d+),\s*(-?\d+),\s*([a-zA-Z]+)\s*\)", r"(\1,\2,\3)", content ) + content = re.sub( + r"\(\s*(-?\d+),\s*(-?\d+),\s*([a-zA-Z]),\s*\{", r"(\1,\2,\3,{", content + ) + + content = re.sub( + r"\(\s*(-?\d+),\s*(-?\d+),\s*([a-zA-Z]+),\s*\{", r"(\1,\2,\3,{", content + ) + + content = re.sub(r"\}\s*\),", r"}),", content) + + content += "\n" # add blank break at the end of the file. + with open(gsFilePath, "w", encoding="utf-8") as file: file.write(content) From 6dcbf389b373f62a9e01f7480b4f2ce79f59494f Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Fri, 17 Jan 2025 21:01:33 +0100 Subject: [PATCH 07/88] Add _findAndReplaceGlyph --- src/fontra_glyphs/backend.py | 85 ++++++++++++++++++++++++++++++------ 1 file changed, 71 insertions(+), 14 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 8baf117..156426c 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -383,7 +383,9 @@ async def putGlyph( self, glyphName: str, glyph: VariableGlyph, codePoints: list[int] ) -> None: # 1. convert VariableGlyph to GSGlyph (but start with a copy of the original) - gsGlyphNew = deepcopy(self.gsFont.glyphs[glyphName]) + gsGlyphNew = variableGlyphToGsGlyph( + glyph, deepcopy(self.gsFont.glyphs[glyphName]) + ) # 2. serialize to text with glyphsLib.writer.Writer(), using io.StringIO or io.BytesIO f = io.StringIO() @@ -391,11 +393,52 @@ async def putGlyph( writer.format_version = self.gsFont.format_version writer.write(gsGlyphNew) - # 3. parse stream into "raw" object - f.seek(0) - rawGlyphData = openstep_plist.load(f, use_numbers=True) - - self._writeRawGlyph(glyphName, rawGlyphData) + # # 3. parse stream into "raw" object + # f.seek(0) + # rawGlyphData = openstep_plist.load(f, use_numbers=True) + + # self._writeRawGlyph(glyphName, rawGlyphData) + + self._findAndReplaceGlyph(glyphName, f) + + def _findAndReplaceGlyph(self, glyphName, f): + glyphChunkIndicator = f"glyphname = {glyphName};" + + # find glyph chunk + glyphChunkStart = None + glyphChunkEnd = None + + with open(self.gsFilePath, "r", encoding="utf-8") as fp: + lines = fp.readlines() + for i, line in enumerate(lines): + if glyphChunkIndicator in line: + for j in range(i, 0, -1): + if "{" in lines[j]: + glyphChunkStart = j + break + + braceLeftCount = 1 + for k in range(i, len(lines)): + if "{" in lines[k]: + braceLeftCount += 1 + if "}" in lines[k]: + braceLeftCount -= 1 + if "}" in lines[k] and braceLeftCount == 0: + glyphChunkEnd = k + break + break + + if glyphChunkStart is None or glyphChunkEnd is None: + # If not found, maybe add as a new glyph at the end of the glyphs list. + print("ERROR: Could not find glyph: ", glyphChunkIndicator) + return + + rawGlyphAsText = f.getvalue() + newLines = ( + lines[:glyphChunkStart] + [rawGlyphAsText[:-2]] + lines[glyphChunkEnd:] + ) + with open(self.gsFilePath, "w", encoding="utf-8") as fp: + fp.writelines(newLines) def _writeRawGlyph(self, glyphName, rawGlyphData): # 4. replace original "raw" object with new "raw" object @@ -459,14 +502,9 @@ def sortKey(glyphData): return rawFontData, rawGlyphsData def _writeRawGlyph(self, glyphName, rawGlyphData): - glyphsPath = pathlib.Path(self.gsFilePath) / "glyphs" - # 5. write glyh specific file with openstep_plist - # TODO: Get the right glyph file name will be challenging, - # because for example the glyph A-cy is stored in the package as A_-cy.glyph - realGlyphName = getGlyphspackageGlyphFileName(glyphName) - glyphPath = f"{glyphsPath}/{realGlyphName}.glyph" - with open(glyphPath, "w", encoding="utf-8") as fp: + filePath = self.getGlyphFilePath(glyphName) + with open(filePath, "w", encoding="utf-8") as fp: openstep_plist.dump( rawGlyphData, fp, @@ -477,7 +515,18 @@ def _writeRawGlyph(self, glyphName, rawGlyphData): ) # 6. fix formatting - saveFileWithGsFormatting(glyphPath) + saveFileWithGsFormatting(filePath) + + def _findAndReplaceGlyph(self, glyphName, f): + filePath = self.getGlyphFilePath(glyphName) + filePath.write_text(f.getvalue(), encoding="utf=8") + + def getGlyphFilePath(self, glyphName): + glyphsPath = pathlib.Path(self.gsFilePath) / "glyphs" + # TODO: Get the right glyph file name might be challenging, + # because for example the glyph A-cy is stored in the package as A_-cy.glyph + realGlyphName = getGlyphspackageGlyphFileName(glyphName) + return glyphsPath / (realGlyphName + ".glyph") def getGlyphspackageGlyphFileName(glyphName): @@ -762,6 +811,7 @@ def gsVerticalMetricsToFontraLineMetricsHorizontal(gsFont, gsMaster): return lineMetricsHorizontal +# The following should be obsolte with _findAndReplaceGlyph def saveFileWithGsFormatting(gsFilePath): # openstep_plist.dump changes the whole formatting, therefore # it's very diffucute to see what has changed. @@ -813,3 +863,10 @@ def saveFileWithGsFormatting(gsFilePath): with open(gsFilePath, "w", encoding="utf-8") as file: file.write(content) + + +def variableGlyphToGsGlyph(variableGlyph, gsGlyph): + # TODO: convert fontra variableGlyph to GlyphsApp glyph + gsGlyph.name = f"{variableGlyph.name}.changed" + + return gsGlyph From d8ac61e4edba260b4c03f69ea042e65bffe96dc6 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Fri, 17 Jan 2025 21:54:38 +0100 Subject: [PATCH 08/88] openstep_plist: Fix order issue with toOrderedDict --- src/fontra_glyphs/backend.py | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 156426c..b3ba591 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -1,7 +1,7 @@ import io import pathlib import re -from collections import defaultdict +from collections import OrderedDict, defaultdict from copy import deepcopy from os import PathLike from typing import Any @@ -105,8 +105,8 @@ def _setupFromPath(self, path: PathLike) -> None: self.gsFont.glyphs = [ glyphsLib.classes.GSGlyph() for i in range(len(rawGlyphsData)) ] - self.rawFontData = rawFontData - self.rawGlyphsData = rawGlyphsData + self.rawFontData = toOrderedDict(rawFontData) + self.rawGlyphsData = toOrderedDict(rawGlyphsData) self.glyphNameToIndex = { glyphData["glyphname"]: i for i, glyphData in enumerate(rawGlyphsData) @@ -393,13 +393,13 @@ async def putGlyph( writer.format_version = self.gsFont.format_version writer.write(gsGlyphNew) - # # 3. parse stream into "raw" object - # f.seek(0) - # rawGlyphData = openstep_plist.load(f, use_numbers=True) + # 3. parse stream into "raw" object + f.seek(0) + rawGlyphData = openstep_plist.load(f, use_numbers=True) - # self._writeRawGlyph(glyphName, rawGlyphData) + self._writeRawGlyph(glyphName, rawGlyphData) - self._findAndReplaceGlyph(glyphName, f) + # self._findAndReplaceGlyph(glyphName, f) def _findAndReplaceGlyph(self, glyphName, f): glyphChunkIndicator = f"glyphname = {glyphName};" @@ -811,7 +811,7 @@ def gsVerticalMetricsToFontraLineMetricsHorizontal(gsFont, gsMaster): return lineMetricsHorizontal -# The following should be obsolte with _findAndReplaceGlyph +# The following should be obsolete with _findAndReplaceGlyph def saveFileWithGsFormatting(gsFilePath): # openstep_plist.dump changes the whole formatting, therefore # it's very diffucute to see what has changed. @@ -870,3 +870,12 @@ def variableGlyphToGsGlyph(variableGlyph, gsGlyph): gsGlyph.name = f"{variableGlyph.name}.changed" return gsGlyph + + +def toOrderedDict(obj): + if isinstance(obj, dict): + return OrderedDict({k: toOrderedDict(v) for k, v in obj.items()}) + elif isinstance(obj, list): + return [toOrderedDict(item) for item in obj] + else: + return obj From 354e7fd5b18a4cb6ac48fef322c6eb102fffda26 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Fri, 17 Jan 2025 21:59:04 +0100 Subject: [PATCH 09/88] Remove _findAndReplaceGlyph idea --- src/fontra_glyphs/backend.py | 66 +++--------------------------------- 1 file changed, 4 insertions(+), 62 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index b3ba591..81aaff8 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -393,54 +393,13 @@ async def putGlyph( writer.format_version = self.gsFont.format_version writer.write(gsGlyphNew) + self._writeRawGlyph(glyphName, f) + + def _writeRawGlyph(self, glyphName, f): # 3. parse stream into "raw" object f.seek(0) rawGlyphData = openstep_plist.load(f, use_numbers=True) - self._writeRawGlyph(glyphName, rawGlyphData) - - # self._findAndReplaceGlyph(glyphName, f) - - def _findAndReplaceGlyph(self, glyphName, f): - glyphChunkIndicator = f"glyphname = {glyphName};" - - # find glyph chunk - glyphChunkStart = None - glyphChunkEnd = None - - with open(self.gsFilePath, "r", encoding="utf-8") as fp: - lines = fp.readlines() - for i, line in enumerate(lines): - if glyphChunkIndicator in line: - for j in range(i, 0, -1): - if "{" in lines[j]: - glyphChunkStart = j - break - - braceLeftCount = 1 - for k in range(i, len(lines)): - if "{" in lines[k]: - braceLeftCount += 1 - if "}" in lines[k]: - braceLeftCount -= 1 - if "}" in lines[k] and braceLeftCount == 0: - glyphChunkEnd = k - break - break - - if glyphChunkStart is None or glyphChunkEnd is None: - # If not found, maybe add as a new glyph at the end of the glyphs list. - print("ERROR: Could not find glyph: ", glyphChunkIndicator) - return - - rawGlyphAsText = f.getvalue() - newLines = ( - lines[:glyphChunkStart] + [rawGlyphAsText[:-2]] + lines[glyphChunkEnd:] - ) - with open(self.gsFilePath, "w", encoding="utf-8") as fp: - fp.writelines(newLines) - - def _writeRawGlyph(self, glyphName, rawGlyphData): # 4. replace original "raw" object with new "raw" object glyphIndex = self.glyphNameToIndex[glyphName] self.rawGlyphsData[glyphIndex] = rawGlyphData @@ -501,23 +460,7 @@ def sortKey(glyphData): return rawFontData, rawGlyphsData - def _writeRawGlyph(self, glyphName, rawGlyphData): - # 5. write glyh specific file with openstep_plist - filePath = self.getGlyphFilePath(glyphName) - with open(filePath, "w", encoding="utf-8") as fp: - openstep_plist.dump( - rawGlyphData, - fp, - unicode_escape=False, - indent=0, - single_line_tuples=True, - escape_newlines=False, - ) - - # 6. fix formatting - saveFileWithGsFormatting(filePath) - - def _findAndReplaceGlyph(self, glyphName, f): + def _writeRawGlyph(self, glyphName, f): filePath = self.getGlyphFilePath(glyphName) filePath.write_text(f.getvalue(), encoding="utf=8") @@ -811,7 +754,6 @@ def gsVerticalMetricsToFontraLineMetricsHorizontal(gsFont, gsMaster): return lineMetricsHorizontal -# The following should be obsolete with _findAndReplaceGlyph def saveFileWithGsFormatting(gsFilePath): # openstep_plist.dump changes the whole formatting, therefore # it's very diffucute to see what has changed. From bf1bfb9123e308526cd8151fd692918a2e30ec2a Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Fri, 17 Jan 2025 23:48:30 +0100 Subject: [PATCH 10/88] Commit current stage. --- src/fontra_glyphs/backend.py | 55 +++++++++++++++++++++++++++++++++++- tests/test_backend.py | 8 +++--- 2 files changed, 58 insertions(+), 5 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 81aaff8..93d038e 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -807,9 +807,62 @@ def saveFileWithGsFormatting(gsFilePath): file.write(content) +def fontraLayerToGSLayer(layer, gsLayer=None): + if gsLayer is None: + gsLayer = glyphsLib.classes.GSLayer() + + gsLayer.width = layer.glyph.xAdvance + + return gsLayer + + +def fontraLayerToGSPaths(layer, gsLayer=None): + if gsLayer is None: + gsLayer = glyphsLib.classes.GSLayer() + + gsPaths = [] + # I have the feeling the following goes into the wrong direction. + # It might should work with a pen and with the help of glyphsLib. + for i, contour in enumerate(layer.glyph.path.unpackedContours()): + gsPath = glyphsLib.classes.GSPath() + gsPath.closed = contour["isClosed"] + gsPath.nodes = [] + + for j, point in enumerate(contour["points"]): + gsNode = glyphsLib.classes.GSNode() + gsNode.position = ( + point["x"], + point["y"], + ) # glyphsLib.classes.Point(point["x"], point["y"]) + gsNode.smooth = point.get("smooth", False) + # gsNode.type = fontraPointTypeToGsNodeType(point.get("type")) + gsPath.nodes.append(gsNode) + gsPaths.append(gsPath) + + return gsPaths + + +def fontraPointTypeToGsNodeType(pointType): + # The type of the node, LINE, CURVE or OFFCURVE + # https://docu.glyphsapp.com/#GSNode.type + if pointType is None: + # Can either be LINE or CURVE, I am currently not sure how to figure this out. + return "LINE" # "CURVE" + elif pointType == "cubic": + return "OFFCURVE" + elif pointType == "quad": + return "QCURVE" + + def variableGlyphToGsGlyph(variableGlyph, gsGlyph): # TODO: convert fontra variableGlyph to GlyphsApp glyph - gsGlyph.name = f"{variableGlyph.name}.changed" + + for i, (layerName, layer) in enumerate(iter(variableGlyph.layers.items())): + gsLayerCopy = deepcopy(gsGlyph.layers[i]) + gsGlyph.layers[i].width = layer.glyph.xAdvance + gsGlyph.layers[i].paths = fontraLayerToGSPaths(layer, gsLayerCopy) + + # gsGlyph.layers[i] = fontraLayerToGSLayer(layer, gsLayerCopy) return gsGlyph diff --git a/tests/test_backend.py b/tests/test_backend.py index 5260044..982f6ad 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -127,10 +127,10 @@ async def test_putGlyph(testFont, referenceFont, glyphName): # so let's monkeypatch the data glyph.customData = {"com.glyphsapp.glyph-color": [120, 220, 20, 4]} - # # for testing change every coordinate by 10 units - # for (layerName, layer) in iter(glyph.layers.items()): - # for i, coordinate in enumerate(layer.glyph.path.coordinates): - # layer.glyph.path.coordinates[i] = coordinate + 10 + # for testing change every coordinate by 10 units + for layerName, layer in iter(glyph.layers.items()): + for i, coordinate in enumerate(layer.glyph.path.coordinates): + layer.glyph.path.coordinates[i] = coordinate + 10 # for testing change xAdvance for layerName, layer in iter(glyph.layers.items()): From c7a66a4969f9d5799cb9f0f09a8d48ef26617a62 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Sun, 19 Jan 2025 02:43:34 +0100 Subject: [PATCH 11/88] Use pen for drawing --- src/fontra_glyphs/backend.py | 54 ++++++------------------------------ 1 file changed, 8 insertions(+), 46 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 93d038e..9f07135 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -807,62 +807,24 @@ def saveFileWithGsFormatting(gsFilePath): file.write(content) -def fontraLayerToGSLayer(layer, gsLayer=None): - if gsLayer is None: - gsLayer = glyphsLib.classes.GSLayer() - +def fontraLayerToGSLayer(layer, gsLayer): gsLayer.width = layer.glyph.xAdvance - return gsLayer - - -def fontraLayerToGSPaths(layer, gsLayer=None): - if gsLayer is None: - gsLayer = glyphsLib.classes.GSLayer() - - gsPaths = [] - # I have the feeling the following goes into the wrong direction. - # It might should work with a pen and with the help of glyphsLib. - for i, contour in enumerate(layer.glyph.path.unpackedContours()): - gsPath = glyphsLib.classes.GSPath() - gsPath.closed = contour["isClosed"] - gsPath.nodes = [] - - for j, point in enumerate(contour["points"]): - gsNode = glyphsLib.classes.GSNode() - gsNode.position = ( - point["x"], - point["y"], - ) # glyphsLib.classes.Point(point["x"], point["y"]) - gsNode.smooth = point.get("smooth", False) - # gsNode.type = fontraPointTypeToGsNodeType(point.get("type")) - gsPath.nodes.append(gsNode) - gsPaths.append(gsPath) - - return gsPaths + # Draw new paths with pen + gsLayer.paths = [] # first: remove all paths + pen = gsLayer.getPointPen() + layer.glyph.path.drawPoints(pen) + gsLayer.drawPoints(pen) -def fontraPointTypeToGsNodeType(pointType): - # The type of the node, LINE, CURVE or OFFCURVE - # https://docu.glyphsapp.com/#GSNode.type - if pointType is None: - # Can either be LINE or CURVE, I am currently not sure how to figure this out. - return "LINE" # "CURVE" - elif pointType == "cubic": - return "OFFCURVE" - elif pointType == "quad": - return "QCURVE" + # TODO: anchors, components, etc. def variableGlyphToGsGlyph(variableGlyph, gsGlyph): # TODO: convert fontra variableGlyph to GlyphsApp glyph for i, (layerName, layer) in enumerate(iter(variableGlyph.layers.items())): - gsLayerCopy = deepcopy(gsGlyph.layers[i]) - gsGlyph.layers[i].width = layer.glyph.xAdvance - gsGlyph.layers[i].paths = fontraLayerToGSPaths(layer, gsLayerCopy) - - # gsGlyph.layers[i] = fontraLayerToGSLayer(layer, gsLayerCopy) + fontraLayerToGSLayer(layer, gsGlyph.layers[i]) return gsGlyph From 62d0e93b6882ed8efffcd7273ad38647d63f9bff Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Sun, 19 Jan 2025 05:01:40 +0100 Subject: [PATCH 12/88] Adding some comments --- src/fontra_glyphs/backend.py | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 9f07135..b015cda 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -383,7 +383,7 @@ async def putGlyph( self, glyphName: str, glyph: VariableGlyph, codePoints: list[int] ) -> None: # 1. convert VariableGlyph to GSGlyph (but start with a copy of the original) - gsGlyphNew = variableGlyphToGsGlyph( + gsGlyphNew = variableGlyphToGSGlyph( glyph, deepcopy(self.gsFont.glyphs[glyphName]) ) @@ -808,27 +808,35 @@ def saveFileWithGsFormatting(gsFilePath): def fontraLayerToGSLayer(layer, gsLayer): - gsLayer.width = layer.glyph.xAdvance - # Draw new paths with pen gsLayer.paths = [] # first: remove all paths pen = gsLayer.getPointPen() layer.glyph.path.drawPoints(pen) gsLayer.drawPoints(pen) - - # TODO: anchors, components, etc. + gsLayer.width = layer.glyph.xAdvance + # gsLayer.components = components # https://docu.glyphsapp.com/#GSLayer.components + # gsLayer.anchors = anchors # https://docu.glyphsapp.com/#GSLayer.anchors -def variableGlyphToGsGlyph(variableGlyph, gsGlyph): +def variableGlyphToGSGlyph(variableGlyph, gsGlyph): # TODO: convert fontra variableGlyph to GlyphsApp glyph for i, (layerName, layer) in enumerate(iter(variableGlyph.layers.items())): fontraLayerToGSLayer(layer, gsGlyph.layers[i]) + # What happens, if the number of layers differ from the original number? + # How do we handle intermediate layers? + # https://docu.glyphsapp.com/#GSGlyph.layers font.glyphs['a'].layers.append(newLayer) + # How do we handle missing masters? + # It might be that someone deletes a not needed layer, which works for fontra, + # but is required for Glyphs. We would need to get the intermediate contours. + return gsGlyph +# The following is obsolete once this is merged: +# https://github.com/fonttools/openstep-plist/pull/35 def toOrderedDict(obj): if isinstance(obj, dict): return OrderedDict({k: toOrderedDict(v) for k, v in obj.items()}) From f76ea61f973b4fb70456342cb470f3eaf95675f2 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Sun, 19 Jan 2025 17:32:43 +0100 Subject: [PATCH 13/88] Add A-cy also to Glyphs 2 and 3 file for unittests --- tests/data/GlyphsUnitTestSans.glyphs | 33 +++++++++++++++++++++++++++ tests/data/GlyphsUnitTestSans3.glyphs | 33 +++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/tests/data/GlyphsUnitTestSans.glyphs b/tests/data/GlyphsUnitTestSans.glyphs index a88827f..042467a 100644 --- a/tests/data/GlyphsUnitTestSans.glyphs +++ b/tests/data/GlyphsUnitTestSans.glyphs @@ -2348,6 +2348,39 @@ rightMetricsKey = "=|V"; topKerningGroup = VTop; bottomKerningGroup = VBottom; unicode = 0056; +}, +{ +glyphname = A-cy; +layers = ( +{ +components = ( +{ +name = A; +} +); +layerId = "C4872ECA-A3A9-40AB-960A-1DB2202F16DE"; +width = 593; +}, +{ +components = ( +{ +name = A; +} +); +layerId = "3E7589AA-8194-470F-8E2F-13C1C581BE24"; +width = 657; +}, +{ +components = ( +{ +name = A; +} +); +layerId = "BFFFD157-90D3-4B85-B99D-9A2F366F03CA"; +width = 753; +} +); +unicode = 1040; } ); instances = ( diff --git a/tests/data/GlyphsUnitTestSans3.glyphs b/tests/data/GlyphsUnitTestSans3.glyphs index 60ca9a0..92810ea 100644 --- a/tests/data/GlyphsUnitTestSans3.glyphs +++ b/tests/data/GlyphsUnitTestSans3.glyphs @@ -2408,6 +2408,39 @@ width = 753; ); metricRight = "=|V"; unicode = 86; +}, +{ +glyphname = "A-cy"; +layers = ( +{ +layerId = "C4872ECA-A3A9-40AB-960A-1DB2202F16DE"; +shapes = ( +{ +ref = A; +} +); +width = 593; +}, +{ +layerId = "3E7589AA-8194-470F-8E2F-13C1C581BE24"; +shapes = ( +{ +ref = A; +} +); +width = 657; +}, +{ +layerId = "BFFFD157-90D3-4B85-B99D-9A2F366F03CA"; +shapes = ( +{ +ref = A; +} +); +width = 753; +} +); +unicode = 1040; } ); instances = ( From b560a8944e4975dc38847e55f8365fa4cfb3a9b4 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Sun, 19 Jan 2025 18:29:03 +0100 Subject: [PATCH 14/88] Rework unittests and fix issue based on self.parsedGlyphNames --- src/fontra_glyphs/backend.py | 10 ++++++--- tests/test_backend.py | 42 ++++++++++++++++++++++++++++-------- 2 files changed, 40 insertions(+), 12 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index b015cda..9eee687 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -393,9 +393,6 @@ async def putGlyph( writer.format_version = self.gsFont.format_version writer.write(gsGlyphNew) - self._writeRawGlyph(glyphName, f) - - def _writeRawGlyph(self, glyphName, f): # 3. parse stream into "raw" object f.seek(0) rawGlyphData = openstep_plist.load(f, use_numbers=True) @@ -405,6 +402,13 @@ def _writeRawGlyph(self, glyphName, f): self.rawGlyphsData[glyphIndex] = rawGlyphData self.rawFontData["glyphs"] = self.rawGlyphsData + self._writeRawGlyph(glyphName, f) + + # 7. Remove glyph from parsed glyph names, because we changed it. + # Next time it needs to be parsed again. + self.parsedGlyphNames.discard(glyphName) + + def _writeRawGlyph(self, glyphName, f): # 5. write whole file with openstep_plist with open(self.gsFilePath, "w", encoding="utf-8") as fp: openstep_plist.dump( diff --git a/tests/test_backend.py b/tests/test_backend.py index 982f6ad..619f624 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -1,4 +1,6 @@ +import os import pathlib +import shutil import pytest from fontra.backends import getFileSystemBackend @@ -22,6 +24,17 @@ def referenceFont(request): return getFileSystemBackend(referenceFontPath) +@pytest.fixture(params=[glyphs2Path, glyphs3Path, glyphsPackagePath]) +def writableTestFont(tmpdir, request): + srcPath = request.param + dstPath = tmpdir / os.path.basename(srcPath) + if os.path.isdir(srcPath): + shutil.copytree(srcPath, dstPath) + else: + shutil.copy(srcPath, dstPath) + return getFileSystemBackend(dstPath) + + expectedAxes = structure( { "axes": [ @@ -119,9 +132,9 @@ async def test_getGlyph(testFont, referenceFont, glyphName): @pytest.mark.asyncio @pytest.mark.parametrize("glyphName", list(expectedGlyphMap)) -async def test_putGlyph(testFont, referenceFont, glyphName): - glyphMap = await testFont.getGlyphMap() - glyph = await testFont.getGlyph(glyphName) +async def test_putGlyph(writableTestFont, testFont, glyphName): + glyphMap = await writableTestFont.getGlyphMap() + glyph = await writableTestFont.getGlyph(glyphName) if glyphName == "A" and "com.glyphsapp.glyph-color" not in glyph.customData: # glyphsLib doesn't read the color attr from Glyphs-2 files, # so let's monkeypatch the data @@ -129,17 +142,28 @@ async def test_putGlyph(testFont, referenceFont, glyphName): # for testing change every coordinate by 10 units for layerName, layer in iter(glyph.layers.items()): + layer.glyph.xAdvance = 500 # for testing change xAdvance for i, coordinate in enumerate(layer.glyph.path.coordinates): layer.glyph.path.coordinates[i] = coordinate + 10 - # for testing change xAdvance - for layerName, layer in iter(glyph.layers.items()): - layer.glyph.xAdvance = 500 + await writableTestFont.putGlyph(glyphName, glyph, glyphMap[glyphName]) - await testFont.putGlyph(glyphName, glyph, glyphMap[glyphName]) + savedGlyph = await writableTestFont.getGlyph(glyphName) + referenceGlyph = await testFont.getGlyph(glyphName) - referenceGlyph = await referenceFont.getGlyph(glyphName) - assert referenceGlyph == glyph + for layerName, layer in iter(referenceGlyph.layers.items()): + assert savedGlyph.layers[layerName].glyph.xAdvance == 500 + + for i, coordinate in enumerate(layer.glyph.path.coordinates): + expectedResult = coordinate + 10 + # The follwing fails currently with: _part.shoulder, _part.stem and a + # I expect this is due to special layers. + # TODO: Fix issue with special layers. + assert ( + savedGlyph.layers[layerName].glyph.path.coordinates[i] == expectedResult + ) + + assert savedGlyph != glyph async def test_getKerning(testFont, referenceFont): From fc5469a7ca7e500e56f4f2941113b550e4e2e115 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Sun, 19 Jan 2025 19:23:46 +0100 Subject: [PATCH 15/88] Fixing the special layer issue with adding a gsLayer.layerId mapping to customData --- src/fontra_glyphs/backend.py | 9 +++++---- tests/test_backend.py | 17 +++++++---------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 9eee687..acc2e4e 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -285,7 +285,7 @@ async def getGlyph(self, glyphName: str) -> VariableGlyph | None: gsLayers = sorted( gsLayers, key=lambda i_gsLayer: masterOrder[i_gsLayer[1].associatedMasterId] ) - + layerIdsMapping = {} seenLocations = [] for i, gsLayer in gsLayers: braceLocation = self._getBraceLayerLocation(gsLayer) @@ -319,6 +319,7 @@ async def getGlyph(self, glyphName: str) -> VariableGlyph | None: ) layers[layerName] = gsLayerToFontraLayer(gsLayer, self.axisNames) + customData["com.glyphsapp.layerIdsMapping"] = layerIdsMapping fixSourceLocations(sources, set(smartLocation)) glyph = VariableGlyph( @@ -825,9 +826,9 @@ def fontraLayerToGSLayer(layer, gsLayer): def variableGlyphToGSGlyph(variableGlyph, gsGlyph): # TODO: convert fontra variableGlyph to GlyphsApp glyph - - for i, (layerName, layer) in enumerate(iter(variableGlyph.layers.items())): - fontraLayerToGSLayer(layer, gsGlyph.layers[i]) + layerIdsMapping = variableGlyph.customData["com.glyphsapp.layerIdsMapping"] + for layerName, gsLayerId in layerIdsMapping.items(): + fontraLayerToGSLayer(variableGlyph.layers[layerName], gsGlyph.layers[gsLayerId]) # What happens, if the number of layers differ from the original number? # How do we handle intermediate layers? diff --git a/tests/test_backend.py b/tests/test_backend.py index 619f624..7080843 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -124,9 +124,13 @@ async def test_getGlyph(testFont, referenceFont, glyphName): if glyphName == "A" and "com.glyphsapp.glyph-color" not in glyph.customData: # glyphsLib doesn't read the color attr from Glyphs-2 files, # so let's monkeypatch the data - glyph.customData = {"com.glyphsapp.glyph-color": [120, 220, 20, 4]} + glyph.customData["com.glyphsapp.glyph-color"] = [120, 220, 20, 4] referenceGlyph = await referenceFont.getGlyph(glyphName) + # TODO: This unit test fails currently, because the fontra referenceFont + # does not contain the customData "com.glyphsapp.layerIdsMapping". + # Before I update the fontra file, I would like to discuss with Just, + # if this is the right approach. test_putGlyph works now. assert referenceGlyph == glyph @@ -135,10 +139,6 @@ async def test_getGlyph(testFont, referenceFont, glyphName): async def test_putGlyph(writableTestFont, testFont, glyphName): glyphMap = await writableTestFont.getGlyphMap() glyph = await writableTestFont.getGlyph(glyphName) - if glyphName == "A" and "com.glyphsapp.glyph-color" not in glyph.customData: - # glyphsLib doesn't read the color attr from Glyphs-2 files, - # so let's monkeypatch the data - glyph.customData = {"com.glyphsapp.glyph-color": [120, 220, 20, 4]} # for testing change every coordinate by 10 units for layerName, layer in iter(glyph.layers.items()): @@ -155,12 +155,9 @@ async def test_putGlyph(writableTestFont, testFont, glyphName): assert savedGlyph.layers[layerName].glyph.xAdvance == 500 for i, coordinate in enumerate(layer.glyph.path.coordinates): - expectedResult = coordinate + 10 - # The follwing fails currently with: _part.shoulder, _part.stem and a - # I expect this is due to special layers. - # TODO: Fix issue with special layers. assert ( - savedGlyph.layers[layerName].glyph.path.coordinates[i] == expectedResult + savedGlyph.layers[layerName].glyph.path.coordinates[i] + == coordinate + 10 ) assert savedGlyph != glyph From eaf855931db06d6336c3388e2e1247bb238eff63 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Sun, 19 Jan 2025 19:43:11 +0100 Subject: [PATCH 16/88] Extend unittest with test_deleteLayer --- src/fontra_glyphs/backend.py | 9 ++++++++- tests/test_backend.py | 15 +++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index acc2e4e..a7f021d 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -828,7 +828,14 @@ def variableGlyphToGSGlyph(variableGlyph, gsGlyph): # TODO: convert fontra variableGlyph to GlyphsApp glyph layerIdsMapping = variableGlyph.customData["com.glyphsapp.layerIdsMapping"] for layerName, gsLayerId in layerIdsMapping.items(): - fontraLayerToGSLayer(variableGlyph.layers[layerName], gsGlyph.layers[gsLayerId]) + if layerName in variableGlyph.layers: + fontraLayerToGSLayer( + variableGlyph.layers[layerName], gsGlyph.layers[gsLayerId] + ) + else: + # Someone removed a layer, for example a special layer. + # Therefore need to be removed from gsGlyph as well. + del gsGlyph.layers[gsLayerId] # What happens, if the number of layers differ from the original number? # How do we handle intermediate layers? diff --git a/tests/test_backend.py b/tests/test_backend.py index 7080843..fdd9612 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -163,6 +163,21 @@ async def test_putGlyph(writableTestFont, testFont, glyphName): assert savedGlyph != glyph +async def test_deleteLayer(writableTestFont): + glyphName = "A" + glyphMap = await writableTestFont.getGlyphMap() + glyph = await writableTestFont.getGlyph(glyphName) + + layerName = "Regular (layer #1)" + del glyph.layers[layerName] + + await writableTestFont.putGlyph(glyphName, glyph, glyphMap[glyphName]) + + savedGlyph = await writableTestFont.getGlyph(glyphName) + + assert layerName not in savedGlyph.layers + + async def test_getKerning(testFont, referenceFont): assert await testFont.getKerning() == await referenceFont.getKerning() From 53a106dcf4cb8abc76010980b6bafb36f29bb716 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Sun, 19 Jan 2025 21:05:33 +0100 Subject: [PATCH 17/88] Add unittest test_addLayer (work in progress) --- src/fontra_glyphs/backend.py | 21 ++++++++++++++++----- tests/test_backend.py | 27 ++++++++++++++++++++++++++- 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index a7f021d..53a7630 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -828,15 +828,26 @@ def variableGlyphToGSGlyph(variableGlyph, gsGlyph): # TODO: convert fontra variableGlyph to GlyphsApp glyph layerIdsMapping = variableGlyph.customData["com.glyphsapp.layerIdsMapping"] for layerName, gsLayerId in layerIdsMapping.items(): - if layerName in variableGlyph.layers: - fontraLayerToGSLayer( - variableGlyph.layers[layerName], gsGlyph.layers[gsLayerId] - ) - else: + if layerName not in variableGlyph.layers: # Someone removed a layer, for example a special layer. # Therefore need to be removed from gsGlyph as well. del gsGlyph.layers[gsLayerId] + for i, (layerName, layer) in enumerate(iter(variableGlyph.layers.items())): + gsLayerId = layerIdsMapping.get(layerName) + if gsLayerId is not None: + # gsLayerExists + fontraLayerToGSLayer(layer, gsGlyph.layers[gsLayerId]) + else: + # gsLayer does not exists, therefore add new layer: + newLayer = glyphsLib.classes.GSLayer() + newLayer.name = layerName + # TODO: the name need probably further modifications + # + best guess for associatedMasterId + # newLayer.associatedMasterId = gsGlyph.layers[0].associatedMasterId + # newLayer.isSpecialLayer = True + gsGlyph.layers.append(newLayer) + # What happens, if the number of layers differ from the original number? # How do we handle intermediate layers? # https://docu.glyphsapp.com/#GSGlyph.layers font.glyphs['a'].layers.append(newLayer) diff --git a/tests/test_backend.py b/tests/test_backend.py index fdd9612..4c300eb 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -4,7 +4,14 @@ import pytest from fontra.backends import getFileSystemBackend -from fontra.core.classes import Axes, FontInfo, structure +from fontra.core.classes import ( + Axes, + FontInfo, + GlyphSource, + Layer, + StaticGlyph, + structure, +) dataDir = pathlib.Path(__file__).resolve().parent / "data" @@ -178,6 +185,24 @@ async def test_deleteLayer(writableTestFont): assert layerName not in savedGlyph.layers +async def test_addLayer(writableTestFont): + glyphName = "a" + glyphMap = await writableTestFont.getGlyphMap() + glyph = await writableTestFont.getGlyph(glyphName) + numGlyphLayers = len(glyph.layers) + + glyph.sources.append( + GlyphSource(name="{166, 100}", location={"weight": 166}, layerName="{166, 100}") + ) + glyph.layers["{166, 100}"] = Layer(glyph=StaticGlyph()) + + await writableTestFont.putGlyph(glyphName, glyph, glyphMap[glyphName]) + + savedGlyph = await writableTestFont.getGlyph(glyphName) + # TODO: we don't have a associated master concept with fontra. + assert len(savedGlyph.layers.keys()) > numGlyphLayers + + async def test_getKerning(testFont, referenceFont): assert await testFont.getKerning() == await referenceFont.getKerning() From c27e22ebdfd646e58d90439aa281ce7acb627a7e Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Mon, 20 Jan 2025 03:21:53 +0100 Subject: [PATCH 18/88] Add support for 'intermediate layer' --- src/fontra_glyphs/backend.py | 22 ++++++++++++++-------- tests/test_backend.py | 21 ++++++++++----------- 2 files changed, 24 insertions(+), 19 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 53a7630..037aa73 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -836,23 +836,29 @@ def variableGlyphToGSGlyph(variableGlyph, gsGlyph): for i, (layerName, layer) in enumerate(iter(variableGlyph.layers.items())): gsLayerId = layerIdsMapping.get(layerName) if gsLayerId is not None: - # gsLayerExists + # gsLayer exists, modify existing gsLayer fontraLayerToGSLayer(layer, gsGlyph.layers[gsLayerId]) else: - # gsLayer does not exists, therefore add new layer: + # gsLayer does not exists, therefore must be 'isSpecialLayer' + # and need to be added as a new layer: newLayer = glyphsLib.classes.GSLayer() newLayer.name = layerName + newLayer.isSpecialLayer = True + + source = variableGlyph.sources[i] + newLayer.attributes["coordinates"] = [ + source.location[axis.name.lower()] + for axis in gsGlyph.parent.axes + if source.location.get(axis.name.lower()) + ] + # TODO: the name need probably further modifications # + best guess for associatedMasterId # newLayer.associatedMasterId = gsGlyph.layers[0].associatedMasterId - # newLayer.isSpecialLayer = True + fontraLayerToGSLayer(layer, newLayer) gsGlyph.layers.append(newLayer) - # What happens, if the number of layers differ from the original number? - # How do we handle intermediate layers? - # https://docu.glyphsapp.com/#GSGlyph.layers font.glyphs['a'].layers.append(newLayer) - # How do we handle missing masters? - # It might be that someone deletes a not needed layer, which works for fontra, + # It might be that someone deletes a not needed master-layer, which works for fontra, # but is required for Glyphs. We would need to get the intermediate contours. return gsGlyph diff --git a/tests/test_backend.py b/tests/test_backend.py index 4c300eb..9953b22 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -1,17 +1,11 @@ import os import pathlib import shutil +from copy import deepcopy import pytest from fontra.backends import getFileSystemBackend -from fontra.core.classes import ( - Axes, - FontInfo, - GlyphSource, - Layer, - StaticGlyph, - structure, -) +from fontra.core.classes import Axes, FontInfo, GlyphSource, Layer, structure dataDir = pathlib.Path(__file__).resolve().parent / "data" @@ -194,13 +188,18 @@ async def test_addLayer(writableTestFont): glyph.sources.append( GlyphSource(name="{166, 100}", location={"weight": 166}, layerName="{166, 100}") ) - glyph.layers["{166, 100}"] = Layer(glyph=StaticGlyph()) + # Copy StaticGlyph of Bold: + glyph.layers["{166, 100}"] = Layer( + glyph=deepcopy(glyph.layers["Bold (layer #2)"].glyph) + ) await writableTestFont.putGlyph(glyphName, glyph, glyphMap[glyphName]) savedGlyph = await writableTestFont.getGlyph(glyphName) - # TODO: we don't have a associated master concept with fontra. - assert len(savedGlyph.layers.keys()) > numGlyphLayers + assert len(savedGlyph.layers) > numGlyphLayers + # TODO: We don't have a associated master concept with fontra, + # therefore the name contains 'Light' (The first master) + assert "Light / {166, 100} (layer #4)" in savedGlyph.layers.keys() async def test_getKerning(testFont, referenceFont): From e343a3de5af80d5cc17eb47fe0af2621285e32bd Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Tue, 21 Jan 2025 21:39:15 +0100 Subject: [PATCH 19/88] putGlyph including components and anchors + unittests --- src/fontra_glyphs/backend.py | 169 +++++++++++++++-------------------- src/fontra_glyphs/utils.py | 115 ++++++++++++++++++++++++ tests/test_backend.py | 118 ++++++++++++++++++++++-- 3 files changed, 300 insertions(+), 102 deletions(-) create mode 100644 src/fontra_glyphs/utils.py diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 037aa73..5725d54 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -1,7 +1,6 @@ import io import pathlib -import re -from collections import OrderedDict, defaultdict +from collections import defaultdict from copy import deepcopy from os import PathLike from typing import Any @@ -37,6 +36,14 @@ to_designspace_axes, ) from glyphsLib.builder.smart_components import Pole +from glyphsLib.types import Transform + +from .utils import ( + getAssociatedMasterId, + getLocation, + saveFileWithGSFormatting, + toOrderedDict, +) rootInfoNames = [ "familyName", @@ -285,7 +292,7 @@ async def getGlyph(self, glyphName: str) -> VariableGlyph | None: gsLayers = sorted( gsLayers, key=lambda i_gsLayer: masterOrder[i_gsLayer[1].associatedMasterId] ) - layerIdsMapping = {} + seenLayerIDs = {} seenLocations = [] for i, gsLayer in gsLayers: braceLocation = self._getBraceLayerLocation(gsLayer) @@ -319,7 +326,7 @@ async def getGlyph(self, glyphName: str) -> VariableGlyph | None: ) layers[layerName] = gsLayerToFontraLayer(gsLayer, self.axisNames) - customData["com.glyphsapp.layerIdsMapping"] = layerIdsMapping + customData["com.glyphsapp.seenLayerIDs"] = seenLayerIDs fixSourceLocations(sources, set(smartLocation)) glyph = VariableGlyph( @@ -422,7 +429,7 @@ def _writeRawGlyph(self, glyphName, f): ) # 6. fix formatting - saveFileWithGsFormatting(self.gsFilePath) + saveFileWithGSFormatting(self.gsFilePath) async def aclose(self) -> None: pass @@ -759,102 +766,47 @@ def gsVerticalMetricsToFontraLineMetricsHorizontal(gsFont, gsMaster): return lineMetricsHorizontal -def saveFileWithGsFormatting(gsFilePath): - # openstep_plist.dump changes the whole formatting, therefore - # it's very diffucute to see what has changed. - # This function is a very bad try to get close to how the formatting - # looks like for a .glyphs file. - # There must be a better solution, but this is better than nothing. - with open(gsFilePath, "r", encoding="utf-8") as file: - content = file.read() - - content = re.sub(r"pos = \(\s*(-?\d+),\s*(-?\d+)\s*\);", r"pos = (\1,\2);", content) - - content = re.sub( - r"pos = \(\s*([\d.]+),\s*([\d.]+)\s*\);", r"pos = (\1,\2);", content - ) - - content = re.sub( - r"\(\s*([\d.]+),\s*(-?\d+),\s*([a-zA-Z])\s*\)", r"(\1,\2,\3)", content - ) - - content = re.sub( - r"origin = \(\s*(-?\d+),\s*(-?\d+)\s*\);", r"origin = (\1,\2);", content - ) - - content = re.sub( - r"target = \(\s*(-?\d+),\s*(-?\d+)\s*\);", r"target = (\1,\2);", content - ) - - content = re.sub( - r"color = \(\s*(\d+),\s*(\d+),\s*(\d+),\s*(\d+)\s*\);", - r"color = (\1,\2,\3,\4);", - content, - ) - - content = re.sub( - r"\(\s*(-?\d+),\s*(-?\d+),\s*([a-zA-Z]+)\s*\)", r"(\1,\2,\3)", content - ) - - content = re.sub( - r"\(\s*(-?\d+),\s*(-?\d+),\s*([a-zA-Z]),\s*\{", r"(\1,\2,\3,{", content - ) - - content = re.sub( - r"\(\s*(-?\d+),\s*(-?\d+),\s*([a-zA-Z]+),\s*\{", r"(\1,\2,\3,{", content - ) - - content = re.sub(r"\}\s*\),", r"}),", content) - - content += "\n" # add blank break at the end of the file. - - with open(gsFilePath, "w", encoding="utf-8") as file: - file.write(content) - - -def fontraLayerToGSLayer(layer, gsLayer): - # Draw new paths with pen - gsLayer.paths = [] # first: remove all paths - pen = gsLayer.getPointPen() - layer.glyph.path.drawPoints(pen) - - gsLayer.drawPoints(pen) - gsLayer.width = layer.glyph.xAdvance - # gsLayer.components = components # https://docu.glyphsapp.com/#GSLayer.components - # gsLayer.anchors = anchors # https://docu.glyphsapp.com/#GSLayer.anchors - - def variableGlyphToGSGlyph(variableGlyph, gsGlyph): - # TODO: convert fontra variableGlyph to GlyphsApp glyph - layerIdsMapping = variableGlyph.customData["com.glyphsapp.layerIdsMapping"] - for layerName, gsLayerId in layerIdsMapping.items(): + # Convert fontra variableGlyph to GlyphsApp glyph + masterIds = [m.id for m in gsGlyph.parent.masters] + seenLayerIDs = variableGlyph.customData["com.glyphsapp.seenLayerIDs"] + for layerName, gsLayerId in seenLayerIDs.items(): if layerName not in variableGlyph.layers: - # Someone removed a layer, for example a special layer. - # Therefore need to be removed from gsGlyph as well. + gsLayerId = seenLayerIDs.get(layerName) + if gsLayerId in masterIds: + # Someone deleted a master, this breaks the compatibility in a .glyphs file. + # There skip deleting master layer. + continue + # Removing non-master-layer: del gsGlyph.layers[gsLayerId] for i, (layerName, layer) in enumerate(iter(variableGlyph.layers.items())): - gsLayerId = layerIdsMapping.get(layerName) + gsLayerId = seenLayerIDs.get(layerName) if gsLayerId is not None: - # gsLayer exists, modify existing gsLayer + # gsLayer exists: modify existing gsLayer fontraLayerToGSLayer(layer, gsGlyph.layers[gsLayerId]) else: - # gsLayer does not exists, therefore must be 'isSpecialLayer' - # and need to be added as a new layer: + # gsLayer does not exist: therefore must be 'isSpecialLayer' + # and need to be created as a new layer: newLayer = glyphsLib.classes.GSLayer() newLayer.name = layerName newLayer.isSpecialLayer = True - source = variableGlyph.sources[i] - newLayer.attributes["coordinates"] = [ - source.location[axis.name.lower()] + location = getLocation(variableGlyph, layerName, gsGlyph.parent.axes) + if location is None: + return + + gsLocation = [ + location[axis.name.lower()] for axis in gsGlyph.parent.axes - if source.location.get(axis.name.lower()) + if location.get(axis.name.lower()) ] + newLayer.attributes["coordinates"] = gsLocation + + associatedMasterId = getAssociatedMasterId(gsGlyph, gsLocation) + if associatedMasterId: + newLayer.associatedMasterId = associatedMasterId - # TODO: the name need probably further modifications - # + best guess for associatedMasterId - # newLayer.associatedMasterId = gsGlyph.layers[0].associatedMasterId fontraLayerToGSLayer(layer, newLayer) gsGlyph.layers.append(newLayer) @@ -864,12 +816,39 @@ def variableGlyphToGSGlyph(variableGlyph, gsGlyph): return gsGlyph -# The following is obsolete once this is merged: -# https://github.com/fonttools/openstep-plist/pull/35 -def toOrderedDict(obj): - if isinstance(obj, dict): - return OrderedDict({k: toOrderedDict(v) for k, v in obj.items()}) - elif isinstance(obj, list): - return [toOrderedDict(item) for item in obj] - else: - return obj +def fontraLayerToGSLayer(layer, gsLayer): + gsLayer.paths = [] + + # Draw new paths with pen + pen = gsLayer.getPointPen() + layer.glyph.path.drawPoints(pen) + + gsLayer.width = layer.glyph.xAdvance + gsLayer.components = [ + fontraComponentToGSComponent(component) for component in layer.glyph.components + ] + gsLayer.anchors = [ + fontraAnchorsToGSAnchor(anchor) for anchor in layer.glyph.anchors + ] + + +def fontraComponentToGSComponent(component): + gsComponent = glyphsLib.classes.GSComponent(component.name) + transformation = component.transformation.toTransform() + if not isinstance(transformation, Transform): + gsComponent.transform = Transform(*transformation) + # gsComponent.smartComponentValues = # TODO: see location={ disambiguateLocalAxisName + return gsComponent + + +def fontraAnchorsToGSAnchor(anchor): + gsAnchor = glyphsLib.classes.GSAnchor() + gsAnchor.name = anchor.name + gsAnchor.position.x = anchor.x + gsAnchor.position.y = anchor.y + if anchor.customData: + gsAnchor.userData = anchor.customData + # TODO: gsAnchor.orientation – If the position of the anchor + # is relative to the LSB (0), center (2) or RSB (1). + # Details: https://docu.glyphsapp.com/#GSAnchor.orientation + return gsAnchor diff --git a/src/fontra_glyphs/utils.py b/src/fontra_glyphs/utils.py new file mode 100644 index 0000000..80964ae --- /dev/null +++ b/src/fontra_glyphs/utils.py @@ -0,0 +1,115 @@ +import re +from collections import OrderedDict + + +# The following is obsolete once this is merged: +# https://github.com/fonttools/openstep-plist/pull/35 +def toOrderedDict(obj): + if isinstance(obj, dict): + return OrderedDict({k: toOrderedDict(v) for k, v in obj.items()}) + elif isinstance(obj, list): + return [toOrderedDict(item) for item in obj] + else: + return obj + + +def getLocationFromLayerName(layerName, gsAxes): + # try to get it from name, eg. Light / {166, 100} (layer #4) + match = re.search(r"\{([^}]+)\}", layerName) + if not match: + return None + listLocation = match.group(1).replace(" ", "").split(",") + listLocationValues = [float(v) for v in listLocation] + return { + gsAxes[i].name.lower(): value + for i, value in enumerate(listLocationValues) + if i < len(gsAxes) + } + + +def getLocationFromSources(sources, layerName): + s = None + for source in sources: + if source.layerName == layerName: + s = source + break + if s is not None: + return {k.lower(): v for k, v in s.location.items()} + + +def getLocation(glyph, layerName, gsAxes): + location = getLocationFromSources(glyph.sources, layerName) + if location: + return location + # This layer is not used by any source: + return getLocationFromLayerName(layerName, gsAxes) + + +def getAssociatedMasterId(gsGlyph, gsLocation): + # Make a best guess of a associatedMasterId + closestMaster = None + closestDistance = float("inf") + for gsLayer in gsGlyph.layers: + gsMaster = gsLayer.master + distance = sum( + abs(gsMaster.axes[i] - gsLocation[i]) + for i in range(len(gsMaster.axes)) + if i < len(gsLocation) + ) + if distance < closestDistance: + closestDistance = distance + closestMaster = gsMaster + return closestMaster.id if closestMaster else None + + +def saveFileWithGSFormatting(gsFilePath): + # openstep_plist.dump changes the whole formatting, therefore + # it's very diffucute to see what has changed. + # This function is a very bad try to get close to how the formatting + # looks like for a .glyphs file. + # There must be a better solution, but this is better than nothing. + with open(gsFilePath, "r", encoding="utf-8") as file: + content = file.read() + + content = re.sub(r"pos = \(\s*(-?\d+),\s*(-?\d+)\s*\);", r"pos = (\1,\2);", content) + + content = re.sub( + r"pos = \(\s*([\d.]+),\s*([\d.]+)\s*\);", r"pos = (\1,\2);", content + ) + + content = re.sub( + r"\(\s*([\d.]+),\s*(-?\d+),\s*([a-zA-Z])\s*\)", r"(\1,\2,\3)", content + ) + + content = re.sub( + r"origin = \(\s*(-?\d+),\s*(-?\d+)\s*\);", r"origin = (\1,\2);", content + ) + + content = re.sub( + r"target = \(\s*(-?\d+),\s*(-?\d+)\s*\);", r"target = (\1,\2);", content + ) + + content = re.sub( + r"color = \(\s*(\d+),\s*(\d+),\s*(\d+),\s*(\d+)\s*\);", + r"color = (\1,\2,\3,\4);", + content, + ) + + content = re.sub( + r"\(\s*(-?\d+),\s*(-?\d+),\s*([a-zA-Z]+)\s*\)", r"(\1,\2,\3)", content + ) + + content = re.sub( + r"\(\s*(-?\d+),\s*(-?\d+),\s*([a-zA-Z]),\s*\{", r"(\1,\2,\3,{", content + ) + + content = re.sub( + r"\(\s*(-?\d+),\s*(-?\d+),\s*([a-zA-Z]+),\s*\{", r"(\1,\2,\3,{", content + ) + + content = re.sub(r"\}\s*\),", r"}),", content) + + content += "\n" # add blank break at the end of the file. + + with open(gsFilePath, "w", encoding="utf-8") as file: + file.write(content) diff --git a/tests/test_backend.py b/tests/test_backend.py index 9953b22..1775bda 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -3,9 +3,24 @@ import shutil from copy import deepcopy +import glyphsLib import pytest from fontra.backends import getFileSystemBackend -from fontra.core.classes import Axes, FontInfo, GlyphSource, Layer, structure +from fontra.core.classes import ( + Anchor, + Axes, + FontInfo, + GlyphSource, + Layer, + StaticGlyph, + structure, +) + +from fontra_glyphs.utils import ( + getAssociatedMasterId, + getLocationFromLayerName, + getLocationFromSources, +) dataDir = pathlib.Path(__file__).resolve().parent / "data" @@ -20,6 +35,11 @@ def testFont(request): return getFileSystemBackend(request.param) +@pytest.fixture(scope="module", params=[glyphs2Path, glyphs3Path, glyphsPackagePath]) +def testGSFont(request): + return glyphsLib.GSFont(request.param) + + @pytest.fixture(scope="module") def referenceFont(request): return getFileSystemBackend(referenceFontPath) @@ -129,7 +149,7 @@ async def test_getGlyph(testFont, referenceFont, glyphName): referenceGlyph = await referenceFont.getGlyph(glyphName) # TODO: This unit test fails currently, because the fontra referenceFont - # does not contain the customData "com.glyphsapp.layerIdsMapping". + # does not contain the customData "com.glyphsapp.seenLayerIDs". # Before I update the fontra file, I would like to discuss with Just, # if this is the right approach. test_putGlyph works now. assert referenceGlyph == glyph @@ -161,7 +181,9 @@ async def test_putGlyph(writableTestFont, testFont, glyphName): == coordinate + 10 ) - assert savedGlyph != glyph + assert len(layer.glyph.path.coordinates) == len( + savedGlyph.layers[layerName].glyph.path.coordinates + ) async def test_deleteLayer(writableTestFont): @@ -176,7 +198,7 @@ async def test_deleteLayer(writableTestFont): savedGlyph = await writableTestFont.getGlyph(glyphName) - assert layerName not in savedGlyph.layers + assert layerName in savedGlyph.layers async def test_addLayer(writableTestFont): @@ -197,9 +219,62 @@ async def test_addLayer(writableTestFont): savedGlyph = await writableTestFont.getGlyph(glyphName) assert len(savedGlyph.layers) > numGlyphLayers - # TODO: We don't have a associated master concept with fontra, - # therefore the name contains 'Light' (The first master) - assert "Light / {166, 100} (layer #4)" in savedGlyph.layers.keys() + assert "Bold / {166, 100} (layer #4)" in savedGlyph.layers.keys() + + +async def test_addLayerWithComponent(writableTestFont): + glyphName = "n" # n is made from components + glyphMap = await writableTestFont.getGlyphMap() + glyph = await writableTestFont.getGlyph(glyphName) + numGlyphLayers = len(glyph.layers) + + glyph.sources.append( + GlyphSource(name="{166, 100}", location={"weight": 166}, layerName="{166, 100}") + ) + # Copy StaticGlyph of Bold: + glyph.layers["{166, 100}"] = Layer( + glyph=deepcopy(glyph.layers["Bold (layer #2)"].glyph) + ) + + await writableTestFont.putGlyph(glyphName, glyph, glyphMap[glyphName]) + + savedGlyph = await writableTestFont.getGlyph(glyphName) + assert len(savedGlyph.layers) > numGlyphLayers + assert "Bold / {166, 100} (layer #3)" in savedGlyph.layers.keys() + + +async def test_removeMasterLayer(writableTestFont): + glyphName = "a" + glyphMap = await writableTestFont.getGlyphMap() + glyph = await writableTestFont.getGlyph(glyphName) + numGlyphLayers = len(glyph.layers) + + # Removing a "master" layer breaks compatibility within a .glyphs file. + del glyph.layers["Bold (layer #2)"] + + await writableTestFont.putGlyph(glyphName, glyph, glyphMap[glyphName]) + + savedGlyph = await writableTestFont.getGlyph(glyphName) + assert len(savedGlyph.layers) == numGlyphLayers + + +async def test_addAnchor(writableTestFont): + glyphName = "a" + glyphMap = await writableTestFont.getGlyphMap() + glyph = await writableTestFont.getGlyph(glyphName) + + layerName = "{ 166, 100 }" + glyph.layers[layerName] = Layer(glyph=StaticGlyph(xAdvance=0)) + glyph.layers[layerName].glyph.anchors.append(Anchor(name="top", x=207, y=746)) + + await writableTestFont.putGlyph(glyphName, glyph, glyphMap[glyphName]) + + savedGlyph = await writableTestFont.getGlyph(glyphName) + + assert ( + glyph.layers[layerName].glyph.anchors + == savedGlyph.layers["Bold / { 166, 100 } (layer #4)"].glyph.anchors + ) async def test_getKerning(testFont, referenceFont): @@ -208,3 +283,32 @@ async def test_getKerning(testFont, referenceFont): async def test_getSources(testFont, referenceFont): assert await testFont.getSources() == await referenceFont.getSources() + + +layerNamesToLocation = [ + ["Light / {166, 100} (layer #4)", {"weight": 166}], + ["{ 166 } (layer #3)", {"weight": 166}], + ["Light / (layer #4)", None], +] + + +@pytest.mark.parametrize("layerName,expected", layerNamesToLocation) +def test_getLocationFromLayerName(layerName, expected): + gsFont = glyphsLib.classes.GSFont() + gsFont.axes = [glyphsLib.classes.GSAxis(name="Weight", tag="wght")] + location = getLocationFromLayerName(layerName, gsFont.axes) + assert location == expected + + +async def test_getLocationFromSources(testFont): + glyphName = "a" + glyph = await testFont.getGlyph(glyphName) + location = getLocationFromSources(glyph.sources, "Regular / {155, 100} (layer #3)") + assert location == {"weight": 155} + + +async def test_getAssociatedMasterId(testGSFont): + gsGlyph = testGSFont.glyphs["a"] + associatedMasterId = getAssociatedMasterId(gsGlyph, [155]) + associatedMaster = gsGlyph.layers[associatedMasterId] + assert associatedMaster.name == "Regular" From b43907a01c92a235344172e03583d7bf12c3bead Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Wed, 22 Jan 2025 10:05:34 +0100 Subject: [PATCH 20/88] Adjust comments, restructure a bit + more unittests --- src/fontra_glyphs/backend.py | 39 +++----- src/fontra_glyphs/utils.py | 14 ++- tests/test_backend.py | 47 +-------- tests/test_utils.py | 183 +++++++++++++++++++++++++++++++++++ 4 files changed, 209 insertions(+), 74 deletions(-) create mode 100644 tests/test_utils.py diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 5725d54..b9061da 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -38,12 +38,7 @@ from glyphsLib.builder.smart_components import Pole from glyphsLib.types import Transform -from .utils import ( - getAssociatedMasterId, - getLocation, - saveFileWithGSFormatting, - toOrderedDict, -) +from .utils import getAssociatedMasterId, getLocation, gsFormatting, toOrderedDict rootInfoNames = [ "familyName", @@ -326,6 +321,7 @@ async def getGlyph(self, glyphName: str) -> VariableGlyph | None: ) layers[layerName] = gsLayerToFontraLayer(gsLayer, self.axisNames) + # NOTE: Remember layerIds like in the following line or add 'identifier' to fontra Layer? customData["com.glyphsapp.seenLayerIDs"] = seenLayerIDs fixSourceLocations(sources, set(smartLocation)) @@ -344,7 +340,6 @@ def _ensureGlyphIsParsed(self, glyphName: str) -> None: glyphIndex = self.glyphNameToIndex[glyphName] rawGlyphData = self.rawGlyphsData[glyphIndex] - # self.rawGlyphsData[glyphIndex] = None self.parsedGlyphNames.add(glyphName) gsGlyph = glyphsLib.classes.GSGlyph() @@ -390,7 +385,7 @@ def _getSmartLocation(self, gsLayer, localAxesByName): async def putGlyph( self, glyphName: str, glyph: VariableGlyph, codePoints: list[int] ) -> None: - # 1. convert VariableGlyph to GSGlyph (but start with a copy of the original) + # 1. convert VariableGlyph to GSGlyph gsGlyphNew = variableGlyphToGSGlyph( glyph, deepcopy(self.gsFont.glyphs[glyphName]) ) @@ -406,8 +401,7 @@ async def putGlyph( rawGlyphData = openstep_plist.load(f, use_numbers=True) # 4. replace original "raw" object with new "raw" object - glyphIndex = self.glyphNameToIndex[glyphName] - self.rawGlyphsData[glyphIndex] = rawGlyphData + self.rawGlyphsData[self.glyphNameToIndex[glyphName]] = rawGlyphData self.rawFontData["glyphs"] = self.rawGlyphsData self._writeRawGlyph(glyphName, f) @@ -418,7 +412,9 @@ async def putGlyph( def _writeRawGlyph(self, glyphName, f): # 5. write whole file with openstep_plist - with open(self.gsFilePath, "w", encoding="utf-8") as fp: + with open(self.gsFilePath, "r+", encoding="utf-8") as fp: + content = gsFormatting(fp.read()) # 6. fix formatting + fp.write(content) openstep_plist.dump( self.rawFontData, fp, @@ -428,9 +424,6 @@ def _writeRawGlyph(self, glyphName, f): escape_newlines=False, ) - # 6. fix formatting - saveFileWithGSFormatting(self.gsFilePath) - async def aclose(self) -> None: pass @@ -767,7 +760,7 @@ def gsVerticalMetricsToFontraLineMetricsHorizontal(gsFont, gsMaster): def variableGlyphToGSGlyph(variableGlyph, gsGlyph): - # Convert fontra variableGlyph to GlyphsApp glyph + # Convert Fontra variableGlyph to GlyphsApp glyph masterIds = [m.id for m in gsGlyph.parent.masters] seenLayerIDs = variableGlyph.customData["com.glyphsapp.seenLayerIDs"] for layerName, gsLayerId in seenLayerIDs.items(): @@ -775,7 +768,7 @@ def variableGlyphToGSGlyph(variableGlyph, gsGlyph): gsLayerId = seenLayerIDs.get(layerName) if gsLayerId in masterIds: # Someone deleted a master, this breaks the compatibility in a .glyphs file. - # There skip deleting master layer. + # Skip deleting master layer. continue # Removing non-master-layer: del gsGlyph.layers[gsLayerId] @@ -788,9 +781,9 @@ def variableGlyphToGSGlyph(variableGlyph, gsGlyph): else: # gsLayer does not exist: therefore must be 'isSpecialLayer' # and need to be created as a new layer: - newLayer = glyphsLib.classes.GSLayer() - newLayer.name = layerName - newLayer.isSpecialLayer = True + gsLayer = glyphsLib.classes.GSLayer() + gsLayer.name = layerName + gsLayer.isSpecialLayer = True location = getLocation(variableGlyph, layerName, gsGlyph.parent.axes) if location is None: @@ -801,14 +794,14 @@ def variableGlyphToGSGlyph(variableGlyph, gsGlyph): for axis in gsGlyph.parent.axes if location.get(axis.name.lower()) ] - newLayer.attributes["coordinates"] = gsLocation + gsLayer.attributes["coordinates"] = gsLocation associatedMasterId = getAssociatedMasterId(gsGlyph, gsLocation) if associatedMasterId: - newLayer.associatedMasterId = associatedMasterId + gsLayer.associatedMasterId = associatedMasterId - fontraLayerToGSLayer(layer, newLayer) - gsGlyph.layers.append(newLayer) + fontraLayerToGSLayer(layer, gsLayer) + gsGlyph.layers.append(gsLayer) # It might be that someone deletes a not needed master-layer, which works for fontra, # but is required for Glyphs. We would need to get the intermediate contours. diff --git a/src/fontra_glyphs/utils.py b/src/fontra_glyphs/utils.py index 80964ae..211ef78 100644 --- a/src/fontra_glyphs/utils.py +++ b/src/fontra_glyphs/utils.py @@ -14,7 +14,8 @@ def toOrderedDict(obj): def getLocationFromLayerName(layerName, gsAxes): - # try to get it from name, eg. Light / {166, 100} (layer #4) + # Get the location based on name, + # for example: Light / {166, 100} (layer #4) match = re.search(r"\{([^}]+)\}", layerName) if not match: return None @@ -41,12 +42,12 @@ def getLocation(glyph, layerName, gsAxes): location = getLocationFromSources(glyph.sources, layerName) if location: return location - # This layer is not used by any source: + # This layerName is not used by any source: return getLocationFromLayerName(layerName, gsAxes) def getAssociatedMasterId(gsGlyph, gsLocation): - # Make a best guess of a associatedMasterId + # Best guess for associatedMasterId closestMaster = None closestDistance = float("inf") for gsLayer in gsGlyph.layers: @@ -62,14 +63,12 @@ def getAssociatedMasterId(gsGlyph, gsLocation): return closestMaster.id if closestMaster else None -def saveFileWithGSFormatting(gsFilePath): +def gsFormatting(content): # openstep_plist.dump changes the whole formatting, therefore # it's very diffucute to see what has changed. # This function is a very bad try to get close to how the formatting # looks like for a .glyphs file. # There must be a better solution, but this is better than nothing. - with open(gsFilePath, "r", encoding="utf-8") as file: - content = file.read() content = re.sub(r"pos = \(\s*(-?\d+),\s*(-?\d+)\s*\);", r"pos = (\1,\2);", content) @@ -111,5 +110,4 @@ def saveFileWithGSFormatting(gsFilePath): content += "\n" # add blank break at the end of the file. - with open(gsFilePath, "w", encoding="utf-8") as file: - file.write(content) + return content diff --git a/tests/test_backend.py b/tests/test_backend.py index 1775bda..2b5f6fc 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -3,7 +3,6 @@ import shutil from copy import deepcopy -import glyphsLib import pytest from fontra.backends import getFileSystemBackend from fontra.core.classes import ( @@ -16,12 +15,6 @@ structure, ) -from fontra_glyphs.utils import ( - getAssociatedMasterId, - getLocationFromLayerName, - getLocationFromSources, -) - dataDir = pathlib.Path(__file__).resolve().parent / "data" glyphs2Path = dataDir / "GlyphsUnitTestSans.glyphs" @@ -35,11 +28,6 @@ def testFont(request): return getFileSystemBackend(request.param) -@pytest.fixture(scope="module", params=[glyphs2Path, glyphs3Path, glyphsPackagePath]) -def testGSFont(request): - return glyphsLib.GSFont(request.param) - - @pytest.fixture(scope="module") def referenceFont(request): return getFileSystemBackend(referenceFontPath) @@ -158,6 +146,7 @@ async def test_getGlyph(testFont, referenceFont, glyphName): @pytest.mark.asyncio @pytest.mark.parametrize("glyphName", list(expectedGlyphMap)) async def test_putGlyph(writableTestFont, testFont, glyphName): + writableTestFont = testFont glyphMap = await writableTestFont.getGlyphMap() glyph = await writableTestFont.getGlyph(glyphName) @@ -243,13 +232,14 @@ async def test_addLayerWithComponent(writableTestFont): assert "Bold / {166, 100} (layer #3)" in savedGlyph.layers.keys() -async def test_removeMasterLayer(writableTestFont): +async def test_deleteMasterLayer(writableTestFont): + # Removing a "master" layer breaks compatibility within a .glyphs file. + # Therefore we need to make sure, that it will be added afterwords. glyphName = "a" glyphMap = await writableTestFont.getGlyphMap() glyph = await writableTestFont.getGlyph(glyphName) numGlyphLayers = len(glyph.layers) - # Removing a "master" layer breaks compatibility within a .glyphs file. del glyph.layers["Bold (layer #2)"] await writableTestFont.putGlyph(glyphName, glyph, glyphMap[glyphName]) @@ -283,32 +273,3 @@ async def test_getKerning(testFont, referenceFont): async def test_getSources(testFont, referenceFont): assert await testFont.getSources() == await referenceFont.getSources() - - -layerNamesToLocation = [ - ["Light / {166, 100} (layer #4)", {"weight": 166}], - ["{ 166 } (layer #3)", {"weight": 166}], - ["Light / (layer #4)", None], -] - - -@pytest.mark.parametrize("layerName,expected", layerNamesToLocation) -def test_getLocationFromLayerName(layerName, expected): - gsFont = glyphsLib.classes.GSFont() - gsFont.axes = [glyphsLib.classes.GSAxis(name="Weight", tag="wght")] - location = getLocationFromLayerName(layerName, gsFont.axes) - assert location == expected - - -async def test_getLocationFromSources(testFont): - glyphName = "a" - glyph = await testFont.getGlyph(glyphName) - location = getLocationFromSources(glyph.sources, "Regular / {155, 100} (layer #3)") - assert location == {"weight": 155} - - -async def test_getAssociatedMasterId(testGSFont): - gsGlyph = testGSFont.glyphs["a"] - associatedMasterId = getAssociatedMasterId(gsGlyph, [155]) - associatedMaster = gsGlyph.layers[associatedMasterId] - assert associatedMaster.name == "Regular" diff --git a/tests/test_utils.py b/tests/test_utils.py new file mode 100644 index 0000000..20099d7 --- /dev/null +++ b/tests/test_utils.py @@ -0,0 +1,183 @@ +import pathlib + +import glyphsLib +import pytest +from fontra.backends import getFileSystemBackend + +from fontra_glyphs.utils import ( + getAssociatedMasterId, + getLocationFromLayerName, + getLocationFromSources, + gsFormatting, +) + +dataDir = pathlib.Path(__file__).resolve().parent / "data" + +glyphs2Path = dataDir / "GlyphsUnitTestSans.glyphs" +glyphs3Path = dataDir / "GlyphsUnitTestSans3.glyphs" +glyphsPackagePath = dataDir / "GlyphsUnitTestSans3.glyphspackage" + + +@pytest.fixture(scope="module", params=[glyphs2Path, glyphs3Path, glyphsPackagePath]) +def testFont(request): + return getFileSystemBackend(request.param) + + +@pytest.fixture(scope="module", params=[glyphs2Path, glyphs3Path, glyphsPackagePath]) +def testGSFont(request): + return glyphsLib.GSFont(request.param) + + +layerNamesToLocation = [ + ["Light / {166, 100} (layer #4)", {"weight": 166}], + ["{ 166 } (layer #3)", {"weight": 166}], + ["Light / (layer #4)", None], +] + + +@pytest.mark.parametrize("layerName,expected", layerNamesToLocation) +def test_getLocationFromLayerName(layerName, expected): + gsFont = glyphsLib.classes.GSFont() + gsFont.axes = [glyphsLib.classes.GSAxis(name="Weight", tag="wght")] + location = getLocationFromLayerName(layerName, gsFont.axes) + assert location == expected + + +async def test_getLocationFromSources(testFont): + glyphName = "a" + glyph = await testFont.getGlyph(glyphName) + location = getLocationFromSources(glyph.sources, "Regular / {155, 100} (layer #3)") + assert location == {"weight": 155} + + +def test_getAssociatedMasterId(testGSFont): + gsGlyph = testGSFont.glyphs["a"] + associatedMasterId = getAssociatedMasterId(gsGlyph, [155]) + associatedMaster = gsGlyph.layers[associatedMasterId] + assert associatedMaster.name == "Regular" + + +contentSnippets = [ + [ + """pos = ( +524, +141 +);""", + "pos = (524,141);", + ], + [ + """pos = ( +-113, +765 +);""", + "pos = (-113,765);", + ], + [ + "customBinaryData = <74686520 62797465 73>;", + "customBinaryData = <746865206279746573>;", + ], + [ + """color = ( +120, +220, +20, +4 +);""", + "color = (120,220,20,4);", + ], + [ + """( +566.99, +700, +l +),""", + "(566.99,700,l),", + ], + [ + """( +191, +700, +l +),""", + "(191,700,l),", + ], + [ + """origin = ( +1, +1 +);""", + "origin = (1,1);", + ], + [ + """target = ( +1, +0 +);""", + "target = (1,0);", + ], + [ + """pos = ( +45, +0 +);""", + "pos = (45,0);", + ], + [ + """pos = ( +-45, +0 +);""", + "pos = (-45,0);", + ], + [ + """( +341, +720, +l, +{""", + "(321,700,l,{", + ][ + """( +268, +153, +ls +),""", + "(268,153,ls),", + ], + [ + """( +268, +153, +o +),""", + "(268,153,o),", + ], + [ + """( +268, +153, +cs +),""", + "(268,153,cs),", + ], + [ + """( +184, +-8, +c +),""", + "(184,-8,c),", + ], + [ + """pos = ( +334.937, +407.08 +);""", + "pos = (334.937,407.08);", + ], +] + + +@pytest.mark.parametrize("content,expected", contentSnippets) +def test_gsFormatting(content, expected): + assert gsFormatting(content) != expected From f6a6728d1e24c5b3f6fcaeeb94554f70610345ad Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Wed, 22 Jan 2025 10:08:24 +0100 Subject: [PATCH 21/88] Fix missing comma and remove 'local hack' --- tests/test_backend.py | 1 - tests/test_utils.py | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_backend.py b/tests/test_backend.py index 2b5f6fc..f809bdf 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -146,7 +146,6 @@ async def test_getGlyph(testFont, referenceFont, glyphName): @pytest.mark.asyncio @pytest.mark.parametrize("glyphName", list(expectedGlyphMap)) async def test_putGlyph(writableTestFont, testFont, glyphName): - writableTestFont = testFont glyphMap = await writableTestFont.getGlyphMap() glyph = await writableTestFont.getGlyph(glyphName) diff --git a/tests/test_utils.py b/tests/test_utils.py index 20099d7..5cfefba 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -136,7 +136,8 @@ def test_getAssociatedMasterId(testGSFont): l, {""", "(321,700,l,{", - ][ + ], + [ """( 268, 153, From 7afe3fa92e4ff9ffaaf398f3015c1ba5b9960e43 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Wed, 22 Jan 2025 11:54:42 +0100 Subject: [PATCH 22/88] Rework gsFormatting --- src/fontra_glyphs/backend.py | 16 +++++++-- src/fontra_glyphs/utils.py | 66 +++++++++++++++--------------------- tests/test_utils.py | 43 +++++++++++++++++++++-- 3 files changed, 81 insertions(+), 44 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index b9061da..59e456e 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -413,8 +413,6 @@ async def putGlyph( def _writeRawGlyph(self, glyphName, f): # 5. write whole file with openstep_plist with open(self.gsFilePath, "r+", encoding="utf-8") as fp: - content = gsFormatting(fp.read()) # 6. fix formatting - fp.write(content) openstep_plist.dump( self.rawFontData, fp, @@ -424,6 +422,20 @@ def _writeRawGlyph(self, glyphName, f): escape_newlines=False, ) + # 6. fix formatting + with open(self.gsFilePath, "r", encoding="utf-8") as fp: + content = gsFormatting(fp.read()) + if self.gsFont.format_version >= 3: + # add blank break at the end of the file. + content += "\n" + with open(self.gsFilePath, "w", encoding="utf-8") as fp: + fp.write(content) + + # This following does not wrok, correctly, there the code above. + # with open(self.gsFilePath, "r+", encoding="utf-8") as fp: + # content = gsFormatting(fp.read()) + # fp.write(content) + async def aclose(self) -> None: pass diff --git a/src/fontra_glyphs/utils.py b/src/fontra_glyphs/utils.py index 211ef78..f8e44f6 100644 --- a/src/fontra_glyphs/utils.py +++ b/src/fontra_glyphs/utils.py @@ -70,44 +70,32 @@ def gsFormatting(content): # looks like for a .glyphs file. # There must be a better solution, but this is better than nothing. - content = re.sub(r"pos = \(\s*(-?\d+),\s*(-?\d+)\s*\);", r"pos = (\1,\2);", content) - - content = re.sub( - r"pos = \(\s*([\d.]+),\s*([\d.]+)\s*\);", r"pos = (\1,\2);", content - ) - - content = re.sub( - r"\(\s*([\d.]+),\s*(-?\d+),\s*([a-zA-Z])\s*\)", r"(\1,\2,\3)", content - ) - - content = re.sub( - r"origin = \(\s*(-?\d+),\s*(-?\d+)\s*\);", r"origin = (\1,\2);", content - ) - - content = re.sub( - r"target = \(\s*(-?\d+),\s*(-?\d+)\s*\);", r"target = (\1,\2);", content - ) - - content = re.sub( - r"color = \(\s*(\d+),\s*(\d+),\s*(\d+),\s*(\d+)\s*\);", - r"color = (\1,\2,\3,\4);", - content, - ) - - content = re.sub( - r"\(\s*(-?\d+),\s*(-?\d+),\s*([a-zA-Z]+)\s*\)", r"(\1,\2,\3)", content - ) - - content = re.sub( - r"\(\s*(-?\d+),\s*(-?\d+),\s*([a-zA-Z]),\s*\{", r"(\1,\2,\3,{", content - ) - - content = re.sub( - r"\(\s*(-?\d+),\s*(-?\d+),\s*([a-zA-Z]+),\s*\{", r"(\1,\2,\3,{", content - ) - - content = re.sub(r"\}\s*\),", r"}),", content) - - content += "\n" # add blank break at the end of the file. + patterns = [ + ( + r"customBinaryData = <\s*([0-9a-fA-F\s]+)\s*>;", + lambda m: f"customBinaryData = <{m.group(1).replace(' ', '')}>;", + ), + (r"\(\s*(-?[\d.]+),\s*(-?[\d.]+)\s*\);", r"(\1,\2);"), + (r"\(\s*(-?[\d.]+),\s*(-?[\d.]+),\s*([a-zA-Z]+)\s*\)", r"(\1,\2,\3)"), + (r"origin = \(\s*(-?[\d.]+),\s*(-?[\d.]+)\s*\);", r"origin = (\1,\2);"), + (r"target = \(\s*(-?[\d.]+),\s*(-?[\d.]+)\s*\);", r"target = (\1,\2);"), + ( + r"color = \(\s*(\d+),\s*(\d+),\s*(\d+),\s*(\d+)\s*\);", + r"color = (\1,\2,\3,\4);", + ), + (r"\(\s*(-?[\d.]+),\s*(-?[\d.]+),\s*([a-zA-Z]+)\s*\)", r"(\1,\2,\3)"), + (r"\(\s*(-?[\d.]+),\s*(-?[\d.]+),\s*([a-zA-Z]+),\s*\{", r"(\1,\2,\3,{"), + (r"\}\s*\),", r"}),"), + (r"anchors = \(\);", r"anchors = (\n);"), + (r"unicode = \(\);", r"unicode = (\n);"), + (r"lib = \{\};", r"lib = {\n};"), + ( + r"verticalStems = \(\s*(-?[\d.]+),(-?[\d.]+)\);", + r"verticalStems = (\n\1,\n\2\n);", + ), + ] + + for pattern, replacement in patterns: + content = re.sub(pattern, replacement, content) return content diff --git a/tests/test_utils.py b/tests/test_utils.py index 5cfefba..2a52750 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -131,8 +131,8 @@ def test_getAssociatedMasterId(testGSFont): ], [ """( -341, -720, +321, +700, l, {""", "(321,700,l,{", @@ -176,9 +176,46 @@ def test_getAssociatedMasterId(testGSFont): );""", "pos = (334.937,407.08);", ], + [ + """pos = ( +-113, +574 +);""", + "pos = (-113,574);", + ], + ["pos = (524,-122);", "pos = (524,-122);"], + [ + "anchors = ();", + """anchors = ( +);""", + ], + [ + "unicode = ();", + """unicode = ( +);""", + ], + [ + "lib = {};", + """lib = { +};""", + ], + [ + "verticalStems = (17,19);", + """verticalStems = ( +17, +19 +);""", + ], + # TODO: The following does not fail in the unittest: diff \n vs \012 + [ + """code = "feature c2sc; +feature smcp; +";""", + """code = "feature c2sc;\012feature smcp;\012";""", + ], ] @pytest.mark.parametrize("content,expected", contentSnippets) def test_gsFormatting(content, expected): - assert gsFormatting(content) != expected + assert gsFormatting(content) == expected From f6fe46c7eabaeea011f0c2411b35641dda75fd7a Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Wed, 22 Jan 2025 12:05:49 +0100 Subject: [PATCH 23/88] Extend Readme file --- README.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/README.md b/README.md index c5b1181..b8b8a2e 100644 --- a/README.md +++ b/README.md @@ -6,3 +6,12 @@ It supports the following features: - Brace layers - Smart components (for now restricted to interpolation: axis values need to be within the minimum and maximum values) + + +## Write + +### Glyph Layer +- Contour (Paths, Nodes) ✅ +- Components ✅ +- Anchors ✅ +- Guidelines From 14144d304bc67e1325b22f477fcf0bdb5359c1bf Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Wed, 22 Jan 2025 12:21:20 +0100 Subject: [PATCH 24/88] Move getGlyphspackageGlyphFileName to utils and add unittest --- src/fontra_glyphs/backend.py | 20 +++++++------------- src/fontra_glyphs/utils.py | 12 ++++++++++++ tests/test_utils.py | 12 ++++++++++++ 3 files changed, 31 insertions(+), 13 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 59e456e..ef9174c 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -38,7 +38,13 @@ from glyphsLib.builder.smart_components import Pole from glyphsLib.types import Transform -from .utils import getAssociatedMasterId, getLocation, gsFormatting, toOrderedDict +from .utils import ( + getAssociatedMasterId, + getGlyphspackageGlyphFileName, + getLocation, + gsFormatting, + toOrderedDict, +) rootInfoNames = [ "familyName", @@ -489,18 +495,6 @@ def getGlyphFilePath(self, glyphName): return glyphsPath / (realGlyphName + ".glyph") -def getGlyphspackageGlyphFileName(glyphName): - nameParts = glyphName.split("-") - firstPart = ( - f"{nameParts[0]}_" - if len(nameParts[0]) == 1 and nameParts[0].isupper() - else nameParts[0] - ) - nameParts[0] = firstPart - - return "-".join(nameParts) - - def _readGlyphMapAndKerningGroups( rawGlyphsData: list, formatVersion: int ) -> tuple[dict[str, list[int]], dict[str, tuple[str, str]]]: diff --git a/src/fontra_glyphs/utils.py b/src/fontra_glyphs/utils.py index f8e44f6..5cef78f 100644 --- a/src/fontra_glyphs/utils.py +++ b/src/fontra_glyphs/utils.py @@ -2,6 +2,18 @@ from collections import OrderedDict +def getGlyphspackageGlyphFileName(glyphName): + nameParts = glyphName.split("-") + firstPart = ( + f"{nameParts[0]}_" + if len(nameParts[0]) == 1 and nameParts[0].isupper() + else nameParts[0] + ) + nameParts[0] = firstPart + + return "-".join(nameParts) + + # The following is obsolete once this is merged: # https://github.com/fonttools/openstep-plist/pull/35 def toOrderedDict(obj): diff --git a/tests/test_utils.py b/tests/test_utils.py index 2a52750..9210a7a 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -6,6 +6,7 @@ from fontra_glyphs.utils import ( getAssociatedMasterId, + getGlyphspackageGlyphFileName, getLocationFromLayerName, getLocationFromSources, gsFormatting, @@ -57,6 +58,17 @@ def test_getAssociatedMasterId(testGSFont): assert associatedMaster.name == "Regular" +glyphNameToGlyphspackageGlyphFileName = [ + ["A-cy", "A_-cy"], + # This list may extend... +] + + +@pytest.mark.parametrize("glyphName,expected", glyphNameToGlyphspackageGlyphFileName) +def test_getGlyphspackageGlyphFileName(glyphName, expected): + assert getGlyphspackageGlyphFileName(glyphName) == expected + + contentSnippets = [ [ """pos = ( From 6d8e7835cd4eda2a3a7e31990fd360d7d14bfefc Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Wed, 22 Jan 2025 12:28:32 +0100 Subject: [PATCH 25/88] Move comment and make it a bit more clear. --- src/fontra_glyphs/backend.py | 2 -- src/fontra_glyphs/utils.py | 4 ++++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index ef9174c..5b6c2c1 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -489,8 +489,6 @@ def _writeRawGlyph(self, glyphName, f): def getGlyphFilePath(self, glyphName): glyphsPath = pathlib.Path(self.gsFilePath) / "glyphs" - # TODO: Get the right glyph file name might be challenging, - # because for example the glyph A-cy is stored in the package as A_-cy.glyph realGlyphName = getGlyphspackageGlyphFileName(glyphName) return glyphsPath / (realGlyphName + ".glyph") diff --git a/src/fontra_glyphs/utils.py b/src/fontra_glyphs/utils.py index 5cef78f..12ba18b 100644 --- a/src/fontra_glyphs/utils.py +++ b/src/fontra_glyphs/utils.py @@ -3,6 +3,10 @@ def getGlyphspackageGlyphFileName(glyphName): + # Get the right glyph file name might be challenging, because for example + # the glyph "A-cy" is stored in the package as A_-cy.glyph. + # I could not find any documentation about this, yet. We may need to figure + # this out over time and extend the unittest. nameParts = glyphName.split("-") firstPart = ( f"{nameParts[0]}_" From 3a210e678a0612a744bd88b75ee5abf13469ef26 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Wed, 22 Jan 2025 16:35:23 +0100 Subject: [PATCH 26/88] Remove seenLayerIDs. Use 'new' layerName (equal to layerId) --- src/fontra_glyphs/backend.py | 38 ++++++++++++++++-------------------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 5b6c2c1..97c7fe4 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -293,7 +293,6 @@ async def getGlyph(self, glyphName: str) -> VariableGlyph | None: gsLayers = sorted( gsLayers, key=lambda i_gsLayer: masterOrder[i_gsLayer[1].associatedMasterId] ) - seenLayerIDs = {} seenLocations = [] for i, gsLayer in gsLayers: braceLocation = self._getBraceLayerLocation(gsLayer) @@ -327,8 +326,6 @@ async def getGlyph(self, glyphName: str) -> VariableGlyph | None: ) layers[layerName] = gsLayerToFontraLayer(gsLayer, self.axisNames) - # NOTE: Remember layerIds like in the following line or add 'identifier' to fontra Layer? - customData["com.glyphsapp.seenLayerIDs"] = seenLayerIDs fixSourceLocations(sources, set(smartLocation)) glyph = VariableGlyph( @@ -766,22 +763,24 @@ def gsVerticalMetricsToFontraLineMetricsHorizontal(gsFont, gsMaster): def variableGlyphToGSGlyph(variableGlyph, gsGlyph): # Convert Fontra variableGlyph to GlyphsApp glyph masterIds = [m.id for m in gsGlyph.parent.masters] - seenLayerIDs = variableGlyph.customData["com.glyphsapp.seenLayerIDs"] - for layerName, gsLayerId in seenLayerIDs.items(): - if layerName not in variableGlyph.layers: - gsLayerId = seenLayerIDs.get(layerName) - if gsLayerId in masterIds: - # Someone deleted a master, this breaks the compatibility in a .glyphs file. - # Skip deleting master layer. - continue - # Removing non-master-layer: - del gsGlyph.layers[gsLayerId] - - for i, (layerName, layer) in enumerate(iter(variableGlyph.layers.items())): - gsLayerId = seenLayerIDs.get(layerName) - if gsLayerId is not None: + for gsLayerId in [gsLayer.layerId for gsLayer in gsGlyph.layers]: + if gsLayerId in variableGlyph.layers: + # This layer will be modified later. + continue + if gsLayerId in masterIds: + # We don't delete master layers. + continue + # Removing non-master-layer: + del gsGlyph.layers[gsLayerId] + + for layerName, layer in iter(variableGlyph.layers.items()): + gsLayer = gsGlyph.layers.get(layerName) + # layerName is equal to gsLayer.layerId if it comes from Glyphsapp, + # otherwise the layer has been newly created within Fontrta. + + if gsLayer is not None: # gsLayer exists: modify existing gsLayer - fontraLayerToGSLayer(layer, gsGlyph.layers[gsLayerId]) + fontraLayerToGSLayer(layer, gsLayer) else: # gsLayer does not exist: therefore must be 'isSpecialLayer' # and need to be created as a new layer: @@ -807,9 +806,6 @@ def variableGlyphToGSGlyph(variableGlyph, gsGlyph): fontraLayerToGSLayer(layer, gsLayer) gsGlyph.layers.append(gsLayer) - # It might be that someone deletes a not needed master-layer, which works for fontra, - # but is required for Glyphs. We would need to get the intermediate contours. - return gsGlyph From 698722fa640a233447b444639cba39f166500174 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Wed, 22 Jan 2025 16:45:23 +0100 Subject: [PATCH 27/88] Update test data "A-cy" + fix test_getGlyph: bug in GlyphsApp 2 file --- tests/data/GlyphsUnitTestSans.glyphs | 2 +- .../GlyphsUnitTestSans3.fontra/glyph-info.csv | 1 + .../glyphs/A-cy^1.json | 58 +++++++++++++++++++ tests/test_backend.py | 2 +- 4 files changed, 61 insertions(+), 2 deletions(-) create mode 100644 tests/data/GlyphsUnitTestSans3.fontra/glyphs/A-cy^1.json diff --git a/tests/data/GlyphsUnitTestSans.glyphs b/tests/data/GlyphsUnitTestSans.glyphs index 042467a..f9e633e 100644 --- a/tests/data/GlyphsUnitTestSans.glyphs +++ b/tests/data/GlyphsUnitTestSans.glyphs @@ -2380,7 +2380,7 @@ layerId = "BFFFD157-90D3-4B85-B99D-9A2F366F03CA"; width = 753; } ); -unicode = 1040; +unicode = 0410; } ); instances = ( diff --git a/tests/data/GlyphsUnitTestSans3.fontra/glyph-info.csv b/tests/data/GlyphsUnitTestSans3.fontra/glyph-info.csv index 3bfd775..676f7b8 100644 --- a/tests/data/GlyphsUnitTestSans3.fontra/glyph-info.csv +++ b/tests/data/GlyphsUnitTestSans3.fontra/glyph-info.csv @@ -1,5 +1,6 @@ glyph name;code points A;U+0041 +A-cy;U+0410 Adieresis;U+00C4 V;U+0056 _part.shoulder; diff --git a/tests/data/GlyphsUnitTestSans3.fontra/glyphs/A-cy^1.json b/tests/data/GlyphsUnitTestSans3.fontra/glyphs/A-cy^1.json new file mode 100644 index 0000000..227f883 --- /dev/null +++ b/tests/data/GlyphsUnitTestSans3.fontra/glyphs/A-cy^1.json @@ -0,0 +1,58 @@ +{ +"name": "A-cy", +"sources": [ +{ +"name": "Light", +"layerName": "C4872ECA-A3A9-40AB-960A-1DB2202F16DE", +"location": { +"Weight": 17 +} +}, +{ +"name": "Regular", +"layerName": "3E7589AA-8194-470F-8E2F-13C1C581BE24", +"location": { +"Weight": 90 +} +}, +{ +"name": "Bold", +"layerName": "BFFFD157-90D3-4B85-B99D-9A2F366F03CA", +"location": { +"Weight": 220 +} +} +], +"layers": { +"3E7589AA-8194-470F-8E2F-13C1C581BE24": { +"glyph": { +"components": [ +{ +"name": "A" +} +], +"xAdvance": 657 +} +}, +"BFFFD157-90D3-4B85-B99D-9A2F366F03CA": { +"glyph": { +"components": [ +{ +"name": "A" +} +], +"xAdvance": 753 +} +}, +"C4872ECA-A3A9-40AB-960A-1DB2202F16DE": { +"glyph": { +"components": [ +{ +"name": "A" +} +], +"xAdvance": 593 +} +} +} +} diff --git a/tests/test_backend.py b/tests/test_backend.py index f809bdf..9bb6788 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -91,7 +91,7 @@ async def test_getAxes(testFont): "m": [109], "n": [110], "V": [86], - "A-cy": [0x0410], + "A-cy": [1040], } From a40a4297c909e39963daaa60e21b8ea8be463ddd Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Wed, 22 Jan 2025 16:50:21 +0100 Subject: [PATCH 28/88] Replace getGlyphspackageGlyphFileName with userNameToFileName (fonttools) --- src/fontra_glyphs/backend.py | 13 ++++--------- src/fontra_glyphs/utils.py | 16 ---------------- tests/test_utils.py | 12 ------------ 3 files changed, 4 insertions(+), 37 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 97c7fe4..1b8cb4a 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -30,6 +30,7 @@ from fontra.core.protocols import WritableFontBackend from fontTools.designspaceLib import DesignSpaceDocument from fontTools.misc.transform import DecomposedTransform +from fontTools.ufoLib.filenames import userNameToFileName from glyphsLib.builder.axes import ( get_axis_definitions, get_regular_master, @@ -38,13 +39,7 @@ from glyphsLib.builder.smart_components import Pole from glyphsLib.types import Transform -from .utils import ( - getAssociatedMasterId, - getGlyphspackageGlyphFileName, - getLocation, - gsFormatting, - toOrderedDict, -) +from .utils import getAssociatedMasterId, getLocation, gsFormatting, toOrderedDict rootInfoNames = [ "familyName", @@ -486,8 +481,8 @@ def _writeRawGlyph(self, glyphName, f): def getGlyphFilePath(self, glyphName): glyphsPath = pathlib.Path(self.gsFilePath) / "glyphs" - realGlyphName = getGlyphspackageGlyphFileName(glyphName) - return glyphsPath / (realGlyphName + ".glyph") + refFileName = userNameToFileName(glyphName, suffix=".glyph") + return glyphsPath / refFileName def _readGlyphMapAndKerningGroups( diff --git a/src/fontra_glyphs/utils.py b/src/fontra_glyphs/utils.py index 12ba18b..f8e44f6 100644 --- a/src/fontra_glyphs/utils.py +++ b/src/fontra_glyphs/utils.py @@ -2,22 +2,6 @@ from collections import OrderedDict -def getGlyphspackageGlyphFileName(glyphName): - # Get the right glyph file name might be challenging, because for example - # the glyph "A-cy" is stored in the package as A_-cy.glyph. - # I could not find any documentation about this, yet. We may need to figure - # this out over time and extend the unittest. - nameParts = glyphName.split("-") - firstPart = ( - f"{nameParts[0]}_" - if len(nameParts[0]) == 1 and nameParts[0].isupper() - else nameParts[0] - ) - nameParts[0] = firstPart - - return "-".join(nameParts) - - # The following is obsolete once this is merged: # https://github.com/fonttools/openstep-plist/pull/35 def toOrderedDict(obj): diff --git a/tests/test_utils.py b/tests/test_utils.py index 9210a7a..2a52750 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -6,7 +6,6 @@ from fontra_glyphs.utils import ( getAssociatedMasterId, - getGlyphspackageGlyphFileName, getLocationFromLayerName, getLocationFromSources, gsFormatting, @@ -58,17 +57,6 @@ def test_getAssociatedMasterId(testGSFont): assert associatedMaster.name == "Regular" -glyphNameToGlyphspackageGlyphFileName = [ - ["A-cy", "A_-cy"], - # This list may extend... -] - - -@pytest.mark.parametrize("glyphName,expected", glyphNameToGlyphspackageGlyphFileName) -def test_getGlyphspackageGlyphFileName(glyphName, expected): - assert getGlyphspackageGlyphFileName(glyphName) == expected - - contentSnippets = [ [ """pos = ( From f25b1b6e949072d4233a459ba7f31f2768325f25 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Wed, 22 Jan 2025 17:27:31 +0100 Subject: [PATCH 29/88] Refactor and fix unittests --- src/fontra_glyphs/backend.py | 15 +++++++++------ src/fontra_glyphs/utils.py | 28 ++-------------------------- tests/test_backend.py | 27 +++++++++++++++------------ tests/test_utils.py | 23 ++++------------------- 4 files changed, 30 insertions(+), 63 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 1b8cb4a..e3fd2fb 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -39,7 +39,12 @@ from glyphsLib.builder.smart_components import Pole from glyphsLib.types import Transform -from .utils import getAssociatedMasterId, getLocation, gsFormatting, toOrderedDict +from .utils import ( + getAssociatedMasterId, + getLocationFromSources, + gsFormatting, + toOrderedDict, +) rootInfoNames = [ "familyName", @@ -769,7 +774,7 @@ def variableGlyphToGSGlyph(variableGlyph, gsGlyph): del gsGlyph.layers[gsLayerId] for layerName, layer in iter(variableGlyph.layers.items()): - gsLayer = gsGlyph.layers.get(layerName) + gsLayer = gsGlyph.layers[layerName] # layerName is equal to gsLayer.layerId if it comes from Glyphsapp, # otherwise the layer has been newly created within Fontrta. @@ -781,12 +786,10 @@ def variableGlyphToGSGlyph(variableGlyph, gsGlyph): # and need to be created as a new layer: gsLayer = glyphsLib.classes.GSLayer() gsLayer.name = layerName + gsLayer.layerId = layerName gsLayer.isSpecialLayer = True - location = getLocation(variableGlyph, layerName, gsGlyph.parent.axes) - if location is None: - return - + location = getLocationFromSources(variableGlyph.sources, layerName) gsLocation = [ location[axis.name.lower()] for axis in gsGlyph.parent.axes diff --git a/src/fontra_glyphs/utils.py b/src/fontra_glyphs/utils.py index f8e44f6..f156393 100644 --- a/src/fontra_glyphs/utils.py +++ b/src/fontra_glyphs/utils.py @@ -13,37 +13,13 @@ def toOrderedDict(obj): return obj -def getLocationFromLayerName(layerName, gsAxes): - # Get the location based on name, - # for example: Light / {166, 100} (layer #4) - match = re.search(r"\{([^}]+)\}", layerName) - if not match: - return None - listLocation = match.group(1).replace(" ", "").split(",") - listLocationValues = [float(v) for v in listLocation] - return { - gsAxes[i].name.lower(): value - for i, value in enumerate(listLocationValues) - if i < len(gsAxes) - } - - def getLocationFromSources(sources, layerName): - s = None + s = sources[0] for source in sources: if source.layerName == layerName: s = source break - if s is not None: - return {k.lower(): v for k, v in s.location.items()} - - -def getLocation(glyph, layerName, gsAxes): - location = getLocationFromSources(glyph.sources, layerName) - if location: - return location - # This layerName is not used by any source: - return getLocationFromLayerName(layerName, gsAxes) + return {k.lower(): v for k, v in s.location.items()} def getAssociatedMasterId(gsGlyph, gsLocation): diff --git a/tests/test_backend.py b/tests/test_backend.py index 9bb6788..ebe81c5 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -1,6 +1,7 @@ import os import pathlib import shutil +import uuid from copy import deepcopy import pytest @@ -179,7 +180,7 @@ async def test_deleteLayer(writableTestFont): glyphMap = await writableTestFont.getGlyphMap() glyph = await writableTestFont.getGlyph(glyphName) - layerName = "Regular (layer #1)" + layerName = "3E7589AA-8194-470F-8E2F-13C1C581BE24" del glyph.layers[layerName] await writableTestFont.putGlyph(glyphName, glyph, glyphMap[glyphName]) @@ -195,19 +196,20 @@ async def test_addLayer(writableTestFont): glyph = await writableTestFont.getGlyph(glyphName) numGlyphLayers = len(glyph.layers) + layerName = str(uuid.uuid4()).upper() glyph.sources.append( - GlyphSource(name="{166, 100}", location={"weight": 166}, layerName="{166, 100}") + GlyphSource(name="SemiBold", location={"weight": 166}, layerName=layerName) ) # Copy StaticGlyph of Bold: - glyph.layers["{166, 100}"] = Layer( - glyph=deepcopy(glyph.layers["Bold (layer #2)"].glyph) + glyph.layers[layerName] = Layer( + glyph=deepcopy(glyph.layers["BFFFD157-90D3-4B85-B99D-9A2F366F03CA"].glyph) ) await writableTestFont.putGlyph(glyphName, glyph, glyphMap[glyphName]) savedGlyph = await writableTestFont.getGlyph(glyphName) assert len(savedGlyph.layers) > numGlyphLayers - assert "Bold / {166, 100} (layer #4)" in savedGlyph.layers.keys() + assert layerName in savedGlyph.layers.keys() async def test_addLayerWithComponent(writableTestFont): @@ -216,19 +218,20 @@ async def test_addLayerWithComponent(writableTestFont): glyph = await writableTestFont.getGlyph(glyphName) numGlyphLayers = len(glyph.layers) + layerName = str(uuid.uuid4()).upper() glyph.sources.append( - GlyphSource(name="{166, 100}", location={"weight": 166}, layerName="{166, 100}") + GlyphSource(name="SemiBold", location={"weight": 166}, layerName=layerName) ) # Copy StaticGlyph of Bold: - glyph.layers["{166, 100}"] = Layer( - glyph=deepcopy(glyph.layers["Bold (layer #2)"].glyph) + glyph.layers[layerName] = Layer( + glyph=deepcopy(glyph.layers["BFFFD157-90D3-4B85-B99D-9A2F366F03CA"].glyph) ) await writableTestFont.putGlyph(glyphName, glyph, glyphMap[glyphName]) savedGlyph = await writableTestFont.getGlyph(glyphName) assert len(savedGlyph.layers) > numGlyphLayers - assert "Bold / {166, 100} (layer #3)" in savedGlyph.layers.keys() + assert layerName in savedGlyph.layers.keys() async def test_deleteMasterLayer(writableTestFont): @@ -239,7 +242,7 @@ async def test_deleteMasterLayer(writableTestFont): glyph = await writableTestFont.getGlyph(glyphName) numGlyphLayers = len(glyph.layers) - del glyph.layers["Bold (layer #2)"] + del glyph.layers["BFFFD157-90D3-4B85-B99D-9A2F366F03CA"] await writableTestFont.putGlyph(glyphName, glyph, glyphMap[glyphName]) @@ -252,7 +255,7 @@ async def test_addAnchor(writableTestFont): glyphMap = await writableTestFont.getGlyphMap() glyph = await writableTestFont.getGlyph(glyphName) - layerName = "{ 166, 100 }" + layerName = str(uuid.uuid4()).upper() glyph.layers[layerName] = Layer(glyph=StaticGlyph(xAdvance=0)) glyph.layers[layerName].glyph.anchors.append(Anchor(name="top", x=207, y=746)) @@ -262,7 +265,7 @@ async def test_addAnchor(writableTestFont): assert ( glyph.layers[layerName].glyph.anchors - == savedGlyph.layers["Bold / { 166, 100 } (layer #4)"].glyph.anchors + == savedGlyph.layers[layerName].glyph.anchors ) diff --git a/tests/test_utils.py b/tests/test_utils.py index 2a52750..37a0c61 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -6,7 +6,6 @@ from fontra_glyphs.utils import ( getAssociatedMasterId, - getLocationFromLayerName, getLocationFromSources, gsFormatting, ) @@ -28,25 +27,11 @@ def testGSFont(request): return glyphsLib.GSFont(request.param) -layerNamesToLocation = [ - ["Light / {166, 100} (layer #4)", {"weight": 166}], - ["{ 166 } (layer #3)", {"weight": 166}], - ["Light / (layer #4)", None], -] - - -@pytest.mark.parametrize("layerName,expected", layerNamesToLocation) -def test_getLocationFromLayerName(layerName, expected): - gsFont = glyphsLib.classes.GSFont() - gsFont.axes = [glyphsLib.classes.GSAxis(name="Weight", tag="wght")] - location = getLocationFromLayerName(layerName, gsFont.axes) - assert location == expected - - async def test_getLocationFromSources(testFont): - glyphName = "a" - glyph = await testFont.getGlyph(glyphName) - location = getLocationFromSources(glyph.sources, "Regular / {155, 100} (layer #3)") + glyph = await testFont.getGlyph("a") + location = getLocationFromSources( + glyph.sources, "1FA54028-AD2E-4209-AA7B-72DF2DF16264" + ) assert location == {"weight": 155} From 1e0ae6aa39d75917389434bcd0ea6a7ff1b5c9d4 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Wed, 22 Jan 2025 17:36:00 +0100 Subject: [PATCH 30/88] rework _writeRawGlyph (dumps) --- src/fontra_glyphs/backend.py | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index e3fd2fb..e730b03 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -416,29 +416,21 @@ async def putGlyph( def _writeRawGlyph(self, glyphName, f): # 5. write whole file with openstep_plist with open(self.gsFilePath, "r+", encoding="utf-8") as fp: - openstep_plist.dump( + content = openstep_plist.dumps( self.rawFontData, - fp, unicode_escape=False, indent=0, single_line_tuples=True, escape_newlines=False, ) - # 6. fix formatting - with open(self.gsFilePath, "r", encoding="utf-8") as fp: - content = gsFormatting(fp.read()) + # 6. fix formatting + content = gsFormatting(content) if self.gsFont.format_version >= 3: - # add blank break at the end of the file. + # add break at the end of the file. content += "\n" - with open(self.gsFilePath, "w", encoding="utf-8") as fp: fp.write(content) - # This following does not wrok, correctly, there the code above. - # with open(self.gsFilePath, "r+", encoding="utf-8") as fp: - # content = gsFormatting(fp.read()) - # fp.write(content) - async def aclose(self) -> None: pass From e4e097e1a52dbee93f80468f8a6ca80de1ce2d14 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Wed, 22 Jan 2025 17:36:16 +0100 Subject: [PATCH 31/88] Remove resolved comment --- tests/test_backend.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/test_backend.py b/tests/test_backend.py index ebe81c5..e505ac4 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -137,10 +137,6 @@ async def test_getGlyph(testFont, referenceFont, glyphName): glyph.customData["com.glyphsapp.glyph-color"] = [120, 220, 20, 4] referenceGlyph = await referenceFont.getGlyph(glyphName) - # TODO: This unit test fails currently, because the fontra referenceFont - # does not contain the customData "com.glyphsapp.seenLayerIDs". - # Before I update the fontra file, I would like to discuss with Just, - # if this is the right approach. test_putGlyph works now. assert referenceGlyph == glyph From f1fc7e0c386817428a5fde1418c84a78390d4e57 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Wed, 22 Jan 2025 18:03:56 +0100 Subject: [PATCH 32/88] Fix variable name, add and remove some comments --- src/fontra_glyphs/backend.py | 7 +++---- src/fontra_glyphs/utils.py | 9 ++++----- tests/test_utils.py | 2 ++ 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index e730b03..361be45 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -293,6 +293,7 @@ async def getGlyph(self, glyphName: str) -> VariableGlyph | None: gsLayers = sorted( gsLayers, key=lambda i_gsLayer: masterOrder[i_gsLayer[1].associatedMasterId] ) + seenLocations = [] for i, gsLayer in gsLayers: braceLocation = self._getBraceLayerLocation(gsLayer) @@ -810,9 +811,7 @@ def fontraLayerToGSLayer(layer, gsLayer): gsLayer.components = [ fontraComponentToGSComponent(component) for component in layer.glyph.components ] - gsLayer.anchors = [ - fontraAnchorsToGSAnchor(anchor) for anchor in layer.glyph.anchors - ] + gsLayer.anchors = [fontraAnchorToGSAnchor(anchor) for anchor in layer.glyph.anchors] def fontraComponentToGSComponent(component): @@ -824,7 +823,7 @@ def fontraComponentToGSComponent(component): return gsComponent -def fontraAnchorsToGSAnchor(anchor): +def fontraAnchorToGSAnchor(anchor): gsAnchor = glyphsLib.classes.GSAnchor() gsAnchor.name = anchor.name gsAnchor.position.x = anchor.x diff --git a/src/fontra_glyphs/utils.py b/src/fontra_glyphs/utils.py index f156393..1ad45c2 100644 --- a/src/fontra_glyphs/utils.py +++ b/src/fontra_glyphs/utils.py @@ -40,11 +40,10 @@ def getAssociatedMasterId(gsGlyph, gsLocation): def gsFormatting(content): - # openstep_plist.dump changes the whole formatting, therefore - # it's very diffucute to see what has changed. - # This function is a very bad try to get close to how the formatting - # looks like for a .glyphs file. - # There must be a better solution, but this is better than nothing. + # TODO: This need a different solution. + # Should be solved in the raw data not via regular expressions. + # The raw data is made out of list. We need to convert some part into tuples. + # For more please see: https://github.com/fonttools/openstep-plist/issues/33 patterns = [ ( diff --git a/tests/test_utils.py b/tests/test_utils.py index 37a0c61..339a8d8 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -36,6 +36,8 @@ async def test_getLocationFromSources(testFont): def test_getAssociatedMasterId(testGSFont): + # TODO: need more complex test with at least two axes, + # then improvement getAssociatedMasterId gsGlyph = testGSFont.glyphs["a"] associatedMasterId = getAssociatedMasterId(gsGlyph, [155]) associatedMaster = gsGlyph.layers[associatedMasterId] From 7d56bac0c52e767cfabe7d71dba8be4a5cc0e9e4 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Thu, 23 Jan 2025 11:17:00 +0100 Subject: [PATCH 33/88] Improve getAssociatedMasterId + unittests --- src/fontra_glyphs/backend.py | 2 +- src/fontra_glyphs/utils.py | 12 +++--- tests/test_utils.py | 77 ++++++++++++++++++++++++++++++------ 3 files changed, 73 insertions(+), 18 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 361be45..a94794f 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -790,7 +790,7 @@ def variableGlyphToGSGlyph(variableGlyph, gsGlyph): ] gsLayer.attributes["coordinates"] = gsLocation - associatedMasterId = getAssociatedMasterId(gsGlyph, gsLocation) + associatedMasterId = getAssociatedMasterId(gsGlyph.parent, gsLocation) if associatedMasterId: gsLayer.associatedMasterId = associatedMasterId diff --git a/src/fontra_glyphs/utils.py b/src/fontra_glyphs/utils.py index 1ad45c2..16f0bd4 100644 --- a/src/fontra_glyphs/utils.py +++ b/src/fontra_glyphs/utils.py @@ -22,12 +22,11 @@ def getLocationFromSources(sources, layerName): return {k.lower(): v for k, v in s.location.items()} -def getAssociatedMasterId(gsGlyph, gsLocation): +def getAssociatedMasterId(gsFont, gsLocation): # Best guess for associatedMasterId - closestMaster = None + closestMasterID = gsFont.masters[0].id # default first master. closestDistance = float("inf") - for gsLayer in gsGlyph.layers: - gsMaster = gsLayer.master + for gsMaster in gsFont.masters: distance = sum( abs(gsMaster.axes[i] - gsLocation[i]) for i in range(len(gsMaster.axes)) @@ -35,8 +34,9 @@ def getAssociatedMasterId(gsGlyph, gsLocation): ) if distance < closestDistance: closestDistance = distance - closestMaster = gsMaster - return closestMaster.id if closestMaster else None + closestMasterID = gsMaster.id + + return closestMasterID def gsFormatting(content): diff --git a/tests/test_utils.py b/tests/test_utils.py index 339a8d8..aa0bcbd 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,8 +1,8 @@ import pathlib -import glyphsLib import pytest from fontra.backends import getFileSystemBackend +from glyphsLib.classes import GSAxis, GSFont, GSFontMaster, GSGlyph, GSLayer from fontra_glyphs.utils import ( getAssociatedMasterId, @@ -22,9 +22,54 @@ def testFont(request): return getFileSystemBackend(request.param) -@pytest.fixture(scope="module", params=[glyphs2Path, glyphs3Path, glyphsPackagePath]) -def testGSFont(request): - return glyphsLib.GSFont(request.param) +def createGSFontMaster(axes=[100, 100], id="DUMMY-MASTER-ID"): + master = GSFontMaster() + master.axes = axes + master.id = id + return master + + +def createGSGlyph(name="GlyphName", unicodes=[], layers=[]): + glyph = GSGlyph() + glyph.name = name + glyph.unicodes = unicodes + glyph.layers = layers + return glyph + + +@pytest.fixture(scope="module") +def testGSFontWW(): + gsFont = GSFont() + gsFont.format_version = 3 + gsFont.axes = [ + GSAxis(name="Optical Size", tag="opsz"), + GSAxis(name="Weight", tag="wght"), + GSAxis(name="Width", tag="wdth"), + ] + gsFont.masters = [ + createGSFontMaster(axes=[12, 50, 100], id="MasterID-TextCondLight"), + createGSFontMaster(axes=[12, 50, 400], id="MasterID-TextCondRegular"), + createGSFontMaster(axes=[12, 50, 900], id="MasterID-TextCondBold"), + createGSFontMaster(axes=[12, 200, 100], id="MasterID-TextWideLight"), + createGSFontMaster(axes=[12, 200, 400], id="MasterID-TextWideRegular"), + createGSFontMaster(axes=[12, 200, 900], id="MasterID-TextWideBold"), + createGSFontMaster(axes=[60, 50, 100], id="MasterID-PosterCondLight"), + createGSFontMaster(axes=[60, 50, 400], id="MasterID-PosterCondRegular"), + createGSFontMaster(axes=[60, 50, 900], id="MasterID-PosterCondBold"), + createGSFontMaster(axes=[60, 200, 100], id="MasterID-PosterWideLight"), + createGSFontMaster(axes=[60, 200, 400], id="MasterID-PosterWideRegular"), + createGSFontMaster(axes=[60, 200, 900], id="MasterID-PosterWideBold"), + ] + gsFont.glyphs.append( + createGSGlyph( + name="A", + unicodes=[ + 0x0041, + ], + layers=[GSLayer()], + ) + ) + return gsFont async def test_getLocationFromSources(testFont): @@ -35,13 +80,23 @@ async def test_getLocationFromSources(testFont): assert location == {"weight": 155} -def test_getAssociatedMasterId(testGSFont): - # TODO: need more complex test with at least two axes, - # then improvement getAssociatedMasterId - gsGlyph = testGSFont.glyphs["a"] - associatedMasterId = getAssociatedMasterId(gsGlyph, [155]) - associatedMaster = gsGlyph.layers[associatedMasterId] - assert associatedMaster.name == "Regular" +expectedAssociatedMasterId = [ + # gsLocation, associatedMasterId + [[14, 155, 900], "MasterID-TextWideBold"], + [[14, 155, 100], "MasterID-TextWideLight"], + [[14, 55, 900], "MasterID-TextCondBold"], + [[14, 55, 110], "MasterID-TextCondLight"], + [[55, 155, 900], "MasterID-PosterWideBold"], + [[55, 155, 100], "MasterID-PosterWideLight"], + [[55, 55, 900], "MasterID-PosterCondBold"], + [[55, 55, 110], "MasterID-PosterCondLight"], + [[30, 100, 399], "MasterID-TextCondRegular"], +] + + +@pytest.mark.parametrize("gsLocation,expected", expectedAssociatedMasterId) +def test_getAssociatedMasterId(testGSFontWW, gsLocation, expected): + assert getAssociatedMasterId(testGSFontWW, gsLocation) == expected contentSnippets = [ From 248fdf1b587e862905d3bc9b54d25a1b2783df05 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Thu, 23 Jan 2025 11:48:24 +0100 Subject: [PATCH 34/88] Raise NotImplementedError for stubs --- src/fontra_glyphs/backend.py | 28 +++++++++------------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index a94794f..f6b1b18 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -172,12 +172,10 @@ async def getGlyphMap(self) -> dict[str, list[int]]: return self.glyphMap async def putGlyphMap(self, value: dict[str, list[int]]) -> None: - print("GlyphsBackend putGlyphMap: ", value) pass async def deleteGlyph(self, glyphName): - print("GlyphsBackend deleteGlyph: ", glyphName) - pass + raise NotImplementedError("deleting glyphs is not yet implemented") async def getFontInfo(self) -> FontInfo: infoDict = {} @@ -195,29 +193,25 @@ async def getFontInfo(self) -> FontInfo: return FontInfo(**infoDict) async def putFontInfo(self, fontInfo: FontInfo): - print("GlyphsBackend putFontInfo: ", fontInfo) - pass + raise NotImplementedError("editing FontInfo is not yet implemented") async def getSources(self) -> dict[str, FontSource]: return gsMastersToFontraFontSources(self.gsFont, self.locationByMasterID) async def putSources(self, sources: dict[str, FontSource]) -> None: - print("GlyphsBackend putSources: ", sources) - pass + raise NotImplementedError("editing FontSources is not yet implemented") async def getAxes(self) -> Axes: return Axes(axes=self.axes) async def putAxes(self, axes: Axes) -> None: - print("GlyphsBackend putAxes: ", axes) - pass + raise NotImplementedError("editing Axes is not yet implemented") async def getUnitsPerEm(self) -> int: return self.gsFont.upm async def putUnitsPerEm(self, value: int) -> None: - print("GlyphsBackend putUnitsPerEm: ", value) - pass + raise NotImplementedError("editing UnitsPerEm is not yet implemented") async def getKerning(self) -> dict[str, Kerning]: # TODO: RTL kerning: https://docu.glyphsapp.com/#GSFont.kerningRTL @@ -239,30 +233,26 @@ async def getKerning(self) -> dict[str, Kerning]: return kerning async def putKerning(self, kerning: dict[str, Kerning]) -> None: - print("GlyphsBackend putKerning: ", kerning) - pass + raise NotImplementedError("editing Kerning is not yet implemented") async def getFeatures(self) -> OpenTypeFeatures: # TODO: extract features return OpenTypeFeatures() async def putFeatures(self, features: OpenTypeFeatures) -> None: - print("GlyphsBackend putFeatures: ", features) - pass + raise NotImplementedError("editing OpenTypeFeatures is not yet implemented") async def getBackgroundImage(self, imageIdentifier: str) -> ImageData | None: return None async def putBackgroundImage(self, imageIdentifier: str, data: ImageData) -> None: - print("GlyphsBackend putBackgroundImage: ", imageIdentifier, data) - pass + raise NotImplementedError("editing BackgroundImage is not yet implemented") async def getCustomData(self) -> dict[str, Any]: return {} async def putCustomData(self, lib): - print("GlyphsBackend putCustomData: ", lib) - pass + raise NotImplementedError("editing CustomData is not yet implemented") async def getGlyph(self, glyphName: str) -> VariableGlyph | None: if glyphName not in self.glyphNameToIndex: From 00753f903d48cff58c27aaf6f0373859cdf5d6a5 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Thu, 23 Jan 2025 11:52:42 +0100 Subject: [PATCH 35/88] Update self.glyphMap within putGlyph --- src/fontra_glyphs/backend.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index f6b1b18..06341e6 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -379,6 +379,10 @@ def _getSmartLocation(self, gsLayer, localAxesByName): async def putGlyph( self, glyphName: str, glyph: VariableGlyph, codePoints: list[int] ) -> None: + assert isinstance(codePoints, list) + assert all(isinstance(cp, int) for cp in codePoints) + self.glyphMap[glyphName] = codePoints + # 1. convert VariableGlyph to GSGlyph gsGlyphNew = variableGlyphToGSGlyph( glyph, deepcopy(self.gsFont.glyphs[glyphName]) From 834dfe7f3d6735c427a6b6159c8218a0df2c293f Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Thu, 23 Jan 2025 14:40:59 +0100 Subject: [PATCH 36/88] replace gsFormatting with convertMatchesToTuples --- src/fontra_glyphs/backend.py | 21 ++-- src/fontra_glyphs/utils.py | 92 +++++++++++------- tests/test_utils.py | 184 +++++------------------------------ 3 files changed, 88 insertions(+), 209 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 06341e6..7362b14 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -40,9 +40,10 @@ from glyphsLib.types import Transform from .utils import ( + convertMatchesToTuples, getAssociatedMasterId, getLocationFromSources, - gsFormatting, + matchTreeFont, toOrderedDict, ) @@ -410,21 +411,19 @@ async def putGlyph( def _writeRawGlyph(self, glyphName, f): # 5. write whole file with openstep_plist - with open(self.gsFilePath, "r+", encoding="utf-8") as fp: - content = openstep_plist.dumps( - self.rawFontData, + result = convertMatchesToTuples(self.rawFontData, matchTreeFont) + self.gsFilePath.write_text( + openstep_plist.dumps( + result, unicode_escape=False, indent=0, single_line_tuples=True, escape_newlines=False, + sort_keys=False, + single_line_empty_objects=False, ) - - # 6. fix formatting - content = gsFormatting(content) - if self.gsFont.format_version >= 3: - # add break at the end of the file. - content += "\n" - fp.write(content) + + "\n" + ) async def aclose(self) -> None: pass diff --git a/src/fontra_glyphs/utils.py b/src/fontra_glyphs/utils.py index 16f0bd4..cf33cf4 100644 --- a/src/fontra_glyphs/utils.py +++ b/src/fontra_glyphs/utils.py @@ -1,4 +1,3 @@ -import re from collections import OrderedDict @@ -39,38 +38,59 @@ def getAssociatedMasterId(gsFont, gsLocation): return closestMasterID -def gsFormatting(content): - # TODO: This need a different solution. - # Should be solved in the raw data not via regular expressions. - # The raw data is made out of list. We need to convert some part into tuples. - # For more please see: https://github.com/fonttools/openstep-plist/issues/33 - - patterns = [ - ( - r"customBinaryData = <\s*([0-9a-fA-F\s]+)\s*>;", - lambda m: f"customBinaryData = <{m.group(1).replace(' ', '')}>;", - ), - (r"\(\s*(-?[\d.]+),\s*(-?[\d.]+)\s*\);", r"(\1,\2);"), - (r"\(\s*(-?[\d.]+),\s*(-?[\d.]+),\s*([a-zA-Z]+)\s*\)", r"(\1,\2,\3)"), - (r"origin = \(\s*(-?[\d.]+),\s*(-?[\d.]+)\s*\);", r"origin = (\1,\2);"), - (r"target = \(\s*(-?[\d.]+),\s*(-?[\d.]+)\s*\);", r"target = (\1,\2);"), - ( - r"color = \(\s*(\d+),\s*(\d+),\s*(\d+),\s*(\d+)\s*\);", - r"color = (\1,\2,\3,\4);", - ), - (r"\(\s*(-?[\d.]+),\s*(-?[\d.]+),\s*([a-zA-Z]+)\s*\)", r"(\1,\2,\3)"), - (r"\(\s*(-?[\d.]+),\s*(-?[\d.]+),\s*([a-zA-Z]+),\s*\{", r"(\1,\2,\3,{"), - (r"\}\s*\),", r"}),"), - (r"anchors = \(\);", r"anchors = (\n);"), - (r"unicode = \(\);", r"unicode = (\n);"), - (r"lib = \{\};", r"lib = {\n};"), - ( - r"verticalStems = \(\s*(-?[\d.]+),(-?[\d.]+)\);", - r"verticalStems = (\n\1,\n\2\n);", - ), - ] - - for pattern, replacement in patterns: - content = re.sub(pattern, replacement, content) - - return content +LEAF = object() + + +def patternsToMatchTree(patterns): + tree = {} + for pattern in patterns: + subtree = tree + for item in pattern[:-1]: + if item not in subtree: + subtree[item] = {} + subtree = subtree[item] + subtree[pattern[-1]] = LEAF + return tree + + +def convertMatchesToTuples(obj, matchTree, path=()): + if isinstance(obj, dict): + assert matchTree is not LEAF, path + return { + k: convertMatchesToTuples( + v, matchTree.get(k, matchTree.get(None, {})), path + (k,) + ) + for k, v in obj.items() + } + elif isinstance(obj, list): + convertToTuple = False + if matchTree is LEAF: + convertToTuple = True + matchTree = {} + seq = [ + convertMatchesToTuples(item, matchTree.get(None, {}), path + (i,)) + for i, item in enumerate(obj) + ] + if convertToTuple: + seq = tuple(seq) + return seq + else: + return obj + + +patterns = [ + ["fontMaster", None, "guides", None, "pos"], + ["glyphs", None, "color"], + ["glyphs", None, "layers", None, "anchors", None, "pos"], + ["glyphs", None, "layers", None, "annotations", None, "pos"], + ["glyphs", None, "layers", None, "background", "shapes", None, "nodes", None], + ["glyphs", None, "layers", None, "guides", None, "pos"], + ["glyphs", None, "layers", None, "hints", None, "origin"], + ["glyphs", None, "layers", None, "hints", None, "target"], + ["glyphs", None, "layers", None, "shapes", None, "nodes", None], + ["glyphs", None, "layers", None, "shapes", None, "pos"], +] + + +matchTreeFont = patternsToMatchTree(patterns) +matchTreeGlyph = matchTreeFont["glyphs"][None] diff --git a/tests/test_utils.py b/tests/test_utils.py index aa0bcbd..3337504 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,13 +1,15 @@ import pathlib +import openstep_plist import pytest from fontra.backends import getFileSystemBackend from glyphsLib.classes import GSAxis, GSFont, GSFontMaster, GSGlyph, GSLayer from fontra_glyphs.utils import ( + convertMatchesToTuples, getAssociatedMasterId, getLocationFromSources, - gsFormatting, + matchTreeFont, ) dataDir = pathlib.Path(__file__).resolve().parent / "data" @@ -99,165 +101,23 @@ def test_getAssociatedMasterId(testGSFontWW, gsLocation, expected): assert getAssociatedMasterId(testGSFontWW, gsLocation) == expected -contentSnippets = [ - [ - """pos = ( -524, -141 -);""", - "pos = (524,141);", - ], - [ - """pos = ( --113, -765 -);""", - "pos = (-113,765);", - ], - [ - "customBinaryData = <74686520 62797465 73>;", - "customBinaryData = <746865206279746573>;", - ], - [ - """color = ( -120, -220, -20, -4 -);""", - "color = (120,220,20,4);", - ], - [ - """( -566.99, -700, -l -),""", - "(566.99,700,l),", - ], - [ - """( -191, -700, -l -),""", - "(191,700,l),", - ], - [ - """origin = ( -1, -1 -);""", - "origin = (1,1);", - ], - [ - """target = ( -1, -0 -);""", - "target = (1,0);", - ], - [ - """pos = ( -45, -0 -);""", - "pos = (45,0);", - ], - [ - """pos = ( --45, -0 -);""", - "pos = (-45,0);", - ], - [ - """( -321, -700, -l, -{""", - "(321,700,l,{", - ], - [ - """( -268, -153, -ls -),""", - "(268,153,ls),", - ], - [ - """( -268, -153, -o -),""", - "(268,153,o),", - ], - [ - """( -268, -153, -cs -),""", - "(268,153,cs),", - ], - [ - """( -184, --8, -c -),""", - "(184,-8,c),", - ], - [ - """pos = ( -334.937, -407.08 -);""", - "pos = (334.937,407.08);", - ], - [ - """pos = ( --113, -574 -);""", - "pos = (-113,574);", - ], - ["pos = (524,-122);", "pos = (524,-122);"], - [ - "anchors = ();", - """anchors = ( -);""", - ], - [ - "unicode = ();", - """unicode = ( -);""", - ], - [ - "lib = {};", - """lib = { -};""", - ], - [ - "verticalStems = (17,19);", - """verticalStems = ( -17, -19 -);""", - ], - # TODO: The following does not fail in the unittest: diff \n vs \012 - [ - """code = "feature c2sc; -feature smcp; -";""", - """code = "feature c2sc;\012feature smcp;\012";""", - ], -] - +@pytest.mark.parametrize("path", [glyphs2Path, glyphs3Path]) +def test_roundtrip_glyphs_file_dumps(path): + root = openstep_plist.loads(path.read_text(), use_numbers=True) + result = convertMatchesToTuples(root, matchTreeFont) + + out = ( + openstep_plist.dumps( + result, + unicode_escape=False, + indent=0, + single_line_tuples=True, + escape_newlines=False, + sort_keys=False, + single_line_empty_objects=False, + ) + + "\n" + ) -@pytest.mark.parametrize("content,expected", contentSnippets) -def test_gsFormatting(content, expected): - assert gsFormatting(content) == expected + for root_line, out_line in zip(path.read_text().splitlines(), out.splitlines()): + assert root_line == out_line From a7fbd4526419bf6e0632850c675c76aa73addcd4 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Thu, 23 Jan 2025 15:27:14 +0100 Subject: [PATCH 37/88] Fix customBinaryData formatting with regEx --- src/fontra_glyphs/backend.py | 6 +++++- src/fontra_glyphs/utils.py | 9 +++++++++ tests/test_utils.py | 5 ++++- 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 7362b14..a5e3929 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -41,6 +41,7 @@ from .utils import ( convertMatchesToTuples, + fixCustomBinaryDataFormatting, getAssociatedMasterId, getLocationFromSources, matchTreeFont, @@ -412,7 +413,7 @@ async def putGlyph( def _writeRawGlyph(self, glyphName, f): # 5. write whole file with openstep_plist result = convertMatchesToTuples(self.rawFontData, matchTreeFont) - self.gsFilePath.write_text( + out = ( openstep_plist.dumps( result, unicode_escape=False, @@ -425,6 +426,9 @@ def _writeRawGlyph(self, glyphName, f): + "\n" ) + out = fixCustomBinaryDataFormatting(out) + self.gsFilePath.write_text(out) + async def aclose(self) -> None: pass diff --git a/src/fontra_glyphs/utils.py b/src/fontra_glyphs/utils.py index cf33cf4..5da0011 100644 --- a/src/fontra_glyphs/utils.py +++ b/src/fontra_glyphs/utils.py @@ -1,3 +1,4 @@ +import re from collections import OrderedDict @@ -94,3 +95,11 @@ def convertMatchesToTuples(obj, matchTree, path=()): matchTreeFont = patternsToMatchTree(patterns) matchTreeGlyph = matchTreeFont["glyphs"][None] + + +def fixCustomBinaryDataFormatting(content): + return re.sub( + r"customBinaryData = <\s*([0-9a-fA-F\s]+)\s*>;", + lambda m: f"customBinaryData = <{m.group(1).replace(' ', '')}>;", + content, + ) diff --git a/tests/test_utils.py b/tests/test_utils.py index 3337504..4980fca 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -7,6 +7,7 @@ from fontra_glyphs.utils import ( convertMatchesToTuples, + fixCustomBinaryDataFormatting, getAssociatedMasterId, getLocationFromSources, matchTreeFont, @@ -101,7 +102,7 @@ def test_getAssociatedMasterId(testGSFontWW, gsLocation, expected): assert getAssociatedMasterId(testGSFontWW, gsLocation) == expected -@pytest.mark.parametrize("path", [glyphs2Path, glyphs3Path]) +@pytest.mark.parametrize("path", [glyphs3Path]) def test_roundtrip_glyphs_file_dumps(path): root = openstep_plist.loads(path.read_text(), use_numbers=True) result = convertMatchesToTuples(root, matchTreeFont) @@ -119,5 +120,7 @@ def test_roundtrip_glyphs_file_dumps(path): + "\n" ) + out = fixCustomBinaryDataFormatting(out) + for root_line, out_line in zip(path.read_text().splitlines(), out.splitlines()): assert root_line == out_line From d41b0b202364297404626b593fcaf30e560f2e67 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Thu, 23 Jan 2025 15:45:50 +0100 Subject: [PATCH 38/88] Adding guideline to get- and putGlyph --- README.md | 2 +- src/fontra_glyphs/backend.py | 17 +++++++++++++++++ tests/test_backend.py | 20 ++++++++++++++++++++ 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index b8b8a2e..04cad85 100644 --- a/README.md +++ b/README.md @@ -14,4 +14,4 @@ It supports the following features: - Contour (Paths, Nodes) ✅ - Components ✅ - Anchors ✅ -- Guidelines +- Guidelines ✅ diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index a5e3929..208a998 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -529,6 +529,9 @@ def gsLayerToFontraLayer(gsLayer, globalAxisNames): ] anchors = [gsAnchorToFontraAnchor(gsAnchor) for gsAnchor in gsLayer.anchors] + guidelines = [ + gsGuidelineToFontraGuideline(gsGuideline) for gsGuideline in gsLayer.guides + ] return Layer( glyph=StaticGlyph( @@ -536,6 +539,7 @@ def gsLayerToFontraLayer(gsLayer, globalAxisNames): path=pen.getPath(), components=components, anchors=anchors, + guidelines=guidelines, ) ) @@ -809,6 +813,9 @@ def fontraLayerToGSLayer(layer, gsLayer): fontraComponentToGSComponent(component) for component in layer.glyph.components ] gsLayer.anchors = [fontraAnchorToGSAnchor(anchor) for anchor in layer.glyph.anchors] + gsLayer.guides = [ + fontraGuidelineToGSGuide(guideline) for guideline in layer.glyph.guidelines + ] def fontraComponentToGSComponent(component): @@ -831,3 +838,13 @@ def fontraAnchorToGSAnchor(anchor): # is relative to the LSB (0), center (2) or RSB (1). # Details: https://docu.glyphsapp.com/#GSAnchor.orientation return gsAnchor + + +def fontraGuidelineToGSGuide(guideline): + gsGuide = glyphsLib.classes.GSGuide() + gsGuide.name = guideline.name + gsGuide.position.x = guideline.x + gsGuide.position.y = guideline.y + gsGuide.angle = guideline.angle + gsGuide.locked = guideline.locked + return gsGuide diff --git a/tests/test_backend.py b/tests/test_backend.py index e505ac4..259175b 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -11,6 +11,7 @@ Axes, FontInfo, GlyphSource, + Guideline, Layer, StaticGlyph, structure, @@ -265,6 +266,25 @@ async def test_addAnchor(writableTestFont): ) +async def test_addGuideline(writableTestFont): + glyphName = "a" + glyphMap = await writableTestFont.getGlyphMap() + glyph = await writableTestFont.getGlyph(glyphName) + + layerName = str(uuid.uuid4()).upper() + glyph.layers[layerName] = Layer(glyph=StaticGlyph(xAdvance=0)) + glyph.layers[layerName].glyph.guidelines.append(Guideline(name="top", x=207, y=746)) + + await writableTestFont.putGlyph(glyphName, glyph, glyphMap[glyphName]) + + savedGlyph = await writableTestFont.getGlyph(glyphName) + + assert ( + glyph.layers[layerName].glyph.guidelines + == savedGlyph.layers[layerName].glyph.guidelines + ) + + async def test_getKerning(testFont, referenceFont): assert await testFont.getKerning() == await referenceFont.getKerning() From 2161e6df3d2cf3b4523c4dede9631e61871e7616 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Thu, 23 Jan 2025 17:19:12 +0100 Subject: [PATCH 39/88] Removing toOrderedDict, not necessary anymore because it's done by Just's openstep-plist --- src/fontra_glyphs/backend.py | 5 ++--- src/fontra_glyphs/utils.py | 12 ------------ 2 files changed, 2 insertions(+), 15 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 208a998..74b209f 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -45,7 +45,6 @@ getAssociatedMasterId, getLocationFromSources, matchTreeFont, - toOrderedDict, ) rootInfoNames = [ @@ -115,8 +114,8 @@ def _setupFromPath(self, path: PathLike) -> None: self.gsFont.glyphs = [ glyphsLib.classes.GSGlyph() for i in range(len(rawGlyphsData)) ] - self.rawFontData = toOrderedDict(rawFontData) - self.rawGlyphsData = toOrderedDict(rawGlyphsData) + self.rawFontData = rawFontData + self.rawGlyphsData = rawGlyphsData self.glyphNameToIndex = { glyphData["glyphname"]: i for i, glyphData in enumerate(rawGlyphsData) diff --git a/src/fontra_glyphs/utils.py b/src/fontra_glyphs/utils.py index 5da0011..10513a5 100644 --- a/src/fontra_glyphs/utils.py +++ b/src/fontra_glyphs/utils.py @@ -1,16 +1,4 @@ import re -from collections import OrderedDict - - -# The following is obsolete once this is merged: -# https://github.com/fonttools/openstep-plist/pull/35 -def toOrderedDict(obj): - if isinstance(obj, dict): - return OrderedDict({k: toOrderedDict(v) for k, v in obj.items()}) - elif isinstance(obj, list): - return [toOrderedDict(item) for item in obj] - else: - return obj def getLocationFromSources(sources, layerName): From 33840a32993102d9600db3ded3d858728c4d1695 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Thu, 23 Jan 2025 17:21:30 +0100 Subject: [PATCH 40/88] Updating fontra data, because of missing guideline --- tests/data/GlyphsUnitTestSans3.fontra/glyphs/A^1.json | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/data/GlyphsUnitTestSans3.fontra/glyphs/A^1.json b/tests/data/GlyphsUnitTestSans3.fontra/glyphs/A^1.json index 13dbe9a..d8d9bc3 100644 --- a/tests/data/GlyphsUnitTestSans3.fontra/glyphs/A^1.json +++ b/tests/data/GlyphsUnitTestSans3.fontra/glyphs/A^1.json @@ -276,6 +276,13 @@ "x": 297, "y": 700 } +], +"guidelines": [ +{ +"name": "", +"x": 45, +"angle": 71.7587 +} ] } } From 35683384a9232f8ca540767c14f16bf56ad2c6e9 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Thu, 23 Jan 2025 17:37:18 +0100 Subject: [PATCH 41/88] Raise error when trying to delete a master layer. Remove numbers from comments. --- src/fontra_glyphs/backend.py | 16 +++++++++------- tests/test_backend.py | 11 ++++++----- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 74b209f..e052593 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -384,33 +384,33 @@ async def putGlyph( assert all(isinstance(cp, int) for cp in codePoints) self.glyphMap[glyphName] = codePoints - # 1. convert VariableGlyph to GSGlyph + # Convert VariableGlyph to GSGlyph gsGlyphNew = variableGlyphToGSGlyph( glyph, deepcopy(self.gsFont.glyphs[glyphName]) ) - # 2. serialize to text with glyphsLib.writer.Writer(), using io.StringIO or io.BytesIO + # Serialize to text with glyphsLib.writer.Writer(), using io.StringIO f = io.StringIO() writer = glyphsLib.writer.Writer(f) writer.format_version = self.gsFont.format_version writer.write(gsGlyphNew) - # 3. parse stream into "raw" object + # Parse stream into "raw" object f.seek(0) rawGlyphData = openstep_plist.load(f, use_numbers=True) - # 4. replace original "raw" object with new "raw" object + # Replace original "raw" object with new "raw" object self.rawGlyphsData[self.glyphNameToIndex[glyphName]] = rawGlyphData self.rawFontData["glyphs"] = self.rawGlyphsData self._writeRawGlyph(glyphName, f) - # 7. Remove glyph from parsed glyph names, because we changed it. + # Remove glyph from parsed glyph names, because we changed it. # Next time it needs to be parsed again. self.parsedGlyphNames.discard(glyphName) def _writeRawGlyph(self, glyphName, f): - # 5. write whole file with openstep_plist + # Write whole file with openstep_plist result = convertMatchesToTuples(self.rawFontData, matchTreeFont) out = ( openstep_plist.dumps( @@ -762,7 +762,9 @@ def variableGlyphToGSGlyph(variableGlyph, gsGlyph): continue if gsLayerId in masterIds: # We don't delete master layers. - continue + raise NotImplementedError( + "Deleting a master layer will cause compatibility issues in GlyphsApp." + ) # Removing non-master-layer: del gsGlyph.layers[gsLayerId] diff --git a/tests/test_backend.py b/tests/test_backend.py index 259175b..7685847 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -180,11 +180,12 @@ async def test_deleteLayer(writableTestFont): layerName = "3E7589AA-8194-470F-8E2F-13C1C581BE24" del glyph.layers[layerName] - await writableTestFont.putGlyph(glyphName, glyph, glyphMap[glyphName]) - - savedGlyph = await writableTestFont.getGlyph(glyphName) - - assert layerName in savedGlyph.layers + with pytest.raises(NotImplementedError) as excinfo: + await writableTestFont.putGlyph(glyphName, glyph, glyphMap[glyphName]) + assert ( + str(excinfo.value) + == "Deleting a master layer will cause compatibility issues in GlyphsApp." + ) async def test_addLayer(writableTestFont): From 0d558d82a001fafa04ee66136b51b90fcae73e3a Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Thu, 23 Jan 2025 18:14:55 +0100 Subject: [PATCH 42/88] Update workflow pytest.yml with Just's openstep-plist --- .github/workflows/pytest.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/pytest.yml b/.github/workflows/pytest.yml index 91004d4..9b1719b 100644 --- a/.github/workflows/pytest.yml +++ b/.github/workflows/pytest.yml @@ -30,6 +30,7 @@ jobs: python -m pip install . --no-deps python -m pip install -r requirements.txt python -m pip install -r requirements-dev.txt + pip install --no-deps git+https://github.com/justvanrossum/openstep-plist.git@glyphsapp-compat - name: Run pre-commit uses: pre-commit/action@v3.0.1 From 5a0b8212f1eb0203914536979ee604078c453ebc Mon Sep 17 00:00:00 2001 From: Just van Rossum Date: Thu, 23 Jan 2025 20:51:07 +0100 Subject: [PATCH 43/88] Return copy, as callers may mutate the result. This fixes googlefonts/fontra#1979 --- src/fontra_glyphs/backend.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index e052593..39d3445 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -203,7 +203,7 @@ async def putSources(self, sources: dict[str, FontSource]) -> None: raise NotImplementedError("editing FontSources is not yet implemented") async def getAxes(self) -> Axes: - return Axes(axes=self.axes) + return Axes(axes=deepcopy(self.axes)) async def putAxes(self, axes: Axes) -> None: raise NotImplementedError("editing Axes is not yet implemented") From 4ed0ac533d17cc95364bddcce44c1ebc2f279397 Mon Sep 17 00:00:00 2001 From: Just van Rossum Date: Thu, 23 Jan 2025 22:03:53 +0100 Subject: [PATCH 44/88] Better deepcopy the glyphMap, as clients may mutate --- src/fontra_glyphs/backend.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 39d3445..c67af6b 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -170,7 +170,7 @@ def _loadFiles(path: PathLike) -> tuple[dict[str, Any], list[Any]]: return rawFontData, rawGlyphsData async def getGlyphMap(self) -> dict[str, list[int]]: - return self.glyphMap + return deepcopy(self.glyphMap) async def putGlyphMap(self, value: dict[str, list[int]]) -> None: pass From eb1a70c9d090bc91364ce3e9a60ce1c89de40656 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Tue, 28 Jan 2025 08:41:08 +0100 Subject: [PATCH 45/88] Remove fixCustomBinaryDataFormatting and fix some unittests Justs openstep-plist takes care of binary_spaces, so need of extra function. --- src/fontra_glyphs/backend.py | 3 +-- tests/test_backend.py | 28 ++++++++++++++-------------- tests/test_utils.py | 4 +--- 3 files changed, 16 insertions(+), 19 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index c67af6b..04cf8e5 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -41,7 +41,6 @@ from .utils import ( convertMatchesToTuples, - fixCustomBinaryDataFormatting, getAssociatedMasterId, getLocationFromSources, matchTreeFont, @@ -421,11 +420,11 @@ def _writeRawGlyph(self, glyphName, f): escape_newlines=False, sort_keys=False, single_line_empty_objects=False, + binary_spaces=False, ) + "\n" ) - out = fixCustomBinaryDataFormatting(out) self.gsFilePath.write_text(out) async def aclose(self) -> None: diff --git a/tests/test_backend.py b/tests/test_backend.py index 7685847..a96b916 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -173,19 +173,18 @@ async def test_putGlyph(writableTestFont, testFont, glyphName): async def test_deleteLayer(writableTestFont): - glyphName = "A" + glyphName = "a" glyphMap = await writableTestFont.getGlyphMap() glyph = await writableTestFont.getGlyph(glyphName) + numGlyphLayers = len(glyph.layers) - layerName = "3E7589AA-8194-470F-8E2F-13C1C581BE24" - del glyph.layers[layerName] + # delete intermediate layer + del glyph.layers["1FA54028-AD2E-4209-AA7B-72DF2DF16264"] - with pytest.raises(NotImplementedError) as excinfo: - await writableTestFont.putGlyph(glyphName, glyph, glyphMap[glyphName]) - assert ( - str(excinfo.value) - == "Deleting a master layer will cause compatibility issues in GlyphsApp." - ) + await writableTestFont.putGlyph(glyphName, glyph, glyphMap[glyphName]) + + savedGlyph = await writableTestFont.getGlyph(glyphName) + assert len(savedGlyph.layers) < numGlyphLayers async def test_addLayer(writableTestFont): @@ -238,14 +237,15 @@ async def test_deleteMasterLayer(writableTestFont): glyphName = "a" glyphMap = await writableTestFont.getGlyphMap() glyph = await writableTestFont.getGlyph(glyphName) - numGlyphLayers = len(glyph.layers) del glyph.layers["BFFFD157-90D3-4B85-B99D-9A2F366F03CA"] - await writableTestFont.putGlyph(glyphName, glyph, glyphMap[glyphName]) - - savedGlyph = await writableTestFont.getGlyph(glyphName) - assert len(savedGlyph.layers) == numGlyphLayers + with pytest.raises(NotImplementedError) as excinfo: + await writableTestFont.putGlyph(glyphName, glyph, glyphMap[glyphName]) + assert ( + str(excinfo.value) + == "Deleting a master layer will cause compatibility issues in GlyphsApp." + ) async def test_addAnchor(writableTestFont): diff --git a/tests/test_utils.py b/tests/test_utils.py index 4980fca..4c39242 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -7,7 +7,6 @@ from fontra_glyphs.utils import ( convertMatchesToTuples, - fixCustomBinaryDataFormatting, getAssociatedMasterId, getLocationFromSources, matchTreeFont, @@ -116,11 +115,10 @@ def test_roundtrip_glyphs_file_dumps(path): escape_newlines=False, sort_keys=False, single_line_empty_objects=False, + binary_spaces=False, ) + "\n" ) - out = fixCustomBinaryDataFormatting(out) - for root_line, out_line in zip(path.read_text().splitlines(), out.splitlines()): assert root_line == out_line From e1452c1c7bf057c9bb6740faa28bba0fc1c19171 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Tue, 28 Jan 2025 09:44:32 +0100 Subject: [PATCH 46/88] Don't raise error if some deletes master layer GlyphsApp will add an empty layer if one is missing. --- src/fontra_glyphs/backend.py | 8 +------- tests/test_backend.py | 17 ----------------- 2 files changed, 1 insertion(+), 24 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 04cf8e5..63ed96c 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -754,17 +754,11 @@ def gsVerticalMetricsToFontraLineMetricsHorizontal(gsFont, gsMaster): def variableGlyphToGSGlyph(variableGlyph, gsGlyph): # Convert Fontra variableGlyph to GlyphsApp glyph - masterIds = [m.id for m in gsGlyph.parent.masters] for gsLayerId in [gsLayer.layerId for gsLayer in gsGlyph.layers]: if gsLayerId in variableGlyph.layers: # This layer will be modified later. continue - if gsLayerId in masterIds: - # We don't delete master layers. - raise NotImplementedError( - "Deleting a master layer will cause compatibility issues in GlyphsApp." - ) - # Removing non-master-layer: + # Removing layer: del gsGlyph.layers[gsLayerId] for layerName, layer in iter(variableGlyph.layers.items()): diff --git a/tests/test_backend.py b/tests/test_backend.py index a96b916..7fe87d9 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -231,23 +231,6 @@ async def test_addLayerWithComponent(writableTestFont): assert layerName in savedGlyph.layers.keys() -async def test_deleteMasterLayer(writableTestFont): - # Removing a "master" layer breaks compatibility within a .glyphs file. - # Therefore we need to make sure, that it will be added afterwords. - glyphName = "a" - glyphMap = await writableTestFont.getGlyphMap() - glyph = await writableTestFont.getGlyph(glyphName) - - del glyph.layers["BFFFD157-90D3-4B85-B99D-9A2F366F03CA"] - - with pytest.raises(NotImplementedError) as excinfo: - await writableTestFont.putGlyph(glyphName, glyph, glyphMap[glyphName]) - assert ( - str(excinfo.value) - == "Deleting a master layer will cause compatibility issues in GlyphsApp." - ) - - async def test_addAnchor(writableTestFont): glyphName = "a" glyphMap = await writableTestFont.getGlyphMap() From 1ec4a25b9dda15665828df18d817d1a477e2fcab Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Tue, 28 Jan 2025 09:46:54 +0100 Subject: [PATCH 47/88] Removing fixCustomBinaryDataFormatting --- src/fontra_glyphs/utils.py | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/fontra_glyphs/utils.py b/src/fontra_glyphs/utils.py index 10513a5..60a6397 100644 --- a/src/fontra_glyphs/utils.py +++ b/src/fontra_glyphs/utils.py @@ -1,6 +1,3 @@ -import re - - def getLocationFromSources(sources, layerName): s = sources[0] for source in sources: @@ -83,11 +80,3 @@ def convertMatchesToTuples(obj, matchTree, path=()): matchTreeFont = patternsToMatchTree(patterns) matchTreeGlyph = matchTreeFont["glyphs"][None] - - -def fixCustomBinaryDataFormatting(content): - return re.sub( - r"customBinaryData = <\s*([0-9a-fA-F\s]+)\s*>;", - lambda m: f"customBinaryData = <{m.group(1).replace(' ', '')}>;", - content, - ) From b00dd7603ffdf2418594708e7ae4ead9ee864160 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Fri, 31 Jan 2025 08:54:44 +0100 Subject: [PATCH 48/88] Removing Just's version of openstep-plist because it's part of the official now --- .github/workflows/pytest.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/pytest.yml b/.github/workflows/pytest.yml index 9b1719b..91004d4 100644 --- a/.github/workflows/pytest.yml +++ b/.github/workflows/pytest.yml @@ -30,7 +30,6 @@ jobs: python -m pip install . --no-deps python -m pip install -r requirements.txt python -m pip install -r requirements-dev.txt - pip install --no-deps git+https://github.com/justvanrossum/openstep-plist.git@glyphsapp-compat - name: Run pre-commit uses: pre-commit/action@v3.0.1 From 2f99a3ab5a0e54403f81f7400d3dbd555b3b1be7 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Tue, 4 Feb 2025 17:52:10 +0100 Subject: [PATCH 49/88] Fix issue with fontra source name --- src/fontra_glyphs/backend.py | 8 +++++++- src/fontra_glyphs/utils.py | 6 ++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 63ed96c..b6d5ba1 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -43,6 +43,7 @@ convertMatchesToTuples, getAssociatedMasterId, getLocationFromSources, + getSourceNameWithLayerName, matchTreeFont, ) @@ -289,7 +290,9 @@ async def getGlyph(self, glyphName: str) -> VariableGlyph | None: braceLocation = self._getBraceLayerLocation(gsLayer) smartLocation = self._getSmartLocation(gsLayer, localAxesByName) masterName = self.gsFont.masters[gsLayer.associatedMasterId].name - if braceLocation or smartLocation: + if gsLayer.userData["xyz.fontra.source-name"]: + sourceName = gsLayer.userData["xyz.fontra.source-name"] + elif braceLocation or smartLocation: sourceName = f"{masterName} / {gsLayer.name}" else: sourceName = gsLayer.name or masterName @@ -777,6 +780,9 @@ def variableGlyphToGSGlyph(variableGlyph, gsGlyph): gsLayer.layerId = layerName gsLayer.isSpecialLayer = True + sourceName = getSourceNameWithLayerName(variableGlyph.sources, layerName) + gsLayer.userData["xyz.fontra.source-name"] = sourceName + location = getLocationFromSources(variableGlyph.sources, layerName) gsLocation = [ location[axis.name.lower()] diff --git a/src/fontra_glyphs/utils.py b/src/fontra_glyphs/utils.py index 60a6397..86aeeca 100644 --- a/src/fontra_glyphs/utils.py +++ b/src/fontra_glyphs/utils.py @@ -7,6 +7,12 @@ def getLocationFromSources(sources, layerName): return {k.lower(): v for k, v in s.location.items()} +def getSourceNameWithLayerName(sources, layerName): + for source in sources: + if source.layerName == layerName: + return source.name + + def getAssociatedMasterId(gsFont, gsLocation): # Best guess for associatedMasterId closestMasterID = gsFont.masters[0].id # default first master. From df72ef5d921c773bca15348c875c9c6baa72b08c Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Wed, 5 Feb 2025 10:09:41 +0100 Subject: [PATCH 50/88] Fix round-trip issue with adding non-master-layer --- src/fontra_glyphs/backend.py | 6 +++--- src/fontra_glyphs/utils.py | 2 +- tests/test_backend.py | 10 ++++------ tests/test_utils.py | 2 +- 4 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index b6d5ba1..fcc943e 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -776,7 +776,6 @@ def variableGlyphToGSGlyph(variableGlyph, gsGlyph): # gsLayer does not exist: therefore must be 'isSpecialLayer' # and need to be created as a new layer: gsLayer = glyphsLib.classes.GSLayer() - gsLayer.name = layerName gsLayer.layerId = layerName gsLayer.isSpecialLayer = True @@ -785,11 +784,12 @@ def variableGlyphToGSGlyph(variableGlyph, gsGlyph): location = getLocationFromSources(variableGlyph.sources, layerName) gsLocation = [ - location[axis.name.lower()] + location[axis.name] for axis in gsGlyph.parent.axes - if location.get(axis.name.lower()) + if location.get(axis.name) ] gsLayer.attributes["coordinates"] = gsLocation + gsLayer.name = "{" + ",".join(str(x) for x in gsLocation) + "}" associatedMasterId = getAssociatedMasterId(gsGlyph.parent, gsLocation) if associatedMasterId: diff --git a/src/fontra_glyphs/utils.py b/src/fontra_glyphs/utils.py index 86aeeca..143e4cf 100644 --- a/src/fontra_glyphs/utils.py +++ b/src/fontra_glyphs/utils.py @@ -4,7 +4,7 @@ def getLocationFromSources(sources, layerName): if source.layerName == layerName: s = source break - return {k.lower(): v for k, v in s.location.items()} + return {k: v for k, v in s.location.items()} def getSourceNameWithLayerName(sources, layerName): diff --git a/tests/test_backend.py b/tests/test_backend.py index 7fe87d9..16384d9 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -191,13 +191,12 @@ async def test_addLayer(writableTestFont): glyphName = "a" glyphMap = await writableTestFont.getGlyphMap() glyph = await writableTestFont.getGlyph(glyphName) - numGlyphLayers = len(glyph.layers) layerName = str(uuid.uuid4()).upper() glyph.sources.append( - GlyphSource(name="SemiBold", location={"weight": 166}, layerName=layerName) + GlyphSource(name="SemiBold", location={"Weight": 166}, layerName=layerName) ) - # Copy StaticGlyph of Bold: + # Copy StaticGlyph from Bold: glyph.layers[layerName] = Layer( glyph=deepcopy(glyph.layers["BFFFD157-90D3-4B85-B99D-9A2F366F03CA"].glyph) ) @@ -205,8 +204,7 @@ async def test_addLayer(writableTestFont): await writableTestFont.putGlyph(glyphName, glyph, glyphMap[glyphName]) savedGlyph = await writableTestFont.getGlyph(glyphName) - assert len(savedGlyph.layers) > numGlyphLayers - assert layerName in savedGlyph.layers.keys() + assert glyph == savedGlyph async def test_addLayerWithComponent(writableTestFont): @@ -217,7 +215,7 @@ async def test_addLayerWithComponent(writableTestFont): layerName = str(uuid.uuid4()).upper() glyph.sources.append( - GlyphSource(name="SemiBold", location={"weight": 166}, layerName=layerName) + GlyphSource(name="SemiBold", location={"Weight": 166}, layerName=layerName) ) # Copy StaticGlyph of Bold: glyph.layers[layerName] = Layer( diff --git a/tests/test_utils.py b/tests/test_utils.py index 4c39242..8deda2b 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -79,7 +79,7 @@ async def test_getLocationFromSources(testFont): location = getLocationFromSources( glyph.sources, "1FA54028-AD2E-4209-AA7B-72DF2DF16264" ) - assert location == {"weight": 155} + assert location == {"Weight": 155} expectedAssociatedMasterId = [ From 3d8a990e5145a6b5a796a2aa3b6b2165ebd6e230 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Wed, 5 Feb 2025 11:13:50 +0100 Subject: [PATCH 51/88] Fix round-trip unittest + fix issue with smartcomonent --- src/fontra_glyphs/backend.py | 3 ++- tests/test_backend.py | 17 ++--------------- 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index fcc943e..3b1ae12 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -823,7 +823,8 @@ def fontraComponentToGSComponent(component): transformation = component.transformation.toTransform() if not isinstance(transformation, Transform): gsComponent.transform = Transform(*transformation) - # gsComponent.smartComponentValues = # TODO: see location={ disambiguateLocalAxisName + for axisName in component.location: + gsComponent.smartComponentValues[axisName] = component.location[axisName] return gsComponent diff --git a/tests/test_backend.py b/tests/test_backend.py index 16384d9..714ecfd 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -143,7 +143,7 @@ async def test_getGlyph(testFont, referenceFont, glyphName): @pytest.mark.asyncio @pytest.mark.parametrize("glyphName", list(expectedGlyphMap)) -async def test_putGlyph(writableTestFont, testFont, glyphName): +async def test_putGlyph(writableTestFont, glyphName): glyphMap = await writableTestFont.getGlyphMap() glyph = await writableTestFont.getGlyph(glyphName) @@ -156,20 +156,7 @@ async def test_putGlyph(writableTestFont, testFont, glyphName): await writableTestFont.putGlyph(glyphName, glyph, glyphMap[glyphName]) savedGlyph = await writableTestFont.getGlyph(glyphName) - referenceGlyph = await testFont.getGlyph(glyphName) - - for layerName, layer in iter(referenceGlyph.layers.items()): - assert savedGlyph.layers[layerName].glyph.xAdvance == 500 - - for i, coordinate in enumerate(layer.glyph.path.coordinates): - assert ( - savedGlyph.layers[layerName].glyph.path.coordinates[i] - == coordinate + 10 - ) - - assert len(layer.glyph.path.coordinates) == len( - savedGlyph.layers[layerName].glyph.path.coordinates - ) + assert glyph == savedGlyph async def test_deleteLayer(writableTestFont): From fa207bf822085b40bbf9749e245e3ea0b14be5e5 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Wed, 5 Feb 2025 12:44:54 +0100 Subject: [PATCH 52/88] duplicate glyph and add as new glyph --- src/fontra_glyphs/backend.py | 57 +++++++++++++++++++++++++----------- tests/test_backend.py | 14 +++++++-- 2 files changed, 51 insertions(+), 20 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 3b1ae12..ec2c198 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -386,6 +386,13 @@ async def putGlyph( assert all(isinstance(cp, int) for cp in codePoints) self.glyphMap[glyphName] = codePoints + # Glyph does not exist: create new one. + if not self.gsFont.glyphs[glyphName]: + gsGlyph = glyphsLib.classes.GSGlyph(glyphName) + gsGlyph.unicodes = codePoints + self.gsFont.glyphs.append(gsGlyph) + self.glyphNameToIndex[glyphName] = len(self.gsFont.glyphs) - 1 + # Convert VariableGlyph to GSGlyph gsGlyphNew = variableGlyphToGSGlyph( glyph, deepcopy(self.gsFont.glyphs[glyphName]) @@ -402,7 +409,10 @@ async def putGlyph( rawGlyphData = openstep_plist.load(f, use_numbers=True) # Replace original "raw" object with new "raw" object - self.rawGlyphsData[self.glyphNameToIndex[glyphName]] = rawGlyphData + if len(self.rawGlyphsData) - 1 < self.glyphNameToIndex[glyphName]: + self.rawGlyphsData.append(rawGlyphData) + else: + self.rawGlyphsData[self.glyphNameToIndex[glyphName]] = rawGlyphData self.rawFontData["glyphs"] = self.rawGlyphsData self._writeRawGlyph(glyphName, f) @@ -756,6 +766,7 @@ def gsVerticalMetricsToFontraLineMetricsHorizontal(gsFont, gsMaster): def variableGlyphToGSGlyph(variableGlyph, gsGlyph): + gsMasterIDsMapping = {m.id: m.name for m in gsGlyph.parent.masters} # Convert Fontra variableGlyph to GlyphsApp glyph for gsLayerId in [gsLayer.layerId for gsLayer in gsGlyph.layers]: if gsLayerId in variableGlyph.layers: @@ -770,30 +781,42 @@ def variableGlyphToGSGlyph(variableGlyph, gsGlyph): # otherwise the layer has been newly created within Fontrta. if gsLayer is not None: - # gsLayer exists: modify existing gsLayer + # gsLayer exists – modify existing gsLayer: fontraLayerToGSLayer(layer, gsLayer) else: - # gsLayer does not exist: therefore must be 'isSpecialLayer' - # and need to be created as a new layer: + print("layerName: ", layerName) + # gsLayer does not exist – create new layer: gsLayer = glyphsLib.classes.GSLayer() gsLayer.layerId = layerName - gsLayer.isSpecialLayer = True sourceName = getSourceNameWithLayerName(variableGlyph.sources, layerName) gsLayer.userData["xyz.fontra.source-name"] = sourceName - location = getLocationFromSources(variableGlyph.sources, layerName) - gsLocation = [ - location[axis.name] - for axis in gsGlyph.parent.axes - if location.get(axis.name) - ] - gsLayer.attributes["coordinates"] = gsLocation - gsLayer.name = "{" + ",".join(str(x) for x in gsLocation) + "}" - - associatedMasterId = getAssociatedMasterId(gsGlyph.parent, gsLocation) - if associatedMasterId: - gsLayer.associatedMasterId = associatedMasterId + # Check if is master id and check sourceName + isMaster = any( + [ + True + for k, v in gsMasterIDsMapping.items() + if gsLayer.layerId == k or sourceName == v + ] + ) + if isMaster: + gsLayer.name = sourceName + else: + location = getLocationFromSources(variableGlyph.sources, layerName) + gsLocation = [ + location[axis.name] + for axis in gsGlyph.parent.axes + if location.get(axis.name) + ] + gsLayer.attributes["coordinates"] = gsLocation + + gsLayer.name = "{" + ",".join(str(x) for x in gsLocation) + "}" + gsLayer.isSpecialLayer = True + + associatedMasterId = getAssociatedMasterId(gsGlyph.parent, gsLocation) + if associatedMasterId: + gsLayer.associatedMasterId = associatedMasterId fontraLayerToGSLayer(layer, gsLayer) gsGlyph.layers.append(gsLayer) diff --git a/tests/test_backend.py b/tests/test_backend.py index 714ecfd..3e532f8 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -159,6 +159,16 @@ async def test_putGlyph(writableTestFont, glyphName): assert glyph == savedGlyph +async def test_duplicateGlyph(writableTestFont): + glyphName = "a.ss01" + glyph = deepcopy(await writableTestFont.getGlyph("a")) + glyph.name = glyphName + await writableTestFont.putGlyph(glyphName, glyph, []) + + savedGlyph = await writableTestFont.getGlyph(glyphName) + assert glyph == savedGlyph + + async def test_deleteLayer(writableTestFont): glyphName = "a" glyphMap = await writableTestFont.getGlyphMap() @@ -198,7 +208,6 @@ async def test_addLayerWithComponent(writableTestFont): glyphName = "n" # n is made from components glyphMap = await writableTestFont.getGlyphMap() glyph = await writableTestFont.getGlyph(glyphName) - numGlyphLayers = len(glyph.layers) layerName = str(uuid.uuid4()).upper() glyph.sources.append( @@ -212,8 +221,7 @@ async def test_addLayerWithComponent(writableTestFont): await writableTestFont.putGlyph(glyphName, glyph, glyphMap[glyphName]) savedGlyph = await writableTestFont.getGlyph(glyphName) - assert len(savedGlyph.layers) > numGlyphLayers - assert layerName in savedGlyph.layers.keys() + assert glyph == savedGlyph async def test_addAnchor(writableTestFont): From e0bc5ca3506d7c18ba97989c7d179de74cc8a13a Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Wed, 5 Feb 2025 14:18:16 +0100 Subject: [PATCH 53/88] Create new glyph + unittest --- src/fontra_glyphs/backend.py | 11 +++++------ tests/test_backend.py | 22 ++++++++++++++++++++++ 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index ec2c198..8c09413 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -766,7 +766,7 @@ def gsVerticalMetricsToFontraLineMetricsHorizontal(gsFont, gsMaster): def variableGlyphToGSGlyph(variableGlyph, gsGlyph): - gsMasterIDsMapping = {m.id: m.name for m in gsGlyph.parent.masters} + gsMasterIDsMapping = {m.name: m.id for m in gsGlyph.parent.masters} # Convert Fontra variableGlyph to GlyphsApp glyph for gsLayerId in [gsLayer.layerId for gsLayer in gsGlyph.layers]: if gsLayerId in variableGlyph.layers: @@ -784,25 +784,24 @@ def variableGlyphToGSGlyph(variableGlyph, gsGlyph): # gsLayer exists – modify existing gsLayer: fontraLayerToGSLayer(layer, gsLayer) else: - print("layerName: ", layerName) # gsLayer does not exist – create new layer: gsLayer = glyphsLib.classes.GSLayer() - gsLayer.layerId = layerName sourceName = getSourceNameWithLayerName(variableGlyph.sources, layerName) gsLayer.userData["xyz.fontra.source-name"] = sourceName - # Check if is master id and check sourceName isMaster = any( [ True - for k, v in gsMasterIDsMapping.items() - if gsLayer.layerId == k or sourceName == v + for name, id in gsMasterIDsMapping.items() + if gsLayer.layerId == id or sourceName == name ] ) if isMaster: gsLayer.name = sourceName + gsLayer.layerId = gsMasterIDsMapping[sourceName] else: + gsLayer.layerId = layerName location = getLocationFromSources(variableGlyph.sources, layerName) gsLocation = [ location[axis.name] diff --git a/tests/test_backend.py b/tests/test_backend.py index 3e532f8..eb8fb1c 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -14,6 +14,7 @@ Guideline, Layer, StaticGlyph, + VariableGlyph, structure, ) @@ -169,6 +170,27 @@ async def test_duplicateGlyph(writableTestFont): assert glyph == savedGlyph +async def test_createNewGlyph(writableTestFont): + glyphName = "a.ss02" + glyph = VariableGlyph(name=glyphName) + + # NOTE: The layer name is equal to the layerID, + # and in case of a master layer, it is the masterID + fontSources = await writableTestFont.getSources() + nameToMasterIDMap = {fontSources[ID].name: ID for ID in fontSources} + layerName = nameToMasterIDMap["Regular"] + + glyph.sources.append( + GlyphSource(name="Regular", location={"Weight": 90}, layerName=layerName) + ) + glyph.layers[layerName] = Layer(glyph=StaticGlyph(xAdvance=333)) + + await writableTestFont.putGlyph(glyphName, glyph, []) + + savedGlyph = await writableTestFont.getGlyph(glyphName) + assert glyph == savedGlyph + + async def test_deleteLayer(writableTestFont): glyphName = "a" glyphMap = await writableTestFont.getGlyphMap() From a39e88a2df8873864b9fd6230072ea46448ddb37 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Wed, 5 Feb 2025 16:59:27 +0100 Subject: [PATCH 54/88] figure out if master layer based on location + use 'Default' for unittest --- src/fontra_glyphs/backend.py | 58 ++++++++++++++++++++++-------------- tests/test_backend.py | 11 ++----- 2 files changed, 37 insertions(+), 32 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 8c09413..cb4cdbd 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -28,6 +28,7 @@ ) from fontra.core.path import PackedPathPointPen from fontra.core.protocols import WritableFontBackend +from fontra.core.varutils import makeDenseLocation, makeSparseLocation from fontTools.designspaceLib import DesignSpaceDocument from fontTools.misc.transform import DecomposedTransform from fontTools.ufoLib.filenames import userNameToFileName @@ -160,6 +161,13 @@ def _setupFromPath(self, path: PathLike) -> None: axes.append(axis) self.axes = axes + self.defaultLocation = {} + for axis in self.axes: + self.defaultLocation[axis.name] = next( + (v for k, v in axis.mapping if k == axis.defaultValue), + axis.defaultValue, + ) + @staticmethod def _loadFiles(path: PathLike) -> tuple[dict[str, Any], list[Any]]: with open(path, "r", encoding="utf-8") as fp: @@ -296,10 +304,13 @@ async def getGlyph(self, glyphName: str) -> VariableGlyph | None: sourceName = f"{masterName} / {gsLayer.name}" else: sourceName = gsLayer.name or masterName - layerName = gsLayer.layerId + layerName = gsLayer.userData["xyz.fontra.layer-name"] or gsLayer.layerId location = { - **self.locationByMasterID[gsLayer.associatedMasterId], + **makeSparseLocation( + self.locationByMasterID[gsLayer.associatedMasterId], + self.defaultLocation, + ), **braceLocation, **smartLocation, } @@ -395,7 +406,7 @@ async def putGlyph( # Convert VariableGlyph to GSGlyph gsGlyphNew = variableGlyphToGSGlyph( - glyph, deepcopy(self.gsFont.glyphs[glyphName]) + self.defaultLocation, glyph, deepcopy(self.gsFont.glyphs[glyphName]) ) # Serialize to text with glyphsLib.writer.Writer(), using io.StringIO @@ -765,8 +776,9 @@ def gsVerticalMetricsToFontraLineMetricsHorizontal(gsFont, gsMaster): return lineMetricsHorizontal -def variableGlyphToGSGlyph(variableGlyph, gsGlyph): - gsMasterIDsMapping = {m.name: m.id for m in gsGlyph.parent.masters} +def variableGlyphToGSGlyph(defaultLocation, variableGlyph, gsGlyph): + gsMasterAxesToIdMapping = {tuple(m.axes): m.id for m in gsGlyph.parent.masters} + gsMasterIdToNameMapping = {m.id: m.name for m in gsGlyph.parent.masters} # Convert Fontra variableGlyph to GlyphsApp glyph for gsLayerId in [gsLayer.layerId for gsLayer in gsGlyph.layers]: if gsLayerId in variableGlyph.layers: @@ -787,27 +799,27 @@ def variableGlyphToGSGlyph(variableGlyph, gsGlyph): # gsLayer does not exist – create new layer: gsLayer = glyphsLib.classes.GSLayer() + sourceLocation = getLocationFromSources(variableGlyph.sources, layerName) + location = makeDenseLocation(sourceLocation, defaultLocation) + gsLocation = [ + location[axis.name] + for axis in gsGlyph.parent.axes + if location.get(axis.name) + ] + sourceName = getSourceNameWithLayerName(variableGlyph.sources, layerName) - gsLayer.userData["xyz.fontra.source-name"] = sourceName - - isMaster = any( - [ - True - for name, id in gsMasterIDsMapping.items() - if gsLayer.layerId == id or sourceName == name - ] - ) - if isMaster: - gsLayer.name = sourceName - gsLayer.layerId = gsMasterIDsMapping[sourceName] + masterId = gsMasterAxesToIdMapping.get(tuple(gsLocation)) + if masterId: + gsLayer.name = gsMasterIdToNameMapping.get(masterId) + gsLayer.layerId = masterId + if gsLayer.name != sourceName: + # for example 'default' instead of 'Regular'. + gsLayer.userData["xyz.fontra.source-name"] = sourceName + if gsLayer.name != layerName: + gsLayer.userData["xyz.fontra.layer-name"] = layerName else: gsLayer.layerId = layerName - location = getLocationFromSources(variableGlyph.sources, layerName) - gsLocation = [ - location[axis.name] - for axis in gsGlyph.parent.axes - if location.get(axis.name) - ] + gsLayer.userData["xyz.fontra.source-name"] = sourceName gsLayer.attributes["coordinates"] = gsLocation gsLayer.name = "{" + ",".join(str(x) for x in gsLocation) + "}" diff --git a/tests/test_backend.py b/tests/test_backend.py index eb8fb1c..63587d7 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -174,15 +174,8 @@ async def test_createNewGlyph(writableTestFont): glyphName = "a.ss02" glyph = VariableGlyph(name=glyphName) - # NOTE: The layer name is equal to the layerID, - # and in case of a master layer, it is the masterID - fontSources = await writableTestFont.getSources() - nameToMasterIDMap = {fontSources[ID].name: ID for ID in fontSources} - layerName = nameToMasterIDMap["Regular"] - - glyph.sources.append( - GlyphSource(name="Regular", location={"Weight": 90}, layerName=layerName) - ) + layerName = str(uuid.uuid4()).upper() + glyph.sources.append(GlyphSource(name="Default", location={}, layerName=layerName)) glyph.layers[layerName] = Layer(glyph=StaticGlyph(xAdvance=333)) await writableTestFont.putGlyph(glyphName, glyph, []) From af1e72d79a64b1a8754807dbfed9034d9cbada59 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Wed, 5 Feb 2025 17:48:40 +0100 Subject: [PATCH 55/88] Rework gsLocation --- src/fontra_glyphs/backend.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index cb4cdbd..c5b9fef 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -33,6 +33,7 @@ from fontTools.misc.transform import DecomposedTransform from fontTools.ufoLib.filenames import userNameToFileName from glyphsLib.builder.axes import ( + AxisDefinitionFactory, get_axis_definitions, get_regular_master, to_designspace_axes, @@ -801,11 +802,17 @@ def variableGlyphToGSGlyph(defaultLocation, variableGlyph, gsGlyph): sourceLocation = getLocationFromSources(variableGlyph.sources, layerName) location = makeDenseLocation(sourceLocation, defaultLocation) - gsLocation = [ - location[axis.name] - for axis in gsGlyph.parent.axes - if location.get(axis.name) - ] + gsLocation = [] + for axis in gsGlyph.parent.axes: + if location.get(axis.name): + gsLocation.append(location[axis.name]) + else: + # This 'else' is necessary for GlyphsApp 2 files, only. + # 'Weight' and 'Width' are always there, + # even if there is no axis specified for it. + factory = AxisDefinitionFactory() + axis_def = factory.get(axis.axisTag, axis.name) + gsLocation.append(axis_def.default_user_loc) sourceName = getSourceNameWithLayerName(variableGlyph.sources, layerName) masterId = gsMasterAxesToIdMapping.get(tuple(gsLocation)) From b458ba9e95f709e5b614492bb90bb92c2395d9aa Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Wed, 5 Feb 2025 17:49:27 +0100 Subject: [PATCH 56/88] Update Fontra test data (dense location) --- tests/data/GlyphsUnitTestSans3.fontra/glyphs/A-cy^1.json | 5 +---- tests/data/GlyphsUnitTestSans3.fontra/glyphs/A^1.json | 5 +---- .../GlyphsUnitTestSans3.fontra/glyphs/Adieresis^1.json | 5 +---- tests/data/GlyphsUnitTestSans3.fontra/glyphs/V^1.json | 5 +---- .../GlyphsUnitTestSans3.fontra/glyphs/_part.shoulder.json | 7 +------ .../data/GlyphsUnitTestSans3.fontra/glyphs/_part.stem.json | 6 +----- tests/data/GlyphsUnitTestSans3.fontra/glyphs/a.json | 5 +---- tests/data/GlyphsUnitTestSans3.fontra/glyphs/a.sc.json | 5 +---- .../data/GlyphsUnitTestSans3.fontra/glyphs/adieresis.json | 5 +---- tests/data/GlyphsUnitTestSans3.fontra/glyphs/dieresis.json | 5 +---- tests/data/GlyphsUnitTestSans3.fontra/glyphs/h.json | 5 +---- tests/data/GlyphsUnitTestSans3.fontra/glyphs/m.json | 5 +---- tests/data/GlyphsUnitTestSans3.fontra/glyphs/n.json | 5 +---- 13 files changed, 13 insertions(+), 55 deletions(-) diff --git a/tests/data/GlyphsUnitTestSans3.fontra/glyphs/A-cy^1.json b/tests/data/GlyphsUnitTestSans3.fontra/glyphs/A-cy^1.json index 227f883..14c46f4 100644 --- a/tests/data/GlyphsUnitTestSans3.fontra/glyphs/A-cy^1.json +++ b/tests/data/GlyphsUnitTestSans3.fontra/glyphs/A-cy^1.json @@ -10,10 +10,7 @@ }, { "name": "Regular", -"layerName": "3E7589AA-8194-470F-8E2F-13C1C581BE24", -"location": { -"Weight": 90 -} +"layerName": "3E7589AA-8194-470F-8E2F-13C1C581BE24" }, { "name": "Bold", diff --git a/tests/data/GlyphsUnitTestSans3.fontra/glyphs/A^1.json b/tests/data/GlyphsUnitTestSans3.fontra/glyphs/A^1.json index d8d9bc3..d7317e7 100644 --- a/tests/data/GlyphsUnitTestSans3.fontra/glyphs/A^1.json +++ b/tests/data/GlyphsUnitTestSans3.fontra/glyphs/A^1.json @@ -10,10 +10,7 @@ }, { "name": "Regular", -"layerName": "3E7589AA-8194-470F-8E2F-13C1C581BE24", -"location": { -"Weight": 90 -} +"layerName": "3E7589AA-8194-470F-8E2F-13C1C581BE24" }, { "name": "Bold", diff --git a/tests/data/GlyphsUnitTestSans3.fontra/glyphs/Adieresis^1.json b/tests/data/GlyphsUnitTestSans3.fontra/glyphs/Adieresis^1.json index c8db159..78cbb25 100644 --- a/tests/data/GlyphsUnitTestSans3.fontra/glyphs/Adieresis^1.json +++ b/tests/data/GlyphsUnitTestSans3.fontra/glyphs/Adieresis^1.json @@ -10,10 +10,7 @@ }, { "name": "Regular", -"layerName": "3E7589AA-8194-470F-8E2F-13C1C581BE24", -"location": { -"Weight": 90 -} +"layerName": "3E7589AA-8194-470F-8E2F-13C1C581BE24" }, { "name": "Bold", diff --git a/tests/data/GlyphsUnitTestSans3.fontra/glyphs/V^1.json b/tests/data/GlyphsUnitTestSans3.fontra/glyphs/V^1.json index 8130455..6789b27 100644 --- a/tests/data/GlyphsUnitTestSans3.fontra/glyphs/V^1.json +++ b/tests/data/GlyphsUnitTestSans3.fontra/glyphs/V^1.json @@ -10,10 +10,7 @@ }, { "name": "Regular", -"layerName": "3E7589AA-8194-470F-8E2F-13C1C581BE24", -"location": { -"Weight": 90 -} +"layerName": "3E7589AA-8194-470F-8E2F-13C1C581BE24" }, { "name": "Bold", diff --git a/tests/data/GlyphsUnitTestSans3.fontra/glyphs/_part.shoulder.json b/tests/data/GlyphsUnitTestSans3.fontra/glyphs/_part.shoulder.json index 83aafd6..121cc08 100644 --- a/tests/data/GlyphsUnitTestSans3.fontra/glyphs/_part.shoulder.json +++ b/tests/data/GlyphsUnitTestSans3.fontra/glyphs/_part.shoulder.json @@ -40,16 +40,12 @@ }, { "name": "Regular", -"layerName": "3E7589AA-8194-470F-8E2F-13C1C581BE24", -"location": { -"Weight": 90 -} +"layerName": "3E7589AA-8194-470F-8E2F-13C1C581BE24" }, { "name": "Regular / NarrowShoulder", "layerName": "595FDB8C-ED41-486A-B76A-0FEFEF8BCDD1", "location": { -"Weight": 90, "shoulderWidth": 0 } }, @@ -57,7 +53,6 @@ "name": "Regular / LowCrotch", "layerName": "65575EEB-523C-4A39-985D-FB9ACFE951AF", "location": { -"Weight": 90, "crotchDepth": -100 } }, diff --git a/tests/data/GlyphsUnitTestSans3.fontra/glyphs/_part.stem.json b/tests/data/GlyphsUnitTestSans3.fontra/glyphs/_part.stem.json index 09b740e..fea7246 100644 --- a/tests/data/GlyphsUnitTestSans3.fontra/glyphs/_part.stem.json +++ b/tests/data/GlyphsUnitTestSans3.fontra/glyphs/_part.stem.json @@ -26,16 +26,12 @@ }, { "name": "Regular", -"layerName": "3E7589AA-8194-470F-8E2F-13C1C581BE24", -"location": { -"Weight": 90 -} +"layerName": "3E7589AA-8194-470F-8E2F-13C1C581BE24" }, { "name": "Regular / TallStem", "layerName": "3E1733D9-3B83-4E6A-B1E9-6381BBE1BD3A", "location": { -"Weight": 90, "height": 100 } }, diff --git a/tests/data/GlyphsUnitTestSans3.fontra/glyphs/a.json b/tests/data/GlyphsUnitTestSans3.fontra/glyphs/a.json index cce9993..012498d 100644 --- a/tests/data/GlyphsUnitTestSans3.fontra/glyphs/a.json +++ b/tests/data/GlyphsUnitTestSans3.fontra/glyphs/a.json @@ -10,10 +10,7 @@ }, { "name": "Regular", -"layerName": "3E7589AA-8194-470F-8E2F-13C1C581BE24", -"location": { -"Weight": 90 -} +"layerName": "3E7589AA-8194-470F-8E2F-13C1C581BE24" }, { "name": "Regular / {155, 100}", diff --git a/tests/data/GlyphsUnitTestSans3.fontra/glyphs/a.sc.json b/tests/data/GlyphsUnitTestSans3.fontra/glyphs/a.sc.json index 3dd18fd..5328312 100644 --- a/tests/data/GlyphsUnitTestSans3.fontra/glyphs/a.sc.json +++ b/tests/data/GlyphsUnitTestSans3.fontra/glyphs/a.sc.json @@ -10,10 +10,7 @@ }, { "name": "Regular", -"layerName": "3E7589AA-8194-470F-8E2F-13C1C581BE24", -"location": { -"Weight": 90 -} +"layerName": "3E7589AA-8194-470F-8E2F-13C1C581BE24" }, { "name": "Bold", diff --git a/tests/data/GlyphsUnitTestSans3.fontra/glyphs/adieresis.json b/tests/data/GlyphsUnitTestSans3.fontra/glyphs/adieresis.json index e61514e..f4709a3 100644 --- a/tests/data/GlyphsUnitTestSans3.fontra/glyphs/adieresis.json +++ b/tests/data/GlyphsUnitTestSans3.fontra/glyphs/adieresis.json @@ -10,10 +10,7 @@ }, { "name": "Regular", -"layerName": "3E7589AA-8194-470F-8E2F-13C1C581BE24", -"location": { -"Weight": 90 -} +"layerName": "3E7589AA-8194-470F-8E2F-13C1C581BE24" }, { "name": "Bold", diff --git a/tests/data/GlyphsUnitTestSans3.fontra/glyphs/dieresis.json b/tests/data/GlyphsUnitTestSans3.fontra/glyphs/dieresis.json index ab0115b..d1cc6e3 100644 --- a/tests/data/GlyphsUnitTestSans3.fontra/glyphs/dieresis.json +++ b/tests/data/GlyphsUnitTestSans3.fontra/glyphs/dieresis.json @@ -10,10 +10,7 @@ }, { "name": "Regular", -"layerName": "3E7589AA-8194-470F-8E2F-13C1C581BE24", -"location": { -"Weight": 90 -} +"layerName": "3E7589AA-8194-470F-8E2F-13C1C581BE24" }, { "name": "Bold", diff --git a/tests/data/GlyphsUnitTestSans3.fontra/glyphs/h.json b/tests/data/GlyphsUnitTestSans3.fontra/glyphs/h.json index e8925e4..32aecad 100644 --- a/tests/data/GlyphsUnitTestSans3.fontra/glyphs/h.json +++ b/tests/data/GlyphsUnitTestSans3.fontra/glyphs/h.json @@ -10,10 +10,7 @@ }, { "name": "Regular", -"layerName": "3E7589AA-8194-470F-8E2F-13C1C581BE24", -"location": { -"Weight": 90 -} +"layerName": "3E7589AA-8194-470F-8E2F-13C1C581BE24" }, { "name": "Bold", diff --git a/tests/data/GlyphsUnitTestSans3.fontra/glyphs/m.json b/tests/data/GlyphsUnitTestSans3.fontra/glyphs/m.json index 3cca04a..c308fa5 100644 --- a/tests/data/GlyphsUnitTestSans3.fontra/glyphs/m.json +++ b/tests/data/GlyphsUnitTestSans3.fontra/glyphs/m.json @@ -10,10 +10,7 @@ }, { "name": "Regular", -"layerName": "3E7589AA-8194-470F-8E2F-13C1C581BE24", -"location": { -"Weight": 90 -} +"layerName": "3E7589AA-8194-470F-8E2F-13C1C581BE24" }, { "name": "Bold", diff --git a/tests/data/GlyphsUnitTestSans3.fontra/glyphs/n.json b/tests/data/GlyphsUnitTestSans3.fontra/glyphs/n.json index 68b738e..ca3a2a8 100644 --- a/tests/data/GlyphsUnitTestSans3.fontra/glyphs/n.json +++ b/tests/data/GlyphsUnitTestSans3.fontra/glyphs/n.json @@ -10,10 +10,7 @@ }, { "name": "Regular", -"layerName": "3E7589AA-8194-470F-8E2F-13C1C581BE24", -"location": { -"Weight": 90 -} +"layerName": "3E7589AA-8194-470F-8E2F-13C1C581BE24" }, { "name": "Bold", From 36127f9f4c75042e090542a6eb5273bf7baf8190 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Wed, 5 Feb 2025 17:58:15 +0100 Subject: [PATCH 57/88] Update getLocationFromSources --- src/fontra_glyphs/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fontra_glyphs/utils.py b/src/fontra_glyphs/utils.py index 143e4cf..5617682 100644 --- a/src/fontra_glyphs/utils.py +++ b/src/fontra_glyphs/utils.py @@ -4,7 +4,7 @@ def getLocationFromSources(sources, layerName): if source.layerName == layerName: s = source break - return {k: v for k, v in s.location.items()} + return s.location def getSourceNameWithLayerName(sources, layerName): From 43ebf28e273ef6da27bd9ccc08a47affd809f774 Mon Sep 17 00:00:00 2001 From: Just van Rossum Date: Wed, 5 Feb 2025 18:18:13 +0100 Subject: [PATCH 58/88] Fix typing --- src/fontra_glyphs/backend.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index c5b9fef..31baeb0 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -146,6 +146,7 @@ def _setupFromPath(self, path: PathLike) -> None: self.gsFont.format_version, ) + axis: FontAxis | DiscreteFontAxis axes: list[FontAxis | DiscreteFontAxis] = [] for dsAxis in dsAxes: axis = FontAxis( From b4b18456b55c798bf4f56c8b98db541af6335a55 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Thu, 13 Feb 2025 16:53:04 +0100 Subject: [PATCH 59/88] Add NotImplementedError: Intermediate layer within a smart glyph is not yet implemented --- src/fontra_glyphs/backend.py | 41 +++++++++++++++++++++++++++++++++--- src/fontra_glyphs/utils.py | 15 +++++++++++++ tests/test_utils.py | 38 +++++++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 3 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 31baeb0..d304ce9 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -47,6 +47,7 @@ getLocationFromSources, getSourceNameWithLayerName, matchTreeFont, + splitLocation, ) rootInfoNames = [ @@ -779,6 +780,7 @@ def gsVerticalMetricsToFontraLineMetricsHorizontal(gsFont, gsMaster): def variableGlyphToGSGlyph(defaultLocation, variableGlyph, gsGlyph): + defaultGlyphLocation = {axis.name: axis.defaultValue for axis in variableGlyph.axes} gsMasterAxesToIdMapping = {tuple(m.axes): m.id for m in gsGlyph.parent.masters} gsMasterIdToNameMapping = {m.id: m.name for m in gsGlyph.parent.masters} # Convert Fontra variableGlyph to GlyphsApp glyph @@ -802,11 +804,16 @@ def variableGlyphToGSGlyph(defaultLocation, variableGlyph, gsGlyph): gsLayer = glyphsLib.classes.GSLayer() sourceLocation = getLocationFromSources(variableGlyph.sources, layerName) - location = makeDenseLocation(sourceLocation, defaultLocation) + fontLocation, glyphLocation = splitLocation( + sourceLocation, variableGlyph.axes + ) + fontLocation = makeDenseLocation(fontLocation, defaultLocation) + glyphLocation = makeDenseLocation(glyphLocation, defaultGlyphLocation) + gsLocation = [] for axis in gsGlyph.parent.axes: - if location.get(axis.name): - gsLocation.append(location[axis.name]) + if fontLocation.get(axis.name): + gsLocation.append(fontLocation[axis.name]) else: # This 'else' is necessary for GlyphsApp 2 files, only. # 'Weight' and 'Width' are always there, @@ -837,6 +844,34 @@ def variableGlyphToGSGlyph(defaultLocation, variableGlyph, gsGlyph): if associatedMasterId: gsLayer.associatedMasterId = associatedMasterId + if glyphLocation: + # We have a smart component. Check if it is an intermediate master/layer, + # because we currently do not support writing this to GlyphsApp files. + isIntermediateLayer = False + + if not masterId: + # If it has glyph axes and is not on any master location, + # it must be an intermediate master. + isIntermediateLayer = True + else: + # If it has glyph axes and is on a master location, + # but any of the glyph axes are not at min or max position, + # is must be an intermediate layer. + if any( + [ + True + for axis in variableGlyph.axes + if glyphLocation[axis.name] + not in [axis.minValue, axis.maxValue] + ] + ): + isIntermediateLayer = True + + if isIntermediateLayer: + raise NotImplementedError( + "Intermediate layer within a smart glyph is not yet implemented" + ) + fontraLayerToGSLayer(layer, gsLayer) gsGlyph.layers.append(gsLayer) diff --git a/src/fontra_glyphs/utils.py b/src/fontra_glyphs/utils.py index 5617682..ee6fb62 100644 --- a/src/fontra_glyphs/utils.py +++ b/src/fontra_glyphs/utils.py @@ -7,6 +7,21 @@ def getLocationFromSources(sources, layerName): return s.location +def splitLocation(location, glyphAxes): + glyphAxisNames = {axis.name for axis in glyphAxes} + + fontLocation = {} + glyphLocation = {} + + for axisName, axisValue in location.items(): + if axisName in glyphAxisNames: + glyphLocation[axisName] = axisValue + else: + fontLocation[axisName] = axisValue + + return fontLocation, glyphLocation + + def getSourceNameWithLayerName(sources, layerName): for source in sources: if source.layerName == layerName: diff --git a/tests/test_utils.py b/tests/test_utils.py index 8deda2b..8fb11cf 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -3,6 +3,7 @@ import openstep_plist import pytest from fontra.backends import getFileSystemBackend +from fontra.core.varutils import makeDenseLocation from glyphsLib.classes import GSAxis, GSFont, GSFontMaster, GSGlyph, GSLayer from fontra_glyphs.utils import ( @@ -10,6 +11,7 @@ getAssociatedMasterId, getLocationFromSources, matchTreeFont, + splitLocation, ) dataDir = pathlib.Path(__file__).resolve().parent / "data" @@ -82,6 +84,42 @@ async def test_getLocationFromSources(testFont): assert location == {"Weight": 155} +expectedLocations = [ + # gsLayerId, expectedFontLocation, expectedGlyphLocation + [ + "C4872ECA-A3A9-40AB-960A-1DB2202F16DE", + {"Weight": 17}, + {"crotchDepth": 0, "shoulderWidth": 100}, + ], + [ + "7C8F98EE-D140-44D5-86AE-E00A730464C0", + {"Weight": 17}, + {"crotchDepth": -100, "shoulderWidth": 100}, + ], + [ + "BA4F7DF9-9552-48BB-A5B8-E2D21D8D086E", + {"Weight": 220}, + {"crotchDepth": -100, "shoulderWidth": 100}, + ], +] + + +@pytest.mark.parametrize( + "gsLayerId,expectedFontLocation,expectedGlyphLocation", expectedLocations +) +async def test_splitLocation( + testFont, gsLayerId, expectedFontLocation, expectedGlyphLocation +): + glyph = await testFont.getGlyph("_part.shoulder") + location = getLocationFromSources(glyph.sources, gsLayerId) + fontLocation, glyphLocation = splitLocation(location, glyph.axes) + glyphLocation = makeDenseLocation( + glyphLocation, {axis.name: axis.defaultValue for axis in glyph.axes} + ) + assert fontLocation == expectedFontLocation + assert glyphLocation == expectedGlyphLocation + + expectedAssociatedMasterId = [ # gsLocation, associatedMasterId [[14, 155, 900], "MasterID-TextWideBold"], From ef5263b51f9727ab1a033ec5623ed92e3e7d0dfd Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Tue, 18 Feb 2025 13:23:47 +0100 Subject: [PATCH 60/88] Round-trip alignment of gsComponent --- src/fontra_glyphs/backend.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index d304ce9..97e3872 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -578,6 +578,12 @@ def gsComponentToFontraComponent(gsComponent, gsLayer, globalAxisNames): for name, value in gsComponent.smartComponentValues.items() }, ) + if gsComponent.alignment: + # The aligment can be 0, but in that case, do not set it. + # See: https://github.com/googlefonts/glyphsLib/blob/c4db6b981d577f456d64ebe9993818770e170454/Lib/glyphsLib/builder/components.py#L88 # noqa: E501 + component.customData["com.glyphsapp.component.alignment"] = ( + gsComponent.alignment + ) return component @@ -902,6 +908,10 @@ def fontraComponentToGSComponent(component): gsComponent.transform = Transform(*transformation) for axisName in component.location: gsComponent.smartComponentValues[axisName] = component.location[axisName] + if "com.glyphsapp.component.alignment" in component.customData: + gsComponent.alignment = component.customData[ + "com.glyphsapp.component.alignment" + ] return gsComponent From 37b3c56ce868d3763990019455ba2dafc6d691ee Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Tue, 18 Feb 2025 13:25:00 +0100 Subject: [PATCH 61/88] Update font data and unittest --- .../GlyphsUnitTestSans3.fontra/glyphs/h.json | 18 ++++++++++ .../GlyphsUnitTestSans3.fontra/glyphs/m.json | 33 +++++++++++++++++-- .../GlyphsUnitTestSans3.fontra/glyphs/n.json | 30 +++++++++++++---- tests/test_backend.py | 11 +++++++ 4 files changed, 83 insertions(+), 9 deletions(-) diff --git a/tests/data/GlyphsUnitTestSans3.fontra/glyphs/h.json b/tests/data/GlyphsUnitTestSans3.fontra/glyphs/h.json index 32aecad..1e34b8b 100644 --- a/tests/data/GlyphsUnitTestSans3.fontra/glyphs/h.json +++ b/tests/data/GlyphsUnitTestSans3.fontra/glyphs/h.json @@ -28,12 +28,18 @@ "name": "_part.stem", "location": { "height": 100 +}, +"customData": { +"com.glyphsapp.component.alignment": -1 } }, { "name": "_part.shoulder", "location": { "crotchDepth": -80.20097 +}, +"customData": { +"com.glyphsapp.component.alignment": -1 } } ], @@ -47,12 +53,18 @@ "name": "_part.stem", "location": { "height": 100 +}, +"customData": { +"com.glyphsapp.component.alignment": -1 } }, { "name": "_part.shoulder", "location": { "crotchDepth": -80.20097 +}, +"customData": { +"com.glyphsapp.component.alignment": -1 } } ], @@ -66,12 +78,18 @@ "name": "_part.stem", "location": { "height": 100 +}, +"customData": { +"com.glyphsapp.component.alignment": -1 } }, { "name": "_part.shoulder", "location": { "crotchDepth": -80.20097 +}, +"customData": { +"com.glyphsapp.component.alignment": -1 } } ], diff --git a/tests/data/GlyphsUnitTestSans3.fontra/glyphs/m.json b/tests/data/GlyphsUnitTestSans3.fontra/glyphs/m.json index c308fa5..7b0e5e8 100644 --- a/tests/data/GlyphsUnitTestSans3.fontra/glyphs/m.json +++ b/tests/data/GlyphsUnitTestSans3.fontra/glyphs/m.json @@ -25,12 +25,18 @@ "glyph": { "components": [ { -"name": "_part.stem" +"name": "_part.stem", +"customData": { +"com.glyphsapp.component.alignment": -1 +} }, { "name": "_part.shoulder", "location": { "shoulderWidth": 0 +}, +"customData": { +"com.glyphsapp.component.alignment": -1 } }, { @@ -40,6 +46,9 @@ }, "location": { "shoulderWidth": 0 +}, +"customData": { +"com.glyphsapp.component.alignment": -1 } } ], @@ -50,12 +59,18 @@ "glyph": { "components": [ { -"name": "_part.stem" +"name": "_part.stem", +"customData": { +"com.glyphsapp.component.alignment": -1 +} }, { "name": "_part.shoulder", "location": { "shoulderWidth": 0 +}, +"customData": { +"com.glyphsapp.component.alignment": -1 } }, { @@ -65,6 +80,9 @@ }, "location": { "shoulderWidth": 0 +}, +"customData": { +"com.glyphsapp.component.alignment": -1 } } ], @@ -75,12 +93,18 @@ "glyph": { "components": [ { -"name": "_part.stem" +"name": "_part.stem", +"customData": { +"com.glyphsapp.component.alignment": -1 +} }, { "name": "_part.shoulder", "location": { "shoulderWidth": 0 +}, +"customData": { +"com.glyphsapp.component.alignment": -1 } }, { @@ -90,6 +114,9 @@ }, "location": { "shoulderWidth": 0 +}, +"customData": { +"com.glyphsapp.component.alignment": -1 } } ], diff --git a/tests/data/GlyphsUnitTestSans3.fontra/glyphs/n.json b/tests/data/GlyphsUnitTestSans3.fontra/glyphs/n.json index ca3a2a8..0ded153 100644 --- a/tests/data/GlyphsUnitTestSans3.fontra/glyphs/n.json +++ b/tests/data/GlyphsUnitTestSans3.fontra/glyphs/n.json @@ -25,10 +25,16 @@ "glyph": { "components": [ { -"name": "_part.shoulder" +"name": "_part.shoulder", +"customData": { +"com.glyphsapp.component.alignment": -1 +} }, { -"name": "_part.stem" +"name": "_part.stem", +"customData": { +"com.glyphsapp.component.alignment": -1 +} } ], "xAdvance": 528 @@ -38,10 +44,16 @@ "glyph": { "components": [ { -"name": "_part.shoulder" +"name": "_part.shoulder", +"customData": { +"com.glyphsapp.component.alignment": -1 +} }, { -"name": "_part.stem" +"name": "_part.stem", +"customData": { +"com.glyphsapp.component.alignment": -1 +} } ], "xAdvance": 560 @@ -51,10 +63,16 @@ "glyph": { "components": [ { -"name": "_part.shoulder" +"name": "_part.shoulder", +"customData": { +"com.glyphsapp.component.alignment": -1 +} }, { -"name": "_part.stem" +"name": "_part.stem", +"customData": { +"com.glyphsapp.component.alignment": -1 +} } ], "xAdvance": 501 diff --git a/tests/test_backend.py b/tests/test_backend.py index 63587d7..2b6cc18 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -138,6 +138,17 @@ async def test_getGlyph(testFont, referenceFont, glyphName): # so let's monkeypatch the data glyph.customData["com.glyphsapp.glyph-color"] = [120, 220, 20, 4] + if ( + glyphName in ["h", "m", "n"] + and "com.glyphsapp.glyph-color" not in glyph.customData + ): + # glyphsLib doesn't read the color alignment from Glyphs-2 files, + # so let's monkeypatch the data + for layerName in glyph.layers: + for component in glyph.layers[layerName].glyph.components: + if "com.glyphsapp.component.alignment" not in component.customData: + component.customData["com.glyphsapp.component.alignment"] = -1 + referenceGlyph = await referenceFont.getGlyph(glyphName) assert referenceGlyph == glyph From 3341bda69faf77380cdd4b49a7cc51989ce5cb9c Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Tue, 18 Feb 2025 13:29:54 +0100 Subject: [PATCH 62/88] Update comment --- tests/test_backend.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_backend.py b/tests/test_backend.py index 2b6cc18..f262e1f 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -142,7 +142,7 @@ async def test_getGlyph(testFont, referenceFont, glyphName): glyphName in ["h", "m", "n"] and "com.glyphsapp.glyph-color" not in glyph.customData ): - # glyphsLib doesn't read the color alignment from Glyphs-2 files, + # glyphsLib doesn't read the component alignment from Glyphs-2 files, # so let's monkeypatch the data for layerName in glyph.layers: for component in glyph.layers[layerName].glyph.components: From b2e08a1514380d5f9b056668d2a80c099b9fbe2b Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Wed, 26 Feb 2025 10:53:13 +0100 Subject: [PATCH 63/88] Adjust formatting of README.md --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 04cad85..919492e 100644 --- a/README.md +++ b/README.md @@ -8,9 +8,9 @@ It supports the following features: - Smart components (for now restricted to interpolation: axis values need to be within the minimum and maximum values) -## Write +### Writing is currently limited to ... -### Glyph Layer +#### Glyph Layer - Contour (Paths, Nodes) ✅ - Components ✅ - Anchors ✅ From 7b96b890b2159eaefcb0b4006006cf7ae7195843 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Wed, 26 Feb 2025 10:55:15 +0100 Subject: [PATCH 64/88] Removing unnecessary reference --- src/fontra_glyphs/backend.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 97e3872..65e1909 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -580,7 +580,6 @@ def gsComponentToFontraComponent(gsComponent, gsLayer, globalAxisNames): ) if gsComponent.alignment: # The aligment can be 0, but in that case, do not set it. - # See: https://github.com/googlefonts/glyphsLib/blob/c4db6b981d577f456d64ebe9993818770e170454/Lib/glyphsLib/builder/components.py#L88 # noqa: E501 component.customData["com.glyphsapp.component.alignment"] = ( gsComponent.alignment ) From 919f2dccc67491e309b0efec882ca1d7e2f65486 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Wed, 26 Feb 2025 10:59:00 +0100 Subject: [PATCH 65/88] Code improvement 'gsComponent.alignment': use .get() + default to 0 --- src/fontra_glyphs/backend.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 65e1909..d50b9e7 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -907,10 +907,9 @@ def fontraComponentToGSComponent(component): gsComponent.transform = Transform(*transformation) for axisName in component.location: gsComponent.smartComponentValues[axisName] = component.location[axisName] - if "com.glyphsapp.component.alignment" in component.customData: - gsComponent.alignment = component.customData[ - "com.glyphsapp.component.alignment" - ] + gsComponent.alignment = component.customData.get( + "com.glyphsapp.component.alignment", 0 + ) return gsComponent From 8cc61e9c7f8e7443c02e3e7047b7077184665363 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Wed, 26 Feb 2025 11:14:06 +0100 Subject: [PATCH 66/88] Fixing typo --- src/fontra_glyphs/backend.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index d50b9e7..1aad94d 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -799,7 +799,7 @@ def variableGlyphToGSGlyph(defaultLocation, variableGlyph, gsGlyph): for layerName, layer in iter(variableGlyph.layers.items()): gsLayer = gsGlyph.layers[layerName] # layerName is equal to gsLayer.layerId if it comes from Glyphsapp, - # otherwise the layer has been newly created within Fontrta. + # otherwise the layer has been newly created within Fontra. if gsLayer is not None: # gsLayer exists – modify existing gsLayer: From aaf6f83d44dfafd7bae5120d3041a2a7ceba953c Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Thu, 13 Feb 2025 15:15:45 +0100 Subject: [PATCH 67/88] Support smart component with intermediate layer --- src/fontra_glyphs/backend.py | 55 ++++++++++++++++++++++++++++++------ 1 file changed, 47 insertions(+), 8 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 1aad94d..199f408 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -372,6 +372,11 @@ def _ensureGlyphIsParsed(self, glyphName: str) -> None: def _getBraceLayerLocation(self, gsLayer): if not gsLayer._is_brace_layer(): return {} + if gsLayer.smartComponentPoleMapping: + # It's a intermediate layer within a smart component, if it has + # _brace_coordinates and smartComponentPoleMapping, + # We need to skip this and get the valid data via _getSmartLocation + return {} return dict( (axis.name, value) @@ -379,13 +384,21 @@ def _getBraceLayerLocation(self, gsLayer): ) def _getSmartLocation(self, gsLayer, localAxesByName): + # If has _brace_coordinates: it is an intermidiate layer in a smart component + coords = gsLayer._brace_coordinates() or [] location = { name: ( - localAxesByName[name].minValue - if poleValue == Pole.MIN - else localAxesByName[name].maxValue + coords[i] + if len(coords) - 1 >= i + else ( + localAxesByName[name].minValue + if poleValue == Pole.MIN + else localAxesByName[name].maxValue + ) + ) + for i, (name, poleValue) in enumerate( + gsLayer.smartComponentPoleMapping.items() ) - for name, poleValue in gsLayer.smartComponentPoleMapping.items() } return { disambiguateLocalAxisName(name, self.axisNames): value @@ -796,6 +809,15 @@ def variableGlyphToGSGlyph(defaultLocation, variableGlyph, gsGlyph): # Removing layer: del gsGlyph.layers[gsLayerId] + # prepare smart component glyph + if variableGlyph.axes and not gsGlyph.smartComponentAxes: + for axis in variableGlyph.axes: + gsAxis = glyphsLib.classes.GSSmartComponentAxis() + gsAxis.name = axis.name + gsAxis.bottomValue = axis.minValue + gsAxis.topValue = axis.maxValue + gsGlyph.smartComponentAxes.append(gsAxis) + for layerName, layer in iter(variableGlyph.layers.items()): gsLayer = gsGlyph.layers[layerName] # layerName is equal to gsLayer.layerId if it comes from Glyphsapp, @@ -807,6 +829,7 @@ def variableGlyphToGSGlyph(defaultLocation, variableGlyph, gsGlyph): else: # gsLayer does not exist – create new layer: gsLayer = glyphsLib.classes.GSLayer() + gsLayer.parent = gsGlyph sourceLocation = getLocationFromSources(variableGlyph.sources, layerName) fontLocation, glyphLocation = splitLocation( @@ -825,11 +848,24 @@ def variableGlyphToGSGlyph(defaultLocation, variableGlyph, gsGlyph): # even if there is no axis specified for it. factory = AxisDefinitionFactory() axis_def = factory.get(axis.axisTag, axis.name) - gsLocation.append(axis_def.default_user_loc) + gsFontLocation.append(axis_def.default_user_loc) + + gsGlyphLocation = [] + for axis in gsGlyph.smartComponentAxes: + if axis.name not in glyphLocation: + # This might be the case if we create a new glyph axis in Fontra + continue + gsGlyphLocation.append(glyphLocation[axis.name]) + pole = ( + Pole.MIN + if axis.bottomValue == glyphLocation[axis.name] + else Pole.MAX + ) + gsLayer.smartComponentPoleMapping[axis.name] = pole sourceName = getSourceNameWithLayerName(variableGlyph.sources, layerName) - masterId = gsMasterAxesToIdMapping.get(tuple(gsLocation)) - if masterId: + masterId = gsMasterAxesToIdMapping.get(tuple(gsFontLocation)) + if masterId and not gsGlyphLocation: gsLayer.name = gsMasterIdToNameMapping.get(masterId) gsLayer.layerId = masterId if gsLayer.name != sourceName: @@ -838,6 +874,7 @@ def variableGlyphToGSGlyph(defaultLocation, variableGlyph, gsGlyph): if gsLayer.name != layerName: gsLayer.userData["xyz.fontra.layer-name"] = layerName else: + gsLocation = gsGlyphLocation or gsFontLocation gsLayer.layerId = layerName gsLayer.userData["xyz.fontra.source-name"] = sourceName gsLayer.attributes["coordinates"] = gsLocation @@ -845,7 +882,9 @@ def variableGlyphToGSGlyph(defaultLocation, variableGlyph, gsGlyph): gsLayer.name = "{" + ",".join(str(x) for x in gsLocation) + "}" gsLayer.isSpecialLayer = True - associatedMasterId = getAssociatedMasterId(gsGlyph.parent, gsLocation) + associatedMasterId = getAssociatedMasterId( + gsGlyph.parent, gsFontLocation + ) if associatedMasterId: gsLayer.associatedMasterId = associatedMasterId From aa2e6a153228e4cc438ff4c023af25f6ae0db298 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Wed, 26 Feb 2025 16:41:11 +0100 Subject: [PATCH 68/88] Fix merge conflict issue --- src/fontra_glyphs/backend.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 199f408..b428047 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -838,10 +838,10 @@ def variableGlyphToGSGlyph(defaultLocation, variableGlyph, gsGlyph): fontLocation = makeDenseLocation(fontLocation, defaultLocation) glyphLocation = makeDenseLocation(glyphLocation, defaultGlyphLocation) - gsLocation = [] + gsFontLocation = [] for axis in gsGlyph.parent.axes: if fontLocation.get(axis.name): - gsLocation.append(fontLocation[axis.name]) + gsFontLocation.append(fontLocation[axis.name]) else: # This 'else' is necessary for GlyphsApp 2 files, only. # 'Weight' and 'Width' are always there, From acd2c3826341956ad55cd13ba3433896679ad8a4 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Wed, 26 Feb 2025 16:48:15 +0100 Subject: [PATCH 69/88] Add glyph axes to existing smart component glyph --- src/fontra_glyphs/backend.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index b428047..3003562 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -810,8 +810,9 @@ def variableGlyphToGSGlyph(defaultLocation, variableGlyph, gsGlyph): del gsGlyph.layers[gsLayerId] # prepare smart component glyph - if variableGlyph.axes and not gsGlyph.smartComponentAxes: - for axis in variableGlyph.axes: + smartComponentAxesNames = [axis.name for axis in gsGlyph.smartComponentAxes] + for axis in variableGlyph.axes: + if axis.name not in smartComponentAxesNames: gsAxis = glyphsLib.classes.GSSmartComponentAxis() gsAxis.name = axis.name gsAxis.bottomValue = axis.minValue From a78250485937cbaf6d53d1e75c43f01566bdea33 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Thu, 27 Feb 2025 15:14:03 +0100 Subject: [PATCH 70/88] Find and fix issue with poleValue being a sting not a number + extend unittests --- src/fontra_glyphs/backend.py | 67 +++++++++++++++++------------------ tests/test_backend.py | 68 ++++++++++++++++++++++++++++++++++++ 2 files changed, 101 insertions(+), 34 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 3003562..7e3582a 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -372,33 +372,19 @@ def _ensureGlyphIsParsed(self, glyphName: str) -> None: def _getBraceLayerLocation(self, gsLayer): if not gsLayer._is_brace_layer(): return {} - if gsLayer.smartComponentPoleMapping: - # It's a intermediate layer within a smart component, if it has - # _brace_coordinates and smartComponentPoleMapping, - # We need to skip this and get the valid data via _getSmartLocation - return {} - return dict( (axis.name, value) for axis, value in zip(self.axes, gsLayer._brace_coordinates()) ) def _getSmartLocation(self, gsLayer, localAxesByName): - # If has _brace_coordinates: it is an intermidiate layer in a smart component - coords = gsLayer._brace_coordinates() or [] location = { name: ( - coords[i] - if len(coords) - 1 >= i - else ( - localAxesByName[name].minValue - if poleValue == Pole.MIN - else localAxesByName[name].maxValue - ) - ) - for i, (name, poleValue) in enumerate( - gsLayer.smartComponentPoleMapping.items() + localAxesByName[name].minValue + if int(poleValue) == Pole.MIN + else localAxesByName[name].maxValue ) + for name, poleValue in gsLayer.smartComponentPoleMapping.items() } return { disambiguateLocalAxisName(name, self.axisNames): value @@ -649,7 +635,7 @@ def gsLocalAxesToFontraLocalAxes(gsGlyph): minValue=axis.bottomValue, defaultValue=( axis.bottomValue - if basePoleMapping[axis.name] == Pole.MIN + if int(basePoleMapping[axis.name]) == Pole.MIN else axis.topValue ), maxValue=axis.topValue, @@ -813,6 +799,14 @@ def variableGlyphToGSGlyph(defaultLocation, variableGlyph, gsGlyph): smartComponentAxesNames = [axis.name for axis in gsGlyph.smartComponentAxes] for axis in variableGlyph.axes: if axis.name not in smartComponentAxesNames: + if axis.defaultValue not in [axis.minValue, axis.maxValue]: + # NOTE: GlyphsApp does not have axis.defaultValue, + # therefore it must be at MIN or MAX. + # https://docu.glyphsapp.com/#GSSmartComponentAxis + raise TypeError( + f"GlyphsApp Backend: Glyph axis '{axis.name}' defaultValue " + "must be at MIN or MAX." + ) gsAxis = glyphsLib.classes.GSSmartComponentAxis() gsAxis.name = axis.name gsAxis.bottomValue = axis.minValue @@ -853,35 +847,36 @@ def variableGlyphToGSGlyph(defaultLocation, variableGlyph, gsGlyph): gsGlyphLocation = [] for axis in gsGlyph.smartComponentAxes: - if axis.name not in glyphLocation: - # This might be the case if we create a new glyph axis in Fontra - continue gsGlyphLocation.append(glyphLocation[axis.name]) pole = ( Pole.MIN if axis.bottomValue == glyphLocation[axis.name] else Pole.MAX ) + # Set pole, only MIN or MAX possible. + # NOTE: In GlyphsApp these are checkboxes, either: on or off. gsLayer.smartComponentPoleMapping[axis.name] = pole sourceName = getSourceNameWithLayerName(variableGlyph.sources, layerName) masterId = gsMasterAxesToIdMapping.get(tuple(gsFontLocation)) - if masterId and not gsGlyphLocation: + + isDefaultLayer = False + if masterId: + if not gsGlyphLocation: + isDefaultLayer = True + elif defaultGlyphLocation == glyphLocation: + isDefaultLayer = True + + if isDefaultLayer: gsLayer.name = gsMasterIdToNameMapping.get(masterId) gsLayer.layerId = masterId - if gsLayer.name != sourceName: - # for example 'default' instead of 'Regular'. - gsLayer.userData["xyz.fontra.source-name"] = sourceName - if gsLayer.name != layerName: - gsLayer.userData["xyz.fontra.layer-name"] = layerName else: - gsLocation = gsGlyphLocation or gsFontLocation + gsLayer.name = sourceName gsLayer.layerId = layerName - gsLayer.userData["xyz.fontra.source-name"] = sourceName - gsLayer.attributes["coordinates"] = gsLocation - - gsLayer.name = "{" + ",".join(str(x) for x in gsLocation) + "}" gsLayer.isSpecialLayer = True + if not gsGlyphLocation: + gsLayer.name = "{" + ",".join(str(x) for x in gsFontLocation) + "}" + gsLayer.attributes["coordinates"] = gsFontLocation associatedMasterId = getAssociatedMasterId( gsGlyph.parent, gsFontLocation @@ -889,6 +884,9 @@ def variableGlyphToGSGlyph(defaultLocation, variableGlyph, gsGlyph): if associatedMasterId: gsLayer.associatedMasterId = associatedMasterId + gsLayer.userData["xyz.fontra.source-name"] = sourceName + gsLayer.userData["xyz.fontra.layer-name"] = layerName + if glyphLocation: # We have a smart component. Check if it is an intermediate master/layer, # because we currently do not support writing this to GlyphsApp files. @@ -914,7 +912,8 @@ def variableGlyphToGSGlyph(defaultLocation, variableGlyph, gsGlyph): if isIntermediateLayer: raise NotImplementedError( - "Intermediate layer within a smart glyph is not yet implemented" + "GlyphsApp Backend: Intermediate layers " + "within smart glyphs are not yet implemented" ) fontraLayerToGSLayer(layer, gsLayer) diff --git a/tests/test_backend.py b/tests/test_backend.py index f262e1f..89b696c 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -10,6 +10,7 @@ Anchor, Axes, FontInfo, + GlyphAxis, GlyphSource, Guideline, Layer, @@ -26,6 +27,13 @@ referenceFontPath = dataDir / "GlyphsUnitTestSans3.fontra" +mappingMasterIDs = { + "Light": "C4872ECA-A3A9-40AB-960A-1DB2202F16DE", + "Regular": "3E7589AA-8194-470F-8E2F-13C1C581BE24", + "Bold": "BFFFD157-90D3-4B85-B99D-9A2F366F03CA", +} + + @pytest.fixture(scope="module", params=[glyphs2Path, glyphs3Path, glyphsPackagePath]) def testFont(request): return getFileSystemBackend(request.param) @@ -195,6 +203,66 @@ async def test_createNewGlyph(writableTestFont): assert glyph == savedGlyph +async def test_createNewSmartGlyph(writableTestFont): + glyphName = "a.smart" + glyphAxis = GlyphAxis(name="Height", minValue=0, maxValue=100, defaultValue=0) + glyph = VariableGlyph(name=glyphName, axes=[glyphAxis]) + + # create a glyph with glyph axis + for sourceName, location in { + "Light": {"Weight": 17}, + "Light-Height": {"Weight": 17, "Height": 100}, + "Regular": {}, + "Regular-Height": {"Height": 100}, + "Bold": {"Weight": 220}, + "Bold-Height": {"Weight": 220, "Height": 100}, + }.items(): + layerName = mappingMasterIDs.get(sourceName) or str(uuid.uuid4()).upper() + glyph.sources.append( + GlyphSource(name=sourceName, location=location, layerName=layerName) + ) + glyph.layers[layerName] = Layer(glyph=StaticGlyph(xAdvance=100)) + + await writableTestFont.putGlyph(glyphName, glyph, []) + + savedGlyph = await writableTestFont.getGlyph(glyphName) + assert glyph == savedGlyph + + +async def test_extendSmartGlyphWithIntermedaiteLayer(writableTestFont): + # This should fail, because not yet implemented. + glyphName = "_part.shoulder" + glyph = await writableTestFont.getGlyph(glyphName) + + layerName = str(uuid.uuid4()).upper() + glyph.sources.append( + GlyphSource( + name="Intermediate Layer", location={"Weight": 99}, layerName=layerName + ) + ) + glyph.layers[layerName] = Layer(glyph=StaticGlyph(xAdvance=100)) + + with pytest.raises( + NotImplementedError, + match="Intermediate layers within smart glyphs are not yet implemented", + ): + await writableTestFont.putGlyph(glyphName, glyph, []) + + +async def test_smartGlyphAddGlyphAxisWithDefaultNotMinOrMax(writableTestFont): + # This should fail, because not yet implemented. + glyphName = "_part.shoulder" + glyph = await writableTestFont.getGlyph(glyphName) + glyphAxis = GlyphAxis(name="Height", minValue=0, maxValue=100, defaultValue=50) + glyph.axes.append(glyphAxis) + + with pytest.raises( + TypeError, + match="Glyph axis 'Height' defaultValue must be at MIN or MAX.", + ): + await writableTestFont.putGlyph(glyphName, glyph, []) + + async def test_deleteLayer(writableTestFont): glyphName = "a" glyphMap = await writableTestFont.getGlyphMap() From c9237ecca302bc00e8c47a8fdf9e20acd8ba8683 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Thu, 27 Feb 2025 15:21:43 +0100 Subject: [PATCH 71/88] Add "GlyphsApp Backend:" to error messages. --- src/fontra_glyphs/backend.py | 39 +++++++++++++++++++++++++++--------- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 7e3582a..5b7f47b 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -187,7 +187,9 @@ async def putGlyphMap(self, value: dict[str, list[int]]) -> None: pass async def deleteGlyph(self, glyphName): - raise NotImplementedError("deleting glyphs is not yet implemented") + raise NotImplementedError( + "GlyphsApp Backend: Deleting glyphs is not yet implemented." + ) async def getFontInfo(self) -> FontInfo: infoDict = {} @@ -205,25 +207,33 @@ async def getFontInfo(self) -> FontInfo: return FontInfo(**infoDict) async def putFontInfo(self, fontInfo: FontInfo): - raise NotImplementedError("editing FontInfo is not yet implemented") + raise NotImplementedError( + "GlyphsApp Backend: Editing FontInfo is not yet implemented." + ) async def getSources(self) -> dict[str, FontSource]: return gsMastersToFontraFontSources(self.gsFont, self.locationByMasterID) async def putSources(self, sources: dict[str, FontSource]) -> None: - raise NotImplementedError("editing FontSources is not yet implemented") + raise NotImplementedError( + "GlyphsApp Backend: Editing FontSources is not yet implemented." + ) async def getAxes(self) -> Axes: return Axes(axes=deepcopy(self.axes)) async def putAxes(self, axes: Axes) -> None: - raise NotImplementedError("editing Axes is not yet implemented") + raise NotImplementedError( + "GlyphsApp Backend: Editing Axes is not yet implemented." + ) async def getUnitsPerEm(self) -> int: return self.gsFont.upm async def putUnitsPerEm(self, value: int) -> None: - raise NotImplementedError("editing UnitsPerEm is not yet implemented") + raise NotImplementedError( + "GlyphsApp Backend: Editing UnitsPerEm is not yet implemented." + ) async def getKerning(self) -> dict[str, Kerning]: # TODO: RTL kerning: https://docu.glyphsapp.com/#GSFont.kerningRTL @@ -245,26 +255,34 @@ async def getKerning(self) -> dict[str, Kerning]: return kerning async def putKerning(self, kerning: dict[str, Kerning]) -> None: - raise NotImplementedError("editing Kerning is not yet implemented") + raise NotImplementedError( + "GlyphsApp Backend: Editing Kerning is not yet implemented." + ) async def getFeatures(self) -> OpenTypeFeatures: # TODO: extract features return OpenTypeFeatures() async def putFeatures(self, features: OpenTypeFeatures) -> None: - raise NotImplementedError("editing OpenTypeFeatures is not yet implemented") + raise NotImplementedError( + "GlyphsApp Backend: Editing OpenTypeFeatures is not yet implemented." + ) async def getBackgroundImage(self, imageIdentifier: str) -> ImageData | None: return None async def putBackgroundImage(self, imageIdentifier: str, data: ImageData) -> None: - raise NotImplementedError("editing BackgroundImage is not yet implemented") + raise NotImplementedError( + "GlyphsApp Backend: Editing BackgroundImage is not yet implemented." + ) async def getCustomData(self) -> dict[str, Any]: return {} async def putCustomData(self, lib): - raise NotImplementedError("editing CustomData is not yet implemented") + raise NotImplementedError( + "GlyphsApp Backend: Editing CustomData is not yet implemented." + ) async def getGlyph(self, glyphName: str) -> VariableGlyph | None: if glyphName not in self.glyphNameToIndex: @@ -372,6 +390,7 @@ def _ensureGlyphIsParsed(self, glyphName: str) -> None: def _getBraceLayerLocation(self, gsLayer): if not gsLayer._is_brace_layer(): return {} + return dict( (axis.name, value) for axis, value in zip(self.axes, gsLayer._brace_coordinates()) @@ -913,7 +932,7 @@ def variableGlyphToGSGlyph(defaultLocation, variableGlyph, gsGlyph): if isIntermediateLayer: raise NotImplementedError( "GlyphsApp Backend: Intermediate layers " - "within smart glyphs are not yet implemented" + "within smart glyphs are not yet implemented." ) fontraLayerToGSLayer(layer, gsLayer) From 917516be7645cc53e882174bbde152417fa57018 Mon Sep 17 00:00:00 2001 From: Just van Rossum Date: Thu, 27 Feb 2025 16:05:10 +0100 Subject: [PATCH 72/88] Fix Enum string conversion on Python 3.10 --- src/fontra_glyphs/backend.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 5b7f47b..c302a79 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -868,9 +868,9 @@ def variableGlyphToGSGlyph(defaultLocation, variableGlyph, gsGlyph): for axis in gsGlyph.smartComponentAxes: gsGlyphLocation.append(glyphLocation[axis.name]) pole = ( - Pole.MIN + int(Pole.MIN) # convert to int for Python <= 3.10 if axis.bottomValue == glyphLocation[axis.name] - else Pole.MAX + else int(Pole.MAX) # convert to int for Python <= 3.10 ) # Set pole, only MIN or MAX possible. # NOTE: In GlyphsApp these are checkboxes, either: on or off. From f693b4833a080db939fb722e8e64a4b7946674f3 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Thu, 27 Feb 2025 16:28:41 +0100 Subject: [PATCH 73/88] Little refactoring + some comments --- src/fontra_glyphs/backend.py | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index c302a79..8d27f16 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -880,28 +880,26 @@ def variableGlyphToGSGlyph(defaultLocation, variableGlyph, gsGlyph): masterId = gsMasterAxesToIdMapping.get(tuple(gsFontLocation)) isDefaultLayer = False + # It is not enough to check if it has a masterId, because in case of a smart component, + # the layer for each glyph axis has the same location as the master layer. if masterId: if not gsGlyphLocation: isDefaultLayer = True elif defaultGlyphLocation == glyphLocation: isDefaultLayer = True - if isDefaultLayer: - gsLayer.name = gsMasterIdToNameMapping.get(masterId) - gsLayer.layerId = masterId - else: - gsLayer.name = sourceName - gsLayer.layerId = layerName - gsLayer.isSpecialLayer = True - if not gsGlyphLocation: - gsLayer.name = "{" + ",".join(str(x) for x in gsFontLocation) + "}" - gsLayer.attributes["coordinates"] = gsFontLocation + gsLayer.name = ( + gsMasterIdToNameMapping.get(masterId) if isDefaultLayer else sourceName + ) + gsLayer.layerId = masterId if isDefaultLayer else layerName + gsLayer.associatedMasterId = getAssociatedMasterId( + gsGlyph.parent, gsFontLocation + ) - associatedMasterId = getAssociatedMasterId( - gsGlyph.parent, gsFontLocation - ) - if associatedMasterId: - gsLayer.associatedMasterId = associatedMasterId + if not isDefaultLayer and not gsGlyphLocation: + # This is an intermediate layer + gsLayer.name = "{" + ",".join(str(x) for x in gsFontLocation) + "}" + gsLayer.attributes["coordinates"] = gsFontLocation gsLayer.userData["xyz.fontra.source-name"] = sourceName gsLayer.userData["xyz.fontra.layer-name"] = layerName @@ -918,7 +916,7 @@ def variableGlyphToGSGlyph(defaultLocation, variableGlyph, gsGlyph): else: # If it has glyph axes and is on a master location, # but any of the glyph axes are not at min or max position, - # is must be an intermediate layer. + # it must be an intermediate layer. if any( [ True From 67c4c3365e01123852f7340b734b0b096b091612 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Thu, 27 Feb 2025 16:53:34 +0100 Subject: [PATCH 74/88] Add unittest for 'Add new glyph axis to existing smart comp glyph' + find solution --- src/fontra_glyphs/backend.py | 12 ++++++++++++ tests/test_backend.py | 12 ++++++++++++ 2 files changed, 24 insertions(+) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 8d27f16..c93889f 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -840,6 +840,18 @@ def variableGlyphToGSGlyph(defaultLocation, variableGlyph, gsGlyph): if gsLayer is not None: # gsLayer exists – modify existing gsLayer: fontraLayerToGSLayer(layer, gsLayer) + # It might be, that we added a new glyph axis within Fontra + # for an existing smart comp glyph, in that case we need to add + # the new axis to gsLayer.smartComponentPoleMapping. + for axis in variableGlyph.axes: + if axis.name in gsLayer.smartComponentPoleMapping: + continue + pole = ( + int(Pole.MIN) # convert to int for Python <= 3.10 + if axis.minValue == defaultGlyphLocation[axis.name] + else int(Pole.MAX) # convert to int for Python <= 3.10 + ) + gsLayer.smartComponentPoleMapping[axis.name] = pole else: # gsLayer does not exist – create new layer: gsLayer = glyphsLib.classes.GSLayer() diff --git a/tests/test_backend.py b/tests/test_backend.py index 89b696c..ec481ef 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -263,6 +263,18 @@ async def test_smartGlyphAddGlyphAxisWithDefaultNotMinOrMax(writableTestFont): await writableTestFont.putGlyph(glyphName, glyph, []) +async def test_smartGlyphAddGlyphAxisWithDefaultAtMinOrMax(writableTestFont): + glyphName = "_part.shoulder" + glyph = await writableTestFont.getGlyph(glyphName) + glyphAxis = GlyphAxis(name="Height", minValue=0, maxValue=100, defaultValue=100) + glyph.axes.append(glyphAxis) + + await writableTestFont.putGlyph(glyphName, glyph, []) + + savedGlyph = await writableTestFont.getGlyph(glyphName) + assert glyph == savedGlyph + + async def test_deleteLayer(writableTestFont): glyphName = "a" glyphMap = await writableTestFont.getGlyphMap() From 8918d8a69d2da1727b3297b04b891e70a6d9f611 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Thu, 27 Feb 2025 17:31:18 +0100 Subject: [PATCH 75/88] Update pattern based on "real" project --- src/fontra_glyphs/utils.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/fontra_glyphs/utils.py b/src/fontra_glyphs/utils.py index ee6fb62..5469a2d 100644 --- a/src/fontra_glyphs/utils.py +++ b/src/fontra_glyphs/utils.py @@ -89,13 +89,22 @@ def convertMatchesToTuples(obj, matchTree, path=()): ["fontMaster", None, "guides", None, "pos"], ["glyphs", None, "color"], ["glyphs", None, "layers", None, "anchors", None, "pos"], + ["glyphs", None, "layers", None, "background", "anchors", None, "pos"], ["glyphs", None, "layers", None, "annotations", None, "pos"], ["glyphs", None, "layers", None, "background", "shapes", None, "nodes", None], + ["glyphs", None, "layers", None, "background", "shapes", None, "pos"], + ["glyphs", None, "layers", None, "background", "shapes", None, "scale"], + ["glyphs", None, "layers", None, "background", "shapes", None, "slant"], ["glyphs", None, "layers", None, "guides", None, "pos"], ["glyphs", None, "layers", None, "hints", None, "origin"], + ["glyphs", None, "layers", None, "hints", None, "scale"], + ["glyphs", None, "layers", None, "background", "hints", None, "origin"], + ["glyphs", None, "layers", None, "background", "hints", None, "scale"], + ["glyphs", None, "layers", None, "background", "hints", None, "place"], ["glyphs", None, "layers", None, "hints", None, "target"], ["glyphs", None, "layers", None, "shapes", None, "nodes", None], ["glyphs", None, "layers", None, "shapes", None, "pos"], + ["glyphs", None, "layers", None, "shapes", None, "scale"], ] From 307e0848520617ae353dbc88a155703591d4feee Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Tue, 4 Mar 2025 11:08:19 +0100 Subject: [PATCH 76/88] To avoid any confusion: Transform as GSTransform --- src/fontra_glyphs/backend.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index c93889f..d2a2f0f 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -39,7 +39,7 @@ to_designspace_axes, ) from glyphsLib.builder.smart_components import Pole -from glyphsLib.types import Transform +from glyphsLib.types import Transform as GSTransform from .utils import ( convertMatchesToTuples, @@ -971,8 +971,8 @@ def fontraLayerToGSLayer(layer, gsLayer): def fontraComponentToGSComponent(component): gsComponent = glyphsLib.classes.GSComponent(component.name) transformation = component.transformation.toTransform() - if not isinstance(transformation, Transform): - gsComponent.transform = Transform(*transformation) + if not isinstance(transformation, GSTransform): + gsComponent.transform = GSTransform(*transformation) for axisName in component.location: gsComponent.smartComponentValues[axisName] = component.location[axisName] gsComponent.alignment = component.customData.get( From dba34b6829a5e332ce552c0246780b23fbd9c53e Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Wed, 5 Mar 2025 10:41:50 +0100 Subject: [PATCH 77/88] Adding a failing unittest for skewing of a component --- tests/test_backend.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/test_backend.py b/tests/test_backend.py index ec481ef..8e5a2f8 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -330,6 +330,20 @@ async def test_addLayerWithComponent(writableTestFont): assert glyph == savedGlyph +async def test_skewComponent(writableTestFont): + glyphName = "Adieresis" # Adieresis is made from components + glyphMap = await writableTestFont.getGlyphMap() + glyph = await writableTestFont.getGlyph(glyphName) + + layerName = mappingMasterIDs.get("Light") + glyph.layers[layerName].glyph.components[0].transformation.skewX = 20 + + await writableTestFont.putGlyph(glyphName, glyph, glyphMap[glyphName]) + + savedGlyph = await writableTestFont.getGlyph(glyphName) + assert glyph == savedGlyph + + async def test_addAnchor(writableTestFont): glyphName = "a" glyphMap = await writableTestFont.getGlyphMap() From dfa397fbff0ef8215f8b3342c6ca654ae8d3feac Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Wed, 5 Mar 2025 10:43:49 +0100 Subject: [PATCH 78/88] Remove redundant 'if' statement --- src/fontra_glyphs/backend.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index d2a2f0f..c6936d2 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -971,8 +971,7 @@ def fontraLayerToGSLayer(layer, gsLayer): def fontraComponentToGSComponent(component): gsComponent = glyphsLib.classes.GSComponent(component.name) transformation = component.transformation.toTransform() - if not isinstance(transformation, GSTransform): - gsComponent.transform = GSTransform(*transformation) + gsComponent.transform = GSTransform(*transformation) for axisName in component.location: gsComponent.smartComponentValues[axisName] = component.location[axisName] gsComponent.alignment = component.customData.get( From c31135293bf3fe8097ef11c5818130cedd058a0d Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Wed, 5 Mar 2025 11:04:15 +0100 Subject: [PATCH 79/88] Raise TypeError when skewing a component + update unittest --- src/fontra_glyphs/backend.py | 10 ++++++++++ tests/test_backend.py | 22 ++++++++++++++-------- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index c6936d2..9122882 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -968,7 +968,17 @@ def fontraLayerToGSLayer(layer, gsLayer): ] +EPSILON = 1e-9 + + def fontraComponentToGSComponent(component): + if ( + abs(component.transformation.skewX) > EPSILON + or abs(component.transformation.skewY) > EPSILON + ): + raise TypeError( + "GlyphsApp Backend: Does not support skewing of components, yet." + ) gsComponent = glyphsLib.classes.GSComponent(component.name) transformation = component.transformation.toTransform() gsComponent.transform = GSTransform(*transformation) diff --git a/tests/test_backend.py b/tests/test_backend.py index 8e5a2f8..c4c1b50 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -330,18 +330,24 @@ async def test_addLayerWithComponent(writableTestFont): assert glyph == savedGlyph -async def test_skewComponent(writableTestFont): +expectedSkewErrors = [ + # skewValue, expectedErrorMatch + [20, "Does not support skewing of components"], + [-0.001, "Does not support skewing of components"], +] + + +@pytest.mark.parametrize("skewValue,expectedErrorMatch", expectedSkewErrors) +async def test_skewComponent(writableTestFont, skewValue, expectedErrorMatch): glyphName = "Adieresis" # Adieresis is made from components glyphMap = await writableTestFont.getGlyphMap() glyph = await writableTestFont.getGlyph(glyphName) - layerName = mappingMasterIDs.get("Light") - glyph.layers[layerName].glyph.components[0].transformation.skewX = 20 - - await writableTestFont.putGlyph(glyphName, glyph, glyphMap[glyphName]) - - savedGlyph = await writableTestFont.getGlyph(glyphName) - assert glyph == savedGlyph + glyph.layers[mappingMasterIDs.get("Light")].glyph.components[ + 0 + ].transformation.skewX = skewValue + with pytest.raises(TypeError, match=expectedErrorMatch): + await writableTestFont.putGlyph(glyphName, glyph, glyphMap[glyphName]) async def test_addAnchor(writableTestFont): From d9eea8ef93a9296f7289577cd4151077113940d8 Mon Sep 17 00:00:00 2001 From: Olli Meier <21055547+ollimeier@users.noreply.github.com> Date: Thu, 6 Mar 2025 08:48:00 +0100 Subject: [PATCH 80/88] Update README.md Co-authored-by: Just van Rossum --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 919492e..9714924 100644 --- a/README.md +++ b/README.md @@ -12,6 +12,6 @@ It supports the following features: #### Glyph Layer - Contour (Paths, Nodes) ✅ -- Components ✅ +- (Smart) Components ✅ - Anchors ✅ - Guidelines ✅ From 3243d31479270313a354f5f52e31c63277772e29 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Thu, 6 Mar 2025 09:43:05 +0100 Subject: [PATCH 81/88] Refactoring: getSourceFromLayerName, return defaultSource --- src/fontra_glyphs/backend.py | 14 +++++++------- src/fontra_glyphs/utils.py | 19 ++++++++----------- tests/test_utils.py | 12 ++++++------ 3 files changed, 21 insertions(+), 24 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 9122882..47f43ec 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -44,8 +44,7 @@ from .utils import ( convertMatchesToTuples, getAssociatedMasterId, - getLocationFromSources, - getSourceNameWithLayerName, + getSourceFromLayerName, matchTreeFont, splitLocation, ) @@ -857,9 +856,9 @@ def variableGlyphToGSGlyph(defaultLocation, variableGlyph, gsGlyph): gsLayer = glyphsLib.classes.GSLayer() gsLayer.parent = gsGlyph - sourceLocation = getLocationFromSources(variableGlyph.sources, layerName) + glyphSource = getSourceFromLayerName(variableGlyph.sources, layerName) fontLocation, glyphLocation = splitLocation( - sourceLocation, variableGlyph.axes + glyphSource.location, variableGlyph.axes ) fontLocation = makeDenseLocation(fontLocation, defaultLocation) glyphLocation = makeDenseLocation(glyphLocation, defaultGlyphLocation) @@ -888,7 +887,6 @@ def variableGlyphToGSGlyph(defaultLocation, variableGlyph, gsGlyph): # NOTE: In GlyphsApp these are checkboxes, either: on or off. gsLayer.smartComponentPoleMapping[axis.name] = pole - sourceName = getSourceNameWithLayerName(variableGlyph.sources, layerName) masterId = gsMasterAxesToIdMapping.get(tuple(gsFontLocation)) isDefaultLayer = False @@ -901,7 +899,9 @@ def variableGlyphToGSGlyph(defaultLocation, variableGlyph, gsGlyph): isDefaultLayer = True gsLayer.name = ( - gsMasterIdToNameMapping.get(masterId) if isDefaultLayer else sourceName + gsMasterIdToNameMapping.get(masterId) + if isDefaultLayer + else glyphSource.name ) gsLayer.layerId = masterId if isDefaultLayer else layerName gsLayer.associatedMasterId = getAssociatedMasterId( @@ -913,7 +913,7 @@ def variableGlyphToGSGlyph(defaultLocation, variableGlyph, gsGlyph): gsLayer.name = "{" + ",".join(str(x) for x in gsFontLocation) + "}" gsLayer.attributes["coordinates"] = gsFontLocation - gsLayer.userData["xyz.fontra.source-name"] = sourceName + gsLayer.userData["xyz.fontra.source-name"] = glyphSource.name gsLayer.userData["xyz.fontra.layer-name"] = layerName if glyphLocation: diff --git a/src/fontra_glyphs/utils.py b/src/fontra_glyphs/utils.py index 5469a2d..e0331cb 100644 --- a/src/fontra_glyphs/utils.py +++ b/src/fontra_glyphs/utils.py @@ -1,10 +1,13 @@ -def getLocationFromSources(sources, layerName): - s = sources[0] +def getSourceFromLayerName(sources, layerName): for source in sources: if source.layerName == layerName: - s = source - break - return s.location + return source + if source.location == {}: + defaultSource = source + + # NOTE: Theoretically it's possible to have a layer with no matching source. + # In that case, fallback to default. + return defaultSource def splitLocation(location, glyphAxes): @@ -22,12 +25,6 @@ def splitLocation(location, glyphAxes): return fontLocation, glyphLocation -def getSourceNameWithLayerName(sources, layerName): - for source in sources: - if source.layerName == layerName: - return source.name - - def getAssociatedMasterId(gsFont, gsLocation): # Best guess for associatedMasterId closestMasterID = gsFont.masters[0].id # default first master. diff --git a/tests/test_utils.py b/tests/test_utils.py index 8fb11cf..c6f11e6 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -9,7 +9,7 @@ from fontra_glyphs.utils import ( convertMatchesToTuples, getAssociatedMasterId, - getLocationFromSources, + getSourceFromLayerName, matchTreeFont, splitLocation, ) @@ -76,12 +76,12 @@ def testGSFontWW(): return gsFont -async def test_getLocationFromSources(testFont): +async def test_getSourceFromLayerName(testFont): glyph = await testFont.getGlyph("a") - location = getLocationFromSources( + glyphSource = getSourceFromLayerName( glyph.sources, "1FA54028-AD2E-4209-AA7B-72DF2DF16264" ) - assert location == {"Weight": 155} + assert glyphSource.location == {"Weight": 155} expectedLocations = [ @@ -111,8 +111,8 @@ async def test_splitLocation( testFont, gsLayerId, expectedFontLocation, expectedGlyphLocation ): glyph = await testFont.getGlyph("_part.shoulder") - location = getLocationFromSources(glyph.sources, gsLayerId) - fontLocation, glyphLocation = splitLocation(location, glyph.axes) + glyphSource = getSourceFromLayerName(glyph.sources, gsLayerId) + fontLocation, glyphLocation = splitLocation(glyphSource.location, glyph.axes) glyphLocation = makeDenseLocation( glyphLocation, {axis.name: axis.defaultValue for axis in glyph.axes} ) From 7c9cadcf79e6c6262ad6b4be85b0a92262ffde32 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Thu, 6 Mar 2025 09:53:40 +0100 Subject: [PATCH 82/88] When assigning self.gsFilePath ensure it is a pathlib.Path --- src/fontra_glyphs/backend.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 47f43ec..d72f4bd 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -103,7 +103,7 @@ def fromPath(cls, path: PathLike) -> WritableFontBackend: def _setupFromPath(self, path: PathLike) -> None: gsFont = glyphsLib.classes.GSFont() - self.gsFilePath = path + self.gsFilePath = pathlib.Path(path) rawFontData, rawGlyphsData = self._loadFiles(path) @@ -517,7 +517,7 @@ def _writeRawGlyph(self, glyphName, f): filePath.write_text(f.getvalue(), encoding="utf=8") def getGlyphFilePath(self, glyphName): - glyphsPath = pathlib.Path(self.gsFilePath) / "glyphs" + glyphsPath = self.gsFilePath / "glyphs" refFileName = userNameToFileName(glyphName, suffix=".glyph") return glyphsPath / refFileName From 21e395c0ecab4d1128e7f335fa006b0962541381 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Thu, 6 Mar 2025 11:31:43 +0100 Subject: [PATCH 83/88] Remove unnecessary int() --- src/fontra_glyphs/backend.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index d72f4bd..9a0604e 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -399,7 +399,7 @@ def _getSmartLocation(self, gsLayer, localAxesByName): location = { name: ( localAxesByName[name].minValue - if int(poleValue) == Pole.MIN + if poleValue == Pole.MIN else localAxesByName[name].maxValue ) for name, poleValue in gsLayer.smartComponentPoleMapping.items() @@ -653,7 +653,7 @@ def gsLocalAxesToFontraLocalAxes(gsGlyph): minValue=axis.bottomValue, defaultValue=( axis.bottomValue - if int(basePoleMapping[axis.name]) == Pole.MIN + if basePoleMapping[axis.name] == Pole.MIN else axis.topValue ), maxValue=axis.topValue, From ec8536d4b9fbb411fe749dccc6f880528f9881e0 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Thu, 6 Mar 2025 11:37:12 +0100 Subject: [PATCH 84/88] Add failing unittest test_smartGlyphRemoveGlyphAxis --- tests/test_backend.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/test_backend.py b/tests/test_backend.py index c4c1b50..3b03799 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -275,6 +275,17 @@ async def test_smartGlyphAddGlyphAxisWithDefaultAtMinOrMax(writableTestFont): assert glyph == savedGlyph +async def test_smartGlyphRemoveGlyphAxis(writableTestFont): + glyphName = "_part.shoulder" + glyph = await writableTestFont.getGlyph(glyphName) + del glyph.axes[0] + + await writableTestFont.putGlyph(glyphName, glyph, []) + + savedGlyph = await writableTestFont.getGlyph(glyphName) + assert glyph == savedGlyph + + async def test_deleteLayer(writableTestFont): glyphName = "a" glyphMap = await writableTestFont.getGlyphMap() From 3db0c210f5f87fde6e73f44a69ea4ffe97b34c7b Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Thu, 6 Mar 2025 12:04:07 +0100 Subject: [PATCH 85/88] Fix issue with deleting a glyph axis from a smart glyph within Fontra --- src/fontra_glyphs/backend.py | 13 +++++++++++++ tests/test_backend.py | 6 ++++++ 2 files changed, 19 insertions(+) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 9a0604e..8cb1a59 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -831,6 +831,14 @@ def variableGlyphToGSGlyph(defaultLocation, variableGlyph, gsGlyph): gsAxis.topValue = axis.maxValue gsGlyph.smartComponentAxes.append(gsAxis) + axisNamesToBeRemoved = [] + for i, axisName in reversed(list(enumerate(smartComponentAxesNames))): + if not defaultGlyphLocation.get(axisName): + # An axis has been removed from the glyph, + # therefore delete axis + del gsGlyph.smartComponentAxes[i] + axisNamesToBeRemoved.append(axisName) + for layerName, layer in iter(variableGlyph.layers.items()): gsLayer = gsGlyph.layers[layerName] # layerName is equal to gsLayer.layerId if it comes from Glyphsapp, @@ -851,6 +859,11 @@ def variableGlyphToGSGlyph(defaultLocation, variableGlyph, gsGlyph): else int(Pole.MAX) # convert to int for Python <= 3.10 ) gsLayer.smartComponentPoleMapping[axis.name] = pole + + for axisName in axisNamesToBeRemoved: + # An axis has been removed from the glyph, therefore we need + # to delete the axis from smartComponentPoleMapping as well. + del gsLayer.smartComponentPoleMapping[axisName] else: # gsLayer does not exist – create new layer: gsLayer = glyphsLib.classes.GSLayer() diff --git a/tests/test_backend.py b/tests/test_backend.py index 3b03799..3f17ce5 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -280,6 +280,12 @@ async def test_smartGlyphRemoveGlyphAxis(writableTestFont): glyph = await writableTestFont.getGlyph(glyphName) del glyph.axes[0] + # We expect we cannot roundtrip a glyph when removing a glyph axis, + # because then some layers locations are not unique anymore. + for i in [8, 5, 2]: + del glyph.layers[glyph.sources[i].layerName] + del glyph.sources[i] + await writableTestFont.putGlyph(glyphName, glyph, []) savedGlyph = await writableTestFont.getGlyph(glyphName) From 09db979e6943223f1ebb7922ca5df721e2ce3f29 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Thu, 6 Mar 2025 12:13:01 +0100 Subject: [PATCH 86/88] Fix a bug (get can return 0) --- src/fontra_glyphs/backend.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index 8cb1a59..c6053d7 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -833,7 +833,7 @@ def variableGlyphToGSGlyph(defaultLocation, variableGlyph, gsGlyph): axisNamesToBeRemoved = [] for i, axisName in reversed(list(enumerate(smartComponentAxesNames))): - if not defaultGlyphLocation.get(axisName): + if axisName not in defaultGlyphLocation: # An axis has been removed from the glyph, # therefore delete axis del gsGlyph.smartComponentAxes[i] From 10443fc8c93dae2adc1aa9614c3ac3033aabb7ee Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Thu, 6 Mar 2025 12:16:35 +0100 Subject: [PATCH 87/88] Add failing unittest test_smartGlyphChangeGlyphAxisValue --- tests/test_backend.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/test_backend.py b/tests/test_backend.py index 3f17ce5..bc1621c 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -292,6 +292,17 @@ async def test_smartGlyphRemoveGlyphAxis(writableTestFont): assert glyph == savedGlyph +async def test_smartGlyphChangeGlyphAxisValue(writableTestFont): + glyphName = "_part.shoulder" + glyph = await writableTestFont.getGlyph(glyphName) + + glyph.axes[1].maxValue = 200 + await writableTestFont.putGlyph(glyphName, glyph, []) + + savedGlyph = await writableTestFont.getGlyph(glyphName) + assert glyph == savedGlyph + + async def test_deleteLayer(writableTestFont): glyphName = "a" glyphMap = await writableTestFont.getGlyphMap() From 6e9d9e1113aabf2dc56033724cdc9ffa69cb82a6 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Thu, 6 Mar 2025 12:34:00 +0100 Subject: [PATCH 88/88] Fix: update glyph axis values --- src/fontra_glyphs/backend.py | 5 +++++ tests/test_backend.py | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/src/fontra_glyphs/backend.py b/src/fontra_glyphs/backend.py index c6053d7..98c6bb4 100644 --- a/src/fontra_glyphs/backend.py +++ b/src/fontra_glyphs/backend.py @@ -839,6 +839,11 @@ def variableGlyphToGSGlyph(defaultLocation, variableGlyph, gsGlyph): del gsGlyph.smartComponentAxes[i] axisNamesToBeRemoved.append(axisName) + # update values, after deleting axis + for i, axis in enumerate(variableGlyph.axes): + gsGlyph.smartComponentAxes[i].bottomValue = axis.minValue + gsGlyph.smartComponentAxes[i].topValue = axis.maxValue + for layerName, layer in iter(variableGlyph.layers.items()): gsLayer = gsGlyph.layers[layerName] # layerName is equal to gsLayer.layerId if it comes from Glyphsapp, diff --git a/tests/test_backend.py b/tests/test_backend.py index bc1621c..b14c91e 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -297,6 +297,10 @@ async def test_smartGlyphChangeGlyphAxisValue(writableTestFont): glyph = await writableTestFont.getGlyph(glyphName) glyph.axes[1].maxValue = 200 + # We expect we cannot roundtrip a glyph when changing a glyph axis min or + # max value without changing the default, because in GlyphsApp there is + # no defaultValue-concept. Therefore we need to change the defaultValue as well. + glyph.axes[1].defaultValue = 200 await writableTestFont.putGlyph(glyphName, glyph, []) savedGlyph = await writableTestFont.getGlyph(glyphName)