Created attachment 30813 [details] [review]
Patch

A bitmap isn't transparent - its foreground/backgound. This really doesn't look
right to me. What gave you the idea it should work like this?

The bitmap I *give* to gtk_widget_shape_combine_mask signifies which pixels in 
the window belong to it, and which are transparent, showing the lower windows 
thru. e.g. if I'd want to write a round clock app, I'd use it to give my 
window a round mask.

The bitmap I *receive* from gdk_pixbuf_render_pixmap_and_mask signifies the 
transparency mask which the pixbuf has. I give it a 255 treshold to receive 
the absolutely transparent pixels (since the pixbuf's alpha channel is 8-bit 
while I want a 1-bit transparent channel).

And, to sum it up, it works :)

Mark, are you going to handle this patch?

Some additional details on the issue of icon translucency.
1. Both GNOME and KDE can alpha-blend icons into the systray.
2. A problem can be observed when the user changes the color scheme (e.g. by
switching theme):
   - On KDE, the existing KDE KSystemTray icons re-blend with the new bg color,
while the existing libegg systray icons don't; newly added libegg icons blend
properly.
   - On GNOME, both KDE and libegg icons blend properly.
3. Why can't libegg be aware of KDE's color scheme changes?
   libegg composes the icon unto a small icon-sized GTK window, which has GTK's
background color.
   Because KDE doesn't provide XSETTINGS notifications, GTK apps can't know in
runtime when the color scheme changes.
   However, KDE modifies gtkrc so newly started GTK apps are notified.
4. Why can KDE icons blend into GNOME's panel, even when color scheme changes?
   Qt uses XRenderCompose to compose the icon unto screen. That's probably what
causes the icon to become correct upon the first X redraw (which
gnome-theme-manager issues).

What should we do now:
1. Apply my patch, so at least we'll have 1-bit transparency (courtesy of
XSHAPE) for libegg icons.
2. Wait for GdkPixbuf to implement XRender-based drawing for translucent pixmaps.

Actually, hold on with the old patch. I have a better one.

Instead of setting a 1-bit shape mask on the window, I'll eliminate the icon
window's bg drawing entirely, making our icon composed on top of the underlying
system tray. That way, we can also paint translucent icons (ie. 8-bit alpha
masks), blending with the systray's color. Esentially, now we'll behave like KDE
does in that regard.

This will cause one regression:
When the theme changes, the icon's background color will not update until the
icon is exposed (ie. received an Expose event). That's due to the unfortunate
fact that gnome-theme-manager doesn't initiate an X refresh. You can see that a
simple 'xrefresh' will solve the graphical artifact.

Created attachment 32092 [details] [review]
Disables icon background painting

Federico, I add you as a follow-up of our IRC discussion. Hope you could give
some insights.

Few concerns were shown by Federico on IRC:

1. An expose event is required for the icon to catch up on its parent's bg color.

   My answer:
   1. gnome-theme-manager might want to force a refresh across X (a-la
xrefresh). A theme change doesn't happen so often so it shouldn't be very expensive.
   2. GtkSockets should send Expose event to their children upon XSETTINGS
change (since the client might not be XSETTINGS-aware).

2. Currently we use XSetWindowBackgroundPixmap in ParentRelative mode to make
the icon's window transparent. This will adopt the parent's declared background
as our own. But what if the parent's theme creates the background not by setting
a pixmap, but by simply drawing it on its own?
   
   My answer:
   Sucks to be us. So far I found no way to have truly transparent-background
tray icon window in X. X always insists to put *something* in the window's
background. I guess we will not be able to do this until xorg's Composite
Extension arrives. Even our icons, when DnDed, lose their translucent sections
and probably get a 1-bit shape mask, courtesy of the SHAPE extension.

   Alternatively, we can capture the underlying window's area. What should be
faster?
   a) 1. Get a pixbuf of the parent window using gdk_pixbuf_get_from_drawable(...)
      2. Compose our icon on it using gdk_draw_pixmap(...)
      3. Use the composed pixmap for our GtkImage
   b) 1. Get a pixbuf of the parent window using gdk_pixbuf_get_from_drawable(...)
      2. Set it as our window background pixmap using
