Created attachment 36684 [details] [review]
Patch too bypass theme engine on applets when using pixmap/colour panel backgrounds.

Adding to my 2.10 TODO list.

I don't know what gtk_style_apply_default_background() does...

A big problem here is that it's sometimes not enough to change the background of
the applet. We often need to change the background of widget in the applet. So
your fix won't work for these applets.

gtk_style_apply_default_background is the function GTK exposes for theme engines
to call directly to apply the default background if they don't override it with
their own method. It is the function which draws the bg_pixmap if any, or fills
with bg colour if not. Calling it is effectively like calling paint_flat_box if
you are running the Default theme.

I realize it won't work for all applets, however it works for most. And the same
method CAN be applied directly into each of those individual applets if need be
I am fairly sure , though I would prefer finding a way which didn't need to
override the style at all of course. :)

Mark: do you have any comment on this one? (this is a test to see if you read
your bugzilla mail ;-)

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

This is too late for 2.10 now, can we try and look at this for 2.10.1?

(I guess Mark doesn't read his bugzilla mail...)

Here's the code every applet is using when it needs to update its background:

	GtkRcStyle *rc_style;
	GtkStyle   *style;

	/* reset style */
	gtk_widget_set_style (widget, NULL);
	rc_style = gtk_rc_style_new ();
	gtk_widget_modify_style (widget, rc_style);
	gtk_rc_style_unref (rc_style);

	switch (type) {
	case PANEL_NO_BACKGROUND:
		break;
	case PANEL_COLOR_BACKGROUND:
		gtk_widget_modify_bg (widget, GTK_STATE_NORMAL, color);
		break;
	case PANEL_PIXMAP_BACKGROUND:
		style = gtk_style_copy (widget->style);
		if (style->bg_pixmap[GTK_STATE_NORMAL])
			g_object_unref (style->bg_pixmap[GTK_STATE_NORMAL]);
		style->bg_pixmap[GTK_STATE_NORMAL] = g_object_ref (pixmap);
		gtk_widget_set_style (widget, style);
		g_object_unref (style);
		break;
	}

It's not in a expose event.

Will adding gtk_style_apply_default_background() here work? Is there something
simple we can do there to bypass the theme engine background? I'm a bit
reluctant to add the patch in libpanel-applet since some applets will also need
to to something like this...

(I'm asking because I can not test right now)

This patch _was_ the simple solution to bypass theme engine backgrounds. It
fixed most applets quickly (so that it could be partially fixed before 2.10).

Yes every applet which does not use the libpanel-applet will need a similar
workaround, but the other obvious solution would be to modify every single
applet to tile the background or draw the color etc itself on expose, and when
you can make one small change to fix most of them....

I still prefer the idea of not using the style at all, but this would require
modifying every single applet, not just the handfull that don't use
libpanel-applet expose, so I put it off for a future rework, leaving this patch
to allow a fix in time for 2.10, which obviously isn't going to happen anyway. 

Is anyone still looking at this bug? This is the one thing that still makes
Gnome look ugly, this ought to be fixed!

I would really like to see this bug fixed. It's annoying to see really nasty
screen artifacts in the system menu and applet area when I want to put a
gradient as the background of gnome-panel. I use clearlooks as the gtk theme by
the way. Please oh great gnome hackers put this patch into cvs.

Obviously not going to happen for 2.10.x any more. This bug has a patch. Vincent?

Sorry for the very long delay.

I'm trying the patch right now, but it doesn't fix anything as far as I can see:
it looks like this should be done in every applet :/ Does it work for you?

Also, I'd be interested to know if there's a better way to handle color/image
backgrounds without using the style?

It worked for me the last time I tried it - but I'll try it again.

Vincent, which themes did you try?

So, I tried again and it doesn't work. This is with Yattacier 3 (http://art.gnome.org/themes/gtk2/598).

Note that there's a new API in libpanel-applet that enables applet authors to specify the widget that will be used to automatically update the background. Maybe it can help in some cases...

Created attachment 68926 [details] [review]
Improved fix for libpanel-applet based applets

Sorry for the very long delay. I never received most of the e-mails from this bug. I assumed no one was even looking at it.

The problem boils down to this -

In a widgets expose event, almost all widgets call the style functions to draw the background. Now, as stated before, if the engine ignores bg_pixmap OR provides its own, this will result in the transparency hack in panel code failing to work.

What this means though is that ANY applet which embeds another widget into itself, will not work on some themes, because that widget calls style draw functions directly. So for example, embeded buttons, or menus, will not work reliably, unless a custom sub-class is made which replaces the expose function with its own.

And as per the earlier example of Yattacier3, my first patch made the assumption that no engine would actually replace bg_pixmap.. it was wrong, but that is easily fixed, by calling gdk_draw_drawable with the pixmap, instead of the default background fill. (Which I probably should have done in the first place).

This new patch still only fixes those applets which don't embed another widget into itself, but that is a good many of them.

Created attachment 69113 [details] [review]
draw background pre-expose

Here is my idea to get this fixed.

This patch does not fiddle with styles or rc styles at all but instead draws the background pre-expose-event.

I made the PanelApplet have this behaviour by default and made panel_applet_set_background_widget ignore (print a note to stderr) PanelApplets.

Created attachment 69115 [details] [review]
bugfixed draw background pre-expose

Created attachment 69266 [details] [review]
some more issues handled

I've discussed the patch with Andrew and he's fine with it.
And I'm running it currently and it seems to work fine with all the applets I'm using and haven't seen any problems.

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

Created attachment 69435 [details] [review]
updated patch

So here's another update with some comments from Vincent handled:
- readded panel_applet_get_background since it's public API (oops)
- added panel_applet_draw_background to public API that draws the correct part of the panel's background to be used in expose signal handlers.

There's one issue with this patch: when you move applets on a transparent panel, you can see some grey lines at the edge of the applet. That's most probably because the pixmap is cached. It should probably be possible to cache a bigger pixmap to mitigate this issue.

(I was testing using the patch from 2006-07-20, but I guess latest patch exhibits the same issue)

hrm...
I'm only seeing this when madly moving applets across my desktop, and I'm only running a 800mHz iBook.
What I see is the gdk_window_move causing the background to be moved, but shortly thereafter it's updated when the next redraw happens.
Since caching a bigger image steals memory (for every applet), it's not visible for me, moving applets is IMO a pretty uncommon operation and it'd break panel_applet_get_background API (there's some assumptons about the returned pixmap in that API), I wonder if it's worth doing. I don't think so, but I'm not running on a kiosk style X server either...

Oh, and this is a thing we can fix after feature freeze ;)

Benjamin: did you have time to work on it to fix the issue I mentioned in comment #22? Or does someone wants to hack on this?