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

Proposed by Andrea Cimitan
Status: Merged
Merged at revision: 177
Proposed branch: lp:~cimi/overlay-scrollbar/fix-compiz-stacking
Merge into: lp:overlay-scrollbar
Diff against target: 107 lines (+76/-0)
1 file modified
os/os-scrollbar.c (+76/-0)
To merge this branch: bzr merge lp:~cimi/overlay-scrollbar/fix-compiz-stacking
Reviewer Review Type Date Requested Status
Loïc Molinari (community) Approve
Review via email: mp+54763@code.launchpad.net

Description of the change

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

To post a comment you must log in.
176. By Andrea Cimitan

renamed XEvent *event to xev

Revision history for this message
Loïc Molinari (loic.molinari) wrote :

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
Revision history for this message
Loïc Molinari (loic.molinari) wrote :

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
Revision history for this message
Andrea Cimitan (cimi) wrote :

> 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.

Revision history for this message
Andrea Cimitan (cimi) wrote :

> 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

177. By Andrea Cimitan

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

Revision history for this message
Loïc Molinari (loic.molinari) wrote :

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

Revision history for this message
Loïc Molinari (loic.molinari) wrote :

Thanks for the error handling bits.

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

Revision history for this message
Andrea Cimitan (cimi) wrote :

> 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

178. By Andrea Cimitan

removed XSync as gdk_flush is a wrapper for it

Revision history for this message
Loïc Molinari (loic.molinari) wrote :

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

Revision history for this message
Loïc Molinari (loic.molinari) wrote :

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

179. By Andrea Cimitan

Added FIXME comment

Revision history for this message
Loïc Molinari (loic.molinari) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'os/os-scrollbar.c'
2--- os/os-scrollbar.c 2011-03-28 16:17:49 +0000
3+++ os/os-scrollbar.c 2011-03-28 17:33:46 +0000
4@@ -97,6 +97,7 @@
5 static gboolean thumb_button_release_event_cb (GtkWidget *widget, GdkEventButton *event, gpointer user_data);
6 static gboolean thumb_enter_notify_event_cb (GtkWidget *widget, GdkEventCrossing *event, gpointer user_data);
7 static gboolean thumb_leave_notify_event_cb (GtkWidget *widget, GdkEventCrossing *event, gpointer user_data);
8+static void thumb_map_cb (GtkWidget *widget, gpointer user_data);
9 static gboolean thumb_motion_notify_event_cb (GtkWidget *widget, GdkEventMotion *event, gpointer user_data);
10 static void thumb_unmap_cb (GtkWidget *widget, gpointer user_data);
11 static void pager_move (OsScrollbar *scrollbar);
12@@ -578,6 +579,8 @@
13 g_signal_handlers_disconnect_by_func (G_OBJECT (priv->thumb),
14 thumb_leave_notify_event_cb, scrollbar);
15 g_signal_handlers_disconnect_by_func (G_OBJECT (priv->thumb),
16+ thumb_map_cb, scrollbar);
17+ g_signal_handlers_disconnect_by_func (G_OBJECT (priv->thumb),
18 thumb_motion_notify_event_cb, scrollbar);
19 g_signal_handlers_disconnect_by_func (G_OBJECT (priv->thumb),
20 thumb_unmap_cb, scrollbar);
21@@ -599,6 +602,8 @@
22 G_CALLBACK (thumb_enter_notify_event_cb), scrollbar);
23 g_signal_connect (G_OBJECT (priv->thumb), "leave-notify-event",
24 G_CALLBACK (thumb_leave_notify_event_cb), scrollbar);
25+ g_signal_connect (G_OBJECT (priv->thumb), "map",
26+ G_CALLBACK (thumb_map_cb), scrollbar);
27 g_signal_connect (G_OBJECT (priv->thumb), "motion-notify-event",
28 G_CALLBACK (thumb_motion_notify_event_cb), scrollbar);
29 g_signal_connect (G_OBJECT (priv->thumb), "unmap",
30@@ -729,6 +734,77 @@
31 return FALSE;
32 }
33
34+static void
35+thumb_map_cb (GtkWidget *widget,
36+ gpointer user_data)
37+{
38+ Display *display;
39+ OsScrollbar *scrollbar;
40+ OsScrollbarPrivate *priv;
41+ XWindowChanges changes;
42+ guint32 xid, xid_parent;
43+ unsigned int value_mask = CWSibling | CWStackMode;
44+ int res;
45+
46+ scrollbar = OS_SCROLLBAR (user_data);
47+ priv = scrollbar->priv;
48+
49+ xid = GDK_WINDOW_XID (gtk_widget_get_window (priv->thumb));
50+ xid_parent = GDK_WINDOW_XID (gtk_widget_get_window (priv->parent));
51+ display = GDK_WINDOW_XDISPLAY (gtk_widget_get_window (GTK_WIDGET (scrollbar)));
52+
53+ changes.sibling = xid_parent;
54+ changes.stack_mode = Above;
55+
56+ gdk_error_trap_push ();
57+ XConfigureWindow (display, xid, value_mask, &changes);
58+
59+ gdk_flush ();
60+ if ((res = gdk_error_trap_pop ()))
61+ {
62+ /* FIXME(Cimi) this code tries to restack the window using
63+ * the _NET_RESTACK_WINDOW atom. See:
64+ * http://standards.freedesktop.org/wm-spec/wm-spec-1.3.html#id2506866
65+ *
66+ * Should work on window managers that supports this, like compiz.
67+ * Unfortunately, metacity doesn't yet, so we might decide to implement
68+ * this atom in metacity/mutter as well.
69+ *
70+ * We need to restack the window because the thumb window can be above
71+ * every window, noticeable when you make the thumb of an unfocused window
72+ * appear, and it could be above other windows (like the focused one).
73+ *
74+ * XConfigureWindow doesn't always work (well, should work only with
75+ * compiz 0.8 or older), because it is not handling the reparenting done
76+ * at the WM level (metacity and compiz >= 0.9 do reparenting). */
77+ Window root = gdk_x11_get_default_root_xwindow ();
78+ XEvent xev;
79+
80+ xev.xclient.type = ClientMessage;
81+ xev.xclient.display = display;
82+ xev.xclient.serial = 0;
83+ xev.xclient.send_event = True;
84+ xev.xclient.window = xid;
85+ xev.xclient.message_type = gdk_x11_get_xatom_by_name ("_NET_RESTACK_WINDOW");
86+ xev.xclient.format = 32;
87+ xev.xclient.data.l[0] = 2;
88+ xev.xclient.data.l[1] = xid_parent;
89+ xev.xclient.data.l[2] = Above;
90+ xev.xclient.data.l[3] = 0;
91+ xev.xclient.data.l[4] = 0;
92+
93+ gdk_error_trap_push ();
94+
95+ XSendEvent (display, root, False,
96+ SubstructureRedirectMask | SubstructureNotifyMask,
97+ &xev);
98+
99+ gdk_flush ();
100+
101+ gdk_error_trap_pop ();
102+ }
103+}
104+
105 static gboolean
106 thumb_motion_notify_event_cb (GtkWidget *widget,
107 GdkEventMotion *event,

Subscribers

People subscribed via source and target branches