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

Clean up unused background images #1841

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft
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
18 changes: 18 additions & 0 deletions src/fontra/backends/designspace.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 7 additions & 0 deletions src/fontra/backends/fontra.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
23 changes: 23 additions & 0 deletions src/fontra/client/core/font-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,10 @@ export class FontController {
});
}

async deleteBackgroundImageData(imageIdentifier) {
await this.font.deleteBackgroundImage(imageIdentifier);
}

getCachedGlyphNames() {
return this._glyphsPromiseCache.keys();
}
Expand Down Expand Up @@ -853,6 +857,25 @@ export class FontController {
if (isRedo) {
undoRecord = reverseUndoRecord(undoRecord);
}

if (
undoRecord?.rollbackChange?.a &&
undoRecord.rollbackChange.a[0] === "backgroundImage"
) {
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(
Expand Down
8 changes: 8 additions & 0 deletions src/fontra/core/fonthandler.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
return await self.backend.deleteBackgroundImage(imageIdentifier)

def _getClientData(self, connection, key, default=None):
return self.clientData[connection.clientUUID].get(key, default)

Expand Down
4 changes: 2 additions & 2 deletions src/fontra/core/protocols.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions src/fontra/views/editor/editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
14 changes: 14 additions & 0 deletions test-py/test_backends_designspace.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Loading