Merge pull request #1794 from googlefonts/put-bg-image-api-tweak

[background image] Get rid of redundant args to putBackgroundImage()

src/fontra/backends/copy.py

@@ -87,15 +87,13 @@ async def _copyFont(
         raise e
 
     if isinstance(destBackend, WriteBackgroundImage):
-        backgroundImageInfos = [info for t in done for info in t.result()]
-        if backgroundImageInfos:
+        backgroundImageIdentifiers = [info for t in done for info in t.result()]
+        if backgroundImageIdentifiers:
             assert isinstance(sourceBackend, ReadBackgroundImage), type(sourceBackend)
-            for glyphName, layerName, imageIdentifier in backgroundImageInfos:
+            for imageIdentifier in backgroundImageIdentifiers:
                 imageData = await sourceBackend.getBackgroundImage(imageIdentifier)
                 if imageData is not None:
-                    await destBackend.putBackgroundImage(
-                        imageIdentifier, glyphName, layerName, imageData
-                    )
+                    await destBackend.putBackgroundImage(imageIdentifier, imageData)
 
     await destBackend.putKerning(await sourceBackend.getKerning())
     await destBackend.putFeatures(await sourceBackend.getFeatures())
@@ -110,7 +108,7 @@ async def copyGlyphs(
     progressInterval: int,
     continueOnError: bool,
 ) -> list:
-    backgroundImageInfos = []
+    backgroundImageIdentifiers = []
 
     while glyphNamesToCopy:
         if progressInterval and not (len(glyphNamesToCopy) % progressInterval):
@@ -140,15 +138,15 @@ async def copyGlyphs(
         }
         glyphNamesToCopy.extend(sorted(componentNames - glyphNamesCopied))
 
-        for layerName, layer in glyph.layers.items():
+        for layer in glyph.layers.values():
             if layer.glyph.backgroundImage is not None:
-                backgroundImageInfos.append(
-                    (glyphName, layerName, layer.glyph.backgroundImage.identifier)
+                backgroundImageIdentifiers.append(
+                    layer.glyph.backgroundImage.identifier
                 )
 
         await destBackend.putGlyph(glyphName, glyph, glyphMap[glyphName])
 
-    return backgroundImageInfos
+    return backgroundImageIdentifiers
 
 
 async def mainAsync() -> None:

src/fontra/backends/designspace.py

@@ -214,6 +214,7 @@ def __init__(self, dsDoc: DesignSpaceDocument) -> None:
         self._glyphDependencies: GlyphDependencies | None = None
         self._backgroundTasksTask: asyncio.Task | None = None
         self._imageMapping = DoubleDict()
+        self._imageDataToWrite: dict[str, ImageData] = {}
         # Set this to true to set "public.truetype.overlap" in each writte .glif's lib:
         self.setOverlapSimpleFlag = False
         self._familyName: str | None = None
@@ -651,11 +652,13 @@ async def putGlyph(
                 if imageInfo is not None:
                     _, imageFileName = imageInfo
                 else:
-                    imageFileName = f"{layer.glyph.backgroundImage.identifier}.png"
+                    imageIdentifier = layer.glyph.backgroundImage.identifier
+                    imageFileName = f"{imageIdentifier}.png"
                     imageInfo = (ufoLayer.path, imageFileName)
-                    self._imageMapping[imageInfo] = (
-                        layer.glyph.backgroundImage.identifier
-                    )
+                    self._imageMapping[imageInfo] = imageIdentifier
+                    imageData = self._imageDataToWrite.pop(imageIdentifier, None)
+                    if imageData is not None:
+                        await self.putBackgroundImage(imageIdentifier, imageData)
 
             drawPointsFunc = populateUFOLayerGlyph(
                 layerGlyph,
@@ -1183,30 +1186,19 @@ async def getBackgroundImage(self, imageIdentifier: str) -> ImageData | None:
 
         return ImageData(type=ImageType.PNG, data=data)
 
-    async def putBackgroundImage(
-        self, imageIdentifier: str, glyphName: str, layerName: str, data: ImageData
-    ) -> None:
-        if glyphName not in self.glyphMap:
-            raise KeyError(glyphName)
-
+    async def putBackgroundImage(self, imageIdentifier: str, data: ImageData) -> None:
         if data.type != ImageType.PNG:
             raise NotImplementedError("convert image to PNG")
 
-        defaultStaticGlyph, defaultUFOGlyph = ufoLayerToStaticGlyph(
-            self.defaultUFOLayer.glyphSet, glyphName
-        )
-
-        layerNameMapping = defaultUFOGlyph.lib.get(LAYER_NAME_MAPPING_LIB_KEY, {})
-        revLayerNameMapping = {v: k for k, v in layerNameMapping.items()}
-        layerName = revLayerNameMapping.get(layerName, layerName)
-        ufoLayer = self.ufoLayers.findItem(fontraLayerName=layerName)
-
-        imageFileName = f"{imageIdentifier}.{data.type.lower()}"
-
-        ufoLayer.reader.writeImage(imageFileName, data.data, validate=True)
-
-        key = (ufoLayer.path, imageFileName)
-        self._imageMapping[key] = imageIdentifier
+        imageInfo = self._imageMapping.reverse.get(imageIdentifier)
+        if imageInfo is None:
+            # We don't yet know in which layer to write this image, let's postpone
+            # until putGlyph() comes across it.
+            self._imageDataToWrite[imageIdentifier] = data
+        else:
+            ufoPath, imageFileName = self._imageMapping.reverse[imageIdentifier]
+            reader = self.ufoManager.getReader(ufoPath)
+            reader.writeImage(imageFileName, data.data, validate=True)
 
     def _getImageIdentifier(self, ufoPath: str, imageFileName: str) -> str:
         key = (ufoPath, imageFileName)

src/fontra/backends/fontra.py

@@ -197,9 +197,7 @@ async def getBackgroundImage(self, imageIdentifier: str) -> ImageData | None:
 
         return None  # Image not found
 
-    async def putBackgroundImage(
-        self, imageIdentifier: str, glyphName: str, layerName: str, data: ImageData
-    ) -> None:
+    async def putBackgroundImage(self, imageIdentifier: str, data: ImageData) -> None:
         fileName = f"{imageIdentifier}.{data.type.lower()}"
         self.backgroundImagesDir.mkdir(exist_ok=True)
         path = self.backgroundImagesDir / fileName

src/fontra/core/protocols.py

@@ -101,9 +101,7 @@ async def getBackgroundImage(self, imageIdentifier: str) -> ImageData | None:
 
 @runtime_checkable
 class WriteBackgroundImage(Protocol):
-    async def putBackgroundImage(
-        self, imageIdentifier: str, glyphName: str, layerName: str, data: ImageData
-    ) -> None:
+    async def putBackgroundImage(self, imageIdentifier: str, data: ImageData) -> None:
         pass
 
     # TODO: since the image data does not itself participate in change messages,

test-py/test_backends_designspace.py

@@ -402,16 +402,17 @@ async def test_putBackgroundImage(writableTestFont):
 
     glyphName = "D"
     imageIdentifier = str(uuid.uuid4())
-    await writableTestFont.putBackgroundImage(
-        imageIdentifier, glyphName, layerName, imageData
-    )
+    await writableTestFont.putBackgroundImage(imageIdentifier, imageData)
+    glyph2 = deepcopy(glyph)
+    glyph2.layers[layerName].glyph.backgroundImage.identifier = imageIdentifier
+    await writableTestFont.putGlyph(glyphName, glyph2, [ord("D")])
 
     imageData2 = await writableTestFont.getBackgroundImage(imageIdentifier)
 
     assert imageData2 == imageData
 
 
-async def test_putBackgroundImage_new_font(testFont, tmpdir):
+async def test_putGlyph_with_backgroundImage_new_font(testFont, tmpdir):
     tmpdir = pathlib.Path(tmpdir)
     newFont = DesignspaceBackend.createFromPath(tmpdir / "test.designspace")