Bug 435234 - Brush Preset History causes malfunctions after closing a document
Summary: Brush Preset History causes malfunctions after closing a document
Status: RESOLVED FIXED
Alias: None
Product: krita
Classification: Applications
Component: Resource Management (other bugs)
Version First Reported In: git master (please specify the git hash!)
Platform: Compiled Sources Linux
: NOR normal
Target Milestone: ---
Assignee: Halla Rempt
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-04-01 20:27 UTC by Lynx3d
Modified: 2021-04-07 12:11 UTC (History)
1 user (show)

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


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Lynx3d 2021-04-01 20:27:39 UTC
As soons as you close a document, various preset related actions stop working correctly.

STEPS TO REPRODUCE
1. Open or create a document
2. Change current brush presets a few times
3. Close the document
4. Open or create a document again
5. Click on any entry in the "Brush Preset History" docker
6. Use the hotkey for "Switch to Previous Preset" ("/" by default)

OBSERVED RESULT
It gets stuck on the same brush preset rather than switching between the last two when pressing "/".
Other side effects are that the brush option sliders of the toolbar doesn't update correctly anymore (while the brush HUD of the popup palette still does).

EXPECTED RESULT
Work like before closing a document


SOFTWARE/OS VERSIONS
Linux/KDE Plasma: Ubuntu 20.04 
KDE Plasma Version: 5.18.5
KDE Frameworks Version: 5.68.0
Qt Version: 5.12.8

ADDITIONAL INFORMATION
git commit: e97414d541bd9de8e080cf4cee0ce3d2c9ab1973

My breakdown so far:
Closing a document makes KisResourceLocator purge all references to currently allocated resources.
This makes resource models return new instances after the purge, because they get it from KisResourceLocator cache.

The Brush Preset History however keeps shared pointers to the old instance. When clicking a history entry, this old one gets passed to KisPaintopBox, which updates current and previous presets in KisCanvasResourceProvider, and then passes it on to the KisPaintOpPresetsEditor.

The editor in turn sets its own resource chooser to this preset, however its resource model sees a new memory instance for this resource ID, fires a change signal with that, and KisPaintopBox in turn writes this new instance as current canvas resource again, overwriting the actual previous preset with the old instance of the current preset because it only goes by pointer values.

Now this could be solved not just matching resources by their shared pointer value etc., but I have my doubts it is acceptable to have the same resource in memory in the first place, not just because of wasted memory, but I'm not familiar enough with how resource changes get synched with the storages to judge if they may lead to inconsistent states.
Comment 1 Halla Rempt 2021-04-07 12:11:25 UTC
Git commit 55ef3d30cd6647c36b033b06a7bc40b40eeac9bb by Halla Rempt.
Committed on 07/04/2021 at 12:11.
Pushed by rempt into branch 'master'.

Purge the resourcecache selectively

A resource object should remain valid for the entire lifetime of
Krita, unless the storage that owns is is _deleted_ -- not just
deactivated.

M  +10   -6    libs/resources/KisResourceLocator.cpp
M  +1    -1    libs/resources/KisResourceLocator.h
M  +1    -1    libs/resources/tests/TestResourceModel.cpp

https://invent.kde.org/graphics/krita/commit/55ef3d30cd6647c36b033b06a7bc40b40eeac9bb