From e0b1d60f01275f770b2cdc8b8664c94658abb27d Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Fri, 27 Sep 2024 21:57:55 +1000 Subject: [PATCH 1/6] Port over SliceLayoutEngine fixes --- .../AreaHelpers/DoLayoutTests.cs | 50 +++++++ .../SliceLayoutEngine/AreaTests.cs | 29 ++++ .../SliceLayoutEngineTests.cs | 39 ++++++ .../SliceLayoutsTests.cs | 131 ++++++++++++------ src/Whim.SliceLayout/Area.cs | 40 ++++++ src/Whim.SliceLayout/SliceLayoutEngine.cs | 37 +++++ src/Whim.SliceLayout/SliceLayouts.cs | 18 ++- 7 files changed, 290 insertions(+), 54 deletions(-) create mode 100644 src/Whim.SliceLayout.Tests/SliceLayoutEngine/AreaTests.cs diff --git a/src/Whim.SliceLayout.Tests/AreaHelpers/DoLayoutTests.cs b/src/Whim.SliceLayout.Tests/AreaHelpers/DoLayoutTests.cs index 74dcbe78e..b04cf1552 100644 --- a/src/Whim.SliceLayout.Tests/AreaHelpers/DoLayoutTests.cs +++ b/src/Whim.SliceLayout.Tests/AreaHelpers/DoLayoutTests.cs @@ -238,6 +238,54 @@ public class DoLayoutTests }, }; + public static TheoryData, ParentArea, IRectangle[]> DoLayout_Column => + new() + { + // Empty + { new Rectangle(0, 0, 100, 100), SliceLayouts.CreateColumnArea(), Array.Empty>() }, + // Single window + { + new Rectangle(0, 0, 100, 100), + SliceLayouts.CreateColumnArea(), + new[] { new Rectangle(0, 0, 100, 100) } + }, + // Three windows + { + new Rectangle(0, 0, 100, 100), + SliceLayouts.CreateColumnArea(), + new[] + { + new Rectangle(0, 0, 100, 33), + new Rectangle(0, 33, 100, 33), + new Rectangle(0, 66, 100, 33), + } + }, + }; + + public static TheoryData, ParentArea, IRectangle[]> DoLayout_Row => + new() + { + // Empty + { new Rectangle(0, 0, 100, 100), SliceLayouts.CreateRowArea(), Array.Empty>() }, + // Single window + { + new Rectangle(0, 0, 100, 100), + SliceLayouts.CreateRowArea(), + new[] { new Rectangle(0, 0, 100, 100) } + }, + // Three windows + { + new Rectangle(0, 0, 100, 100), + SliceLayouts.CreateRowArea(), + new[] + { + new Rectangle(0, 0, 33, 100), + new Rectangle(33, 0, 33, 100), + new Rectangle(66, 0, 33, 100), + } + }, + }; + [Theory] [MemberAutoSubstituteData(nameof(DoLayout_PrimaryStack))] [MemberAutoSubstituteData(nameof(DoLayout_MultiColumn))] @@ -245,6 +293,8 @@ public class DoLayoutTests [MemberAutoSubstituteData(nameof(DoLayout_OverflowColumn))] [MemberAutoSubstituteData(nameof(DoLayout_OverflowRow))] [MemberAutoSubstituteData(nameof(DoLayout_Nested))] + [MemberAutoSubstituteData(nameof(DoLayout_Column))] + [MemberAutoSubstituteData(nameof(DoLayout_Row))] internal void DoLayout( IRectangle rectangle, ParentArea area, diff --git a/src/Whim.SliceLayout.Tests/SliceLayoutEngine/AreaTests.cs b/src/Whim.SliceLayout.Tests/SliceLayoutEngine/AreaTests.cs new file mode 100644 index 000000000..d44c5c16b --- /dev/null +++ b/src/Whim.SliceLayout.Tests/SliceLayoutEngine/AreaTests.cs @@ -0,0 +1,29 @@ +using Xunit; + +namespace Whim.SliceLayout.Tests; + +public class AreaTests +{ + [Fact] + public void ParentArea_Equals() + { + // Given + ParentArea area1 = new(isRow: true, (1, new OverflowArea(isRow: true))); + ParentArea area2 = new(isRow: true, (1, new OverflowArea(isRow: true))); + + // Then + Assert.Equal(area1, area2); + } + + [Fact] + public void ParentArea_GetHashCode() + { + // Given + ParentArea area1 = new(isRow: true, (1, new OverflowArea(isRow: true))); + ParentArea area2 = new(isRow: true, (1, new OverflowArea(isRow: true))); + + // Then + Assert.NotEqual(0, area1.GetHashCode()); + Assert.Equal(area1.GetHashCode(), area2.GetHashCode()); + } +} diff --git a/src/Whim.SliceLayout.Tests/SliceLayoutEngine/SliceLayoutEngineTests.cs b/src/Whim.SliceLayout.Tests/SliceLayoutEngine/SliceLayoutEngineTests.cs index 63a97770c..41c15691b 100644 --- a/src/Whim.SliceLayout.Tests/SliceLayoutEngine/SliceLayoutEngineTests.cs +++ b/src/Whim.SliceLayout.Tests/SliceLayoutEngine/SliceLayoutEngineTests.cs @@ -64,6 +64,45 @@ public void Count(int windowCount, int minimizedWindowCount, IContext ctx, Slice Assert.Equal(windowCount + minimizedWindowCount, sut.Count); } + [Theory, AutoSubstituteData] + public void Equals_Null(IContext ctx, SliceLayoutPlugin plugin) + { + // Given + ILayoutEngine sut = new SliceLayoutEngine(ctx, plugin, identity, SampleSliceLayouts.CreateNestedLayout()); + + // When + bool equals = sut.Equals(null); + + // Then + Assert.False(equals); + } + + [Theory, AutoSubstituteData] + public void Equals_Same(IContext ctx, SliceLayoutPlugin plugin) + { + // Given + ILayoutEngine sut = new SliceLayoutEngine(ctx, plugin, identity, SampleSliceLayouts.CreateNestedLayout()); + + // When + bool equals = sut.Equals(sut); + + // Then + Assert.True(equals); + } + + [Theory, AutoSubstituteData] + public void SliceLayoutEngine_GetHashCode(IContext ctx, SliceLayoutPlugin plugin) + { + // Given + ILayoutEngine sut = new SliceLayoutEngine(ctx, plugin, identity, SampleSliceLayouts.CreateNestedLayout()); + + // When + int hashCode = sut.GetHashCode(); + + // Then + Assert.NotEqual(0, hashCode); + } + #region AddWindow [Theory, AutoSubstituteData] public void AddWindow(IContext ctx, SliceLayoutPlugin plugin, IWindow window) diff --git a/src/Whim.SliceLayout.Tests/SliceLayoutsTests.cs b/src/Whim.SliceLayout.Tests/SliceLayoutsTests.cs index fece2512b..2e1c3a9b0 100644 --- a/src/Whim.SliceLayout.Tests/SliceLayoutsTests.cs +++ b/src/Whim.SliceLayout.Tests/SliceLayoutsTests.cs @@ -1,4 +1,3 @@ -using FluentAssertions; using Whim.TestUtils; using Xunit; @@ -14,17 +13,18 @@ public void CreateColumnLayout(IContext ctx, ISliceLayoutPlugin plugin) ILayoutEngine sut = SliceLayouts.CreateColumnLayout(ctx, plugin, identity); // Then - new SliceLayoutEngine( - ctx, - plugin, - identity, - new ParentArea(isRow: false, (1, new SliceArea(order: 0, maxChildren: 0))) - ) - { - Name = "Column", - } - .Should() - .BeEquivalentTo(sut); + Assert.Equal( + new SliceLayoutEngine( + ctx, + plugin, + identity, + new ParentArea(isRow: false, (1, new OverflowArea(isRow: false))) + ) + { + Name = "Column", + }, + sut + ); } [Theory, AutoSubstituteData] @@ -35,18 +35,18 @@ public void CreateRowLayout(IContext ctx, ISliceLayoutPlugin plugin) ILayoutEngine sut = SliceLayouts.CreateRowLayout(ctx, plugin, identity); // Then - // Then - new SliceLayoutEngine( - ctx, - plugin, - identity, - new ParentArea(isRow: true, (1, new SliceArea(order: 0, maxChildren: 0))) - ) - { - Name = "Row", - } - .Should() - .BeEquivalentTo(sut); + Assert.Equal( + new SliceLayoutEngine( + ctx, + plugin, + identity, + new ParentArea(isRow: true, (1, new OverflowArea(isRow: true))) + ) + { + Name = "Row", + }, + sut + ); } [Theory, AutoSubstituteData] @@ -57,17 +57,18 @@ public void CreatePrimaryStackLayout(IContext ctx, ISliceLayoutPlugin plugin) ILayoutEngine sut = SliceLayouts.CreatePrimaryStackLayout(ctx, plugin, identity); // Then - new SliceLayoutEngine( - ctx, - plugin, - identity, - new ParentArea(isRow: true, (0.5, new SliceArea(order: 0, maxChildren: 0)), (0.5, new OverflowArea())) - ) - { - Name = "Primary stack", - } - .Should() - .BeEquivalentTo(sut); + Assert.Equal( + new SliceLayoutEngine( + ctx, + plugin, + identity, + new ParentArea(isRow: true, (0.5, new SliceArea(order: 0, maxChildren: 1)), (0.5, new OverflowArea())) + ) + { + Name = "Primary stack", + }, + sut + ); } [Fact] @@ -77,14 +78,56 @@ public void CreateMultiColumnArea_MultipleOverflows() ParentArea sut = SliceLayouts.CreateMultiColumnArea([2, 1, 0, 0]); // Then - new ParentArea( - isRow: true, - (0.25, new SliceArea(order: 0, maxChildren: 2)), - (0.25, new SliceArea(order: 1, maxChildren: 1)), - (0.25, new SliceArea(order: 2, maxChildren: 0)), - (0.25, new OverflowArea()) - ) - .Should() - .BeEquivalentTo(sut); + Assert.Equal( + new ParentArea( + isRow: true, + (0.25, new SliceArea(order: 0, maxChildren: 2)), + (0.25, new SliceArea(order: 1, maxChildren: 1)), + (0.25, new SliceArea(order: 2, maxChildren: 0)), + (0.25, new OverflowArea()) + ), + sut + ); + } + + // TODO: SecondaryPrimaryArea + [Theory] + [InlineAutoSubstituteData(1, 2)] + [InlineAutoSubstituteData(3, 1)] + public void CreateSecondaryPrimaryLayout( + uint primaryCount, + uint secondaryCount, + IContext ctx, + ISliceLayoutPlugin plugin + ) + { + // Given + LayoutEngineIdentity identity = new(); + ILayoutEngine sut = SliceLayouts.CreateSecondaryPrimaryLayout( + ctx, + plugin, + identity, + primaryCount, + secondaryCount + ); + + // Then + Assert.Equal( + new SliceLayoutEngine( + ctx, + plugin, + identity, + new ParentArea( + isRow: true, + (0.25, new SliceArea(order: 1, maxChildren: secondaryCount)), + (0.5, new SliceArea(order: 0, maxChildren: primaryCount)), + (0.25, new OverflowArea()) + ) + ) + { + Name = "Secondary primary", + }, + sut + ); } } diff --git a/src/Whim.SliceLayout/Area.cs b/src/Whim.SliceLayout/Area.cs index 0e2e0d172..1592b6b93 100644 --- a/src/Whim.SliceLayout/Area.cs +++ b/src/Whim.SliceLayout/Area.cs @@ -1,4 +1,6 @@ +using System; using System.Collections.Immutable; +using System.Linq; namespace Whim.SliceLayout; @@ -65,6 +67,44 @@ internal ParentArea(bool isRow, ImmutableList weights, ImmutableList + /// Determines whether the specified object is equal to the current object. This compares + /// the contents of the weights and children. + /// + /// + /// + public virtual bool Equals(ParentArea? other) + { + if (other is null) + { + return false; + } + + if (ReferenceEquals(this, other)) + { + return true; + } + + return IsRow == other.IsRow && Weights.SequenceEqual(other.Weights) && Children.SequenceEqual(other.Children); + } + + /// + public override int GetHashCode() + { + // Get the hash code of the weights and children + int hash = HashCode.Combine(IsRow); + foreach (double weight in Weights) + { + hash = HashCode.Combine(hash, weight); + } + foreach (IArea child in Children) + { + hash = HashCode.Combine(hash, child); + } + + return hash; + } } /// diff --git a/src/Whim.SliceLayout/SliceLayoutEngine.cs b/src/Whim.SliceLayout/SliceLayoutEngine.cs index 23bfbac37..b829d0aa3 100644 --- a/src/Whim.SliceLayout/SliceLayoutEngine.cs +++ b/src/Whim.SliceLayout/SliceLayoutEngine.cs @@ -1,3 +1,4 @@ +using System; using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; @@ -103,6 +104,42 @@ ParentArea rootArea _prunedRootArea = _rootArea.Prune(_windows.Count); } + /// + public virtual bool Equals(SliceLayoutEngine? other) + { + if (other is null) + { + return false; + } + + if (ReferenceEquals(this, other)) + { + return true; + } + + return _windows.SequenceEqual(other._windows) + && _minimizedWindows.SequenceEqual(other._minimizedWindows) + && _rootArea.Equals(other._rootArea) + && Name == other.Name + && Identity.Equals(other.Identity); + } + + /// + public override int GetHashCode() + { + int hash = HashCode.Combine(_context, _plugin, Name, Identity); + foreach (IWindow window in _windows) + { + hash = HashCode.Combine(hash, window); + } + foreach (IWindow window in _minimizedWindows) + { + hash = HashCode.Combine(hash, window); + } + hash = HashCode.Combine(hash, _rootArea); + return hash; + } + /// public ILayoutEngine AddWindow(IWindow window) { diff --git a/src/Whim.SliceLayout/SliceLayouts.cs b/src/Whim.SliceLayout/SliceLayouts.cs index 42b492273..028c7926c 100644 --- a/src/Whim.SliceLayout/SliceLayouts.cs +++ b/src/Whim.SliceLayout/SliceLayouts.cs @@ -45,18 +45,14 @@ public static class SliceLayouts /// /// /// - /// /// public static ILayoutEngine CreateColumnLayout( IContext context, ISliceLayoutPlugin plugin, - LayoutEngineIdentity identity, - bool leftToRight = true - ) => - new SliceLayoutEngine(context, plugin, identity, new(isRow: false, (1, new OverflowArea()))) - { - Name = "Column", - }; + LayoutEngineIdentity identity + ) => new SliceLayoutEngine(context, plugin, identity, CreateColumnArea()) { Name = "Column" }; + + internal static ParentArea CreateColumnArea() => new(isRow: false, (1, new OverflowArea())); /// /// Creates a row layout, where windows are stacked horizontally. @@ -103,7 +99,9 @@ public static ILayoutEngine CreateRowLayout( IContext context, ISliceLayoutPlugin plugin, LayoutEngineIdentity identity - ) => new SliceLayoutEngine(context, plugin, identity, new(isRow: true, (1, new OverflowArea()))) { Name = "Row" }; + ) => new SliceLayoutEngine(context, plugin, identity, CreateRowArea()) { Name = "Row" }; + + internal static ParentArea CreateRowArea() => new(isRow: true, (1, new OverflowArea(isRow: true))); /// /// Creates a primary stack layout, where the first window takes up half the screen, and the @@ -165,7 +163,7 @@ internal static ParentArea CreatePrimaryStackArea() => new(isRow: true, (0.5, new SliceArea(order: 0, maxChildren: 1)), (0.5, new OverflowArea())); /// - /// Creates a multi-column layout with the given number of columns. + /// Creates a multi-column layout with the given number of windows in each column. ///
/// For example, new uint[] { 2, 1, 0 } will create a layout with 3 columns, where the /// first column has 2 rows, the second column has 1 row, and the third column has infinite rows. From effc408381b8f60be58304da67b61841ef6cce83 Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Sat, 28 Sep 2024 00:06:49 +1000 Subject: [PATCH 2/6] Bump coverage --- .../SliceLayoutEngine/AreaTests.cs | 24 +++++++++++++++++++ .../SliceLayoutEngineTests.cs | 2 ++ 2 files changed, 26 insertions(+) diff --git a/src/Whim.SliceLayout.Tests/SliceLayoutEngine/AreaTests.cs b/src/Whim.SliceLayout.Tests/SliceLayoutEngine/AreaTests.cs index d44c5c16b..63bf99d21 100644 --- a/src/Whim.SliceLayout.Tests/SliceLayoutEngine/AreaTests.cs +++ b/src/Whim.SliceLayout.Tests/SliceLayoutEngine/AreaTests.cs @@ -15,6 +15,30 @@ public void ParentArea_Equals() Assert.Equal(area1, area2); } + public static TheoryData ParentArea_NotEqual => + new() + { + { + new(isRow: true, (1, new OverflowArea(isRow: true))), + new(isRow: false, (1, new OverflowArea(isRow: true))) + }, + { + new(isRow: true, (1, new OverflowArea(isRow: true))), + new(isRow: true, (1, new OverflowArea(isRow: false))) + }, + { + new(isRow: true, (1, new OverflowArea(isRow: true))), + new(isRow: true, (1, new OverflowArea(isRow: true)), (1, new OverflowArea(isRow: true))) + }, + }; + + [Theory, MemberData(nameof(ParentArea_NotEqual))] + public void ParentArea_Equals_NotEqual(ParentArea area1, ParentArea area2) + { + // Then + Assert.NotEqual(area1, area2); + } + [Fact] public void ParentArea_GetHashCode() { diff --git a/src/Whim.SliceLayout.Tests/SliceLayoutEngine/SliceLayoutEngineTests.cs b/src/Whim.SliceLayout.Tests/SliceLayoutEngine/SliceLayoutEngineTests.cs index 41c15691b..66f538022 100644 --- a/src/Whim.SliceLayout.Tests/SliceLayoutEngine/SliceLayoutEngineTests.cs +++ b/src/Whim.SliceLayout.Tests/SliceLayoutEngine/SliceLayoutEngineTests.cs @@ -95,6 +95,8 @@ public void SliceLayoutEngine_GetHashCode(IContext ctx, SliceLayoutPlugin plugin { // Given ILayoutEngine sut = new SliceLayoutEngine(ctx, plugin, identity, SampleSliceLayouts.CreateNestedLayout()); + sut = sut.AddWindow(Substitute.For()); + sut = sut.MinimizeWindowStart(Substitute.For()); // When int hashCode = sut.GetHashCode(); From ea962819a53ef59bcdb1eab7c6208863d1baa972 Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Sat, 28 Sep 2024 00:21:34 +1000 Subject: [PATCH 3/6] Change timeout --- .../Store/MonitorSector/MonitorEventListenerTests.cs | 4 ++-- src/Whim/Store/MonitorSector/MonitorEventListener.cs | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Whim.Tests/Store/MonitorSector/MonitorEventListenerTests.cs b/src/Whim.Tests/Store/MonitorSector/MonitorEventListenerTests.cs index 997641b70..0a279349e 100644 --- a/src/Whim.Tests/Store/MonitorSector/MonitorEventListenerTests.cs +++ b/src/Whim.Tests/Store/MonitorSector/MonitorEventListenerTests.cs @@ -98,7 +98,7 @@ internal void WindowMessageMonitor_DpiChanged(IContext ctx, IInternalContext int internal async Task WindowMessageMonitor_SessionChanged(IContext ctx, IInternalContext internalCtx) { // Given the listener is initialized - MonitorEventListener listener = new(ctx, internalCtx); + MonitorEventListener listener = new(ctx, internalCtx, 500); listener.Initialize(); NativeManagerUtils.SetupTryEnqueue(ctx); @@ -108,7 +108,7 @@ internal async Task WindowMessageMonitor_SessionChanged(IContext ctx, IInternalC ctx.Store.MonitorEvents, _windowMessageArgs ); - await Task.Delay(5200); + await Task.Delay(2000); // Then a dispatch to the store was triggered ctx.Store.Received(1).Dispatch(new MonitorsChangedTransform()); diff --git a/src/Whim/Store/MonitorSector/MonitorEventListener.cs b/src/Whim/Store/MonitorSector/MonitorEventListener.cs index a3c54a8b8..b46e2b0a5 100644 --- a/src/Whim/Store/MonitorSector/MonitorEventListener.cs +++ b/src/Whim/Store/MonitorSector/MonitorEventListener.cs @@ -2,10 +2,11 @@ namespace Whim; -internal class MonitorEventListener(IContext ctx, IInternalContext internalCtx) : IDisposable +internal class MonitorEventListener(IContext ctx, IInternalContext internalCtx, int delayMs = 5000) : IDisposable { private readonly IContext _ctx = ctx; private readonly IInternalContext _internalCtx = internalCtx; + private readonly int _delayMs = delayMs; private bool _disposedValue; public void Initialize() @@ -29,7 +30,7 @@ private void WindowMessageMonitor_SessionChanged(object? sender, WindowMessageMo // This gives Windows some to figure out the correct working area. _ctx.NativeManager.TryEnqueue(async () => { - await Task.Delay(5000).ConfigureAwait(true); + await Task.Delay(_delayMs).ConfigureAwait(true); WindowMessageMonitor_MonitorsChanged(sender, e); }); } From 02eec8f296471420effa1b5af84a3c1720176719 Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Sat, 28 Sep 2024 00:27:27 +1000 Subject: [PATCH 4/6] More coverage --- .../SliceLayoutEngine/SliceLayoutEngineTests.cs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/Whim.SliceLayout.Tests/SliceLayoutEngine/SliceLayoutEngineTests.cs b/src/Whim.SliceLayout.Tests/SliceLayoutEngine/SliceLayoutEngineTests.cs index 66f538022..8819fd54b 100644 --- a/src/Whim.SliceLayout.Tests/SliceLayoutEngine/SliceLayoutEngineTests.cs +++ b/src/Whim.SliceLayout.Tests/SliceLayoutEngine/SliceLayoutEngineTests.cs @@ -90,6 +90,20 @@ public void Equals_Same(IContext ctx, SliceLayoutPlugin plugin) Assert.True(equals); } + [Theory, AutoSubstituteData] + public void Equals_DifferentType(IContext ctx, SliceLayoutPlugin plugin) + { + // Given + ILayoutEngine a = new SliceLayoutEngine(ctx, plugin, identity, SampleSliceLayouts.CreateNestedLayout()); + ILayoutEngine b = SliceLayouts.CreateColumnLayout(ctx, plugin, identity); + + // When + bool equals = a.Equals(b); + + // Then + Assert.False(equals); + } + [Theory, AutoSubstituteData] public void SliceLayoutEngine_GetHashCode(IContext ctx, SliceLayoutPlugin plugin) { From 742e11f8ef179d347be500789adb5f8fc2553164 Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Tue, 1 Oct 2024 21:33:03 +1000 Subject: [PATCH 5/6] Add a sanity check test --- src/Whim.SliceLayout.Tests/SliceLayoutsTests.cs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/Whim.SliceLayout.Tests/SliceLayoutsTests.cs b/src/Whim.SliceLayout.Tests/SliceLayoutsTests.cs index 2e1c3a9b0..ca973606a 100644 --- a/src/Whim.SliceLayout.Tests/SliceLayoutsTests.cs +++ b/src/Whim.SliceLayout.Tests/SliceLayoutsTests.cs @@ -49,6 +49,19 @@ public void CreateRowLayout(IContext ctx, ISliceLayoutPlugin plugin) ); } + [Theory, AutoSubstituteData] + public void RowNotEqualToColumn(IContext ctx, ISliceLayoutPlugin plugin) + { + // A sanity check to ensure that the two layouts are not equal. + // Given + LayoutEngineIdentity identity = new(); + ILayoutEngine row = SliceLayouts.CreateRowLayout(ctx, plugin, identity); + ILayoutEngine column = SliceLayouts.CreateColumnLayout(ctx, plugin, identity); + + // Then + Assert.NotEqual(row, column); + } + [Theory, AutoSubstituteData] public void CreatePrimaryStackLayout(IContext ctx, ISliceLayoutPlugin plugin) { From 2b948be5f6637965694163025ecb7644a6fd41fe Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Tue, 1 Oct 2024 22:47:49 +1000 Subject: [PATCH 6/6] Switch from custom Equals to the record's auto-generated equality This avoids any excessive deep equality checks for sequences --- .../SliceLayoutEngine/AreaTests.cs | 20 +-- .../SliceLayoutsTests.cs | 145 +++++++++--------- src/Whim.SliceLayout/Area.cs | 40 ----- src/Whim.SliceLayout/SliceLayoutEngine.cs | 57 ++----- 4 files changed, 90 insertions(+), 172 deletions(-) diff --git a/src/Whim.SliceLayout.Tests/SliceLayoutEngine/AreaTests.cs b/src/Whim.SliceLayout.Tests/SliceLayoutEngine/AreaTests.cs index 63bf99d21..e5af3ddeb 100644 --- a/src/Whim.SliceLayout.Tests/SliceLayoutEngine/AreaTests.cs +++ b/src/Whim.SliceLayout.Tests/SliceLayoutEngine/AreaTests.cs @@ -1,3 +1,4 @@ +using FluentAssertions; using Xunit; namespace Whim.SliceLayout.Tests; @@ -12,7 +13,7 @@ public void ParentArea_Equals() ParentArea area2 = new(isRow: true, (1, new OverflowArea(isRow: true))); // Then - Assert.Equal(area1, area2); + area1.Should().BeEquivalentTo(area2); } public static TheoryData ParentArea_NotEqual => @@ -30,6 +31,10 @@ public void ParentArea_Equals() new(isRow: true, (1, new OverflowArea(isRow: true))), new(isRow: true, (1, new OverflowArea(isRow: true)), (1, new OverflowArea(isRow: true))) }, + { + new(isRow: true, (1, new OverflowArea(isRow: true))), + new(isRow: true, (1, new SliceArea(order: 0, maxChildren: 0))) + }, }; [Theory, MemberData(nameof(ParentArea_NotEqual))] @@ -37,17 +42,6 @@ public void ParentArea_Equals_NotEqual(ParentArea area1, ParentArea area2) { // Then Assert.NotEqual(area1, area2); - } - - [Fact] - public void ParentArea_GetHashCode() - { - // Given - ParentArea area1 = new(isRow: true, (1, new OverflowArea(isRow: true))); - ParentArea area2 = new(isRow: true, (1, new OverflowArea(isRow: true))); - - // Then - Assert.NotEqual(0, area1.GetHashCode()); - Assert.Equal(area1.GetHashCode(), area2.GetHashCode()); + area1.Should().NotBeEquivalentTo(area2); } } diff --git a/src/Whim.SliceLayout.Tests/SliceLayoutsTests.cs b/src/Whim.SliceLayout.Tests/SliceLayoutsTests.cs index ca973606a..fa13f9f45 100644 --- a/src/Whim.SliceLayout.Tests/SliceLayoutsTests.cs +++ b/src/Whim.SliceLayout.Tests/SliceLayoutsTests.cs @@ -1,3 +1,4 @@ +using FluentAssertions; using Whim.TestUtils; using Xunit; @@ -10,21 +11,15 @@ public void CreateColumnLayout(IContext ctx, ISliceLayoutPlugin plugin) { // Given LayoutEngineIdentity identity = new(); - ILayoutEngine sut = SliceLayouts.CreateColumnLayout(ctx, plugin, identity); + SliceLayoutEngine sut = (SliceLayoutEngine)SliceLayouts.CreateColumnLayout(ctx, plugin, identity); // Then - Assert.Equal( - new SliceLayoutEngine( - ctx, - plugin, - identity, - new ParentArea(isRow: false, (1, new OverflowArea(isRow: false))) - ) - { - Name = "Column", - }, - sut - ); + new SliceLayoutEngine(ctx, plugin, identity, new ParentArea(isRow: false, (1, new OverflowArea(isRow: false)))) + { + Name = "Column", + } + .Should() + .BeEquivalentTo(sut); } [Theory, AutoSubstituteData] @@ -32,34 +27,41 @@ public void CreateRowLayout(IContext ctx, ISliceLayoutPlugin plugin) { // Given LayoutEngineIdentity identity = new(); - ILayoutEngine sut = SliceLayouts.CreateRowLayout(ctx, plugin, identity); + SliceLayoutEngine sut = (SliceLayoutEngine)SliceLayouts.CreateRowLayout(ctx, plugin, identity); // Then - Assert.Equal( - new SliceLayoutEngine( - ctx, - plugin, - identity, - new ParentArea(isRow: true, (1, new OverflowArea(isRow: true))) - ) - { - Name = "Row", - }, - sut - ); + new SliceLayoutEngine(ctx, plugin, identity, new ParentArea(isRow: true, (1, new OverflowArea(isRow: true)))) + { + Name = "Row", + } + .Should() + .BeEquivalentTo(sut); } [Theory, AutoSubstituteData] - public void RowNotEqualToColumn(IContext ctx, ISliceLayoutPlugin plugin) + public void SanityCheck_RowNotEqualToColumn(IContext ctx, ISliceLayoutPlugin plugin) { // A sanity check to ensure that the two layouts are not equal. // Given LayoutEngineIdentity identity = new(); - ILayoutEngine row = SliceLayouts.CreateRowLayout(ctx, plugin, identity); - ILayoutEngine column = SliceLayouts.CreateColumnLayout(ctx, plugin, identity); + SliceLayoutEngine row = (SliceLayoutEngine)SliceLayouts.CreateRowLayout(ctx, plugin, identity); + SliceLayoutEngine column = (SliceLayoutEngine)SliceLayouts.CreateColumnLayout(ctx, plugin, identity); + + // Then + row.Should().NotBeEquivalentTo(column); + } + + [Theory, AutoSubstituteData] + public void SanityCheck_RowsNotEqual(IContext ctx, ISliceLayoutPlugin plugin) + { + // Given + LayoutEngineIdentity identity = new(); + SliceLayoutEngine row1 = + new(ctx, plugin, identity, new ParentArea(isRow: true, (1, new OverflowArea(isRow: true)))); + SliceLayoutEngine row2 = new(ctx, plugin, identity, new ParentArea(isRow: true, (1, new SliceArea(0, 0)))); // Then - Assert.NotEqual(row, column); + row1.Should().NotBeEquivalentTo(row2); } [Theory, AutoSubstituteData] @@ -67,21 +69,20 @@ public void CreatePrimaryStackLayout(IContext ctx, ISliceLayoutPlugin plugin) { // Given LayoutEngineIdentity identity = new(); - ILayoutEngine sut = SliceLayouts.CreatePrimaryStackLayout(ctx, plugin, identity); + SliceLayoutEngine sut = (SliceLayoutEngine)SliceLayouts.CreatePrimaryStackLayout(ctx, plugin, identity); // Then - Assert.Equal( - new SliceLayoutEngine( - ctx, - plugin, - identity, - new ParentArea(isRow: true, (0.5, new SliceArea(order: 0, maxChildren: 1)), (0.5, new OverflowArea())) - ) - { - Name = "Primary stack", - }, - sut - ); + new SliceLayoutEngine( + ctx, + plugin, + identity, + new ParentArea(isRow: true, (0.5, new SliceArea(order: 0, maxChildren: 1)), (0.5, new OverflowArea())) + ) + { + Name = "Primary stack", + } + .Should() + .BeEquivalentTo(sut); } [Fact] @@ -91,19 +92,17 @@ public void CreateMultiColumnArea_MultipleOverflows() ParentArea sut = SliceLayouts.CreateMultiColumnArea([2, 1, 0, 0]); // Then - Assert.Equal( - new ParentArea( - isRow: true, - (0.25, new SliceArea(order: 0, maxChildren: 2)), - (0.25, new SliceArea(order: 1, maxChildren: 1)), - (0.25, new SliceArea(order: 2, maxChildren: 0)), - (0.25, new OverflowArea()) - ), - sut - ); + new ParentArea( + isRow: true, + (0.25, new SliceArea(order: 0, maxChildren: 2)), + (0.25, new SliceArea(order: 1, maxChildren: 1)), + (0.25, new SliceArea(order: 2, maxChildren: 0)), + (0.25, new OverflowArea()) + ) + .Should() + .BeEquivalentTo(sut); } - // TODO: SecondaryPrimaryArea [Theory] [InlineAutoSubstituteData(1, 2)] [InlineAutoSubstituteData(3, 1)] @@ -116,31 +115,25 @@ ISliceLayoutPlugin plugin { // Given LayoutEngineIdentity identity = new(); - ILayoutEngine sut = SliceLayouts.CreateSecondaryPrimaryLayout( + SliceLayoutEngine sut = (SliceLayoutEngine) + SliceLayouts.CreateSecondaryPrimaryLayout(ctx, plugin, identity, primaryCount, secondaryCount); + + // Then + new SliceLayoutEngine( ctx, plugin, identity, - primaryCount, - secondaryCount - ); - - // Then - Assert.Equal( - new SliceLayoutEngine( - ctx, - plugin, - identity, - new ParentArea( - isRow: true, - (0.25, new SliceArea(order: 1, maxChildren: secondaryCount)), - (0.5, new SliceArea(order: 0, maxChildren: primaryCount)), - (0.25, new OverflowArea()) - ) + new ParentArea( + isRow: true, + (0.25, new SliceArea(order: 1, maxChildren: secondaryCount)), + (0.5, new SliceArea(order: 0, maxChildren: primaryCount)), + (0.25, new OverflowArea()) ) - { - Name = "Secondary primary", - }, - sut - ); + ) + { + Name = "Secondary primary", + } + .Should() + .BeEquivalentTo(sut); } } diff --git a/src/Whim.SliceLayout/Area.cs b/src/Whim.SliceLayout/Area.cs index 1592b6b93..0e2e0d172 100644 --- a/src/Whim.SliceLayout/Area.cs +++ b/src/Whim.SliceLayout/Area.cs @@ -1,6 +1,4 @@ -using System; using System.Collections.Immutable; -using System.Linq; namespace Whim.SliceLayout; @@ -67,44 +65,6 @@ internal ParentArea(bool isRow, ImmutableList weights, ImmutableList - /// Determines whether the specified object is equal to the current object. This compares - /// the contents of the weights and children. - /// - /// - /// - public virtual bool Equals(ParentArea? other) - { - if (other is null) - { - return false; - } - - if (ReferenceEquals(this, other)) - { - return true; - } - - return IsRow == other.IsRow && Weights.SequenceEqual(other.Weights) && Children.SequenceEqual(other.Children); - } - - /// - public override int GetHashCode() - { - // Get the hash code of the weights and children - int hash = HashCode.Combine(IsRow); - foreach (double weight in Weights) - { - hash = HashCode.Combine(hash, weight); - } - foreach (IArea child in Children) - { - hash = HashCode.Combine(hash, child); - } - - return hash; - } } /// diff --git a/src/Whim.SliceLayout/SliceLayoutEngine.cs b/src/Whim.SliceLayout/SliceLayoutEngine.cs index b829d0aa3..6c0e368ad 100644 --- a/src/Whim.SliceLayout/SliceLayoutEngine.cs +++ b/src/Whim.SliceLayout/SliceLayoutEngine.cs @@ -27,7 +27,6 @@ public partial record SliceLayoutEngine : ILayoutEngine private readonly IContext _context; private readonly ImmutableList _windows; private readonly ImmutableList _minimizedWindows; - private readonly ParentArea _rootArea; private readonly ISliceLayoutPlugin _plugin; /// @@ -54,10 +53,18 @@ public partial record SliceLayoutEngine : ILayoutEngine /// private IWindowState[]? _cachedWindowStates; + // The following two parent areas are important for equality comparisons. The record will automatically use + // these in its auto-generated equality method. + + /// + /// The raw root area, with no pruning - see . + /// + public ParentArea RootArea { get; } + /// /// The root area, with any empty areas pruned. /// - private readonly ParentArea _prunedRootArea; + public ParentArea PrunedRootArea { get; } private SliceLayoutEngine( SliceLayoutEngine engine, @@ -73,8 +80,8 @@ ImmutableList minimizedWindows _windows = windows; _minimizedWindows = minimizedWindows; - (_rootArea, _windowAreas) = engine._rootArea.SetStartIndexes(); - _prunedRootArea = _rootArea.Prune(_windows.Count); + (RootArea, _windowAreas) = engine.RootArea.SetStartIndexes(); + PrunedRootArea = RootArea.Prune(_windows.Count); } /// @@ -100,44 +107,8 @@ ParentArea rootArea _windows = []; _minimizedWindows = []; - (_rootArea, _windowAreas) = rootArea.SetStartIndexes(); - _prunedRootArea = _rootArea.Prune(_windows.Count); - } - - /// - public virtual bool Equals(SliceLayoutEngine? other) - { - if (other is null) - { - return false; - } - - if (ReferenceEquals(this, other)) - { - return true; - } - - return _windows.SequenceEqual(other._windows) - && _minimizedWindows.SequenceEqual(other._minimizedWindows) - && _rootArea.Equals(other._rootArea) - && Name == other.Name - && Identity.Equals(other.Identity); - } - - /// - public override int GetHashCode() - { - int hash = HashCode.Combine(_context, _plugin, Name, Identity); - foreach (IWindow window in _windows) - { - hash = HashCode.Combine(hash, window); - } - foreach (IWindow window in _minimizedWindows) - { - hash = HashCode.Combine(hash, window); - } - hash = HashCode.Combine(hash, _rootArea); - return hash; + (RootArea, _windowAreas) = rootArea.SetStartIndexes(); + PrunedRootArea = RootArea.Prune(_windows.Count); } /// @@ -197,7 +168,7 @@ private IWindowState[] DoNormalLayout(IRectangle rectangle) // Get the rectangles for each window SliceRectangleItem[] items = new SliceRectangleItem[_windows.Count]; - _prunedRootArea.DoParentLayout(rectangle, items); + PrunedRootArea.DoParentLayout(rectangle, items); // Get the window states IWindowState[] windowStates = new IWindowState[_windows.Count];