Merge pull request #23572 from bdach/avoid-display-settings-feedback

Work around issues with bindable/event feedback in layout settings

osu.Game/Overlays/Settings/Sections/Graphics/LayoutSettings.cs

@@ -193,6 +193,12 @@ namespace osu.Game.Overlays.Settings.Sections.Graphics
 
             currentDisplay.BindValueChanged(display => Schedule(() =>
             {
+                // there is no easy way to perform an atomic swap on the `resolutions` list, which is bound to the dropdown via `ItemSource`.
+                // this means, if the old resolution isn't saved, the `RemoveRange()` call below will cause `resolutionDropdown.Current` to reset value,
+                // therefore making it impossible to select any dropdown value other than "Default".
+                // to circumvent this locally, store the old value here, so that we can attempt to restore it later.
+                var oldResolution = resolutionDropdown.Current.Value;
+
                 resolutions.RemoveRange(1, resolutions.Count - 1);
 
                 if (display.NewValue != null)
@@ -204,6 +210,9 @@ namespace osu.Game.Overlays.Settings.Sections.Graphics
                                                 .Distinct());
                 }
 
+                if (resolutions.Contains(oldResolution))
+                    resolutionDropdown.Current.Value = oldResolution;
+
                 updateDisplaySettingsVisibility();
             }), true);
 
@@ -244,7 +253,8 @@ namespace osu.Game.Overlays.Settings.Sections.Graphics
         {
             Scheduler.AddOnce(d =>
             {
-                displayDropdown.Items = d;
+                if (!displayDropdown.Items.SequenceEqual(d, DisplayListComparer.DEFAULT))
+                    displayDropdown.Items = d;
                 updateDisplaySettingsVisibility();
             }, displays);
         }
@@ -376,5 +386,43 @@ namespace osu.Game.Overlays.Settings.Sections.Graphics
                 }
             }
         }
+
+        /// <summary>
+        /// Contrary to <see cref="Display.Equals(osu.Framework.Platform.Display?)"/>, this comparer disregards the value of <see cref="Display.Bounds"/>.
+        /// We want to just show a list of displays, and for the purposes of settings we don't care about their bounds when it comes to the list.
+        /// However, <see cref="IWindow.DisplaysChanged"/> fires even if only the resolution of the current display was changed
+        /// (because it causes the bounds of all displays to also change).
+        /// We're not interested in those changes, so compare only the rest that we actually care about.
+        /// This helps to avoid a bindable/event feedback loop, in which a resolution change
+        /// would trigger a display "change", which would in turn reset resolution again.
+        /// </summary>
+        private class DisplayListComparer : IEqualityComparer<Display>
+        {
+            public static readonly DisplayListComparer DEFAULT = new DisplayListComparer();
+
+            public bool Equals(Display? x, Display? y)
+            {
+                if (ReferenceEquals(x, y)) return true;
+                if (ReferenceEquals(x, null)) return false;
+                if (ReferenceEquals(y, null)) return false;
+
+                return x.Index == y.Index
+                       && x.Name == y.Name
+                       && x.DisplayModes.SequenceEqual(y.DisplayModes);
+            }
+
+            public int GetHashCode(Display obj)
+            {
+                var hashCode = new HashCode();
+
+                hashCode.Add(obj.Index);
+                hashCode.Add(obj.Name);
+                hashCode.Add(obj.DisplayModes.Length);
+                foreach (var displayMode in obj.DisplayModes)
+                    hashCode.Add(displayMode);
+
+                return hashCode.ToHashCode();
+            }
+        }
     }
 }