gdk_window_set_back_pixmap(...)
      3. Proceed to use our original icon for the GtkImage
   
   Then again, we ask ourselves: How do we know when to capture the underlying
window?
   I think the answer to this is: Leave the hacks alone; wait for Composite.
   
   Also, we might add a shape to the icon's window, in the fully-transparent
areas. It's a little but it helps.

---

So, what do you guys think? If none of you have time for this, I hope you could
point some other developer at it who could direct me. As you understand, I've
put some research into this and wouldn't like it to go to waste.

[1] I refer to the Xlib function but I really use the GDK, of course.

Guys, it's been two weeks now. None of you is available to maintain libegg, and
you can't find replacement -- so, well, in the spirit of community effort and
trusting other developers which show technical knowledge and common sense, why
won't you simply let me commit it?

(I've been testing the patch for the last few weeks and it's stable. I'm using
it with my Mozilla extension - http://moztraybiff.mozdev.org/ )

Anders, could you look at this patch? I'd really love to get it reviewed.

What can I say... This seems to be the realm of Adûnaphel The Quiet :-)

Grr. Please don't let a few days of my work bitrot. It's been half a year
already. Would someone please review it or just let it silently in?

(When it comes to beta-testing, this code was already in my mozTrayBiff TB
extension for ages...)

I'm going to mark this for 2.12, because we need to analyse this patch and the
problem. It sh*ts me to tears.

Let's just commit this already.

Let's not just yet. Some analysis of this patch by Ryan Lortie has revealed some
rather unwanted assumptions. It turns off GDK drawing to start with, so the
applet can never move or change size.

Ryan has been working with the KDE people on an extension to the spec to solve
the problem without turning off drawing. I have CC'ed him here.

Turns out my patch has a slight problem: When asking the tray icon to change the
pixmap, the old pixmap remains as garbage underneath cause the parent is not
re-exposed.

Gah, we need COMPOSITE...

Bug 320034 gives another go at the same problem.

Created attachment 59290 [details] [review]
better patch

Here's a patch that fixes the redrawing problem; the documentation for
gtk_widget_set_double_buffered() points out that you if you turn it off,
you also need to handle clearing the background in your expose handler.

Fixing the tray protocol or using composite is still a better fix, but this
is an easy band-aid for gnome tray icons looking bad in the kde systray.

Isn't it enough to set the background of the tray icon window to parent-relative and turn on APP_PAINTABLE?  Why do you need to turn off double-buffering?

I've tested this patch with KDE tray manager and transparency is not working.

Federico: Actually, it turns out you don't even need APP_PAINTABLE if you have
the expose_event handler.

If you don't turn off DOUBLE_BUFFERED, you get a black background. It looks
like this is because of gdk_window_set_bg_pattern, which doesn't have a case
for GDK_PARENT_RELATIVE_BG against an out-of-proc parent, so it ignores that
and uses the background color instead. It could probably just do nothing in
the GDK_PARENT_RELATIVE_BG && !private->parent case?

Frederic: I'm testing libegg's teststatusicon program against kdebase 3.5.0
using gtk-qt-engine 0.6cvs20060116 for the Gtk theme (although other themes
seem to work fine too). And I've tested with both a panel background image
and with a translucent panel covering a desktop background image, and in
both cases, the gnome icon seems to behave the same as the KDE ones. What
are you using and what effect are you seeing? (Eg, are you getting a black bg
or a theme-background-color bg?)

I've tested with our own Mandriva applications (which are using eggtrayicon.c, through perl-Gtk2) and with gnome-obex-server (gnome-bluetooth).

With or without any patch, background around GTK2 tray icon is the default background from theme engine (in my test with Mandriva Galaxy theme, grey).

i'm testing with kde 3.4.2, with either a transparent panel or a default panel

