FCR001 / cantata

Automatically exported from code.google.com/p/cantata
GNU General Public License v3.0
0 stars 0 forks source link

No tray icon with Qt5 build #483

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Build cantata against Qt5
2. Start it in either Plasma 4.x, or Plasma Next

What is the expected output? What do you see instead?
icon and proper tooltip should be present, instead there's
http://i.imgur.com/7cgPGiB.png

What version of the product are you using? On what operating system?
openSUSE Factory, latest, todays trunk of cantata

Please provide any additional information below.

Original issue reported on code.google.com by schums...@gmail.com on 20 May 2014 at 11:20

GoogleCodeExporter commented 9 years ago
Just tested on Ubuntu 14.04 with KDE 4.13, and the tray icon did appear (and in 
the correct place). Cantata uses QSystemTrayIcon for Qt4 (not KDE) and Qt5 
builds. Does a Qt4 (pure Qt4, not KDE) build function correctly for you?

However, Qt5 does appear to have issues with the system tray, e.g .see:
 https://bugreports.qt-project.org/browse/QTBUG-31762

Original comment by craig.p....@gmail.com on 21 May 2014 at 11:06

GoogleCodeExporter commented 9 years ago
will try...
one additional note - icon is missing only with Plasma; it is there with e.g. 
lxqt (dunno which tray mechanism they use)
with Plasma i see org.kde.StatusNotifierItem-.... registered, but as shown, no 
icon

Original comment by schums...@gmail.com on 21 May 2014 at 11:58

GoogleCodeExporter commented 9 years ago
hm, if i unload statusnotifierwatcher kded module, i see constant
KStatusNotifierWatcher not reachable
QSystemTrayIcon::setVisible: No Icon set
service is "org.kde.StatusNotifierItem-23289-$(n+1)"
Registering a client interface to the KStatusNotifierWatcher

and cantata is sucking half of CPU... (off course, no icon again)

Original comment by schums...@gmail.com on 21 May 2014 at 12:03

GoogleCodeExporter commented 9 years ago
Which Qt5 version? Ubuntu has 5.2.1 

I'm 90% sure this is a Qt issue. As I said, it works for me, the Qt4 and Qt5 
versions use the same Cantata code. Does Qt4 work for you? If so, then its a 
Qt5 issue - not much I can do.

Original comment by craig.p....@gmail.com on 21 May 2014 at 12:34

GoogleCodeExporter commented 9 years ago
yep, could be - pure Qt4 version also works...

Original comment by schums...@gmail.com on 21 May 2014 at 12:41

GoogleCodeExporter commented 9 years ago
ah, using Qt 5.3

Original comment by schums...@gmail.com on 21 May 2014 at 12:46

GoogleCodeExporter commented 9 years ago
To be honest, Qt5 has so many QtWidgets bugs. I myself have reported 3 
(QTBUG-36862, QTBUG-36863, and QTBUG-36864) Not sure if I'd actually call Qt5 
ready for desktop use.

Anyway, this has to be a Qt5 issue - the Cantata code for both Qt4 and Qt5 in 
this case is *exactly* the same. If Qt4 works, but not Qt5, then it is the Qt5 
code that is wrong.

Under Unity, the Qt5 tray icon is appearing in the top-left corner! Not even in 
the system tray!

Original comment by craig.p....@gmail.com on 21 May 2014 at 12:56

GoogleCodeExporter commented 9 years ago
Hi,

couple notes about QSystemTrayIcon in Qt5:

 * versions prior 5.4 (I think) are using old xembed implementation for systray icons; in Plasma Next (the 5 series) we've decided to not support xembed icons anymore - they bring looots of issues and we believe it's just not worth it.

 * versions after 5.4 will use systray icon from a platform theme (if available). Plasma's platform theme uses KStatusNotifierItem as a backend for QSystemTrayIcon.

 * we kinda hope that distributions will apply that patch even to Qt 5.3, it works properly and improves things about 9000%.

(there is similar patch for Qt4, done by Canonical, which is shipped in *buntu 
and openSuse)

