Problem with fetching embedded resources by MD5
Closed, ResolvedPublic

Description

Problem

  1. When loading/saving a resource with loadFromDevice()/saveToDevice() MD5 of the resource changes. Therefore, we cannot use resource->md5() when writing the embedded resource tag.
  1. We have a lot of presets in the default bundle, whose MD5 links are broken because of this problem.
  1. Right now we have a policy: if resource is not found by MD5, then we don't even check filename and name tags. It causes the embedded resource to be fetched all the time, because it cannot find the resource it has already loaded from embedded storage. It introduces huge delays on painting. See this bug https://bugs.kde.org/show_bug.cgi?id=443629

Proposed solution consists of two stages:

Stage 1:

  1. When loading an embedded resource, first try to fetch with the saved MD5 tag.
  2. If it fails, try to calculate MD5 of the embedded data and use it for fetching. This calculation takes about 4ms, which is quite a lot, so we should evaluate if this step is really needed.
  3. If it fails, fallback to filename and name to fetch the resource.

At this stage this bug will be "fixed": https://bugs.kde.org/show_bug.cgi?id=443629 Though we will not be guarded from this problem in the future. With every update of libpng and/or Qt MD5 links will become broken again, and we will basically use name/filename as keys. Therefore, I propose the second stage:

Stage 2:

  1. If resource is not present in the database, KoResource::embeddedResources() returns QByteArray with data of the resource instead of a built up KoResource object. It will let us preserve resource's MD5.
  2. This QByteArray is then passed to a modified version of KisResourceLocator::importResourceFromFile(), which copies the resource into the storage.
  3. For optionally returning QByteArray we can use a std::expected-like structure (needs research to check if it is possible).
  4. When saving an embedded resource, we should either generate MD5 for the actual saved data (instead of using resource->md5()) or add something like KisResourceLocator::exportResourceToFile(), which just copies underlying data from the storage and, therefore, preserves MD5.
rempt added a comment.Oct 14 2021, 1:59 PM

Yes, this sounds like a good plan.

rempt added a comment.Oct 14 2021, 2:01 PM

Oh, I probably would drop item 2 in stage 1.

Okay, thanks! I'll try to implement that tomorrow :)

dkazakov closed this task as Resolved.Dec 10 2021, 8:42 AM
dkazakov claimed this task.

The task is implemented and merged now! :)