[mutter] MonitorXrandr: check the event timestamps before reconfiguring

[Giovanni Campagna <gcampagna src gnome org>]

commit 9678a412e2266e4994ad7ec2fb0bcf67d256bbed
Author: Giovanni Campagna <gcampagn redhat com>
Date:   Mon Sep 2 15:04:05 2013 +0200

    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  |   39 ++++++++++++++++++++++++++++
 src/core/monitor-private.h |    5 +++
 src/core/monitor-xrandr.c  |   46 +++++++++++++++++++++++++++++++++
 src/core/monitor.c         |   61 ++++++-------------------------------------
 4 files changed, 99 insertions(+), 52 deletions(-)
---
diff --git a/src/core/monitor-config.c b/src/core/monitor-config.c
index 420c670..c223a38 100644
--- a/src/core/monitor-config.c
+++ b/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);
 
diff --git a/src/core/monitor-private.h b/src/core/monitor-private.h
index b0fd70c..beef289 100644
--- a/src/core/monitor-private.h
+++ b/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
diff --git a/src/core/monitor-xrandr.c b/src/core/monitor-xrandr.c
index bcceff8..5e02c0a 100644
--- a/src/core/monitor-xrandr.c
+++ b/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;
 }
 
diff --git a/src/core/monitor.c b/src/core/monitor.c
index 6aea003..4f29ea1 100644
--- a/src/core/monitor.c
+++ b/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
-free_output_array (MetaOutput *old_outputs,
-                   int         n_old_outputs)
+void
+meta_monitor_manager_free_output_array (MetaOutput *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
-invalidate_logical_config (MetaMonitorManager *manager)
+void
+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;
 }