That said, I've just tried Cantata-Qt5 using Qt 5.3 and the above-mentioned 
patch in Plasma Next. A KStatusNotifierItem is properly created through the 
platform theme, but there is no icon, just an empty pixmap (really just blank 
pixmap). I've looked into all the codes and I /think/ this might be Cantata's 
bug - applying the diff below to Cantata shows normal working icon in the 
Plasma Next systray:

diff --git a/gui/trayitem.cpp b/gui/trayitem.cpp
index 50bf2cc..afb3cf3 100644
--- a/gui/trayitem.cpp
+++ b/gui/trayitem.cpp
@@ -144,7 +144,7 @@ void TrayItem::setup()
         icon.addFile(iconFile);
     }
     #endif
-    trayItem->setIcon(icon.isNull() ? Icons::self()->appIcon : icon);
+    trayItem->setIcon(QIcon::fromTheme("kde"));
     trayItem->setToolTip(i18n("Cantata"));
     trayItem->show();
     connect(trayItem, SIGNAL(activated(QSystemTrayIcon::ActivationReason)), this, SLOT(trayItemClicked(QSystemTrayIcon::ActivationReason)));

---
...so it leads me to think that the "icon.isNull() ? Icons::self()->appIcon : 
icon" does not produce valid/usable icon. 

I didn't test it any further though.

Original comment by martin.k...@gmail.com on 12 Jun 2014 at 4:54

GoogleCodeExporter commented 9 years ago
Thanks. But the line of code is the same for Qt4 as Qt5 - if the Qt4 code 
works, then how can the error be on Cantata's side? Does changing:

trayItem->setIcon(icon.isNull() ? Icons::self()->appIcon : icon);

to:

trayItem->setIcon(Icons::self()->appIcon);

work??? If not, then it must be because this appIcon only has a single embedded 
SVG as the source. Is the issue that the SVG is embedded within the cantata 
executable? Would it be better if the SVG was installed, to disk, and 
icon.addFile("/path/to/svg") was used instead?

...

However, the Qt5 'works' for me; in KDE4 the icon is in the sys-tray, under 
Unity an icon appears top-right.

Original comment by craig.p....@gmail.com on 13 Jun 2014 at 12:48

GoogleCodeExporter commented 9 years ago
> However, the Qt5 'works' for me; in KDE4 the icon is in the sys-tray, under 
Unity an icon appears top-right.

Yes, that's expected; as per my first two notes, it will use xembed which is 
supported both in KDE4 and Unity (Qt4 apps are however transformed into 
StatusNotifierItem by default in Unity), but not anymore in Plasma 5 (we have 
to draw the line somewhere; we also expect Unity will use the patch mentioned 
in my second note).

That said, this might be a combination of issues. The QIcon supplied by cantata 
returns empty for availableSizes(), which might be expected for SVGs as it 
kinda makes sense, but I dunno. What our platform theme plugin does is looking 
if the icon has a name and if not, it takes all pixmaps for all 
availableSizes() and stores them (as they need to be transferred over DBus). 
So, Cantata's QIcon has no name and no availableSizes(), so the icon /is/ 
actually empty. 

Quite possibly just setting the icon name to "cantata" would work as I see 
there are png icons being installed by Cantata; that would however likely 
ignore the SVG completely. In anycase, I'll try to add a check in our code for 
icon being empty and try to load application icon instead. That should improve 
things a bit :)

Anyways, I would strongly suggest you do not embed the icon or simply install 
it as well to the standard icons dir. Installing it into default dir would 
allow to have it simply loaded by doing QIcon("cantata.svg") (or 
QIcon::fromTheme("cantata.svg");).

Original comment by martin.k...@gmail.com on 13 Jun 2014 at 1:30

GoogleCodeExporter commented 9 years ago
Yeah, I'm going to change the code so that appIcon is just 
QIcon::fromTheme("cantata") - this way it /should/ just work. As you said, the 
PNGs are installed anyway...

I'll update the code later. Thanks for the *very* quick reply!

Original comment by craig.p....@gmail.com on 13 Jun 2014 at 2:13

GoogleCodeExporter commented 9 years ago
Glad I could help :)

Original comment by martin.k...@gmail.com on 13 Jun 2014 at 2:15