Merge pull request #5439 from Susko3/fix-Displays-allocation

Fix `Displays` querying on every access and add `DisplaysChanged`

osu.Framework.Tests/Visual/Platform/TestSceneBorderless.cs

@@ -4,6 +4,7 @@
 #nullable disable
 
 using System;
+using System.Collections.Generic;
 using System.Drawing;
 using System.Linq;
 using osu.Framework.Allocation;
@@ -149,9 +150,8 @@ private void load(FrameworkConfigManager config, GameHost host)
                 return;
             }
 
-            refreshScreens();
-
-            AddStep("set up screens", refreshScreens);
+            window.DisplaysChanged += onDisplaysChanged;
+            refreshScreens(window.Displays);
 
             const string desc2 = "Check whether the window size is one pixel wider than the screen in each direction";
 
@@ -181,14 +181,19 @@ private void load(FrameworkConfigManager config, GameHost host)
             }
         }
 
-        private void refreshScreens()
+        private void onDisplaysChanged(IEnumerable<Display> displays)
+        {
+            Scheduler.AddOnce(refreshScreens, displays);
+        }
+
+        private void refreshScreens(IEnumerable<Display> displays)
         {
             screenContainer.Remove(windowContainer, false);
             screenContainer.Clear();
 
             var bounds = new RectangleI();
 
-            foreach (var display in window.Displays)
+            foreach (var display in displays)
             {
                 screenContainer.Add(createScreen(display, display.Name));
                 bounds = RectangleI.Union(bounds, new RectangleI(display.Bounds.X, display.Bounds.Y, display.Bounds.Width, display.Bounds.Height));
@@ -238,5 +243,13 @@ protected override void Update()
             currentClientSize.Text = $"Client size: {window?.ClientSize}";
             currentDisplay.Text = $"Current Display: {window?.CurrentDisplayBindable.Value.Name}";
         }
+
+        protected override void Dispose(bool isDisposing)
+        {
+            if (window != null)
+                window.DisplaysChanged -= onDisplaysChanged;
+
+            base.Dispose(isDisposing);
+        }
     }
 }

osu.Framework.Tests/Visual/Platform/TestSceneFullscreen.cs

@@ -3,6 +3,7 @@
 
 #nullable disable
 
+using System.Collections.Generic;
 using System.Drawing;
 using System.Linq;
 using NUnit.Framework;
@@ -72,12 +73,21 @@ private void load(GameHost host)
             if (window == null)
                 return;
 
-            displaysDropdown.Items = window.Displays;
+            window.DisplaysChanged += onDisplaysChanged;
+            updateDisplays(window.Displays);
+
             displaysDropdown.Current.BindTo(window.CurrentDisplayBindable);
 
             supportedWindowModes.Text = $"Supported Window Modes: {string.Join(", ", window.SupportedWindowModes)}";
         }
 
+        private void onDisplaysChanged(IEnumerable<Display> displays)
+        {
+            Scheduler.AddOnce(updateDisplays, displays);
+        }
+
+        private void updateDisplays(IEnumerable<Display> displays) => displaysDropdown.Items = displays;
+
         [Test]
         public void TestScreenModeSwitch()
         {
@@ -156,5 +166,13 @@ private void testResolution(int w, int h)
         {
             AddStep($"set to {w}x{h}", () => sizeFullscreen.Value = new Size(w, h));
         }
+
+        protected override void Dispose(bool isDisposing)
+        {
+            if (window != null)
+                window.DisplaysChanged -= onDisplaysChanged;
+
+            base.Dispose(isDisposing);
+        }
     }
 }

osu.Framework/Platform/Display.cs

@@ -1,8 +1,6 @@
 // Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
-#nullable disable
-
 using System;
 using System.Drawing;
 using System.Linq;
@@ -17,7 +15,7 @@ public sealed class Display : IEquatable<Display>
         /// <summary>
         /// The name of the display, if available. Usually the manufacturer.
         /// </summary>
-        public string Name { get; }
+        public string? Name { get; }
 
         /// <summary>
         /// The current rectangle of the display in screen space.
@@ -36,7 +34,7 @@ public sealed class Display : IEquatable<Display>
         /// </summary>
         public int Index { get; }
 
