Merge lp:~cimi/overlay-scrollbar/fix-compiz-stacking into lp:overlay-scrollbar

[Andrea Cimitan]

this should fix the stacking issues with compiz, we still need to patch metacity to support _NET_RESTACK_WINDOW

[lp:~cimi/overlay-scrollbar/fix-compiz-stacking]

176. By Andrea Cimitan on 2011-03-24

renamed XEvent *event to xev

[Loïc Molinari (loic.molinari)]

Hey Cimi, I've got some question about that one.

Can you please explain why that patch is needed?

Why have you actually chosen to implement it that way?

Isn't that EWMH atom already wrapped as a simple GDK function?

review: Needs Information

[Loïc Molinari (loic.molinari)]

I think it would be safer to wrap all the XLib code with the GDK X11 error handler (gdk_error_trap_push and gdk_error_trap_pop).

review: Needs Fixing

[Andrea Cimitan (cimi)]

> Hey Cimi, I've got some question about that one.
>
> Can you please explain why that patch is needed?
>
> Why have you actually chosen to implement it that way?
>
> Isn't that EWMH atom already wrapped as a simple GDK function?

I've decided to implement it in that way, because it's the only one I got it working with compiz :-) sam spilsbury approved it.
Do you have other ideas? Another one I had was to use XQueryTree to get the real parent, but was working for metacity (with bugs) and not for compiz, and was more expensive.

[Andrea Cimitan (cimi)]

> Isn't that EWMH atom already wrapped as a simple GDK function?

you mean gdk_window_restack?
I've tried with
gdk_window_restack (gtk_widget_get_window (priv->thumb), gtk_widget_get_window (priv->parent), TRUE);

doesn't work with both compiz and metacity

[lp:~cimi/overlay-scrollbar/fix-compiz-stacking]

177. By Andrea Cimitan on 2011-03-28

wrap x11 calls with gdk error trapping code (push, flush, pop)

[Loïc Molinari (loic.molinari)]

What's currently used in trunk in order to do that and what's the problem?

[Loïc Molinari (loic.molinari)]

Thanks for the error handling bits.

Note that gdk_flush() is actually a wrapper for XSync(), so that last one is useless there.

[Andrea Cimitan (cimi)]

> What's currently used in trunk in order to do that and what's the problem?
nothing is used in trunk: the issue is that, if your window is not the focused one (something above it), and you make the thumb appear, the thumb will be above everything.
we need to restack the thumb window so it will be on the same layer of its toplevel

[lp:~cimi/overlay-scrollbar/fix-compiz-stacking]

178. By Andrea Cimitan on 2011-03-28

removed XSync as gdk_flush is a wrapper for it

[Loïc Molinari (loic.molinari)]

OK. Looking at the _NET_RESTACK_WINDOW spec¹, it seems like a hack, but I don't see another solution for now.

¹ http://standards.freedesktop.org/wm-spec/wm-spec-1.3.html#id2506866

[Loïc Molinari (loic.molinari)]

Can you add a FIXME(Cimi) in the code explaining why you do that, what it fixes and what are the remaining issues?

[lp:~cimi/overlay-scrollbar/fix-compiz-stacking]

179. By Andrea Cimitan on 2011-03-28

Added FIXME comment

[Loïc Molinari (loic.molinari)]

review: Approve

=== modified file 'os/os-scrollbar.c'
--- os/os-scrollbar.c	2011-03-28 16:17:49 +0000
+++ os/os-scrollbar.c	2011-03-28 17:33:46 +0000
@@ -97,6 +97,7 @@
 static gboolean thumb_button_release_event_cb (GtkWidget *widget, GdkEventButton *event, gpointer user_data);
 static gboolean thumb_enter_notify_event_cb (GtkWidget *widget, GdkEventCrossing *event, gpointer user_data);
 static gboolean thumb_leave_notify_event_cb (GtkWidget *widget, GdkEventCrossing *event, gpointer user_data);
+static void thumb_map_cb (GtkWidget *widget, gpointer user_data);
 static gboolean thumb_motion_notify_event_cb (GtkWidget *widget, GdkEventMotion *event, gpointer user_data);
 static void thumb_unmap_cb (GtkWidget *widget, gpointer user_data);
 static void pager_move (OsScrollbar *scrollbar);
