[krita] [Bug 431945] Comics Manager memory leak in Krita 4.4.2
https://bugs.kde.org/show_bug.cgi?id=431945 Dmitry Kazakov changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED Latest Commit||https://invent.kde.org/grap ||hics/krita/commit/0f532 ||9b6bdbf8b960677b9154ec749d8 ||6810 --- Comment #7 from Dmitry Kazakov --- Git commit 0f5329b6bdbf8b960677b9154ec749d86810 by Dmitry Kazakov, on behalf of Anestis Papazoglou. Committed on 25/06/2021 at 09:36. Pushed by dkazakov into branch 'master'. Fix excessive memory usage in Comics Manager docker Reverts back to using preview.png for thumbnails instead of mergedimage.png to avoid excessive memory usage and processing time when dealing with high resolution images. This partially reverts behaviour introduced in: b8b657d2ebf323a2007e870950c131ce5ee1e6f9 M +3-3 plugins/python/comics_project_management_tools/comics_project_manager_docker.py https://invent.kde.org/graphics/krita/commit/0f5329b6bdbf8b960677b9154ec749d86810 -- You are receiving this mail because: You are watching all bug changes.
[krita] [Bug 431945] Comics Manager memory leak in Krita 4.4.2
https://bugs.kde.org/show_bug.cgi?id=431945 mourgos changed: What|Removed |Added CC||papazoglou.anes...@gmail.co ||m --- Comment #6 from mourgos --- (In reply to Tiar from comment #5) > Looks like it's inevitable: performance is more important... you're welcome > to make a MR, but please: > 1) add a comment explaining why preview.png is used instead of > mergedimage.png (so no one else makes the same mistake again), > 2) keep other changes; for example the devicePixelRatioF() stuff (it keeps > it looking at least somewhat nice on 4k screens). (I'm afraid the pixel art > won't look good there anyway, so it might be a good idea to remove that part > that scales them up using FastTransformation; it will be blurred terribly > anyway, unfortunately... SmoothTransformation at least will have only one > kind of artifacts). > 3) It would be good if you checked if after your changes the pages in the > page viewer are still rendered nicely with high resolution. > > Thank you :) I have created a (quite minimal) MR: https://invent.kde.org/graphics/krita/-/merge_requests/920 I just changed the source image for the thumbnails in the docker, so the page viewer is unaffected as far as I can tell. Unfortunately I don't own any HiDPI displays, so I wouldn't be able to tell the difference if HiDPI support regressed. I don't anticipate the change to break anything, but if anyone with a HiDPI monitor could double check it would be greatly appreciated. As a future improvement, I think it would be possible to have higher resolution thumbnails if we allowed some preprocessing and additional storage in the comic project folder. We could generate dedicated higher res previews (512px) from the merged image every time we detected a change in a .kra file, and store it in a dedicated folder (e.g. /thumbnails). This could then be used to populate the thumbnail list instead of loading from the .kra files. I did not attempt to do that yet, because it would add quite a bit of complexity to do it right, and I first wanted a fix for the current problem. The main issue is that there is a nasty corner case when the .kra file gets modified without loading the project in the comic manager first, which can lead to stale thumbnails (this is fixable but a proper solution is complex). That and the fact that we'd need to create an additional folder on disk, which might be undesirable. However, if you are OK with the idea I could try to implement that once I get some free time. -- You are receiving this mail because: You are watching all bug changes.
[krita] [Bug 431945] Comics Manager memory leak in Krita 4.4.2
https://bugs.kde.org/show_bug.cgi?id=431945 --- Comment #5 from Tiar --- Looks like it's inevitable: performance is more important... you're welcome to make a MR, but please: 1) add a comment explaining why preview.png is used instead of mergedimage.png (so no one else makes the same mistake again), 2) keep other changes; for example the devicePixelRatioF() stuff (it keeps it looking at least somewhat nice on 4k screens). (I'm afraid the pixel art won't look good there anyway, so it might be a good idea to remove that part that scales them up using FastTransformation; it will be blurred terribly anyway, unfortunately... SmoothTransformation at least will have only one kind of artifacts). 3) It would be good if you checked if after your changes the pages in the page viewer are still rendered nicely with high resolution. Thank you :) -- You are receiving this mail because: You are watching all bug changes.
[krita] [Bug 431945] Comics Manager memory leak in Krita 4.4.2
https://bugs.kde.org/show_bug.cgi?id=431945 wolthera changed: What|Removed |Added Ever confirmed|0 |1 Status|REPORTED|ASSIGNED --- Comment #4 from wolthera --- Thanks! I had totally not seen this particular commit had changed the _source_ of the images, so I was completely puzzled, as I couldn't figure out which commit would've changed anything significantly. Guess I'll be semi-reverting tiar's commit. -- You are receiving this mail because: You are watching all bug changes.
[krita] [Bug 431945] Comics Manager memory leak in Krita 4.4.2
https://bugs.kde.org/show_bug.cgi?id=431945 --- Comment #3 from mourgos --- I managed to find some free time to investigate this and I believe I have found the cause of the issue, and it isn't a memory leak but rather an (unforseen?) change in behaviour. Looking at the history, I think the culprit is this commit: https://invent.kde.org/graphics/krita/-/commit/b8b657d2ebf323a2007e870950c131ce5ee1e6f9. In order to have higher resolution images to display, comics_project_manager_docker.fill_pages() was changed to read the mergedimage.png rather than preview.png. The problem is, that when loaded in memory (and as such decompressed), a merged image can take a significant amount of RAM. In my case, that's ~130MiB per file. Given that a comic/graphic novel has dozens of images, this quickly balloons to multiple gigabytes of ram just for page previews. Unsurprisingly, loading that many big images also takes a lot of time, which causes the delay I described before. I think this can be seen as a regression in functionality, as it uses up valuable RAM that would be otherwise be used for actual drawing. If the comic book is large enough, it might even exhaust all of the available RAM just by opening the project json file! In terms of solutions to this, I see two possible avenues to fix this: 1) Revert back to the old behaviour of loading preview.png instead of mergedimage.png. This will fix the memory/loading time issues, but will lead to smaller page previews (in my opinion not a huge issue). 2) Alternatively, if larger size page previews are a necessity, the mergedimage could be resized on the fly to something smaller before being used as an icon. This would make memory usage more sensible without sacrificing much in terms of visual quality, but it would increase the loading time even more given that we now would need to also resize multiple large images. Any thoughts on this? This might be a niche of a bug, but it prevents me from upgrading from 4.4.1, so I'd be more than willing to propose an MR if you are happy with one of the two options :) -- You are receiving this mail because: You are watching all bug changes.
[krita] [Bug 431945] Comics Manager memory leak in Krita 4.4.2
https://bugs.kde.org/show_bug.cgi?id=431945 Yncke changed: What|Removed |Added CC||yn...@hotmail.com -- You are receiving this mail because: You are watching all bug changes.
[krita] [Bug 431945] Comics Manager memory leak in Krita 4.4.2
https://bugs.kde.org/show_bug.cgi?id=431945 --- Comment #2 from mourgos --- The bug seems to still be present in Krita 4.4.3-beta2. -- You are receiving this mail because: You are watching all bug changes.
[krita] [Bug 431945] Comics Manager memory leak in Krita 4.4.2
https://bugs.kde.org/show_bug.cgi?id=431945 wolthera changed: What|Removed |Added CC||griffinval...@gmail.com --- Comment #1 from wolthera --- Er, those two lines are for when you are actually opening the krita file and when you are resizing batch resizing all files. Neither is happening when you just open the config file, and we did not change anything in the python code for that docker for 4.4.1. It uses fill_pages when loading the data, and there it loads it from the zip files, and those get shutdown too, so I don't know what is causing this. -- You are receiving this mail because: You are watching all bug changes.
