Merge lp:~sil2100/compiz-core/fglrx_win_focus into lp:compiz-core

[Łukasz Zemczak]

Notice: this is just a proposition to be discussed. The proposed change seemingly fixes the stated problem, but I will proceed with analysing the very root cause of the problem anyway. Since it seems there might be a problem with damage events (again) on fglrx.

= Problem description =

This fix was mostly designed to cope with the problem of invalid decorations for active/inactive windows on the fglrx driver (LP: 781384). On fglrx and compiz with gtk-window-decorator, most of the time windows don't change the decoration with focus change. An inactive window can have an active decoration and vice versa. Even window title change doesn't force the decoration to change. This is very irritating on fglrx systems.

The decorator (gtk-window-decorator) is seemingly redrawing the decorations on the pixmap on focus change correctly, but probably due to some problems with damage events, the modified pixmap is not refreshed. And due to the way that the decorator is currently implemented, compiz is not explicitly notified that the decoration has been changed. The decorations can be correctly refreshed by resizing the window, as gwd calls XChangeProperty() senting PropertyNotify events to compiz.

= The fix =

The proposed fix eliminates the problem by sending PropertyNotify events to compiz every time the decoration is redrawn (both for cairo and metacity). This way, compiz explicitly knows about the decoration change and can update the decoration. We do this by not clearing the prop_xid variable on every update. From what I consulted with Sam, this doesn't really seem to have any noticible performance implications.

= Test coverage =

After the fix, active/inactive window decorations are correct. The decorations are also redrawn on mouse hover over window decoration buttons.

[Sam Spilsbury (smspillaz)]

I'm more inclined to take this as a distro patch than to have it upstream.

Resetting not resetting prop_xid means that for every redraw on the decoration we will have two events coming in to the decor plugin to update the decoration texture, both damage events on the pixmap and the PropertyNotify events. The PropertyNotify events also call through to DecorWindow::update which is a slightly more expensive codepath.

[Łukasz Zemczak (sil2100)]

This bug actually makes me wonder. Since the very reason for this bug seems to be strange - it doesn't seem to be a problem with just damage events. I'm still a newcomer in this regard, but I think if the core reason would be unsent damage events, shouldn't any damage/redraws from other applications in the area of the decoration and/or above the decoration force a redraw of the pixmap? While nothing explicit can really force the decoration redraw I noticed.

I also noticed one thing. Sometimes, completely at random, certain windows 'unlock' themselves. Suddenly from unknown reasons, selected windows have their decorations updated correctly. But only selected windows. Others on screen still have locked up decorations that don't get refreshed - resizing them refreshes the decoration right after resize, but they still don't react to anything later on. But those selected windows correctly update, correctly redraw on button hover, on title change. Why?

Maybe some pixmaps are just allocated invalidly? And don't get modified on redraw?

[Daniel van Vugt (vanvugt)]

review: Abstain

[Łukasz Zemczak (sil2100)]

Actually, this is still not it.

[Alan Griffiths (alan-griffiths)]

<sil2100> ah, btw. could you reject one of my merges?
<sil2100> https://code.launchpad.net/~sil2100/compiz-core/fglrx_win_focus/+merge/94997
<sil2100> Since this fix is generally invalid

review: Disapprove

Unmerged revisions

3021. By Łukasz Zemczak on 2012-02-24

When drawing decorations, don't reset the prop_xid value of the decorator, so that we send notification about the change of the decoration pixmap to X through PropertyNotify events.

=== modified file 'gtk/window-decorator/cairo.c'
--- gtk/window-decorator/cairo.c	2011-10-13 09:53:38 +0000
+++ gtk/window-decorator/cairo.c	2012-02-28 16:38:26 +0000
@@ -707,10 +707,7 @@
     }
 
     if (d->prop_xid)
-    {
 	decor_update_window_property (d);
-	d->prop_xid = 0;
-    }
 }
 
 static void

=== modified file 'gtk/window-decorator/metacity.c'
--- gtk/window-decorator/metacity.c	2012-02-16 06:51:37 +0000
+++ gtk/window-decorator/metacity.c	2012-02-28 16:38:26 +0000
@@ -932,7 +932,6 @@
 					   bottom_region,
 					   left_region,
 					   right_region);
-	d->prop_xid = 0;
     }
 
     if (top_region)