(In reply to comment #22)
> Federico: Actually, it turns out you don't even need APP_PAINTABLE if you have
> the expose_event handler.

Oh, okay.  If you override the stock gtk_window_expose_event(), yeah - that one only calls gtk_paint_flat_box() if the widget is not APP_PAINTABLE.

> If you don't turn off DOUBLE_BUFFERED, you get a black background. It looks
> like this is because of gdk_window_set_bg_pattern, which doesn't have a case
> for GDK_PARENT_RELATIVE_BG against an out-of-proc parent, so it ignores that
> and uses the background color instead. It could probably just do nothing in
> the GDK_PARENT_RELATIVE_BG && !private->parent case?

Interesting.  The problem is that we need to paint *something* to the double-buffer pixmap --- that function is the one that sets up a GC to clear that pixmap.  If we just do nothing, it would leave garbage in the newly-created pixmap.  I guess we need to push this down to the gdkwindow-x11 layer to find the background pixmap for all the foreign parents in that case.

Frederic: I haven't tried rebuilding it, but it looks like in gnome-bluetooth,
the problem is that the EggTrayIcon contains a GnomebtSpinner, which is based
on GtkEventBox, and so it paints its own solid-colored window on top of
EggTrayIcon's transparent one. So you need to apply an equivalent to
patch 59290 above to gnome-bluetooth/src/spinner.gob as well. Probably the
gtk-perl apps have similar issues.

Maybe eggtrayicon should do those tweaks itself if you pass in a windowed
widget?

Federico: I'm not sure we *can* find the foreign parent's pixmap.
XSetWindowBackgroundPixmap doesn't seem to have any obvious inverse. I guess
we have to just fall back to not double-buffering?

You are right : I've checked gtk-perl code :
gtkadd(my $icon = Gtk2::TrayIcon->new("MdkApplet"),
       gtkadd($eventbox = Gtk2::EventBox->new,
              gtkpack($img = Gtk2::Image->new)
             )
      );

I guess the issue will probably be similar with most (if not all) applications. If it can be fixed at the eggtrayicon level, it would probably be better (I don't expect every maintainers to be able to test and fix it, moreover when using standard gtk widgets).

Created attachment 59957 [details] [review]
even better patch (transparentifies child as well)

Updated patch with two new fixes:

  - makes the child widget transparent as well (if it's a windowed widget).
    This could break some things, but it seems unlikely, and they can just
    set APP_PAINTABLE in that case to make the tray icon not transparentify
    them.

  - re-transparentifies the EggTrayIcon and its child on a style_set, in
    case the style engine un-transparentified it (which at least gtk-qt-engine
    does).

confirmed, new patch works fine with our perl-Gtk applications.. congratulations..

funny thing : with latest patch, transparency works great with kicker but not with gnome-panel notification area (it wasn't working without the patch)..

Correct. The notification area itself isn't transparent, which is bug 100600
(which AFAIK no one is working on).

Wasn't aware of this bug :-)

Is someone working on a patch for GtkStatusIcon too? :-) Or is it working there?

It is presumably not working in GtkStatusIcon. I'm not currently working
on that, because we're not shipping gtk 2.10 yet. If no one else has fixed it
by the time we start hacking on SUSE 10.2, I probably will.

A lot of the code in the patch is to deal with themes doing the wrong thing.
For GtkTrayIcon, the default rcstyle should give it a parent-relative
background, and any theme that (unintentionally) overrides that should be
fixed. The hack to transparentify the child as well doesn't need to be there,
because GtkStatusIcon doesn't add an EventBox, it just listens for clicks on
the GtkPlug itself. So after setting the default style, the only things you
should need from this patch are setting APP_PAINTABLE and !DOUBLE_BUFFERED,
and overriding expose_event.

Does it make sense to commit this to random tray-using modules (gossip for example)? Or can it cause trouble if the notification area itself isn't patched as well?

it should cause any regression (I've already integrated it in some mandriva packages) and it still needed for correct tray icon transparency with KDE kicker :)

s/should/should not/ 

Ok, thanks!

Applied it to gnome-python-extras. Fixed the no transparency problem for me with Quod Libet :).
Any reason this isn't commited yet?

*** Bug 306974 has been marked as a duplicate of this bug. ***

Fixed in the development version. The fix will be available in the next major release. Thank you for your bug report.
Fixed in the development version. The fix will be available in the next major release. Thank you for your bug report.