Skip to content

Commit

Permalink
Fix SliceLayouts.CreateColumnLayout creating a row layout (#1039)
Browse files Browse the repository at this point in the history
The tests previously succeeded because:

- the `SliceLayoutEngine` record didn't expose the root area, which meant they were not included in the generated `record` `Equals`
- apparently casting `ILayoutEngine` to `SliceLayoutEngine` matters 🤷
  • Loading branch information
dalyIsaac authored Oct 1, 2024
1 parent 6b195f7 commit 0c9707a
Show file tree
Hide file tree
Showing 8 changed files with 246 additions and 38 deletions.
50 changes: 50 additions & 0 deletions src/Whim.SliceLayout.Tests/AreaHelpers/DoLayoutTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -238,13 +238,63 @@ public class DoLayoutTests
},
};

public static TheoryData<IRectangle<int>, ParentArea, IRectangle<int>[]> DoLayout_Column =>
new()
{
// Empty
{ new Rectangle<int>(0, 0, 100, 100), SliceLayouts.CreateColumnArea(), Array.Empty<IRectangle<int>>() },
// Single window
{
new Rectangle<int>(0, 0, 100, 100),
SliceLayouts.CreateColumnArea(),
new[] { new Rectangle<int>(0, 0, 100, 100) }
},
// Three windows
{
new Rectangle<int>(0, 0, 100, 100),
SliceLayouts.CreateColumnArea(),
new[]
{
new Rectangle<int>(0, 0, 100, 33),
new Rectangle<int>(0, 33, 100, 33),
new Rectangle<int>(0, 66, 100, 33),
}
},
};

public static TheoryData<IRectangle<int>, ParentArea, IRectangle<int>[]> DoLayout_Row =>
new()
{
// Empty
{ new Rectangle<int>(0, 0, 100, 100), SliceLayouts.CreateRowArea(), Array.Empty<IRectangle<int>>() },
// Single window
{
new Rectangle<int>(0, 0, 100, 100),
SliceLayouts.CreateRowArea(),
new[] { new Rectangle<int>(0, 0, 100, 100) }
},
// Three windows
{
new Rectangle<int>(0, 0, 100, 100),
SliceLayouts.CreateRowArea(),
new[]
{
new Rectangle<int>(0, 0, 33, 100),
new Rectangle<int>(33, 0, 33, 100),
new Rectangle<int>(66, 0, 33, 100),
}
},
};

[Theory]
[MemberAutoSubstituteData(nameof(DoLayout_PrimaryStack))]
[MemberAutoSubstituteData(nameof(DoLayout_MultiColumn))]
[MemberAutoSubstituteData(nameof(DoLayout_SecondaryPrimary))]
[MemberAutoSubstituteData(nameof(DoLayout_OverflowColumn))]
[MemberAutoSubstituteData(nameof(DoLayout_OverflowRow))]
[MemberAutoSubstituteData(nameof(DoLayout_Nested))]
[MemberAutoSubstituteData(nameof(DoLayout_Column))]
[MemberAutoSubstituteData(nameof(DoLayout_Row))]
internal void DoLayout(
IRectangle<int> rectangle,
ParentArea area,
Expand Down
47 changes: 47 additions & 0 deletions src/Whim.SliceLayout.Tests/SliceLayoutEngine/AreaTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
using FluentAssertions;
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
area1.Should().BeEquivalentTo(area2);
}

public static TheoryData<ParentArea, ParentArea> 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)))
},
{
new(isRow: true, (1, new OverflowArea(isRow: true))),
new(isRow: true, (1, new SliceArea(order: 0, maxChildren: 0)))
},
};

[Theory, MemberData(nameof(ParentArea_NotEqual))]
public void ParentArea_Equals_NotEqual(ParentArea area1, ParentArea area2)
{
// Then
Assert.NotEqual(area1, area2);
area1.Should().NotBeEquivalentTo(area2);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,61 @@ 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 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)
{
// Given
ILayoutEngine sut = new SliceLayoutEngine(ctx, plugin, identity, SampleSliceLayouts.CreateNestedLayout());
sut = sut.AddWindow(Substitute.For<IWindow>());
sut = sut.MinimizeWindowStart(Substitute.For<IWindow>());

// When
int hashCode = sut.GetHashCode();

// Then
Assert.NotEqual(0, hashCode);
}

