Summary: | [Brush-Presets-Docker] flickers and slow down all Krita canvas performances in certain conditions | ||
---|---|---|---|
Product: | [Applications] krita | Reporter: | David REVOY <info> |
Component: | Dockers | Assignee: | Stefano Bonicatti <smjert> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | freebox64, halla, kde, mr.glaceon, smjert |
Priority: | NOR | ||
Version: | 2.9 | ||
Target Milestone: | --- | ||
Platform: | Compiled Sources | ||
OS: | Linux | ||
Latest Commit: | Version Fixed In: | ||
Sentry Crash Report: | |||
Attachments: | This patch sets fixed scrollbars to test |
Description
David REVOY
2015-03-09 09:23:41 UTC
Hm... It must have something to do with the brush docker relayout constantly. I haven't been able to reproduce the flicker yet, but that might just be because my desktop is too fast to show it. As to whether it's fixable -- it might even be a Qt issue. Or something weird in our layout code in the resource chooser. I can confirm both: the flickering of the UI, and the slowdowns of the brushes. It seems that the issue doesn't show up with the Favorite presets tag. (Ubuntu 14.04 Unity - Krita 2.9.1). ...well, actually it shows up also using the "Favorite Presets" tag if many brush presets are present. Created attachment 91542 [details]
This patch sets fixed scrollbars to test
I spent some hours yesterday evening to try to tackle this issue down. I'm not entirely sure whether it is the cause or simply a symptom but it might be that the problem lies in updateView() in KoResourceItemView. It calculates the width of the columns from the viewports width and then sets them. However, when the scrollbars are set to ScrollBarAsNeeded, they don't know what to do and there seems to be some looping going on because when the scrollbars are showing themselves they cause a resize that in turn calls updateView() again etc.
This patch hides the horizontal scrollbar and always shows the vertical one when using fixed columns for testing purposes. Unfortunately I wasn't able to reproduce the slowdown issue but it might be well worth it to test the patch if you are affected.
To make a proper fix would require some time, but it would be good to know to anyone who wants to fix it whether the patch goes into the right direction.
Yes, that sounds very likely. Hi Mmolch, I tested the patch, and here is the report : 1. I could still resize the brush preset docker to make it flickers. 2. But the performances/big slowdown is now not as dramatic as in calligra/2.9 : It's slow but the stroke end and no missing end-tail, or brush decoration outline changing to another brush. So, I notice an improvement. 3. A side effect: Krita main windows could not be resized in my WM ( on XFCE right now ), just maximised or the windowed floating mode was locked in a mini position. I reverted the patch after test. *** Bug 348778 has been marked as a duplicate of this bug. *** I've compiled Krita and calligra with -O0 and -g, then i've put a kRealBacktrace inside KoResourceItemView::updateView and that helped a lot pinpointing the issue. What i saw is that alternately there was a call path coming from KoResourceItemChooser::baseLengthChanged and one coming from KoResourceItemView::resizeEvent. A third one, but that was not repeating so much times indefinitely, but basicly only when actually resizing by hand, comes from KoResourceItemChooser::setSynced, called by KisPresetChooser::resizeEvent. Now KoResourceItemChooser::baseLengthChanged is called by KoResourceItemChooser::updateView which is connected to the signal sigSizeChanged which is emitted by KoResourceItemView::resizeEvent, as you can see here then we have already 2 calls of updateView, happening together, that comes from the resizeEvent (one through a signal, one through a direct call). Now given that the profiler shows a high amount of time spent in viewportEvent (because it end up calling the scaling of the images of the brushes) and that updateView (of the Ko[...]View) when calling setColumnWidth or setRowWidth end up generating a new viewportEvent (the stack traces always show that Err while assigning to myself this issue i didn't saw i had a comment half wrote from yesterday. Disregard that comment... a better explanation can be given by simply checking, in updateView, the rowHeight and columnWidth, current and desired, and when the scrollbar appears. mmolch basically said it, as soon as scrollbar appears, updateView code tries to fit the tiles horizontally, by changing the columnWidth and the rowHeight (because tiles are squares), but this tiles height reduction is enough to not need the scrollbars anymore, so they hide and cause a viewport expansion in width.. which causes an expansion of tiles (width and height), and again the tiles height is too much to fit in the viewport, so the scrollbar appears again and the loop continues. Now i see only two ways of fixing this: 1) mmolch way, always hide the horizontal bar (this isn't needed anyway because it should always fit horizontally) and show the vertical bar so that no width resize happens due to scrollbar appearing. 2) Always leave some margin on the right side to account for scrollbars... even if this is a bit ugly (tiles not perfectly fitting) and maybe difficult to pre-calculate the expected scrollbar size. In the end the cleanest way for me is 1, though i don't get why David still gets flicker, can you make a short video with that issue and the patch applied + the main window resizing issue you have? Because when i tested i had no more slowdowns and i was able to resize the main windows just fine. >> In the end the cleanest way for me is 1
Ok , I suggest to push the patch in 2.9 and master to ease further test. ( It can't hurt afaik ) It will be easier for me to see how the docker behaves on a week of painting and this way I can keep track of other commit too. As soon as I can reproduce, I'll screencapture.
The problem is that the patch is in Ko.. class, that means that comes from calligra and has to be reviewed first. So i can open the review and prepare a branch with it but "nothing else". https://git.reviewboard.kde.org/r/124042/ here is the review and krita-presetdockerflickering-bonicatti is the branch to test the patch. Hello, and thank you for the patch. It works for me on Ubuntu 14.04 - Unity. As an aside, I think that it could possibly solve a little issue currently affecting the Palette docker with a large number of swatches. Steps: 1. Ctrl+mouse-wheel over the swatches to enlarge their sizes and so adjusting the total width of the rows to fit to the opening (without triggering the horizontal scrollbar). 2. Click-and-drag the top or the bottom edge of the Palette docker, to reduce the height of the docker so to trigger the vertical scrollbars. Result: The swatches are not resized to fit to the new width of the opening and the horizontal scrollbar appears. Hope you don't mind for this digression. Thanks Git commit 2c592c5872a9e35e0b4c17a1c3a97da02a183c89 by Stefano Bonicatti. Committed on 20/06/2015 at 08:53. Pushed by stefanobonicatti into branch 'calligra/2.9'. Fix preset docker flickering and slowdown When vertically resizing the preset docker and the vertical scrollbar appears, the preset tiles are resized to fit the viewport with their width, but this also reduces their height not needing the scrollbar anymore. The scrollbar disappearing lets the viewport expand and so the tiles, in height, making the scrollbar needed again and this loops forever generating a considerable slowdown. The fix consists in always showing the vertical scrollbar so that no resizing happens due to them. To avoid unnecessary work and event spam we check in KoResourceItemChooser::setSynced function if the value passed corresponds to its current status already. REVIEW: 124042 Thanks to mmolch for the initial patch. M +3 -0 libs/widgets/KoResourceItemChooser.cpp M +15 -1 libs/widgets/KoResourceItemView.cpp http://commits.kde.org/calligra/2c592c5872a9e35e0b4c17a1c3a97da02a183c89 Hey, I updated my install of calligra/2.9, and I noticed I can't reproduce this bug anymore. Thank you! A bit of bug triage here on my threads , this one have to be marked as 'RESO' too :3 |