Merge lp:~vanvugt/unity/fix-687567-unity3.0 into lp:unity/3.0

[Daniel van Vugt]

Fix white "flashes" seen in the panel menus during scrubbing, or when they're unmapped (LP: #687567)

The problem, it seems, was that we were calling gdk/gtk/X11 functions in the middle of a GDK event filter. Removing these function calls and replacing them with a simpler, more optimized solution solves the problem.

I'm proposing this fix to unity 3.0 first because it was developed and tested on natty. I will forward port it to unity trunk (requires some conflict resolution) after this version has been approved.

[Daniel van Vugt (vanvugt)]

And before someone suggests the bug was a timing issue, it wasn't. I tried inserting delays in the affected code and it made no difference. Only removing the gdk/gtk/X calls fixed the problem.

[Neil J. Patel (njpatel)]

*hattips*

Very, very nice merge :) I couldn't compile old Unity on my system so couldn't test your branch but I did a quick port of your work to the latest unity (diff: http://paste.ubuntu.com/643341/) and it works perfectly!

If you want to merge-propose something for trunk, I'll compile and test this on my natty desktop this evening.

[Neil J. Patel (njpatel)]

Works perfectly, approved.

review: Approve

=== modified file 'services/panel-service.c'
--- services/panel-service.c	2011-04-10 23:51:05 +0000
+++ services/panel-service.c	2011-07-13 14:24:24 +0000
@@ -55,6 +55,10 @@
   guint32  last_menu_move_id;
   gint32   last_x;
   gint32   last_y;
+  gint     last_left;
+  gint     last_top;
+  gint     last_right;
+  gint     last_bottom;
   guint32  last_menu_button;
 
   gint     last_menu_x;
@@ -220,84 +224,29 @@
   if (!GTK_IS_WIDGET (self->priv->last_menu))
     return ret;
 
-  if (e->type == 5 && self->priv->last_menu_button != 0) //FocusChange
+  // Don't call any gdk/gtk/X functions in here or you risk creating graphical
+  // glitches (LP: #687567) and of course slowing down event processing.
+  if (e->type == ButtonRelease && self->priv->last_menu_button != 0)
     {
-      gint       x=0, y=0, width=0, height=0, depth=0, x_root=0, y_root=0;
-      GdkWindow *window = gtk_widget_get_window (GTK_WIDGET (self->priv->last_menu));
-      if (window == NULL)
-        return GDK_FILTER_CONTINUE;
-
-      Window     xwindow = gdk_x11_drawable_get_xid (GDK_DRAWABLE (window));
-
-      if (xwindow == 0)
-        return GDK_FILTER_CONTINUE;
-
-      Window     root = 0, child = 0;
-      int        win_x=0, win_y = 0;
-      guint32    mask_return = 0;
-
-      XQueryPointer (gdk_x11_display_get_xdisplay (gdk_display_get_default ()),
-                     xwindow,
-                     &root,
-                     &child,
-                     &x_root,
-                     &y_root,
-                     &win_x,
-                     &win_y,
-                     &mask_return);
-
-      gdk_window_get_geometry (window, &x, &y, &width, &height, &depth);
-      gdk_window_get_origin (window, &x, &y);
-
-      if (x_root > x
-          && x_root < x + width
-          && y_root > y
-          && y_root < y + height)
-        {
-          ret = GDK_FILTER_CONTINUE;
-        }
-      else
+      if (e->xbutton.x_root < self->priv->last_left ||
+          e->xbutton.x_root > self->priv->last_right ||
+          e->xbutton.y_root < self->priv->last_top ||
+          e->xbutton.y_root > self->priv->last_bottom)
         {
           ret = GDK_FILTER_REMOVE;
         }
 
       self->priv->last_menu_button = 0;
     }
-
-  // FIXME: THIS IS HORRIBLE AND WILL BE CHANGED BEFORE RELEASE
-  // ITS A WORKAROUND SO I CAN TEST THE PANEL SCRUBBING
-  // DONT HATE ME
-  // --------------------------------------------------------------------------
-  else if (e->type == 6)
+  else if (e->type == MotionNotify)
     {
-      int       x_root=0, y_root=0;
-      GdkWindow *window = gtk_widget_get_window (GTK_WIDGET (self->priv->last_menu));
-      Window     xwindow = gdk_x11_drawable_get_xid (GDK_DRAWABLE (window));
-      Window     root = 0, child = 0;
-      int        win_x=0, win_y = 0;
-      guint32    mask_return = 0;
-
-      XQueryPointer (gdk_x11_display_get_xdisplay (gdk_display_get_default ()),
-                     xwindow,
-                     &root,
-                     &child,
-                     &x_root,
-                     &y_root,
-                     &win_x,
-                     &win_y,
-                     &mask_return);
-
-      self->priv->last_menu_x = x_root;
-      self->priv->last_menu_y = y_root;
-
-      if (y_root <= self->priv->last_y)
+      self->priv->last_menu_x = e->xmotion.x_root;
+      self->priv->last_menu_y = e->xmotion.y_root;
+      if (self->priv->last_menu_y <= self->priv->last_y)
         {
           g_signal_emit (self, _service_signals[ACTIVE_MENU_POINTER_MOTION], 0);
         }
     }
-  // /DONT HATE ME
-  // /FIXME
-  // --------------------------------------------------------------------------
   return ret;
 }
 
@@ -852,6 +801,10 @@
   priv->last_menu_id = 0;
   priv->last_menu_move_id = 0;
   priv->last_entry = NULL;
+  priv->last_left = 0;
+  priv->last_right = 0;
+  priv->last_top = 0;
+  priv->last_bottom = 0;
 
   g_signal_emit (self, _service_signals[ENTRY_ACTIVATED], 0, "");
 }
@@ -1149,6 +1102,24 @@
 
       indicator_object_entry_activate (object, entry, CurrentTime);
       gtk_menu_popup (priv->last_menu, NULL, NULL, positon_menu, self, 0, CurrentTime);
+      GdkWindow *gdkwin = gtk_widget_get_window (GTK_WIDGET (priv->last_menu));
+      if (gdkwin != NULL)
+        {
+          gint left, top, width, height;
+          gdk_window_get_geometry (gdkwin, NULL, NULL, &width, &height, NULL);
+          gdk_window_get_origin (gdkwin, &left, &top);
+          priv->last_left = left;
+          priv->last_right = left + width - 1;
+          priv->last_top = top;
+          priv->last_bottom = top + height - 1;
+        }
+      else
+        {
+          priv->last_left = 0;
+          priv->last_right = 0;
+          priv->last_top = 0;
+          priv->last_bottom = 0;
+        }
 
       g_signal_emit (self, _service_signals[ENTRY_ACTIVATED], 0, entry_id);
     }