From 9678a412e2266e4994ad7ec2fb0bcf67d256bbed Mon Sep 17 00:00:00 2001 From: Giovanni Campagna Date: Mon, 2 Sep 2013 15:04:05 +0200 Subject: [PATCH] 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 420c670b6..c223a38d1 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 (¤t->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 b0fd70c4c..beef289bb 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 bcceff80f..5e02c0ac6 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 6aea00384..4f29ea19b 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; }