Bug 487866 - Importing KPP files with embedded resources fails.
Summary: Importing KPP files with embedded resources fails.
Status: RESOLVED FIXED
Alias: None
Product: krita
Classification: Applications
Component: Resource Management (show other bugs)
Version: 5.2.2
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Dmitry Kazakov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-06-01 04:57 UTC by quytelda
Modified: 2024-10-24 11:36 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
Example KPP file that fails to load (50.70 KB, image/png)
2024-06-01 04:57 UTC, quytelda
Details
Full output log when reproducing the bug (7.82 KB, text/x-log)
2024-06-01 04:59 UTC, quytelda
Details

Note You need to log in before you can comment on or make changes to this bug.
Description quytelda 2024-06-01 04:57:35 UTC
Created attachment 170033 [details]
Example KPP file that fails to load

SUMMARY

Importing KPP files with embedded resources fails.

I have an example KPP file (attached) that contains two embedded resources (a brush tip and a pattern). It was created using Krita 5.2.2, but cannot be imported correctly into a fresh Krita installation. I also tried several other brushes with embedded resources and they also fail to load, but brushes without embedded resources are loaded fine. Interestingly, the embedded resources do import correctly if the preset is imported as part of a resource bundle.

STEPS TO REPRODUCE
1. Go to Settings -> Manage Resources and press the "Import Resources" button to import a new brush preset.
2. Choose a brush preset that contains embedded resources.
3. Open a new file, select the brush tool, and choose the imported preset.

OBSERVED RESULT
The imported preset does not use the embedded resources. Several error messages are printed to the console after the preset is selected:

```
SAFE ASSERT (krita): "data" in file /usr/src/debug/krita/krita-5.2.2/plugins/paintops/libpaintop/kis_brush_option_widget.cpp, line 111
Failed to load embedded resource KoResourceSignature("patterns", "3ca1bcf8dc1bc90b5a788d89793d2a89", "hourglass.png", "hourglass.png")
createLocalResourcesSnapshot: failed to load a linked resource: KoResourceSignature("brushes", "b877c93efe4540891304ae3662e9ce58", "egg.png", "")
createLocalResourcesSnapshot: Could not import embedded resource KoResourceSignature("patterns", "3ca1bcf8dc1bc90b5a788d89793d2a89", "hourglass.png", "hourglass.png")
```

EXPECTED RESULT
The imported preset should import the embedded resources correctly.

SOFTWARE/OS VERSIONS
Arch Linux (up-to-date)
Krita 5.2.2 (package: krita-5.2.2-9)
Desktop: GNOME (Wayland)
Linux/KDE Plasma: 
(available in About System)
Qt Version: 5.15.14

ADDITIONAL INFORMATION
At first I thought the KPP files were the problem, so I inspected their contents manually. I can confirm that they do actually contain the embedded resources and that the md5 checksum values match.

I also tried downgrading to Krita 5.1 but the bug still remained.
Comment 1 quytelda 2024-06-01 04:59:18 UTC
Created attachment 170034 [details]
Full output log when reproducing the bug
Comment 2 Freya Lupen 2024-07-01 14:33:10 UTC
Confirmed on 5.2.3, importing shape.kpp and selecting it gives all those warnings.

I think it might be related to an issue in how "Texture/Pattern/PatternMD5" is being saved.
In the test brush shape.kpp, it's a string "<![CDATA[XXX]]>" where XXX is unprintable control characters that break XML parsing.
In the default brush "g) Dry Textured Creases" it's  a bytearray "4N3Zfs+z3ckn4Al2hcS8OA==".
But if you save a copy of that default brush, it also gets saved as a cstring, and it also shows the same behavior.
Related bug reports: BUG:456586, BUG:456197
Comment 3 Dmitry Kazakov 2024-10-24 11:33:15 UTC
Git commit 072460560b704e9ab36f488b35081bf49c7b9edd by Dmitry Kazakov.
Committed on 24/10/2024 at 11:27.
Pushed by dkazakov into branch 'master'.

Fix loading .kpp files with embedded top-level resources

The patch fixes multiple issues with loading embedded (side-loaded) resources
from the .kpp files:

1) Side-loaded resources were **never** loaded (since the very beginning of 5.0),
    because `reader.text("embedded_resources").toInt()` was read from a wrong
    place. The number of resources is stored in XML data, not in the PNG's metadata
    field.

    Now Krita doesn't check the resource count at all and just iterates over the XML to
    find "resource" elements.

2) A new KoResource::sideLoadedResources() method is introduced. It returns a one-time-
    available resources which are loaded alongside the main resource. These side-loaded
    resources can be expelled from memory by calling clearSideLoadedResources(), which
    is called by KisResourceLocator right after loading the resources into the memory.

