tests/monitor-unit-tests: Add test to for ignoring connector IDs

In some cases the connector ID should be ignored. Add a flag to modify
the generated connector ID, and test whether the stored configuration
still applies with the changed IDs.

https://gitlab.gnome.org/GNOME/mutter/issues/932

src/backends/meta-monitor-config-manager.c

@@ -410,6 +410,25 @@ is_lid_closed (MetaMonitorManager *monitor_manager)
     return meta_backend_is_lid_closed (backend);
 }
 
+gboolean
+meta_config_key_edid_sufficient_for_specs (GList *monitor_specs)
+{
+  GList *l;
+
+  for (l = monitor_specs; l && l->next; l = l->next)
+    {
+      MetaMonitorSpec *spec = l->data;
+      MetaMonitorSpec *spec_next = l->next->data;
+
+      if (meta_monitor_spec_equals (spec, spec_next, TRUE))
+        return FALSE;
+
+      /* XXX: Should we reject e.g. only 0 serial numbers? */
+    }
+
+  return TRUE;
+}
+
 MetaMonitorsConfigKey *
 meta_create_monitors_config_key_for_current_state (MetaMonitorManager *monitor_manager)
 {
@@ -439,7 +458,8 @@ meta_create_monitors_config_key_for_current_state (MetaMonitorManager *monitor_m
 
   config_key = g_new0 (MetaMonitorsConfigKey, 1);
   *config_key = (MetaMonitorsConfigKey) {
-    .monitor_specs = monitor_specs
+    .monitor_specs = monitor_specs,
+    .edid_sufficient = meta_config_key_edid_sufficient_for_specs (monitor_specs)
   };
 
   return config_key;
@@ -923,7 +943,8 @@ find_logical_config_for_builtin_display_rotation (MetaMonitorConfigManager *conf
 
           monitor_config = logical_monitor_config->monitor_configs->data;
           if (meta_monitor_spec_equals (meta_monitor_get_spec (panel),
-                                        monitor_config->monitor_spec))
+                                        monitor_config->monitor_spec,
+                                        config_manager->monitor_manager->edid_sufficient))
             return logical_monitor_config;
         }
     }
@@ -1356,7 +1377,8 @@ meta_monitors_config_key_new (GList *logical_monitor_configs,
 
   config_key = g_new0 (MetaMonitorsConfigKey, 1);
   *config_key = (MetaMonitorsConfigKey) {
-    .monitor_specs = monitor_specs
+    .monitor_specs = monitor_specs,
+    .edid_sufficient = meta_config_key_edid_sufficient_for_specs (monitor_specs)
   };
 
   return config_key;
@@ -1382,10 +1404,12 @@ meta_monitors_config_key_hash (gconstpointer data)
     {
       MetaMonitorSpec *monitor_spec = l->data;
 
-      hash ^= (g_str_hash (monitor_spec->connector) ^
+      hash ^= (g_str_hash (monitor_spec->vendor) ^
-               g_str_hash (monitor_spec->vendor) ^
                g_str_hash (monitor_spec->product) ^
                g_str_hash (monitor_spec->serial));
+
+      if (!config_key->edid_sufficient)
+        hash ^= g_str_hash (monitor_spec->connector);
     }
 
   return hash;
@@ -1399,6 +1423,9 @@ meta_monitors_config_key_equal (gconstpointer data_a,
   const MetaMonitorsConfigKey *config_key_b = data_b;
   GList *l_a, *l_b;
 
+  if (config_key_a->edid_sufficient != config_key_b->edid_sufficient)
+    return FALSE;
+
   for (l_a = config_key_a->monitor_specs, l_b = config_key_b->monitor_specs;
        l_a && l_b;
        l_a = l_a->next, l_b = l_b->next)
@@ -1406,7 +1433,8 @@ meta_monitors_config_key_equal (gconstpointer data_a,
       MetaMonitorSpec *monitor_spec_a = l_a->data;
       MetaMonitorSpec *monitor_spec_b = l_b->data;
 
-      if (!meta_monitor_spec_equals (monitor_spec_a, monitor_spec_b))
+      if (!meta_monitor_spec_equals (monitor_spec_a, monitor_spec_b,
+                                     config_key_a->edid_sufficient))
         return FALSE;
     }
 
@@ -1471,7 +1499,8 @@ meta_monitors_config_new (MetaMonitorManager           *monitor_manager,
 
       monitor_spec = meta_monitor_get_spec (monitor);
       if (meta_logical_monitor_configs_have_monitor (logical_monitor_configs,
-                                                     monitor_spec))
+                                                     monitor_spec,
+                                                     monitor_manager->edid_sufficient))
         continue;
 
       disabled_monitor_specs =
@@ -1674,7 +1703,8 @@ has_adjacent_neighbour (MetaMonitorsConfig       *config,
 
 gboolean
 meta_logical_monitor_configs_have_monitor (GList           *logical_monitor_configs,
-                                           MetaMonitorSpec *monitor_spec)
+                                           MetaMonitorSpec *monitor_spec,
+                                           gboolean         edid_sufficient)
 {
   GList *l;
 
@@ -1688,7 +1718,8 @@ meta_logical_monitor_configs_have_monitor (GList           *logical_monitor_conf
           MetaMonitorConfig *monitor_config = k->data;
 
           if (meta_monitor_spec_equals (monitor_spec,
-                                        monitor_config->monitor_spec))
+                                        monitor_config->monitor_spec,
+                                        edid_sufficient))
             return TRUE;
         }
     }
