Fix window jump and size change while moving a floating window in bet…

…ween monitors (#1001) Avoid the "window jump + size change" when you move a window in between monitors. `NormalizeRectangle` now clamps to the monitor's bounds. --------- Co-authored-by: Isaac Daly <[email protected]>
dalyIsaac · Aug 31, 2024 · ba02b68 · ba02b68
1 parent 8b991bd
commit ba02b68
Show file tree

Hide file tree

Showing 11 changed files with 238 additions and 121 deletions.
diff --git a/src/Whim.FloatingWindow.Tests/FloatingLayoutEngineTests.cs b/src/Whim.FloatingWindow.Tests/FloatingLayoutEngineTests.cs
@@ -1,29 +1,9 @@
-using AutoFixture;
-using NSubstitute;
+using NSubstitute;
 using Whim.TestUtils;
-using Windows.Win32.Foundation;
 using Xunit;
 
 namespace Whim.FloatingWindow.Tests;
 
-public class FloatingLayoutEngineCustomization : ICustomization
-{
-	public void Customize(IFixture fixture)
-	{
-		IContext context = fixture.Freeze<IContext>();
-		IMonitor monitor = fixture.Freeze<IMonitor>();
-
-		context.MonitorManager.GetMonitorAtPoint(Arg.Any<IRectangle<int>>()).Returns(monitor);
-		monitor.WorkingArea.Returns(new Rectangle<int>() { Width = 1000, Height = 1000 });
-		context
-			.NativeManager.DwmGetWindowRectangle(Arg.Any<HWND>())
-			.Returns(new Rectangle<int>() { Width = 100, Height = 100 });
-
-		fixture.Inject(context);
-		fixture.Inject(monitor);
-	}
-}
-
 public class FloatingLayoutEngineTests
 {
 	private static readonly LayoutEngineIdentity identity = new();
@@ -42,7 +22,7 @@ public void Name(IContext context)
 	}
 
 	#region AddWindow
-	[Theory, AutoSubstituteData<FloatingLayoutEngineCustomization>]
+	[Theory, AutoSubstituteData<FloatingWindowCustomization>]
 	public void AddWindow(IContext context, IWindow window)
 	{
 		// Given
@@ -56,7 +36,22 @@ public void AddWindow(IContext context, IWindow window)
 		Assert.Equal(1, newLayoutEngine.Count);
 	}
 
-	[Theory, AutoSubstituteData<FloatingLayoutEngineCustomization>]
+	[Theory, AutoSubstituteData<FloatingWindowCustomization>]
+	public void AddWindow_UntrackedWindow(IContext context)
+	{
+		// Given a window which isn't tracked by the store
+		IWindow window = Substitute.For<IWindow>();
+		ILayoutEngine engine = new FloatingLayoutEngine(context, identity);
+
+		// When
+		ILayoutEngine newLayoutEngine = engine.AddWindow(window);
+
+		// Then
+		Assert.Equal(0, engine.Count);
+		Assert.Same(engine, newLayoutEngine);
+	}
+
+	[Theory, AutoSubstituteData<FloatingWindowCustomization>]
 	public void AddWindow_WindowAlreadyPresent(IContext context, IWindow window)
 	{
 		// Given
@@ -72,7 +67,7 @@ public void AddWindow_WindowAlreadyPresent(IContext context, IWindow window)
 	#endregion
 
 	#region RemoveWindow
-	[Theory, AutoSubstituteData<FloatingLayoutEngineCustomization>]
+	[Theory, AutoSubstituteData<FloatingWindowCustomization>]
 	public void Remove(IContext context, IWindow window)
 	{
 		// Given
@@ -86,7 +81,7 @@ public void Remove(IContext context, IWindow window)
 		Assert.Equal(0, newLayoutEngine.Count);
 	}
 
-	[Theory, AutoSubstituteData<FloatingLayoutEngineCustomization>]
+	[Theory, AutoSubstituteData<FloatingWindowCustomization>]
 	public void Remove_NoChanges(IContext context, IWindow window)
 	{
 		// Given
@@ -102,7 +97,7 @@ public void Remove_NoChanges(IContext context, IWindow window)
 	#endregion RemoveWindow
 
 	#region Contains
-	[Theory, AutoSubstituteData<FloatingLayoutEngineCustomization>]
+	[Theory, AutoSubstituteData<FloatingWindowCustomization>]
 	public void Contains(IContext context, IWindow window)
 	{
 		// Given
@@ -115,7 +110,7 @@ public void Contains(IContext context, IWindow window)
 		Assert.True(contains);
 	}
 
-	[Theory, AutoSubstituteData<FloatingLayoutEngineCustomization>]
+	[Theory, AutoSubstituteData<FloatingWindowCustomization>]
 	public void Contains_False(IContext context, IWindow window)
 	{
 		// Given
@@ -130,7 +125,7 @@ public void Contains_False(IContext context, IWindow window)
 	#endregion
 
 	#region DoLayout
-	[Theory, AutoSubstituteData<FloatingLayoutEngineCustomization>]
+	[Theory, AutoSubstituteData<FloatingWindowCustomization>]
 	public void DoLayout_Empty(IContext context)
 	{
 		// Given
@@ -145,15 +140,15 @@ public void DoLayout_Empty(IContext context)
 		Assert.Empty(windowStates);
 	}
 
-	[Theory, AutoSubstituteData<FloatingLayoutEngineCustomization>]
-	public void DoLayout_KeepWindowSize(
-		IContext context,
-		IWindow windowNormal,
-		IWindow windowMinimized,
-		IWindow windowMaximized
-	)
+	[Theory, AutoSubstituteData<FloatingWindowCustomization>]
+	internal void DoLayout_KeepWindowSize(IContext context, MutableRootSector root)
 	{
 		// Given
+		IWindow[] allWindows = root.WindowSector.Windows.Values.ToArray();
+		IWindow windowNormal = allWindows[0];
+		IWindow windowMinimized = allWindows[1];
+		IWindow windowMaximized = allWindows[2];
+
 		windowMinimized.IsMinimized.Returns(true);
 		windowMaximized.IsMaximized.Returns(true);
 		ILayoutEngine engine = new FloatingLayoutEngine(context, identity)
@@ -189,7 +184,7 @@ public void GetFirstWindow_Null(IContext context)
 		Assert.Null(result);
 	}
 
-	[Theory, AutoSubstituteData<FloatingLayoutEngineCustomization>]
+	[Theory, AutoSubstituteData<FloatingWindowCustomization>]
 	public void GetFirstWindow_SingleWindow(IContext context, IWindow window)
 	{
 		// Given
@@ -204,7 +199,7 @@ public void GetFirstWindow_SingleWindow(IContext context, IWindow window)
 	#endregion
 
 	#region WindowRelated
-	[Theory, AutoSubstituteData<FloatingLayoutEngineCustomization>]
+	[Theory, AutoSubstituteData<FloatingWindowCustomization>]
 	public void MoveWindowToPoint(IContext context, IWindow window)
 	{
 		// Given
@@ -218,7 +213,7 @@ public void MoveWindowToPoint(IContext context, IWindow window)
 		Assert.Equal(engine, newEngine);
 	}
 
-	[Theory, AutoSubstituteData<FloatingLayoutEngineCustomization>]
+	[Theory, AutoSubstituteData<FloatingWindowCustomization>]
 	public void MoveWindowEdgesInDirection(IContext context, IWindow window)
 	{
 		// Given
@@ -232,7 +227,7 @@ public void MoveWindowEdgesInDirection(IContext context, IWindow window)
 		Assert.Equal(engine, newEngine);
 	}
 
-	[Theory, AutoSubstituteData<FloatingLayoutEngineCustomization>]
+	[Theory, AutoSubstituteData<FloatingWindowCustomization>]
 	public void MinimizeWindowStart(IContext context, IWindow window)
 	{
 		// Given
@@ -245,7 +240,7 @@ public void MinimizeWindowStart(IContext context, IWindow window)
 		Assert.Equal(engine, newEngine);
 	}
 
-	[Theory, AutoSubstituteData<FloatingLayoutEngineCustomization>]
+	[Theory, AutoSubstituteData<FloatingWindowCustomization>]
 	public void MinimizeWindowEnd(IContext context, IWindow window)
 	{
 		// Given
@@ -258,7 +253,7 @@ public void MinimizeWindowEnd(IContext context, IWindow window)
 		Assert.Equal(engine, newEngine);
 	}
 
-	[Theory, AutoSubstituteData<FloatingLayoutEngineCustomization>]
+	[Theory, AutoSubstituteData<FloatingWindowCustomization>]
 	public void FocusWindowInDirection(IContext context, IWindow window)
 	{
 		// Given
@@ -271,7 +266,7 @@ public void FocusWindowInDirection(IContext context, IWindow window)
 		Assert.Equal(engine, newEngine);
 	}
 
-	[Theory, AutoSubstituteData<FloatingLayoutEngineCustomization>]
+	[Theory, AutoSubstituteData<FloatingWindowCustomization>]
 	public void SwapWindowInDirection(IContext context, IWindow window)
 	{
 		// Given
@@ -285,7 +280,7 @@ public void SwapWindowInDirection(IContext context, IWindow window)
 	}
 	#endregion
 
-	[Theory, AutoSubstituteData<FloatingLayoutEngineCustomization>]
+	[Theory, AutoSubstituteData<FloatingWindowCustomization>]
 	public void PerformCustomAction(IContext context, IWindow window)
 	{
 		// Given

diff --git a/src/Whim.FloatingWindow.Tests/FloatingWindowCustomization.cs b/src/Whim.FloatingWindow.Tests/FloatingWindowCustomization.cs
@@ -0,0 +1,46 @@
+using System.Diagnostics.CodeAnalysis;
+using AutoFixture;
+using NSubstitute;
+using Whim.TestUtils;
+using Windows.Win32.Foundation;
+using Windows.Win32.Graphics.Gdi;
+
+namespace Whim.FloatingWindow.Tests;
+
+public class FloatingWindowCustomization : ICustomization
+{
+	[SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope")]
+	public void Customize(IFixture fixture)
+	{
+		IContext ctx = fixture.Freeze<IContext>();
+		IInternalContext internalCtx = fixture.Freeze<IInternalContext>();
+
+		Store store = new(ctx, internalCtx);
+		ctx.Store.Returns(store);
+
+		IWindow window1 = StoreTestUtils.CreateWindow((HWND)1);
+		IWindow window2 = StoreTestUtils.CreateWindow((HWND)2);
+		IWindow window3 = StoreTestUtils.CreateWindow((HWND)3);
+		Workspace workspace = StoreTestUtils.CreateWorkspace(ctx);
+
+		IMonitor monitor = StoreTestUtils.CreateMonitor((HMONITOR)123);
+		monitor.WorkingArea.Returns(new Rectangle<int>() { Width = 1000, Height = 1000 });
+
+		StoreTestUtils.SetupMonitorAtPoint(ctx, internalCtx, store._root.MutableRootSector, monitor);
+
+		StoreTestUtils.PopulateThreeWayMap(ctx, store._root.MutableRootSector, monitor, workspace, window1);
+		StoreTestUtils.PopulateThreeWayMap(ctx, store._root.MutableRootSector, monitor, workspace, window2);
+		StoreTestUtils.PopulateThreeWayMap(ctx, store._root.MutableRootSector, monitor, workspace, window3);
+
+		ctx.NativeManager.DwmGetWindowRectangle(Arg.Any<HWND>())
+			.Returns(new Rectangle<int>() { Width = 100, Height = 100 });
+
+		fixture.Inject(monitor);
+		fixture.Inject(store._root);
+		fixture.Inject(store._root.MutableRootSector);
+		fixture.Inject(workspace);
+
+		// Only inject the first window. Other windows can be retrieved from the WindowSector.
+		fixture.Inject(window1);
+	}
+}