Merge pull request #8341 from RussKie/Constrain_child_windows_to_moni…

…tor_bounds

GitUI/GitExtensionsForm.cs

@@ -109,7 +109,7 @@ protected virtual void RestorePosition()
 
             _needsPositionRestore = false;
 
-            var workingArea = _getScreensWorkingArea();
+            IReadOnlyList<Rectangle> workingArea = _getScreensWorkingArea();
             if (!workingArea.Any(screen => screen.IntersectsWith(position.Rect)))
             {
                 if (position.State == FormWindowState.Maximized)
@@ -133,21 +133,17 @@ protected virtual void RestorePosition()
 
             if (Owner is null || !windowCentred)
             {
-                var location = DpiUtil.Scale(position.Rect.Location, originalDpi: position.DeviceDpi);
+                Point calculatedLocation = DpiUtil.Scale(position.Rect.Location, originalDpi: position.DeviceDpi);
 
-                if (WindowPositionManager.FindWindowScreen(location, workingArea) is Rectangle rect)
-                {
-                    location.Y = rect.Y;
-                }
-
-                DesktopLocation = location;
+                DesktopLocation = WindowPositionManager.FitWindowOnScreen(new Rectangle(calculatedLocation, Size), workingArea);
             }
             else
             {
                 // Calculate location for modal form with parent
-                Location = new Point(
+                Point calculatedLocation = new Point(
                     Owner.Left + (Owner.Width / 2) - (Width / 2),
                     Owner.Top + (Owner.Height / 2) - (Height / 2));
+                Location = WindowPositionManager.FitWindowOnScreen(new Rectangle(calculatedLocation, Size), workingArea);
             }
 
             if (WindowState != position.State)

GitUI/WindowPositionManager.cs

@@ -1,10 +1,9 @@
 using System;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Drawing;
-using System.Linq;
 using System.Windows.Forms;
 using GitExtUtils.GitUI;
-using JetBrains.Annotations;
 
 namespace GitUI
 {
@@ -29,35 +28,81 @@ internal sealed class WindowPositionManager : IWindowPositionManager
     {
         private static WindowPositionList _windowPositionList;
 
-        /// <summary>
-        /// Determines which screen a given point belongs to.
-        /// </summary>
-        [Pure]
-        public static Rectangle? FindWindowScreen(Point location, IEnumerable<Rectangle> desktopWorkingArea)
+        public static Point FitWindowOnScreen(Rectangle calculatedWindowBounds, IEnumerable<Rectangle> workingArea)
         {
-            var distance = new SortedDictionary<float, Rectangle>();
+            // Ensure the window with its new size and location will be accessible to the user
+            // If all fails, we'll display the window the (0, 0)
+            Point location = Point.Empty;
+            foreach (Rectangle screen in workingArea)
+            {
+                bool isDisplayed = IsDisplayedOn10Percent(screen, calculatedWindowBounds);
+                if (isDisplayed)
+                {
+                    location = calculatedWindowBounds.Location;
+                    break;
+                }
+            }
+
+            return location;
+        }
+
+        private static bool IsDisplayedOn10Percent(Rectangle screen, Rectangle window)
+        {
+            if (screen.IsEmpty || window.IsEmpty)
+            {
+                return false;
+            }
+
+            // We insist that any window to cover at least 10% of a screen realestate both horizontally and vertically
+            // However, check if the window is smaller than the minimum presence requirement.
+            // If so, adjust the requirements to the size of the window.
+            const float MinimumScreenPresence = 0.1f; // 10%
+            int requiredHeight = Math.Min((int)(screen.Height * MinimumScreenPresence), window.Height);
+            int requireWidth = Math.Min((int)(screen.Width * MinimumScreenPresence), window.Width);
 
-            foreach (var rect in desktopWorkingArea)
+            Point p;
+            if (screen.Contains(window.Location))
             {
-                if (rect.Contains(location) && !distance.ContainsKey(0.0f))
+                p = new Point(window.Left + requireWidth, window.Top + requiredHeight);
+                bool leftTop = screen.Contains(p);
+                if (leftTop)
                 {
-                    return null; // title in screen
+                    Debug.WriteLine($"{screen.ToString()} contains {p} (L, T)");
+                    return true;
                 }
+            }
 
-                int midPointX = rect.X + (rect.Width / 2);
-                int midPointY = rect.Y + (rect.Height / 2);
-                var d = (float)Math.Sqrt(((location.X - midPointX) * (location.X - midPointX)) +
-                                         ((location.Y - midPointY) * (location.Y - midPointY)));
+            if (screen.Contains(new Point(window.Left + (window.Width / 2), window.Top)))
+            {
+                p = new Point(window.Left + (window.Width / 2) - requireWidth, window.Top + requiredHeight);
+                bool middleTop = screen.Contains(p);
+                if (middleTop)
+                {
+                    Debug.WriteLine($"{screen.ToString()} contains {p} (W/2-, T)");
+                    return true;
+                }
 
-                // In a very unlikely scenario where a user has several monitors, which are arranged in a particular way
-                // and a form's window happens to be in exact middle between monitors, if we don't check - it will crash
-                if (!distance.ContainsKey(d))
+                p = new Point(window.Left + (window.Width / 2) + requireWidth, window.Top + requiredHeight);
+                middleTop = screen.Contains(p);
+                if (middleTop)
+                {
+                    Debug.WriteLine($"{screen.ToString()} contains {p} (W/2+, T)");
+                    return true;
+                }
+            }
+
+            if (screen.Contains(new Point(window.Right, window.Top)))
+            {
+                p = new Point(window.Right - requireWidth, window.Top + requiredHeight);
+                bool rightTop = screen.Contains(p);
+                if (rightTop)
                 {
-                    distance.Add(d, rect);
+                    Debug.WriteLine($"{screen.ToString()} contains {p} (R, T)");
+                    return true;
                 }
             }
 
-            return distance.FirstOrDefault().Value;
+            return false;
         }
 
         /// <summary>
@@ -138,4 +183,4 @@ public void SavePosition(Form form)
             }
         }
     }
-}
+}