@@ -1701,7 +1732,8 @@ meta_monitors_config_is_monitor_enabled (MetaMonitorsConfig *config,
                                          MetaMonitorSpec    *monitor_spec)
 {
   return meta_logical_monitor_configs_have_monitor (config->logical_monitor_configs,
-                                                    monitor_spec);
+                                                    monitor_spec,
+                                                    config->key->edid_sufficient);
 }
 
 gboolean

src/backends/meta-monitor-config-manager.h

@@ -48,7 +48,9 @@ typedef struct _MetaLogicalMonitorConfig
 
 typedef struct _MetaMonitorsConfigKey
 {
+  /* monitor_specs needs to be sorted using meta_monitor_spec_compare. */
   GList *monitor_specs;
+  gboolean edid_sufficient;
 } MetaMonitorsConfigKey;
 
 typedef enum _MetaMonitorsConfigFlag
@@ -159,6 +161,9 @@ META_EXPORT_TEST
 gboolean meta_monitors_config_key_equal (gconstpointer config_key_a,
                                          gconstpointer config_key_b);
 
+META_EXPORT_TEST
+gboolean meta_config_key_edid_sufficient_for_specs (GList *monitor_specs);
+
 META_EXPORT_TEST
 void meta_monitors_config_key_free (MetaMonitorsConfigKey *config_key);
 
@@ -173,7 +178,8 @@ MetaMonitorsConfigKey * meta_create_monitors_config_key_for_current_state (MetaM
 
 META_EXPORT_TEST
 gboolean meta_logical_monitor_configs_have_monitor (GList           *logical_monitor_configs,
-                                                    MetaMonitorSpec *monitor_spec);
+                                                    MetaMonitorSpec *monitor_spec,
+                                                    gboolean         edid_sufficient);
 
 META_EXPORT_TEST
 gboolean meta_verify_monitor_mode_spec (MetaMonitorModeSpec *monitor_mode_spec,

src/backends/meta-monitor-manager-private.h

@@ -122,6 +122,7 @@ struct _MetaMonitorManager
   int screen_height;
 
   GList *monitors;
+  gboolean edid_sufficient;
 
   GList *logical_monitors;
   MetaLogicalMonitor *primary_logical_monitor;

src/backends/meta-monitor-manager.c

@@ -2475,7 +2475,8 @@ meta_monitor_manager_get_monitor_from_spec (MetaMonitorManager *manager,
       MetaMonitor *monitor = l->data;
 
       if (meta_monitor_spec_equals (meta_monitor_get_spec (monitor),
-                                    monitor_spec))
+                                    monitor_spec,
+                                    manager->edid_sufficient))
         return monitor;
     }
 
