On one of my machines, all system tray icons placed by xembedsniproxy are blank. This is probably the same issue as <https://github.com/davidedmundson/xembed-sni-proxy/issues/28>. I believe I have some additional information (see below), but since the github repository is closed, I am filing the bug here.
Steps to Reproduce:
1. Start an X session with plasmashell and xembedsniproxy.
2. Run an application using a legacy tray icon, such as pidgin or hexchat.
3. Look at the system tray.
For all applications with legacy system tray icons, the icons displayed are blank. They still react to mouse events, such as toggling visibility on left click and showing a context menu on right click.
There should be non-transparent icons in the system tray for these applications.
I have two fairly similarly set-up machines running Plasma 5 (both running Fedora 23), but only one exhibits this problem. The biggest difference that comes to mind is that the unproblematic machine uses the i915 graphics driver, whereas the problematic one uses an nvidia graphics chip (though the problem occurs both using the nouveau and the proprietary drivers).
On the problematic machine, xembedsniproxy will print debug messages such as
kde.xembedsniproxy: Skip transparent xembed icon for 60819945 "Pidgin"
Other than this, I don't observe any significant differences in log output.
Replacing the (in any case fine looking) transparency detection code in SNIProxy::update() with a simple "isTransparentImage = false;" doesn't alleviate the problem; the icon received through XCB actually appears to be transparent.
I still don't have any new info to go on. I'm kinda stuck
Inspecting the image data received through XCB, I can see that the image is there, only the alpha channel is set to transparent for each pixel.
If I add
for (int i = 0; i < image->width * image->height; i++)
image->data[i * image->bpp / 8 - 1] = 0xFF;
before the conversion to a QImage in SNIProxy::getImageNonComposite(), the icons become visible (though of course with a transparent background).
I suppose this can function as a workaround for those affected and willing to compile their own version of xembedsniproxy.
For clarification, I meant to say the icons become visible with a black background, not with a transparent one.
Created attachment 95921 [details]
Patch for xembed-sni-proxy-0~git20151104-ded1538
Found a nice image load function in another kde commit which works for me
Using Rakyn's patch, I get visible icons with a black background on both of my systems, i.e. including the one where icon display worked well previously. This is because for the icons grabbed those two systems, xcbImage->depth is 24, whereas xcbImage->bpp is 32. This is in line with the comments in the xcb-util-image source code:
uint8_t depth; /**< Depth in bits. Valid depths
* are 1, 4, 8, 16, 24 for z format,
* 1 for xy-bitmap-format, anything
* for xy-pixmap-format.
uint8_t bpp; /**< Storage per pixel in bits.
* Must be >= depth. Valid bpp
* are 1, 4, 8, 16, 24, 32 for z
* format, 1 for xy-bitmap format,
* anything for xy-pixmap-format.
Effectively, what Rakyn's code does in this case, is simply use QImage::Format_RGB32 always and ignore the alpha channel completely (basically, what I did with my hacky workaround above). Once one changes xcbImage->depth to xcbImage->bpp in the switch statement, the old behaviour of the unpatched version returns.
*** Bug 355418 has been marked as a duplicate of this bug. ***
*** Bug 356430 has been marked as a duplicate of this bug. ***
Patch looks good.
In theory I specific what format embedded clients should be using, but they also seem to just ignore it.
In future, if you can put patches on git.reviewboard.kde.org it's a bit easier for me to comment especially if we have some back and forth. (like to say that method should be const)
and otherwise they get lost.
Rakyn, what's your surname. I need it for the commit author.
*** Bug 356514 has been marked as a duplicate of this bug. ***
Sorry for a duplicate I submitted. The same problem here. Arch Linux, Nvidia driver. I will be glad to provide all the info needed to resolve it completely.
Created attachment 96044 [details]
Patch for xembed-sni-proxy-0~git20151104-ded1538
(In reply to David Edmundson from comment #9)
> Rakyn, what's your surname. I need it for the commit author.
Added sort of artificial alpha mask to the icons,
posting the patch at this place one last time.
This patch is still a regression on machines where transparency worked fine in the first place. It is visible e.g. in the hexchat icon, where createHeuristicMap will leave the black spots in the middle of the icon unchanged and not detect the semitransparency at the edges of the icon (leaving a black seam).
Can you name a few applications which worked fine before please?
I would like to check.
I don't see how it's a regression? your new section only applies to images where depth == 24, so they wouldn't have had any transparency anyway?
My usual test suite is
xchat, tuxguitar, spotify
gtk, java, qt
then I get someone else to test Steam via Wine as that normally behaves weirdly.
I've just run the 3 above. First two are fine, spotify isn't showing up at all for me, but that's not related to your changes.
Like I said in comment #5, depth is 24 even for transparent images (bpp is 32), both in practice and according to xcb-util-image documentation.
I'm currently preparing a patch that only uses Rakyn's conversion routine when the previous method didn't work, which should give us the best of both worlds.
Created attachment 96045 [details]
Try naive conversion, only then use alternative copying method
This patch doesn't regress on my machine where the current version works already, though I suppose it should be checked whether it reintroduces the WINE bug mentioned in the source code. The Battle.net launcher icon works fine on my machine though.
One could probably also use createMaskFromColor(qRgba(0, 0, 0, 0)) instead of createHeuristicMask() in Rakyn's copying routine, which gives somewhat better results with the hexchat icon, but might of course be worse for other icons that use the black colour intentionally.
Seems OK to me.
- I don't like how update() has a call to isTransparentImage even when we potentially we've already done that.
can we make getImageNonComposite have a second isTransparentImage after convertFromNative, and if applicable return a null QImage. Then remove the check from update(). It groups the logic together
- don't leave commented out code
+ //QImage image(xcbImage->data, xcbImage->width, xcbImage->height, format);
- why did you remove the braces in isTransparentImage ?
- make methods const
Works fine here too.
There is a typo in na(t)ive.
Sorry no typo, i think it is just my poor english.
David, I believe I've addressed your points, and I've posted the new version of the patch as review request #126336: https://git.reviewboard.kde.org/r/126336/
Rakyn, yes, I mean naive as opposed to sophisticated/elaborate. It should be clearer in the latest version of the patch. :)
Git commit 41df1bdb8b478eb27fb424d201c075c76ec0ed5a by David Edmundson, on behalf of Benedikt Gollatz.
Committed on 13/12/2015 at 20:49.
Pushed by davidedmundson into branch 'Plasma/5.5'.
Mitigate failed icon grabbing in xembed-sni-proxy
If grabbed icons are blank, try to salvage the copied data as well as
possible while leaving setups where image grabbing works fine alone.
Based on a patch by Rakyn Barker.
M +110 -33 xembed-sni-proxy/sniproxy.cpp
M +4 -1 xembed-sni-proxy/sniproxy.h