UnitTests/GitUI.Tests/GitExtensionsFormTests.cs

@@ -149,7 +149,7 @@ public void RestorePosition_should_scale_sizable_window_if_different_dpi(int sav
 
         [TestCase(-1000, 100, /* -1000 + (800 - 300)/2 */ -750, /* 100 + (600-200)/2 */300)]
         [TestCase(0, 0, /* 0 + (800 - 300)/2 */ 250, /* 0 + (600-200)/2 */200)]
-        [TestCase(1000, -400, /* 1000 + (800 - 300)/2 */ 1250, /* -400 + (600-200)/2 */ -200)]
+        [TestCase(1000, -400, /* falls off the screen */ 0, /* falls off the screen */ 0)]
         public void RestorePosition_should_position_window_with_Owner_set_and_CenterParent(int ownerFormTop, int ownerFormLeft, int expectFormTop, int expectedFormLeft)
         {
             if (DpiUtil.IsNonStandard)

UnitTests/GitUI.Tests/WindowPositionManagerTests.cs

@@ -10,26 +10,64 @@ namespace GitUITests
     public class WindowPositionManagerTests
     {
         [Test]
-        public void FindWindowScreen_should_return_empty_rect_if_no_screen_supplied()
+        public void IsDisplayedOn10Percent_should_return_false_if_no_screen_supplied()
         {
-            WindowPositionManager.FindWindowScreen(new Point(10, 10), Array.Empty<Rectangle>()).Should().Be(Rectangle.Empty);
+            WindowPositionManagerTestAccessor.IsDisplayedOn10Percent(Rectangle.Empty, new Rectangle(1, 1, 1, 1))
+                .Should().BeFalse();
         }
 
         [Test]
-        public void FindWindowScreen_should_not_crash_if_point_in_dead_middle_between_monitors()
+        public void IsDisplayedOn10Percent_should_return_false_if_no_window_supplied()
         {
-            // imagine 3 monitors 1920x1080 positioned side-by-side, the middle monitor is the main one (0,0)
-            // the other two are positioned on the same level (Y=0)
-            // Place the point at the dead center
-            var point = new Point(960, 540);
-            var screens = new[]
+            WindowPositionManagerTestAccessor.IsDisplayedOn10Percent(new Rectangle(1, 1, 1, 1), Rectangle.Empty)
+                .Should().BeFalse();
+        }
+
+        [TestCase(500, 500)] // normal window
+        [TestCase(50, 50)] // less than 10% visibility
+        public void IsDisplayedOn10Percent_should_return_true_if_window_fully_fit_on_screen(int width, int height)
+        {
+            Rectangle screen = new(0, 0, 1920, 1080);
+            Rectangle window = new(0, 0, width, height);
+
+            WindowPositionManagerTestAccessor.IsDisplayedOn10Percent(screen, window)
+                .Should().BeTrue();
+        }
+
+        //// clipped on the left
+        [TestCase(-500, 100, 500, 500, false)] // less than 10% visible horizontally
+        [TestCase(-192, 100, 500, 500, true)] // 10% visible horizontally
+        //// clipped on the top
+        [TestCase(50, -50, 500, 500, false)]
+        //// clipped on the right
+        [TestCase(1727, 100, 500, 500, true)] // 10% visible horizontally, (1920 * 90% - 1)
+        [TestCase(1728, 100, 500, 500, false)] // less than 10% visible horizontally, (1920 * 90%)
+        //// clipped at the bottom
+        [TestCase(100, 971, 500, 500, true)] // 10% visible vertically, (1080 * 90% - 1)
+        [TestCase(100, 972, 500, 500, false)] // 10% visible vertically, (1080 * 90%)
+        //// clipped on both the left and right
+        [TestCase(-500, 100, 2100, 500, true)] // more than 10% visible horizontally
+        [TestCase(-2700, 100, 5000, 500, false)] // unexpected case! neither left-middle-rights points are visible
+        public void IsDisplayedOn10Percent_should_return_expected_if_window_partially_fit_on_screen(int x, int y, int width, int height, bool expected)
+        {
+            Rectangle screen = new(0, 0, 1920, 1080);
+            Rectangle window = new(x, y, width, height);
+
+            WindowPositionManagerTestAccessor.IsDisplayedOn10Percent(screen, window)
+                .Should().Be(expected);
+        }
+
+        private class WindowPositionManagerTestAccessor : TestAccessor<WindowPositionManager>
+        {
+            // Accessor for static members
+            private static readonly dynamic Static = typeof(WindowPositionManager).TestAccessor().Dynamic;
+
+            public WindowPositionManagerTestAccessor(WindowPositionManager instance)
+                : base(instance)
             {
-                new Rectangle(-1920, 0, 1920, 1080),
-                new Rectangle(1920, 0, 1920, 1080),
-                new Rectangle(0, 0, 1920, 1080)
-            };
+            }
 
-            WindowPositionManager.FindWindowScreen(point, screens).Should().Be(null);
+            public static bool IsDisplayedOn10Percent(Rectangle screen, Rectangle window) => Static.IsDisplayedOn10Percent(screen, window);
         }
     }
-}
+}