When the 'Share' button is clicked and the Kipi plugins are not loaded yet the menu is supposed to show Loading... but D13289 disabled that behaviour.
Details
- Reviewers
rkflx - Group Reviewers
Gwenview - Commits
- R260:e5e5b9a888e8: Fix 'Loading...' action not showing
Diff Detail
- Repository
- R260 Gwenview
- Branch
- bug/share-menu-first-use
- Lint
No Linters Available - Unit
No Unit Test Coverage
Yeah, it causes the button menu to show, even when you only clicked on Plugins in the menu bar ;)
- Call d->updateMenu() twice when the loading is finished (i.e. both inside and outside if (d->mKIPIInterface->isLoadingFinished()) or twice outside).
This causes some flickering for me, and conceptually sounds a bit odd. Why is it working the second time, but not the first? It might be worth investigating what parts of the second call will do the trick.
The second solution seems to work fine but I couldn't find the real cause of the problem. What do you think?
I think we should at least try to find the real cause and not blindly try things.
That's really interesting, maybe we can make it work based on that.
I'd like to add two observations:
- When I remove clear(), everything will work just fine (of course there will be one superfluous Loading... entry).
- If I turn off compositing via ⇧+Alt+F12, the menu will also show every time.
What might be happening here is that clear() gets the "window" of the menu in such a state that KWin's compositor won't update it properly afterwards, or there is some kind of race. Technically it's there, but we don't see it. Note that this only affects compositing on X11, on Wayland I don't see the problem in the first place. Also, it's independent of the chosen widget style or the window decoration.
BTW, I'm mostly testing in a VM. Are you testing on real hardware?
Thoughts?
BTW, I'm mostly testing in a VM. Are you testing on real hardware?
Yes, I am doing the tests on real hardware (with Arch Linux | KDE Plasma 5.12.5 | KDE Frameworks 5.46.0 | Qt 5.11.0 | Linux 4.16.13)
Here is a video showing that the menu still works even if it's invisible:
I also noticed that it's not always broken but It works sometimes.
I found that someone had a similar problem with QMenu (on this thread) and I noticed that they were using Kubuntu (hence, most likely KDE) and the original implementation of KIPIExportAction::init() is logically correct. It seems like the problem is not related to the implementation of the Share menu but it looks like some sort of bug related to QMenu on KDE and this also points to that direction:
If I turn off compositing via ⇧+Alt+F12, the menu will also show every time.
[...]
Note that this only affects compositing on X11, on Wayland I don't see the problem in the first place
I think that D13289 and D13312 don't really solve the problem but they're just hacks which hide the actual problem.
I kept digging and I found that it might be related to this code (in KIPIInterface::loadOnePlugin() which is in app/kipiinterface.cpp):
QMetaObject::invokeMethod(this, "loadOnePlugin", Qt::QueuedConnection);
If loadOnePlugin() is called synchronously the Share menu works fine (without D13289 or D13312). Could it be some sort of thread issue? But then why does the problem disappear when compositing is disabled?
For me it seem Gwenview is not at fault, and until we fix the issue elsewhere, here we only can work around the problem.
At least it's somewhat comforting to know that others have the problem too. BTW, I already tried setUpdatesEnabled(false) before you posted the link, does not help.
Using git blame on that line, I end up in a7fdfb42fad0, which reads "Do not block the UI while loading KIPI plugins". And indeed, if I follow your suggestion to use recursion (i.e. the direct message call) instead of iterating and coming back to the event loop (which invokeMethod achieves), the UI blocks, which results in Loading... not showing. (Add qDebug() before and after the call to see what I mean.)
Therefore I don't think we should go in that direction.
Could it be some sort of thread issue? But then why does the problem disappear when compositing is disabled?
Sounds like there is some kind of race, which does not work out in our favour in some situations.
For the moment, I think showing the menu after the first click is most important. Now we have to decide between flickering and not showing Loading.... Since machines are plenty fast nowadays (see your remark above), I'd say we can go with the latter until we have a proper fix to also improve it for slower machines.
I'd suggest to do the following:
- Write a small test case app, similar to what's in the forum post.
- Submit that as a bug to KWin (it might also be a Qt bug, though).
- Change your patch to add a TODO to the code, with a short explanation and a reference to the bug number as well as the discussion over here.
Write a small test case app, similar to what's in the forum post.
I actually tried to reproduce the bug by clearing a QMenu multiple times in a row but no luck, until I followed your suggestion and I used a QTimer to clear the menu which caused the exact same bug. It seems like it only happens when the menu is cleared through a slot triggered outside of the main event loop.
Submit that as a bug to KWin (it might also be a Qt bug, though).
I filed this bug in which I attached videos of the test program which reproduces the bug.
Great, thank you so much for your investigations and filing the bug!
The patch looks a bit odd after your latest update: You only seem to add the comment, but the addition of d->updateMenu(); is not shown (while it should be, as currently that line is not in the repo).
Normally arc diff should do the right thing, i.e. upload the diff between HEAD and master, but maybe you did something different?
Nice, LGTM now. I'll wait a couple of days whether we get any more updates on the bug, and then commit on your behalf.