@@ -578,6 +579,8 @@
       g_signal_handlers_disconnect_by_func (G_OBJECT (priv->thumb),
                                             thumb_leave_notify_event_cb, scrollbar);
       g_signal_handlers_disconnect_by_func (G_OBJECT (priv->thumb),
+                                            thumb_map_cb, scrollbar);
+      g_signal_handlers_disconnect_by_func (G_OBJECT (priv->thumb),
                                             thumb_motion_notify_event_cb, scrollbar);
       g_signal_handlers_disconnect_by_func (G_OBJECT (priv->thumb),
                                             thumb_unmap_cb, scrollbar);
@@ -599,6 +602,8 @@
                         G_CALLBACK (thumb_enter_notify_event_cb), scrollbar);
       g_signal_connect (G_OBJECT (priv->thumb), "leave-notify-event",
                         G_CALLBACK (thumb_leave_notify_event_cb), scrollbar);
+      g_signal_connect (G_OBJECT (priv->thumb), "map",
+                        G_CALLBACK (thumb_map_cb), scrollbar);
       g_signal_connect (G_OBJECT (priv->thumb), "motion-notify-event",
                         G_CALLBACK (thumb_motion_notify_event_cb), scrollbar);
       g_signal_connect (G_OBJECT (priv->thumb), "unmap",
@@ -729,6 +734,77 @@
   return FALSE;
 }
 
+static void
+thumb_map_cb (GtkWidget *widget,
+              gpointer   user_data)
+{
+  Display *display;
+  OsScrollbar *scrollbar;
+  OsScrollbarPrivate *priv;
+  XWindowChanges changes;
+  guint32 xid, xid_parent;
+  unsigned int value_mask = CWSibling | CWStackMode;
+  int res;
+
+  scrollbar = OS_SCROLLBAR (user_data);
+  priv = scrollbar->priv;
+
+  xid = GDK_WINDOW_XID (gtk_widget_get_window (priv->thumb));
+  xid_parent = GDK_WINDOW_XID (gtk_widget_get_window (priv->parent));
+  display = GDK_WINDOW_XDISPLAY (gtk_widget_get_window (GTK_WIDGET (scrollbar)));
+
+  changes.sibling = xid_parent;
+  changes.stack_mode = Above;
+
+  gdk_error_trap_push ();
+  XConfigureWindow (display, xid, value_mask, &changes);
+
+  gdk_flush ();
+  if ((res = gdk_error_trap_pop ()))
+    {
+      /* FIXME(Cimi) this code tries to restack the window using
+       * the _NET_RESTACK_WINDOW atom. See:
+       * http://standards.freedesktop.org/wm-spec/wm-spec-1.3.html#id2506866
+       * 
+       * Should work on window managers that supports this, like compiz.
+       * Unfortunately, metacity doesn't yet, so we might decide to implement
+       * this atom in metacity/mutter as well. 
+       * 
+       * We need to restack the window because the thumb window can be above
+       * every window, noticeable when you make the thumb of an unfocused window
+       * appear, and it could be above other windows (like the focused one).
+       * 
+       * XConfigureWindow doesn't always work (well, should work only with
+       * compiz 0.8 or older), because it is not handling the reparenting done
+       * at the WM level (metacity and compiz >= 0.9 do reparenting). */
+      Window root = gdk_x11_get_default_root_xwindow ();
+      XEvent xev;
+
+      xev.xclient.type = ClientMessage;
+      xev.xclient.display = display;
+      xev.xclient.serial = 0;
+      xev.xclient.send_event = True;
+      xev.xclient.window = xid;
+      xev.xclient.message_type = gdk_x11_get_xatom_by_name ("_NET_RESTACK_WINDOW");
+      xev.xclient.format = 32;
+      xev.xclient.data.l[0] = 2;
+      xev.xclient.data.l[1] = xid_parent;
+      xev.xclient.data.l[2] = Above;
+      xev.xclient.data.l[3] = 0;
+      xev.xclient.data.l[4] = 0;
+
+      gdk_error_trap_push ();
+
+      XSendEvent (display, root, False,
+                  SubstructureRedirectMask | SubstructureNotifyMask,
+                  &xev);
+
+      gdk_flush ();
+
+      gdk_error_trap_pop ();
+    }
+}
+
 static gboolean
 thumb_motion_notify_event_cb (GtkWidget      *widget,
                               GdkEventMotion *event,