3) KisResourceLocator now calls loadRequiredResources() in all code paths that load the
    resource into memory (to make sure all its embedded resources are loaded). Previously
    it required a Krita restart to load the embedded resources, cause it happened only during
    the normal lazy-loading.

4) Fixed KisEmbeddedTextureData::loadLinkedPattern() to return FailedLink resource
    instead of invalid EmbeddedResource, when the actual "patternBase64" tag is missing
    (it is missing in 5.0 version of the presets)

5) Adds a unittest that checks all these peculiarities (see KisPaintOpPresetTest)

NOTE:
Bug 456197 is "kind of fixed" with this patch, because side-loaded resources are now
loaded correctly. Though the problem may still be valid for other resource types that
do not side-load the dependencies.
Related: bug 456586, bug 456197

M  +50   -30   libs/image/brushengine/kis_paintop_preset.cpp
M  +3    -0    libs/image/brushengine/kis_paintop_preset.h
M  +1    -0    libs/image/tests/CMakeLists.txt
A  +108  -0    libs/image/tests/KisPaintOpPresetTest.cpp     [License: GPL(v2.0+)]
A  +21   -0    libs/image/tests/KisPaintOpPresetTest.h     [License: GPL(v2.0+)]
A  +-    --    libs/image/tests/data/test-embedded-resources-2.2.kpp
A  +-    --    libs/image/tests/data/test-embedded-resources-5.0.kpp
M  +39   -21   libs/resources/KisResourceLocator.cpp
M  +17   -0    libs/resources/KoResource.cpp
M  +28   -0    libs/resources/KoResource.h
M  +6    -1    plugins/paintops/libpaintop/KisEmbeddedTextureData.cpp

https://invent.kde.org/graphics/krita/-/commit/072460560b704e9ab36f488b35081bf49c7b9edd
Comment 4 Dmitry Kazakov 2024-10-24 11:36:20 UTC
Git commit 869cad0ce67c7786058de47c42909a9dd285597f by Dmitry Kazakov.
Committed on 24/10/2024 at 11:35.
Pushed by dkazakov into branch 'krita/5.2'.

Fix loading .kpp files with embedded top-level resources

The patch fixes multiple issues with loading embedded (side-loaded) resources
from the .kpp files:

1) Side-loaded resources were **never** loaded (since the very beginning of 5.0),
    because `reader.text("embedded_resources").toInt()` was read from a wrong
    place. The number of resources is stored in XML data, not in the PNG's metadata
    field.

    Now Krita doesn't check the resource count at all and just iterates over the XML to
    find "resource" elements.

2) A new KoResource::sideLoadedResources() method is introduced. It returns a one-time-
    available resources which are loaded alongside the main resource. These side-loaded
    resources can be expelled from memory by calling clearSideLoadedResources(), which
    is called by KisResourceLocator right after loading the resources into the memory.

3) KisResourceLocator now calls loadRequiredResources() in all code paths that load the
    resource into memory (to make sure all its embedded resources are loaded). Previously
    it required a Krita restart to load the embedded resources, cause it happened only during
    the normal lazy-loading.

4) Fixed KisEmbeddedTextureData::loadLinkedPattern() to return FailedLink resource
    instead of invalid EmbeddedResource, when the actual "patternBase64" tag is missing
    (it is missing in 5.0 version of the presets)

5) Adds a unittest that checks all these peculiarities (see KisPaintOpPresetTest)

NOTE:
Bug 456197 is "kind of fixed" with this patch, because side-loaded resources are now
loaded correctly. Though the problem may still be valid for other resource types that
do not side-load the dependencies.
Related: bug 456586, bug 456197

M  +50   -30   libs/image/brushengine/kis_paintop_preset.cpp
M  +3    -0    libs/image/brushengine/kis_paintop_preset.h
M  +1    -0    libs/image/tests/CMakeLists.txt
A  +108  -0    libs/image/tests/KisPaintOpPresetTest.cpp     [License: GPL(v2.0+)]
A  +21   -0    libs/image/tests/KisPaintOpPresetTest.h     [License: GPL(v2.0+)]
A  +-    --    libs/image/tests/data/test-embedded-resources-2.2.kpp
A  +-    --    libs/image/tests/data/test-embedded-resources-5.0.kpp
M  +39   -21   libs/resources/KisResourceLocator.cpp
M  +17   -0    libs/resources/KoResource.cpp
M  +28   -0    libs/resources/KoResource.h
M  +6    -1    plugins/paintops/libpaintop/KisEmbeddedTextureData.cpp

https://invent.kde.org/graphics/krita/-/commit/869cad0ce67c7786058de47c42909a9dd285597f