@@ -2621,6 +2622,7 @@ rebuild_monitors (MetaMonitorManager *manager)
 {
   GList *gpus;
   GList *l;
+  GList *specs;
 
   if (manager->monitors)
     {
@@ -2659,6 +2661,15 @@ rebuild_monitors (MetaMonitorManager *manager)
             }
         }
     }
+
+  specs = NULL;
+  for (l = manager->monitors; l; l = l->next)
+    specs = g_list_prepend (specs, meta_monitor_get_spec (l->data));
+
+  specs = g_list_sort (specs,
+                       (GCompareFunc) meta_monitor_spec_compare);
+  manager->edid_sufficient = meta_config_key_edid_sufficient_for_specs (specs);
+  g_list_free (specs);
 }
 
 void

src/backends/meta-monitor.c

@@ -126,12 +126,13 @@ meta_monitor_spec_clone (MetaMonitorSpec *monitor_spec)
 
 gboolean
 meta_monitor_spec_equals (MetaMonitorSpec *monitor_spec,
-                          MetaMonitorSpec *other_monitor_spec)
+                          MetaMonitorSpec *other_monitor_spec,
+                          gboolean         edid_only)
 {
-  return (g_str_equal (monitor_spec->connector, other_monitor_spec->connector) &&
+  return (g_str_equal (monitor_spec->vendor, other_monitor_spec->vendor) &&
-          g_str_equal (monitor_spec->vendor, other_monitor_spec->vendor) &&
           g_str_equal (monitor_spec->product, other_monitor_spec->product) &&
-          g_str_equal (monitor_spec->serial, other_monitor_spec->serial));
+          g_str_equal (monitor_spec->serial, other_monitor_spec->serial) &&
+          (edid_only || g_str_equal (monitor_spec->connector, other_monitor_spec->connector)));
 }
 
 int
