SUMMARY This issue is rather hard to describe, but I've made a recording of it. (Unfortunately I had to use a phone camera, recording in X doesn't work properly.) https://ipfs.io/ipfs/QmSLSkmnwsSyBVq8e9wgF8zuH74LFz43AWXYNLJ56jMpY2/krunner_jank.mp4 You can see from watching the video that there are several sources of visual "jank". Step through the one starting at 9 seconds with me, frame by frame. 1. First the basic krunner menu appears. 2. "Colors - System Settings" appears (weird, it's not the thing that ultimately gets highlighted) 3. Very briefly, the krunner menu gets shifted about an inch down the screen. 4. krunner gets shifted back to the top of the screen, and the full size menu appears, but now a file (breeze-settings.svg) is selected. This is weird, because it's an option halfway down the screen and krunner should definitely know it won't be the default selection 5. The selection is (finally) updated to be the first menu choice, System Settings. Sometimes the scrollbar flashes on and then off too. You can see different versions of these issues by stepping through the other parts of the recording. All of these issues are combined with some very fast repaints, which looks like "flashing", which is already annoying even for me, possibly a serious problem for someone with certain forms of epilepsy. I'm almost certain it wasn't like this in 5.19. It's possible that krunner was made faster than before with this new version, so now the repaints happen more quickly so more "flashing" is noticeable. SOFTWARE/OS VERSIONS Linux: Arch Linux 5.9.0-arch1-1 KDE Plasma Version: 5.20.0 (no option for this in the version box yet) KDE Frameworks Version: 5.75.0 Qt Version: 5.15.1 HARDWARE Intel HD Graphics 3000 Compositing enabled with OpenGL 3.1
To clarify what I meant by '"janky" when loading new results': the specific animation jank in the video happens when I toggle krunner with a search query already loaded, but it *also* appears when I type in a new search query, sometimes after every new character. It's really bad looking, I can provide another video if necessary.
By my understanding there are two issues here 1: The flashing. When you type the runners get queried for each letter typed. And if for one letter there are plenty of results, but for the next letter less the window resizes and it looks like it is flashing. Funnily I had talked to the former KRunner maintainer about this yesterday. 2: The selection kindof jumps around when new entries append. This might be because some runners take longer/get queried delayed or are simply later in the queue. (I have added some new properties to the KRunner lib which will help with the last mentioned issue). Can you please try out if the issue also exists in the in the Search plasmoid? Of course there it only appears when typing ;) And the Application Launcher has the advantage that the window is not resizing.
*** Bug 358252 has been marked as a duplicate of this bug. ***
(In reply to Alexander Lohnau from comment #2) > By my understanding there are two issues here There's a third issue as well, visible in the video, which I described as stage "3". I have krunner docked at the top, but for a brief instant (a couple of frames) the textbox will sometimes appear lower down the screen by an inch or more, then flash back to the correct position. > Can you please try out if the issue also exists in the in the Search > plasmoid? Of course there it only appears when typing ;) It does not, as far as I can tell. In fact the painting strategy appears to be the exact opposite of krunner: after I type each new character, the results freeze until *all* new search results are loaded in. krunner repaints *every* time a new section gets loaded (e.g. one paint for files, one paint for applications, etc). The selection is *always* the first (default) option. Furthermore, my normal typing speed (or even typing deliberately a bit slower) is actually fast enough that ordinarily the search widget does not repaint *at all* until I've stopped typing. But (maybe because it's loading results from each runner individually instead of waiting for them all to respond?) krunner frequently updates while I'm still typing. This exacerbates the "flashing" issue.
*** Bug 374593 has been marked as a duplicate of this bug. ***
I did some research into this. We have a few problems, the most notable is Plasma::Dialog and size hints. It goes like this: contentHeight updates so the layout implicit height changes so the root item implicitHeight changes Dialog gets notified *HAS A FRICKIN' TIMER* Then the dialog resizes Layout then gets squashed ListView gets adjusted --- Because we now have the busy indicator we're rendering frames constantly which means we can have a frame queued at the point where the frame happens. It's even likely given the main thread has been stalled adding results for so long. The problematic part is the docs say the queued event is required (presumably for some legacy reason) so maybe it's reason to introduce Dialog2
It is annoying because I have to wait for the window to stabilize to find out which option is selected. This often leads to unintentional opening of things.
(In reply to José from comment #7) > It is annoying because I have to wait for the window to stabilize to find > out which option is selected. This often leads to unintentional opening of > things. A workaround would be to disable runners you don't use anyways. Especially if they produce a lot of results.
*** Bug 430326 has been marked as a duplicate of this bug. ***
A possibly relevant merge request was started @ https://invent.kde.org/plasma/milou/-/merge_requests/36
(In reply to Alexander Lohnau from comment #2) > 2: The selection kindof jumps around when new entries append. This will be fixed by https://invent.kde.org/plasma/milou/-/merge_requests/37 > This might be > because some runners take longer/get queried delayed or are simply later in > the queue. (I have added some new properties to the KRunner lib which will > help with the last mentioned issue). The reason for the bug is not that, it is detailed inside the MR
A possibly relevant merge request was started @ https://invent.kde.org/frameworks/plasma-framework/-/merge_requests/421
Git commit 320676ded0d47b52d8d910f1371f908fb6d67083 by David Edmundson, on behalf of Eduardo Cruz. Committed on 13/01/2022 at 11:53. Pushed by davidedmundson into branch 'master'. PlasmaQuick::Dialog - Fix flickering issues when resizing (specially in krunner) The goal is to fix the flickering issue with KRunner as reported in [BUG 427672](https://bugs.kde.org/show_bug.cgi?id=427672). The bug report deals with 2 issues: the quick reordering of the results list (that will be fixed by plasma/milou!37) and the quick rendering/flashing of frames with components in wrong positions. This MR deals with the latter. In PlasmaQuick::Dialog we have the slots `updateMinimumWidth`, `updateMinimumHeight`, `updateMaximumWidth`, `updateMaximumHeight` connected to their corresponding signals on the underlying layout object. Theses slots used to start a 0ms instantly-expired timer to condensate all four operations into one single execution of the method `updateLayoutParameters()`. It was seemingly done in an attempt to avoid flickering. However, it was apparently causing it instead of preventing it. We had no guarantee that the timer's action will be the next event executed in the GUI thread event loop. By returning control of the GUI thread to the event loop, we are vulnerable to the event loop deciding that a frame update is more important than the expired timer's action, and that's what was happening: a frame was being rendered before that timer's action was executed, and the frame was rendered with incorrect layout parameters, causing KRunner to be displayed in crazy ways for a few milliseconds. This code is old, 7 years ago. Maybe this timer mechanism was actually useful back then: there was a bug inside QT that caused flickering as reported in [this QT bug report](https://bugreports.qt.io/browse/QTBUG-46074), but is is now already solved since QT 5.12.5. I have deleted that timer and now I call `updateLayoutParameters()` instantly for each of the four slots. It is true that this method is now running 4 times, however that doesn't seem to be any problem at all. They run in the GUI thread, in the same pass of the event loop, so at the end of the loop's current pass we will reach our goal of having all layout parameters up-to-date before it gets a chance to decide to render the next frame. This reference helped me to understand more of the frame rendering cycle: https://doc.qt.io/qt-5/qtquick-visualcanvas-scenegraph.html#threaded-render-loop-threaded With this patch applied, the flickering situation in KRunner is MUCH better, but not completely fixed. Some kinds of flicker seem to be gone at all: for example we will never see the search textbox being rendered out of place, nor the quick repositioning of the results lists. But we sometimes still see some result being rendered in the location of the textbox when we press backspace successively. I'd say the problem is now 90% fixed, there must be other causes of flicker lurking across the stack but it doesn't seem to be in PlasmaQuick::Dialog. M +11 -21 src/plasmaquick/dialog.cpp https://invent.kde.org/frameworks/plasma-framework/commit/320676ded0d47b52d8d910f1371f908fb6d67083
This should be fully fixed now in Plasma 5.24.
(In reply to Nate Graham from comment #14) > This should be fully fixed now in Plasma 5.24. This bug report addresses two issues: the frame rendering flickering and the quick crazy reordering of the results. The quick reordering is not fixed yet, the MR with the fix is here: https://invent.kde.org/plasma/milou/-/merge_requests/37 IMO it is ready to be merged, we just need somebody to review & merge it. So I'm reopening until the quick crazy reordering problem is fixed too.
Git commit 58728acc19933a3d2a93d8cecfc77606dbf8dda9 by Alexander Lohnau, on behalf of Eduardo Cruz. Committed on 08/02/2022 at 16:53. Pushed by alex into branch 'master'. Avoid sorting old results based on new query input string `SortProxyModel` reorders the results provided by krunner. It uses the query input string to judge the associated results and sort them. It works great, however we have a little bug that causes a nonsensical list to be shown for the a few milliseconds right after we input a new query string. At the time `setQueryString()` is called, we still haven't received any matches result list from krunner for this new query. Milou still has the previous query's results, which are completely dissociated from the new query input. So at this point it was doing its logic using the old results list from the previous input string, but evaluating them against the newly provided input string, which resulted in bogus behavior. This example might make it more clear: 1 - We type string "fire" and wait for the query to finish and the final results are displayed. We get this result set:  2- We type one more letter, "f", so now we have "firef" as input. `SortProxyModel::setQueryString()` would set "firef" as its new `m_words` and it would call its own `invalidate()`, however at this moment the results list for "firef" haven't been provided by krunner yet: Milou still has the results list for "fire", our previous query. So `SortProxyModel` is processing the results from "fire" using the string "firef" as criteria to judge its sorting, and this results in a quick exhibition of this bogus list to the user. This is only shown for 250ms. It took some tries to take a screenshot of it, but here it is:  As we can see, we have some bogus results like "Command line: run fire" and "System Settings: Firewall" that have no business being there for the input string "firef". They are there because they are still the results from the old "fire" query. Also the ordering is different from the first list since the proxy judged the relevancy in a different manner because it's evaluating that list against the string "firef" and not "fire". 3 - 250ms later, krunner will have provided its results for "firef", and `SortProxyModel` gets in action again, now sanely sorting the "firef" result against its associated "firef" input string, and we get this final correct list:  So I fixed it by not calling `SortProxyModel::setQueryString()` anymore when we have a query string change. Now we call it whenever we have a matches result set change. So the `SortProxyModel` will maintain the old query string until we receive the first result set for the new query string. That method's inner logic will detect whenever a true query string change happened and will properly call `invalidate()` when necessary. With this patch applied, the bogus list in step 2 above will never be generated and it won't be shown to the user. M +7 -1 lib/resultsmodel.cpp M +2 -0 lib/runnerresultsmodel.cpp M +2 -0 lib/runnerresultsmodel.h https://invent.kde.org/plasma/milou/commit/58728acc19933a3d2a93d8cecfc77606dbf8dda9
Git commit a458e4c7a7b02bf533f463d1be19138e195d8d22 by Nate Graham, on behalf of Eduardo Cruz. Committed on 08/02/2022 at 16:58. Pushed by ngraham into branch 'Plasma/5.24'. Avoid sorting old results based on new query input string `SortProxyModel` reorders the results provided by krunner. It uses the query input string to judge the associated results and sort them. It works great, however we have a little bug that causes a nonsensical list to be shown for the a few milliseconds right after we input a new query string. At the time `setQueryString()` is called, we still haven't received any matches result list from krunner for this new query. Milou still has the previous query's results, which are completely dissociated from the new query input. So at this point it was doing its logic using the old results list from the previous input string, but evaluating them against the newly provided input string, which resulted in bogus behavior. This example might make it more clear: 1 - We type string "fire" and wait for the query to finish and the final results are displayed. We get this result set:  2- We type one more letter, "f", so now we have "firef" as input. `SortProxyModel::setQueryString()` would set "firef" as its new `m_words` and it would call its own `invalidate()`, however at this moment the results list for "firef" haven't been provided by krunner yet: Milou still has the results list for "fire", our previous query. So `SortProxyModel` is processing the results from "fire" using the string "firef" as criteria to judge its sorting, and this results in a quick exhibition of this bogus list to the user. This is only shown for 250ms. It took some tries to take a screenshot of it, but here it is:  As we can see, we have some bogus results like "Command line: run fire" and "System Settings: Firewall" that have no business being there for the input string "firef". They are there because they are still the results from the old "fire" query. Also the ordering is different from the first list since the proxy judged the relevancy in a different manner because it's evaluating that list against the string "firef" and not "fire". 3 - 250ms later, krunner will have provided its results for "firef", and `SortProxyModel` gets in action again, now sanely sorting the "firef" result against its associated "firef" input string, and we get this final correct list:  So I fixed it by not calling `SortProxyModel::setQueryString()` anymore when we have a query string change. Now we call it whenever we have a matches result set change. So the `SortProxyModel` will maintain the old query string until we receive the first result set for the new query string. That method's inner logic will detect whenever a true query string change happened and will properly call `invalidate()` when necessary. With this patch applied, the bogus list in step 2 above will never be generated and it won't be shown to the user. (cherry picked from commit 58728acc19933a3d2a93d8cecfc77606dbf8dda9) M +7 -1 lib/resultsmodel.cpp M +2 -0 lib/runnerresultsmodel.cpp M +2 -0 lib/runnerresultsmodel.h https://invent.kde.org/plasma/milou/commit/a458e4c7a7b02bf533f463d1be19138e195d8d22
Git commit 6a2fd0704694f0a711a2052c827b0a900e2ed52c by Alexander Lohnau. Committed on 12/02/2022 at 14:56. Pushed by alex into branch 'master'. Remove douplicate timeout logic in RunnerResultsModel KRunner internally aggregates the matches and only emits the matches changed signal every 250 ms. M +0 -20 lib/runnerresultsmodel.cpp M +0 -2 lib/runnerresultsmodel.h https://invent.kde.org/plasma/milou/commit/6a2fd0704694f0a711a2052c827b0a900e2ed52c
With that merged, this should be fully fixed for Plasma 5.25 now. ...Unless we want to backport that last commit to 5.24.
We can't backport that one to plasma 5.24 because it depends on https://invent.kde.org/frameworks/krunner/-/merge_requests/82 which only got merged into krunner 5.91, and plasma 5.24 must remain compatible with frameworks 5.90.