#region AddWindow
[Theory, AutoSubstituteData]
public void AddWindow(IContext ctx, SliceLayoutPlugin plugin, IWindow window)
Expand Down
83 changes: 66 additions & 17 deletions src/Whim.SliceLayout.Tests/SliceLayoutsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,10 @@ 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
new SliceLayoutEngine(
ctx,
plugin,
identity,
new ParentArea(isRow: false, (1, new SliceArea(order: 0, maxChildren: 0)))
)
new SliceLayoutEngine(ctx, plugin, identity, new ParentArea(isRow: false, (1, new OverflowArea(isRow: false))))
{
Name = "Column",
}
Expand All @@ -32,36 +27,56 @@ 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
// Then
new SliceLayoutEngine(
ctx,
plugin,
identity,
new ParentArea(isRow: true, (1, new SliceArea(order: 0, maxChildren: 0)))
)
new SliceLayoutEngine(ctx, plugin, identity, new ParentArea(isRow: true, (1, new OverflowArea(isRow: true))))
{
Name = "Row",
}
.Should()
.BeEquivalentTo(sut);
}

[Theory, AutoSubstituteData]
public void SanityCheck_RowNotEqualToColumn(IContext ctx, ISliceLayoutPlugin plugin)
{
// A sanity check to ensure that the two layouts are not equal.
// Given
LayoutEngineIdentity identity = new();
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
row1.Should().NotBeEquivalentTo(row2);
}

[Theory, AutoSubstituteData]
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
new SliceLayoutEngine(
ctx,
plugin,
identity,
new ParentArea(isRow: true, (0.5, new SliceArea(order: 0, maxChildren: 0)), (0.5, new OverflowArea()))
new ParentArea(isRow: true, (0.5, new SliceArea(order: 0, maxChildren: 1)), (0.5, new OverflowArea()))
)
{
Name = "Primary stack",
Expand All @@ -87,4 +102,38 @@ public void CreateMultiColumnArea_MultipleOverflows()
.Should()
.BeEquivalentTo(sut);
}

[Theory]
[InlineAutoSubstituteData(1, 2)]
[InlineAutoSubstituteData(3, 1)]
public void CreateSecondaryPrimaryLayout(
uint primaryCount,
uint secondaryCount,
IContext ctx,
ISliceLayoutPlugin plugin
)
{
// Given
LayoutEngineIdentity identity = new();
SliceLayoutEngine sut = (SliceLayoutEngine)
SliceLayouts.CreateSecondaryPrimaryLayout(ctx, plugin, identity, primaryCount, secondaryCount);

// Then
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",
}
.Should()
.BeEquivalentTo(sut);
}
}
22 changes: 15 additions & 7 deletions src/Whim.SliceLayout/SliceLayoutEngine.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
Expand Down Expand Up @@ -26,7 +27,6 @@ public partial record SliceLayoutEngine : ILayoutEngine
private readonly IContext _context;
private readonly ImmutableList<IWindow> _windows;
private readonly ImmutableList<IWindow> _minimizedWindows;
private readonly ParentArea _rootArea;
private readonly ISliceLayoutPlugin _plugin;

/// <inheritdoc />
Expand All @@ -53,10 +53,18 @@ public partial record SliceLayoutEngine : ILayoutEngine
/// </summary>
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.

/// <summary>
/// The raw root area, with no pruning - see <see cref="PrunedRootArea"/>.
/// </summary>
public ParentArea RootArea { get; }

/// <summary>
/// The root area, with any empty areas pruned.
/// </summary>
private readonly ParentArea _prunedRootArea;
public ParentArea PrunedRootArea { get; }

private SliceLayoutEngine(
SliceLayoutEngine engine,
Expand All @@ -72,8 +80,8 @@ ImmutableList<IWindow> 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);
}

/// <summary>
Expand All @@ -99,8 +107,8 @@ ParentArea rootArea
_windows = [];
_minimizedWindows = [];

(_rootArea, _windowAreas) = rootArea.SetStartIndexes();
_prunedRootArea = _rootArea.Prune(_windows.Count);
(RootArea, _windowAreas) = rootArea.SetStartIndexes();
PrunedRootArea = RootArea.Prune(_windows.Count);
}

/// <inheritdoc />
Expand Down Expand Up @@ -160,7 +168,7 @@ private IWindowState[] DoNormalLayout(IRectangle<int> 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];
Expand Down
Loading

0 comments on commit 0c9707a

Please sign in to comment.