From bd40ebe91af6b8223d44844924ec5bf587abdf88 Mon Sep 17 00:00:00 2001 From: RussKie Date: Mon, 20 Jul 2020 12:10:38 +1000 Subject: [PATCH] Constrain child windows to monitor bounds If a parent window is at a corner of a desktop, and it is smaller than a child window, it is possible to end up in a situation where the child window is positioned outside of visible and reachable area. Coupled with the fact that locations and dimensions of most windows get persisted this may lead to a situation where the window may become permanently inaccessible until the settings are reset. --- GitUI/GitExtensionsForm.cs | 14 ++- GitUI/WindowPositionManager.cs | 87 ++++++++++++++----- .../GitUI.Tests/GitExtensionsFormTests.cs | 2 +- .../GitUI.Tests/WindowPositionManagerTests.cs | 66 +++++++++++--- 4 files changed, 124 insertions(+), 45 deletions(-) diff --git a/GitUI/GitExtensionsForm.cs b/GitUI/GitExtensionsForm.cs index c95389b9f2d..19d1a85c23d 100644 --- a/GitUI/GitExtensionsForm.cs +++ b/GitUI/GitExtensionsForm.cs @@ -109,7 +109,7 @@ protected virtual void RestorePosition() _needsPositionRestore = false; - var workingArea = _getScreensWorkingArea(); + IReadOnlyList 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) diff --git a/GitUI/WindowPositionManager.cs b/GitUI/WindowPositionManager.cs index 33e53c859be..d59f1ccaf86 100644 --- a/GitUI/WindowPositionManager.cs +++ b/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; - /// - /// Determines which screen a given point belongs to. - /// - [Pure] - public static Rectangle? FindWindowScreen(Point location, IEnumerable desktopWorkingArea) + public static Point FitWindowOnScreen(Rectangle calculatedWindowBounds, IEnumerable workingArea) { - var distance = new SortedDictionary(); + // 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; } /// @@ -138,4 +183,4 @@ public void SavePosition(Form form) } } } -} \ No newline at end of file +} diff --git a/UnitTests/GitUI.Tests/GitExtensionsFormTests.cs b/UnitTests/GitUI.Tests/GitExtensionsFormTests.cs index 44bd10dab32..029351e2d23 100644 --- a/UnitTests/GitUI.Tests/GitExtensionsFormTests.cs +++ b/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) diff --git a/UnitTests/GitUI.Tests/WindowPositionManagerTests.cs b/UnitTests/GitUI.Tests/WindowPositionManagerTests.cs index 636dde57893..4e8bfc9f17e 100644 --- a/UnitTests/GitUI.Tests/WindowPositionManagerTests.cs +++ b/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()).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 + { + // 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); } } -} \ No newline at end of file +}