From 8111d95e20665077ad8bc9fe2f01bfecf8f09c7a Mon Sep 17 00:00:00 2001
From: Isaac Daly <isaac.daly@outlook.com>
Date: Thu, 2 Nov 2023 08:37:55 +1100
Subject: [PATCH] Lazily evaluate Bounds and WorkingArea (#561)

Lazily evaluate `Bounds` and `WorkingArea`, to handle RDP returning some incorrect values during display changes.
---
 src/Whim.Tests/Monitor/MonitorTests.cs | 19 ++++++-
 src/Whim/Monitor/Monitor.cs            | 74 ++++++++++++++++++--------
 2 files changed, 69 insertions(+), 24 deletions(-)

diff --git a/src/Whim.Tests/Monitor/MonitorTests.cs b/src/Whim.Tests/Monitor/MonitorTests.cs
index fa7001fb2..26aa4497c 100644
--- a/src/Whim.Tests/Monitor/MonitorTests.cs
+++ b/src/Whim.Tests/Monitor/MonitorTests.cs
@@ -112,7 +112,6 @@ internal void CreateMonitor_IsPrimaryHMonitor(IInternalContext internalCtx, HMON
 	internal void CreateMonitor_MultipleMonitors(IInternalContext internalCtx, HMONITOR hmonitor)
 	{
 		// Given
-
 		internalCtx.CoreNativeManager.HasMultipleMonitors().Returns(true);
 
 		internalCtx.CoreNativeManager
@@ -164,6 +163,24 @@ internal void CreateMonitor_MultipleMonitors(IInternalContext internalCtx, HMONI
 		Assert.Equal(150, monitor.ScaleFactor);
 	}
 
+	[Theory, AutoSubstituteData<MonitorCustomization>]
+	internal void CreateMonitor_CreationFailed(IInternalContext internalCtx, HMONITOR hmonitor)
+	{
+		// Given
+		bool isPrimaryHMonitor = false;
+		internalCtx.CoreNativeManager.HasMultipleMonitors().Returns(true);
+		internalCtx.CoreNativeManager.GetMonitorInfoEx(Arg.Any<HMONITOR>()).Returns((_) => null);
+
+		// When
+		Monitor monitor = new(internalCtx, hmonitor, isPrimaryHMonitor);
+
+		// Then
+		Assert.Equal("NOT A DISPLAY", monitor.Name);
+		Assert.False(monitor.IsPrimary);
+		Assert.Equal(new Location<int>(), monitor.Bounds);
+		Assert.Equal(new Location<int>(), monitor.WorkingArea);
+	}
+
 	[Theory, AutoSubstituteData<MonitorCustomization>]
 	internal void Equals_Failure(IInternalContext internalCtx, HMONITOR hmonitor)
 	{
diff --git a/src/Whim/Monitor/Monitor.cs b/src/Whim/Monitor/Monitor.cs
index 1f9c03fca..88f5ec8c6 100644
--- a/src/Whim/Monitor/Monitor.cs
+++ b/src/Whim/Monitor/Monitor.cs
@@ -22,45 +22,30 @@ public Monitor(IInternalContext internalContext, HMONITOR monitor, bool isPrimar
 
 	public bool IsPrimary { get; private set; }
 
-	public ILocation<int> Bounds { get; private set; }
+	// Bounds and WorkingArea are lazily evaluated because sometimes they return incorrect values
+	// inside RDP sessions, during display changes. This is a workaround for that.
+	public ILocation<int> Bounds => GetBounds();
 
-	public ILocation<int> WorkingArea { get; private set; }
+	public ILocation<int> WorkingArea => GetWorkingArea();
 
 	public int ScaleFactor { get; private set; }
 
-	[MemberNotNull(nameof(Bounds), nameof(IsPrimary), nameof(Name), nameof(WorkingArea), nameof(ScaleFactor))]
+	[MemberNotNull(nameof(IsPrimary), nameof(Name), nameof(ScaleFactor))]
 	internal unsafe void Update(bool isPrimaryHMonitor)
 	{
-		if (!_internalContext.CoreNativeManager.HasMultipleMonitors() || isPrimaryHMonitor)
+		IsPrimary = !_internalContext.CoreNativeManager.HasMultipleMonitors() || isPrimaryHMonitor;
+		if (IsPrimary)
 		{
-			// Single monitor system.
-			Bounds = new Location<int>()
-			{
-				X = _internalContext.CoreNativeManager.GetVirtualScreenLeft(),
-				Y = _internalContext.CoreNativeManager.GetVirtualScreenTop(),
-				Width = _internalContext.CoreNativeManager.GetVirtualScreenWidth(),
-				Height = _internalContext.CoreNativeManager.GetVirtualScreenHeight()
-			};
-
-			IsPrimary = true;
 			Name = "DISPLAY";
-
-			_internalContext.CoreNativeManager.GetPrimaryDisplayWorkArea(out RECT rect);
-			WorkingArea = rect.ToLocation();
 		}
 		else if (_internalContext.CoreNativeManager.GetMonitorInfoEx(_hmonitor) is MONITORINFOEXW infoEx)
 		{
 			// Multiple monitor system.
-			Bounds = infoEx.monitorInfo.rcMonitor.ToLocation();
-			WorkingArea = infoEx.monitorInfo.rcWork.ToLocation();
-			IsPrimary = false;
 			Name = infoEx.GetDeviceName();
 		}
 		else
 		{
-			Bounds = new Location<int>();
-			WorkingArea = new Location<int>();
-			IsPrimary = false;
+			Logger.Error($"Failed to get name for monitor {_hmonitor}");
 			Name = "NOT A DISPLAY";
 		}
 
@@ -75,6 +60,49 @@ internal unsafe void Update(bool isPrimaryHMonitor)
 		ScaleFactor = (int)((double)effectiveDpiX / 96 * 100);
 	}
 
+	private ILocation<int> GetBounds()
+	{
+		if (IsPrimary)
+		{
+			return new Location<int>()
+			{
+				X = _internalContext.CoreNativeManager.GetVirtualScreenLeft(),
+				Y = _internalContext.CoreNativeManager.GetVirtualScreenTop(),
+				Width = _internalContext.CoreNativeManager.GetVirtualScreenWidth(),
+				Height = _internalContext.CoreNativeManager.GetVirtualScreenHeight()
+			};
+		}
+		else if (_internalContext.CoreNativeManager.GetMonitorInfoEx(_hmonitor) is MONITORINFOEXW infoEx)
+		{
+			// Multiple monitor system.
+			return infoEx.monitorInfo.rcMonitor.ToLocation();
+		}
+		else
+		{
+			Logger.Error($"Failed to get bounds for monitor {_hmonitor}");
+			return new Location<int>();
+		}
+	}
+
+	private ILocation<int> GetWorkingArea()
+	{
+		if (IsPrimary)
+		{
+			_internalContext.CoreNativeManager.GetPrimaryDisplayWorkArea(out RECT rect);
+			return rect.ToLocation();
+		}
+		else if (_internalContext.CoreNativeManager.GetMonitorInfoEx(_hmonitor) is MONITORINFOEXW infoEx)
+		{
+			// Multiple monitor system.
+			return infoEx.monitorInfo.rcWork.ToLocation();
+		}
+		else
+		{
+			Logger.Error($"Failed to get working area for monitor {_hmonitor}");
+			return new Location<int>();
+		}
+	}
+
 	/// <inheritdoc/>
 	public override bool Equals(object? other) => other is Monitor monitor && _hmonitor == monitor._hmonitor;