From 357bc2f819a57f0f5777b20fbdaf41bc3d41bb57 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Thu, 5 Dec 2024 08:08:03 +0100 Subject: [PATCH 1/5] First draft of: clean up background image data --- src/fontra/backends/designspace.py | 18 ++++++++++++++++++ src/fontra/client/core/font-controller.js | 19 +++++++++++++++++++ src/fontra/core/fonthandler.py | 8 ++++++++ src/fontra/core/protocols.py | 4 ++-- src/fontra/views/editor/editor.js | 3 +++ test-py/test_backends_designspace.py | 14 ++++++++++++++ 6 files changed, 64 insertions(+), 2 deletions(-) diff --git a/src/fontra/backends/designspace.py b/src/fontra/backends/designspace.py index 497d0a4e7..c8f4a0ae8 100644 --- a/src/fontra/backends/designspace.py +++ b/src/fontra/backends/designspace.py @@ -1200,6 +1200,24 @@ async def putBackgroundImage(self, imageIdentifier: str, data: ImageData) -> Non reader = self.ufoManager.getReader(ufoPath) reader.writeImage(imageFileName, data.data, validate=True) + async def deleteBackgroundImage(self, imageIdentifier: str) -> None: + imageInfo = self._imageMapping.reverse.get(imageIdentifier) + if imageInfo is None: + return + + ufoPath, imageFileName = self._imageMapping.reverse[imageIdentifier] + # a temp folder is not needed if we get back the image from cache. + + # # before we remove the image: copy to temporary folder + # # so that we can restore it if needed + # temp_dir = pathlib.Path(ufoPath) / "temp" + # temp_dir.mkdir(exist_ok=True) + + reader = self.ufoManager.getReader(ufoPath) + # reader.copyFromReader(reader, f"images/{imageFileName}", f"temp/{imageFileName}") + + reader.removeImage(imageFileName) + def _getImageIdentifier(self, ufoPath: str, imageFileName: str) -> str: key = (ufoPath, imageFileName) imageIdentifier = self._imageMapping.get(key) diff --git a/src/fontra/client/core/font-controller.js b/src/fontra/client/core/font-controller.js index fa67618e2..601caeade 100644 --- a/src/fontra/client/core/font-controller.js +++ b/src/fontra/client/core/font-controller.js @@ -264,6 +264,10 @@ export class FontController { }); } + async deleteBackgroundImageData(imageIdentifier) { + await this.font.deleteBackgroundImage(imageIdentifier); + } + getCachedGlyphNames() { return this._glyphsPromiseCache.keys(); } @@ -853,6 +857,21 @@ export class FontController { if (isRedo) { undoRecord = reverseUndoRecord(undoRecord); } + + // the following code uses the cached images to put it back to the backend + if ( + !isRedo && + undoRecord?.rollbackChange?.a && + undoRecord.rollbackChange.a[0] === "backgroundImage" + ) { + const backgroundImage = undoRecord.rollbackChange["a"][1]; + // get cached image via getBackgroundImage + const image = await this.getBackgroundImage(backgroundImage.identifier); + // if image saved in the cache, put it back to the backend + if (image) { + await this.putBackgroundImageData(backgroundImage.identifier, image.src); + } + } // Hmmm, would be nice to have this abstracted more await this.applyChange(undoRecord.rollbackChange); const error = await this.editFinal( diff --git a/src/fontra/core/fonthandler.py b/src/fontra/core/fonthandler.py index d484d8c41..5698ebf17 100644 --- a/src/fontra/core/fonthandler.py +++ b/src/fontra/core/fonthandler.py @@ -283,6 +283,14 @@ async def getBackgroundImage( type=imageData.type, data=base64.b64encode(imageData.data).decode("ascii") ) + @remoteMethod + async def deleteBackgroundImage( + self, imageIdentifier: str, *, connection=None + ) -> dict | None: + if not hasattr(self.backend, "deleteBackgroundImage"): + return None + await self.backend.deleteBackgroundImage(imageIdentifier) + def _getClientData(self, connection, key, default=None): return self.clientData[connection.clientUUID].get(key, default) diff --git a/src/fontra/core/protocols.py b/src/fontra/core/protocols.py index 0010cebb5..43d9e0422 100644 --- a/src/fontra/core/protocols.py +++ b/src/fontra/core/protocols.py @@ -106,8 +106,8 @@ async def putBackgroundImage(self, imageIdentifier: str, data: ImageData) -> Non # TODO: since the image data does not itself participate in change messages, # we may depend on the backend itself to purge unused images. - # async def deleteBackgroundImage(self, imageIdentifier: str) -> None: - # pass + async def deleteBackgroundImage(self, imageIdentifier: str) -> None: + pass @runtime_checkable diff --git a/src/fontra/views/editor/editor.js b/src/fontra/views/editor/editor.js index 82d23515d..af312fda8 100644 --- a/src/fontra/views/editor/editor.js +++ b/src/fontra/views/editor/editor.js @@ -2435,6 +2435,9 @@ export class EditorController { if (backgroundImageSelection) { // TODO: don't delete if bg images are locked // (even though we shouldn't be able to select them) + this.fontController.deleteBackgroundImageData( + layerGlyph.backgroundImage.identifier + ); layerGlyph.backgroundImage = undefined; } } diff --git a/test-py/test_backends_designspace.py b/test-py/test_backends_designspace.py index ac3c5fcef..f985d1629 100644 --- a/test-py/test_backends_designspace.py +++ b/test-py/test_backends_designspace.py @@ -414,6 +414,20 @@ async def test_putBackgroundImage(writableTestFont): assert imageData2 == imageData +async def test_deleteBackgroundImage(writableTestFont): + glyph = await writableTestFont.getGlyph("C") + for layerName, layer in glyph.layers.items(): + bgImage = layer.glyph.backgroundImage + if bgImage is not None: + break + + await writableTestFont.deleteBackgroundImage(bgImage.identifier) + + imageData3 = await writableTestFont.getBackgroundImage(bgImage.identifier) + + assert imageData3 is None + + async def test_putGlyph_with_backgroundImage_new_font(testFont, tmpdir): tmpdir = pathlib.Path(tmpdir) newFont = DesignspaceBackend.createFromPath(tmpdir / "test.designspace") From 9a4338544b2885bb94c0715c6d2e0332a0ca6c6c Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Thu, 5 Dec 2024 08:24:26 +0100 Subject: [PATCH 2/5] Adding deleteBackgroundImage to FontraBackend --- src/fontra/backends/fontra.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/fontra/backends/fontra.py b/src/fontra/backends/fontra.py index 21ef76211..c54a29ede 100644 --- a/src/fontra/backends/fontra.py +++ b/src/fontra/backends/fontra.py @@ -203,6 +203,13 @@ async def putBackgroundImage(self, imageIdentifier: str, data: ImageData) -> Non path = self.backgroundImagesDir / fileName path.write_bytes(data.data) + async def deleteBackgroundImage(self, imageIdentifier: str) -> None: + for imageType in [ImageType.PNG, ImageType.JPEG]: + fileName = f"{imageIdentifier}.{imageType.lower()}" + path = self.backgroundImagesDir / fileName + if path.is_file(): + path.unlink() + async def getCustomData(self) -> dict[str, Any]: return deepcopy(self.fontData.customData) From 87b24a89cbe95ad32ceaaa30a5abd73f46a589b3 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Thu, 5 Dec 2024 08:42:15 +0100 Subject: [PATCH 3/5] Make redo delete the image --- src/fontra/client/core/font-controller.js | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/fontra/client/core/font-controller.js b/src/fontra/client/core/font-controller.js index 601caeade..0fe1640bc 100644 --- a/src/fontra/client/core/font-controller.js +++ b/src/fontra/client/core/font-controller.js @@ -858,20 +858,24 @@ export class FontController { undoRecord = reverseUndoRecord(undoRecord); } - // the following code uses the cached images to put it back to the backend if ( - !isRedo && undoRecord?.rollbackChange?.a && undoRecord.rollbackChange.a[0] === "backgroundImage" ) { - const backgroundImage = undoRecord.rollbackChange["a"][1]; - // get cached image via getBackgroundImage - const image = await this.getBackgroundImage(backgroundImage.identifier); - // if image saved in the cache, put it back to the backend - if (image) { - await this.putBackgroundImageData(backgroundImage.identifier, image.src); + if (!isRedo) { + // the following code uses the cached image to put it back to the backend + const backgroundImage = undoRecord.rollbackChange["a"][1]; + const image = await this.getBackgroundImage(backgroundImage.identifier); + if (image) { + await this.putBackgroundImageData(backgroundImage.identifier, image.src); + } + } else { + const backgroundImage = undoRecord.change["a"][1]; + // if we do redo, we need to delete the image from the backend + await this.deleteBackgroundImageData(backgroundImage.identifier); } } + // Hmmm, would be nice to have this abstracted more await this.applyChange(undoRecord.rollbackChange); const error = await this.editFinal( From 36963fcfad7c8da04548a0aaf5874fc23231679c Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Thu, 5 Dec 2024 11:18:05 +0100 Subject: [PATCH 4/5] Add missing return --- src/fontra/core/fonthandler.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/fontra/core/fonthandler.py b/src/fontra/core/fonthandler.py index 5698ebf17..5db9ed495 100644 --- a/src/fontra/core/fonthandler.py +++ b/src/fontra/core/fonthandler.py @@ -290,6 +290,7 @@ async def deleteBackgroundImage( if not hasattr(self.backend, "deleteBackgroundImage"): return None await self.backend.deleteBackgroundImage(imageIdentifier) + return def _getClientData(self, connection, key, default=None): return self.clientData[connection.clientUUID].get(key, default) From 45c173d2b243901f7a70e8fe049c06abef9d8089 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Thu, 5 Dec 2024 11:40:27 +0100 Subject: [PATCH 5/5] Fix return value expected --- src/fontra/core/fonthandler.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/fontra/core/fonthandler.py b/src/fontra/core/fonthandler.py index 5db9ed495..fdad64162 100644 --- a/src/fontra/core/fonthandler.py +++ b/src/fontra/core/fonthandler.py @@ -289,8 +289,7 @@ async def deleteBackgroundImage( ) -> dict | None: if not hasattr(self.backend, "deleteBackgroundImage"): return None - await self.backend.deleteBackgroundImage(imageIdentifier) - return + return await self.backend.deleteBackgroundImage(imageIdentifier) def _getClientData(self, connection, key, default=None): return self.clientData[connection.clientUUID].get(key, default)