@@ -140,10 +141,6 @@ meta_monitor_spec_compare (MetaMonitorSpec *monitor_spec_a,
 {
   int ret;
 
-  ret = strcmp (monitor_spec_a->connector, monitor_spec_b->connector);
-  if (ret != 0)
-    return ret;
-
   ret = strcmp (monitor_spec_a->vendor, monitor_spec_b->vendor);
   if (ret != 0)
     return ret;
@@ -152,7 +149,15 @@ meta_monitor_spec_compare (MetaMonitorSpec *monitor_spec_a,
   if (ret != 0)
     return ret;
 
-  return strcmp (monitor_spec_a->serial, monitor_spec_b->serial);
+  ret = strcmp (monitor_spec_a->serial, monitor_spec_b->serial);
+  if (ret != 0)
+    return ret;
+
+  ret = strcmp (monitor_spec_a->connector, monitor_spec_b->connector);
+  if (ret != 0)
+    return ret;
+
+  return 0;
 }
 
 void

src/backends/meta-monitor.h

@@ -272,7 +272,8 @@ MetaMonitorSpec * meta_monitor_spec_clone (MetaMonitorSpec *monitor_id);
 
 META_EXPORT_TEST
 gboolean meta_monitor_spec_equals (MetaMonitorSpec *monitor_id,
-                                   MetaMonitorSpec *other_monitor_id);
+                                   MetaMonitorSpec *other_monitor_id,
+                                   gboolean         edid_only);
 
 META_EXPORT_TEST
 int meta_monitor_spec_compare (MetaMonitorSpec *monitor_spec_a,

src/tests/monitor-configs/lid-switch.xml

@@ -30,7 +30,7 @@
 	  <connector>DP-1</connector>
 	  <vendor>MetaProduct's Inc.</vendor>
 	  <product>MetaMonitor</product>
-	  <serial>0x123456</serial>
+	  <serial>0x654321</serial>
 	</monitorspec>
 	<mode>
 	  <width>1024</width>
@@ -54,7 +54,7 @@
 	  <connector>DP-1</connector>
 	  <vendor>MetaProduct's Inc.</vendor>
 	  <product>MetaMonitor</product>
-	  <serial>0x123456</serial>
+	  <serial>0x654321</serial>
 	</monitorspec>
 	<mode>
 	  <width>1024</width>

src/tests/monitor-store-unit-tests.c

@@ -108,7 +108,8 @@ create_config_key_from_expect (MonitorStoreTestConfiguration *expect_config)
 
   config_key = g_new0 (MetaMonitorsConfigKey, 1);
   *config_key = (MetaMonitorsConfigKey) {
-    .monitor_specs = monitor_specs
+    .monitor_specs = monitor_specs,
+    .edid_sufficient = meta_config_key_edid_sufficient_for_specs (monitor_specs)
   };
 
   return config_key;

src/tests/monitor-unit-tests.c

@@ -89,7 +89,8 @@
 typedef enum _MonitorTestFlag
 {
   MONITOR_TEST_FLAG_NONE,
-  MONITOR_TEST_FLAG_NO_STORED
+  MONITOR_TEST_FLAG_NO_STORED,
+  MONITOR_TEST_FLAG_OFFSET_DP_CONNECTOR,
 } MonitorTestFlag;
 
 typedef struct _MonitorTestCaseMode
@@ -924,6 +925,9 @@ create_monitor_test_setup (MonitorTestCase *test_case,
   else
     hotplug_mode_update = FALSE;
 
+  if (flags & MONITOR_TEST_FLAG_OFFSET_DP_CONNECTOR)
+    n_normal_panels = 100;
+
   test_setup = g_new0 (MetaMonitorTestSetup, 1);
 
   test_setup->modes = NULL;
@@ -5279,6 +5283,15 @@ meta_test_monitor_custom_oneoff (void)
   emulate_hotplug (test_setup);
 
   check_monitor_configuration (&test_case);
+
+  /* Same test after hotplugging the monitor to a different port */
+  test_setup = create_monitor_test_setup (&test_case,
+                                          MONITOR_TEST_FLAG_NONE |
+                                          MONITOR_TEST_FLAG_OFFSET_DP_CONNECTOR);
+  set_custom_monitor_config ("oneoff.xml");
+  emulate_hotplug (test_setup);
+
+  check_monitor_configuration (&test_case);
 }
 
 static void
@@ -5310,6 +5323,7 @@ meta_test_monitor_custom_lid_switch_config (void)
           .crtc = -1,
           .modes = { 0 },
           .n_modes = 1,
+          .serial = "0x654321",
           .preferred_mode = 0,
           .possible_crtcs = { 1 },
           .n_possible_crtcs = 1,
@@ -5410,6 +5424,8 @@ meta_test_monitor_custom_lid_switch_config (void)
   MetaMonitorTestSetup *test_setup;
   MetaBackend *backend = meta_get_backend ();
 
+  g_debug ("Initial configuration (only internal screen)");
+
   test_setup = create_monitor_test_setup (&test_case,
                                           MONITOR_TEST_FLAG_NONE);
   set_custom_monitor_config ("lid-switch.xml");
@@ -5417,6 +5433,7 @@ meta_test_monitor_custom_lid_switch_config (void)
   check_monitor_configuration (&test_case);
 
   /* External monitor connected */
+  g_debug ("External monitor connected");
 
   test_case.setup.n_outputs = 2;
   test_case.expect.n_monitors = 2;
@@ -5437,6 +5454,7 @@ meta_test_monitor_custom_lid_switch_config (void)
   check_monitor_configuration (&test_case);
 
   /* Lid was closed */
+  g_debug ("Closing lid");
 
   test_case.expect.crtcs[0].current_mode = -1;
   test_case.expect.crtcs[1].transform = META_MONITOR_TRANSFORM_90;
@@ -5455,6 +5473,7 @@ meta_test_monitor_custom_lid_switch_config (void)
   check_monitor_configuration (&test_case);
 
   /* Lid was opened */
+  g_debug ("Opening lid");
 
   test_case.expect.crtcs[0].current_mode = 0;
   test_case.expect.crtcs[0].transform = META_MONITOR_TRANSFORM_NORMAL;