-        public Display(int index, string name, Rectangle bounds, DisplayMode[] displayModes)
+        public Display(int index, string? name, Rectangle bounds, DisplayMode[] displayModes)
         {
             Index = index;
             Name = name;
@@ -46,12 +44,15 @@ public Display(int index, string name, Rectangle bounds, DisplayMode[] displayMo
 
         public override string ToString() => $"Name: {Name ?? "Unknown"}, Bounds: {Bounds}, DisplayModes: {DisplayModes.Length}, Index: {Index}";
 
-        public bool Equals(Display other)
+        public bool Equals(Display? other)
         {
             if (ReferenceEquals(null, other)) return false;
             if (ReferenceEquals(this, other)) return true;
 
-            return Index == other.Index;
+            return Index == other.Index
+                   && Name == other.Name
+                   && Bounds == other.Bounds
+                   && DisplayModes.SequenceEqual(other.DisplayModes);
         }
 
         /// <summary>

osu.Framework/Platform/IWindow.cs

@@ -120,6 +120,12 @@ public interface IWindow : IDisposable
         /// </summary>
         IEnumerable<Display> Displays { get; }
 
+        /// <summary>
+        /// Invoked when <see cref="Displays"/> has changed.
+        /// </summary>
+        [CanBeNull]
+        event Action<IEnumerable<Display>> DisplaysChanged;
+
         /// <summary>
         /// Gets the <see cref="Display"/> that has been set as "primary" or "default" in the operating system.
         /// </summary>

osu.Framework/Platform/OsuTKWindow.cs

@@ -82,6 +82,10 @@ public event Action KeymapChanged { add { } remove { } }
 
         public virtual IEnumerable<Display> Displays => new[] { DisplayDevice.GetDisplay(DisplayIndex.Primary).ToDisplay() };
 
+#pragma warning disable CS0067
+        public event Action<IEnumerable<Display>> DisplaysChanged;
+#pragma warning restore CS0067
+
         public virtual Display PrimaryDisplay => Displays.FirstOrDefault(d => d.Index == (int)DisplayDevice.Default.GetIndex());
 
         public Bindable<Display> CurrentDisplayBindable { get; } = new Bindable<Display>();
@@ -176,7 +180,8 @@ protected OsuTKWindow([NotNull] IGameWindow osuTKGameWindow)
         /// <para>Note that this will use the default <see cref="GameWindow"/> implementation, which is not compatible with every platform.</para>
         /// </summary>
         protected OsuTKWindow(int width, int height)
-            : this(new GameWindow(width, height, new GraphicsMode(GraphicsMode.Default.ColorFormat, GraphicsMode.Default.Depth, GraphicsMode.Default.Stencil, GraphicsMode.Default.Samples, GraphicsMode.Default.AccumulatorFormat, 3)))
+            : this(new GameWindow(width, height,
+                new GraphicsMode(GraphicsMode.Default.ColorFormat, GraphicsMode.Default.Depth, GraphicsMode.Default.Stencil, GraphicsMode.Default.Samples, GraphicsMode.Default.AccumulatorFormat, 3)))
         {
         }
 

osu.Framework/Platform/SDL2DesktopWindow.cs

@@ -351,6 +351,10 @@ private void handleSDLEvent(SDL.SDL_Event e)
                     handleQuitEvent(e.quit);
                     break;
 
+                case SDL.SDL_EventType.SDL_DISPLAYEVENT:
+                    handleDisplayEvent(e.display);
+                    break;
+
                 case SDL.SDL_EventType.SDL_WINDOWEVENT:
                     handleWindowEvent(e.window);
                     break;

osu.Framework/Platform/SDL2DesktopWindow_Windowing.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Drawing;
 using System.Linq;
 using osu.Framework.Bindables;
@@ -18,6 +19,9 @@ public partial class SDL2DesktopWindow
     {
         private void setupWindowing(FrameworkConfigManager config)
         {
+            updateDisplays();
+
+            DisplaysChanged += _ => CurrentDisplayBindable.Default = PrimaryDisplay;
             CurrentDisplayBindable.Default = PrimaryDisplay;
             CurrentDisplayBindable.ValueChanged += evt =>
             {
@@ -259,10 +263,68 @@ public WindowState WindowState
 
         public float Scale = 1;
 
+        #region Displays (mostly self-contained)
+
         /// <summary>
         /// Queries the physical displays and their supported resolutions.
         /// </summary>
-        public IEnumerable<Display> Displays => Enumerable.Range(0, SDL.SDL_GetNumVideoDisplays()).Select(displayFromSDL);
+        public IEnumerable<Display> Displays { get; private set; } = null!;
+
+        public event Action<IEnumerable<Display>>? DisplaysChanged;
+
+        // ReSharper disable once UnusedParameter.Local
+        private void handleDisplayEvent(SDL.SDL_DisplayEvent evtDisplay) => updateDisplays();
+
+        /// <summary>
+        /// Updates <see cref="Displays"/> with the latest display information reported by SDL.
+        /// </summary>
+        /// <remarks>
+        /// Has no effect on values of
+        /// <see cref="currentDisplay"/> /
+        /// <see cref="CurrentDisplay"/> /
+        /// <see cref="CurrentDisplayBindable"/>.
+        /// </remarks>
+        private void updateDisplays()
+        {
+            Displays = getSDLDisplays();
+            DisplaysChanged?.Invoke(Displays);
+        }
+
+        /// <summary>
+        /// Asserts that the current <see cref="Displays"/> match the actual displays as reported by SDL.
+        /// </summary>
+        /// <remarks>
+        /// This assert is not fatal, as the <see cref="Displays"/> will get updated sooner or later
+        /// in <see cref="handleDisplayEvent"/> or <see cref="handleWindowEvent"/>.
+        /// </remarks>
+        [Conditional("DEBUG")]
+        private void assertDisplaysMatchSDL()
+        {
+            var actualDisplays = getSDLDisplays();
+            Debug.Assert(actualDisplays.SequenceEqual(Displays), $"Stored {nameof(Displays)} don't match actual displays",
+                $"Stored displays:\n  {string.Join("\n  ", Displays)}\n\nActual displays:\n  {string.Join("\n  ", actualDisplays)}");
+        }
+
+        private IEnumerable<Display> getSDLDisplays()
+        {
+            return Enumerable.Range(0, SDL.SDL_GetNumVideoDisplays()).Select(displayFromSDL).ToArray();
+
+            static Display displayFromSDL(int displayIndex)
+            {
+                var displayModes = Enumerable.Range(0, SDL.SDL_GetNumDisplayModes(displayIndex))
+                                             .Select(modeIndex =>
+                                             {
+                                                 SDL.SDL_GetDisplayMode(displayIndex, modeIndex, out var mode);
+                                                 return mode.ToDisplayMode(displayIndex);
+                                             })
+                                             .ToArray();
+
+                SDL.SDL_GetDisplayBounds(displayIndex, out var rect);
+                return new Display(displayIndex, SDL.SDL_GetDisplayName(displayIndex), new Rectangle(rect.x, rect.y, rect.w, rect.h), displayModes);
+            }
+        }
+
+        #endregion
 
         /// <summary>
         /// Gets the <see cref="Display"/> that has been set as "primary" or "default" in the operating system.
@@ -343,6 +405,8 @@ private void handleWindowEvent(SDL.SDL_WindowEvent evtWindow)
                             storeWindowPositionToConfig();
                     }
 
+                    // we may get a SDL_WINDOWEVENT_MOVED when the resolution of a display changes.
+                    updateDisplays();
                     break;
 
                 case SDL.SDL_WindowEventID.SDL_WINDOWEVENT_SIZE_CHANGED:
@@ -362,6 +426,10 @@ private void handleWindowEvent(SDL.SDL_WindowEvent evtWindow)
                 case SDL.SDL_WindowEventID.SDL_WINDOWEVENT_RESTORED:
                 case SDL.SDL_WindowEventID.SDL_WINDOWEVENT_FOCUS_GAINED:
                     Focused = true;
+                    // displays can change without a SDL_DISPLAYEVENT being sent, eg. changing resolution.
+                    // force update displays when gaining keyboard focus to always have up-to-date information.
+                    // eg. this covers scenarios when changing resolution outside of the game, and then tabbing in.
+                    updateDisplays();
                     break;
 
                 case SDL.SDL_WindowEventID.SDL_WINDOWEVENT_MINIMIZED:
@@ -372,6 +440,8 @@ private void handleWindowEvent(SDL.SDL_WindowEvent evtWindow)
                 case SDL.SDL_WindowEventID.SDL_WINDOWEVENT_CLOSE:
                     break;
             }
+
+            assertDisplaysMatchSDL();
         }
 
         /// <summary>
@@ -633,20 +703,6 @@ private SDL.SDL_DisplayMode getClosestDisplayMode(Size size, int refreshRate, in
             throw new InvalidOperationException("couldn't retrieve valid display mode");
         }
 
-        private static Display displayFromSDL(int displayIndex)
-        {
-            var displayModes = Enumerable.Range(0, SDL.SDL_GetNumDisplayModes(displayIndex))
-                                         .Select(modeIndex =>
-                                         {
-                                             SDL.SDL_GetDisplayMode(displayIndex, modeIndex, out var mode);
-                                             return mode.ToDisplayMode(displayIndex);
-                                         })
-                                         .ToArray();
-
-            SDL.SDL_GetDisplayBounds(displayIndex, out var rect);
-            return new Display(displayIndex, SDL.SDL_GetDisplayName(displayIndex), new Rectangle(rect.x, rect.y, rect.w, rect.h), displayModes);
-        }
-
         #endregion
 
         #region Events