MonitorXrandr: check the event timestamps before reconfiguring

If, checking the event timestamps, we see that a new configuration
was explicitly requested by an another XRandR client, don't proceed to
apply the intended configuration again, even if looking at the
EDIDs it appears that the outputs changed.
This works around some buggy Xorg drivers (qxl, vbox) that generate
a new serial number everytime the user resizes the host window.

https://bugzilla.gnome.org/show_bug.cgi?id=706735

src/core/monitor-config.c

@@ -55,6 +55,8 @@ typedef struct {
   char *serial;
 } MetaOutputKey;
 
+/* Keep this structure packed, so that we
+   can use memcmp */
 typedef struct {
   gboolean enabled;
   MetaRectangle rect;
@@ -147,6 +149,13 @@ output_key_equal (const MetaOutputKey *one,
     strcmp (one->serial, two->serial) == 0;
 }
 
+static gboolean
+output_config_equal (const MetaOutputConfig *one,
+                     const MetaOutputConfig *two)
+{
+  return memcmp (one, two, sizeof (MetaOutputConfig)) == 0;
+}
+
 static unsigned int
 config_hash (gconstpointer data)
 {
@@ -180,6 +189,30 @@ config_equal (gconstpointer one,
   return ok;
 }
 
+static gboolean
+config_equal_full (gconstpointer one,
+                   gconstpointer two)
+{
+  const MetaConfiguration *c_one = one;
+  const MetaConfiguration *c_two = two;
+  unsigned int i;
+  gboolean ok;
+
+  if (c_one->n_outputs != c_two->n_outputs)
+    return FALSE;
+
+  ok = TRUE;
+  for (i = 0; i < c_one->n_outputs && ok; i++)
+    {
+      ok = output_key_equal (&c_one->keys[i],
+                             &c_two->keys[i]);
+      ok = ok && output_config_equal (&c_one->outputs[i],
+                                      &c_two->outputs[i]);
+    }
+
+  return ok;
+}
+
 static void
 meta_monitor_config_init (MetaMonitorConfig *self)
 {
@@ -1243,6 +1276,12 @@ meta_monitor_config_update_current (MetaMonitorConfig  *self,
       init_config_from_output (&current->outputs[i], &outputs[i]);
     }
 
+  if (self->current && config_equal_full (current, self->current))
+    {
+      config_free (current);
+      return;
+    }
+
   if (self->current && !self->current_is_stored)
     config_free (self->current);
 

src/core/monitor-private.h

@@ -301,6 +301,8 @@ GType meta_monitor_manager_get_type (void);
 void                meta_monitor_manager_initialize (void);
 MetaMonitorManager *meta_monitor_manager_get  (void);
 
+void                meta_monitor_manager_rebuild_derived   (MetaMonitorManager *manager);
+
 MetaMonitorInfo    *meta_monitor_manager_get_monitor_infos (MetaMonitorManager *manager,
 							    unsigned int       *n_infos);
 
@@ -379,6 +381,9 @@ void               meta_monitor_config_restore_previous (MetaMonitorConfig  *con
 void               meta_crtc_info_free   (MetaCRTCInfo   *info);
 void               meta_output_info_free (MetaOutputInfo *info);
 
+void               meta_monitor_manager_free_output_array (MetaOutput *old_outputs,
+                                                           int         n_old_outputs);
+
 /* Returns true if transform causes width and height to be inverted
    This is true for the odd transforms in the enum */
 static inline gboolean

src/core/monitor-xrandr.c

@@ -974,11 +974,57 @@ meta_monitor_manager_xrandr_handle_xevent (MetaMonitorManager *manager,
 					   XEvent             *event)
 {
   MetaMonitorManagerXrandr *manager_xrandr = META_MONITOR_MANAGER_XRANDR (manager);
+  MetaOutput *old_outputs;
+  MetaCRTC *old_crtcs;
+  MetaMonitorMode *old_modes;
+  int n_old_outputs;
 
   if ((event->type - manager_xrandr->rr_event_base) != RRScreenChangeNotify)
     return FALSE;
 
   XRRUpdateConfiguration (event);
+
+  /* Save the old structures, so they stay valid during the update */
+  old_outputs = manager->outputs;
+  n_old_outputs = manager->n_outputs;
+  old_modes = manager->modes;
+  old_crtcs = manager->crtcs;
+
+  manager->serial++;
+  meta_monitor_manager_xrandr_read_current (manager);
+
+  /* Check if the current intended configuration has the same outputs
+     as the new real one, or if the event is a result of an XRandR call.
+     If so, we can go straight to rebuild the logical config and tell
+     the outside world.
+     Otherwise, this event was caused by hotplug, so give a chance to
+     MetaMonitorConfig.
+
+     Note that we need to check both the timestamps and the list of
+     outputs, because the X server might emit spurious events with
+     new configTimestamps (bug 702804), and the driver may have
+     changed the EDID for some other reason (old broken qxl and vbox
+     drivers...).
+  */
+  if (manager_xrandr->resources->timestamp >= manager_xrandr->resources->configTimestamp ||
+      meta_monitor_config_match_current (manager->config, manager))
+    {
+      /* This will be a no-op if the change was from our side, as
+         we already called it in the DBus method handler */
+      meta_monitor_config_update_current (manager->config, manager);
+
+      meta_monitor_manager_rebuild_derived (manager);
+    }
+  else
+    {
+      if (!meta_monitor_config_apply_stored (manager->config, manager))
+        meta_monitor_config_make_default (manager->config, manager);
+    }
+
+  meta_monitor_manager_free_output_array (old_outputs, n_old_outputs);
+  g_free (old_modes);
+  g_free (old_crtcs);
+
   return TRUE;
 }
 

src/core/monitor.c

@@ -59,9 +59,6 @@ static void meta_monitor_manager_display_config_init (MetaDBusDisplayConfigIface
 G_DEFINE_TYPE_WITH_CODE (MetaMonitorManager, meta_monitor_manager, META_DBUS_TYPE_DISPLAY_CONFIG_SKELETON,
                          G_IMPLEMENT_INTERFACE (META_DBUS_TYPE_DISPLAY_CONFIG, meta_monitor_manager_display_config_init));
 
-static void free_output_array (MetaOutput *old_outputs,
-                               int         n_old_outputs);
-static void invalidate_logical_config (MetaMonitorManager *manager);
 static void initialize_dbus_interface (MetaMonitorManager *manager);
 
 static void
@@ -349,7 +346,7 @@ apply_config_dummy (MetaMonitorManager *manager,
   manager->screen_width = screen_width;
   manager->screen_height = screen_height;
 
-  invalidate_logical_config (manager);
+  meta_monitor_manager_rebuild_derived (manager);
 }
 
 static GBytes *
@@ -527,7 +524,7 @@ meta_monitor_manager_constructed (GObject *object)
 
       read_current_config (manager);
 
-      free_output_array (old_outputs, n_old_outputs);
+      meta_monitor_manager_free_output_array (old_outputs, n_old_outputs);
       g_free (old_modes);
       g_free (old_crtcs);
     }
@@ -558,9 +555,9 @@ meta_monitor_manager_set_power_save_mode (MetaMonitorManager *manager,
   manager->power_save_mode = mode;
 }
 
-static void
+void
-free_output_array (MetaOutput *old_outputs,
+meta_monitor_manager_free_output_array (MetaOutput *old_outputs,
-                   int         n_old_outputs)
+                                        int         n_old_outputs)
 {
   int i;
 
@@ -583,7 +580,7 @@ meta_monitor_manager_finalize (GObject *object)
 {
   MetaMonitorManager *manager = META_MONITOR_MANAGER (object);
 
-  free_output_array (manager->outputs, manager->n_outputs);
+  meta_monitor_manager_free_output_array (manager->outputs, manager->n_outputs);
   g_free (manager->monitor_infos);
   g_free (manager->modes);
   g_free (manager->crtcs);
@@ -1476,8 +1473,8 @@ meta_monitor_manager_get_screen_limits (MetaMonitorManager *manager,
   *height = manager->max_screen_height;
 }
 
-static void
+void
-invalidate_logical_config (MetaMonitorManager *manager)
+meta_monitor_manager_rebuild_derived (MetaMonitorManager *manager)
 {
   MetaMonitorInfo *old_monitor_infos;
 
@@ -1498,51 +1495,11 @@ meta_monitor_manager_handle_xevent (MetaMonitorManager *manager,
                                     XEvent             *event)
 {
   MetaMonitorManagerClass *klass;
-  MetaOutput *old_outputs;
-  MetaCRTC *old_crtcs;
-  MetaMonitorMode *old_modes;
-  int n_old_outputs;
-  gboolean changed;
 
   klass = META_MONITOR_MANAGER_GET_CLASS (manager);
   if (klass->handle_xevent)
-    changed = klass->handle_xevent (manager, event);
+    return klass->handle_xevent (manager, event);
   else
-    changed = FALSE;
-
-  if (!changed)
     return FALSE;
-
-  /* Save the old structures, so they stay valid during the update */
-  old_outputs = manager->outputs;
-  n_old_outputs = manager->n_outputs;
-  old_modes = manager->modes;
-  old_crtcs = manager->crtcs;
-
-  read_current_config (manager);
-
-  /* Check if the current intended configuration has the same outputs
-     as the new real one. If so, this was a result of an ApplyConfiguration
-     call (or a change from ourselves), and we can go straight to rebuild
-     the logical config and tell the outside world.
-
-     Otherwise, this event was caused by hotplug, so give a chance to
-     MetaMonitorConfig.
-  */
-  if (meta_monitor_config_match_current (manager->config, manager))
-    {
-      invalidate_logical_config (manager);
-    }
-  else
-    {
-      if (!meta_monitor_config_apply_stored (manager->config, manager))
-        meta_monitor_config_make_default (manager->config, manager);
-    }
-
-  free_output_array (old_outputs, n_old_outputs);
-  g_free (old_modes);
-  g_free (old_crtcs);
-
-  return TRUE;
 }