Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't create redundant UFO layers #780

Merged
merged 7 commits into from
Sep 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 27 additions & 13 deletions src/fontra/backends/designspace.py
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ async def putGlyph(self, glyphName, glyph, unicodes):
localSources = []
for source in glyph.sources:
sourceInfo = self._prepareUFOSourceLayer(
source, localDefaultLocation, revLayerNameMapping
glyphName, source, localDefaultLocation, revLayerNameMapping
)
if sourceInfo.sourceName != source.name:
sourceNameMapping[sourceInfo.sourceName] = source.name
Expand Down Expand Up @@ -349,7 +349,9 @@ async def putGlyph(self, glyphName, glyph, unicodes):
if ufoLayer is None:
# This layer is not used by any source and we haven't seen it
# before. Let's create a new layer in the default UFO.
ufoLayer = self._newUFOLayer(self.defaultUFOLayer.path, layerName)
ufoLayer = self._newUFOLayer(
glyphName, self.defaultUFOLayer.path, layerName
)
if ufoLayer.fontraLayerName != layerName:
layerNameMapping[ufoLayer.fontraLayerName] = layerName
layerName = ufoLayer.fontraLayerName
Expand Down Expand Up @@ -397,7 +399,9 @@ async def putGlyph(self, glyphName, glyph, unicodes):

self.savedGlyphModificationTimes[glyphName] = modTimes

def _prepareUFOSourceLayer(self, source, localDefaultLocation, revLayerNameMapping):
def _prepareUFOSourceLayer(
self, glyphName, source, localDefaultLocation, revLayerNameMapping
):
sparseLocalLocation = {
name: source.location[name]
for name, value in localDefaultLocation.items()
Expand All @@ -412,7 +416,7 @@ def _prepareUFOSourceLayer(self, source, localDefaultLocation, revLayerNameMappi
locationTuple=tuplifyLocation(globalLocation)
)
if dsSource is None:
dsSource = self._createDSSource(source, globalLocation)
dsSource = self._createDSSource(glyphName, source, globalLocation)

if sparseLocalLocation:
ufoLayer = self.ufoLayers.findItem(
Expand All @@ -423,7 +427,7 @@ def _prepareUFOSourceLayer(self, source, localDefaultLocation, revLayerNameMappi

if ufoLayer is None:
ufoPath = dsSource.layer.path
ufoLayer = self._newUFOLayer(ufoPath, source.layerName)
ufoLayer = self._newUFOLayer(glyphName, ufoPath, source.layerName)
ufoLayerName = ufoLayer.name
else:
ufoLayerName = ufoLayer.name
Expand All @@ -446,7 +450,7 @@ def _prepareUFOSourceLayer(self, source, localDefaultLocation, revLayerNameMappi
localSourceDict=localSourceDict,
)

def _createDSSource(self, source, globalLocation):
def _createDSSource(self, glyphName, source, globalLocation):
manager = self.ufoManager
atPole, notAtPole = splitLocationByPolePosition(
globalLocation, self.axisPolePositions
Expand Down Expand Up @@ -487,7 +491,9 @@ def _createDSSource(self, source, globalLocation):
poleDSSource = self.defaultDSSource
assert poleDSSource is not None
ufoPath = poleDSSource.layer.path
ufoLayer = self._newUFOLayer(poleDSSource.layer.path, source.layerName)
ufoLayer = self._newUFOLayer(
glyphName, poleDSSource.layer.path, source.layerName
)
ufoLayerName = ufoLayer.name

self.dsDoc.addSourceDescriptor(
Expand All @@ -507,13 +513,21 @@ def _createDSSource(self, source, globalLocation):

return dsSource

def _newUFOLayer(self, ufoPath, suggestedLayerName):
def _newUFOLayer(self, glyphName, ufoPath, suggestedLayerName):
reader = self.ufoManager.getReader(ufoPath)
makeUniqueName = uniqueNameMaker(reader.getLayerNames())
ufoLayerName = makeUniqueName(suggestedLayerName)
# Create the new UFO layer now
_ = self.ufoManager.getGlyphSet(ufoPath, ufoLayerName)
reader.writeLayerContents()
existingLayerNames = set(reader.getLayerNames())
ufoLayerName = suggestedLayerName
count = 0
# getGlyphSet() will create the layer if it doesn't already exist
while glyphName in self.ufoManager.getGlyphSet(ufoPath, ufoLayerName):
# The glyph already exists in the layer, which means there is
# a conflict. Let's make up a layer name in which the glyph
# does not exist.
count += 1
ufoLayerName = f"{suggestedLayerName}#{count}"

if ufoLayerName not in existingLayerNames:
reader.writeLayerContents()

ufoLayer = UFOLayer(
manager=self.ufoManager,
Expand Down
22 changes: 22 additions & 0 deletions test-py/test_backend_designspace.py
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,28 @@ async def test_newFileSystemBackend(tmpdir, testFont):
assert glyph == referenceGlyph


async def test_writeCorrectLayers(tmpdir, testFont):
# Check that no redundant layers are written
tmpdir = pathlib.Path(tmpdir)
dsPath = tmpdir / "Test.designspace"
font = newFileSystemBackend(dsPath)

axes = await testFont.getGlobalAxes()
await font.putGlobalAxes(axes)
glyphMap = await testFont.getGlyphMap()
glyph = await testFont.getGlyph("A")
await font.putGlyph("A", glyph, glyphMap["A"])
await font.putGlyph("A.alt", glyph, [])

assert [
"fontinfo.plist",
"glyphs",
"glyphs.M_utatorS_ansL_ightC_ondensed_support",
"layercontents.plist",
"metainfo.plist",
] == fileNamesFromDir(tmpdir / "Test_Regular.ufo")


def fileNamesFromDir(path):
return sorted(p.name for p in path.iterdir())

Expand Down