Browse Source

Aligner code cleanup

Tig 1 năm trước cách đây
mục cha
commit
6785828b11

+ 77 - 68
Terminal.Gui/Drawing/Aligner.cs

@@ -3,7 +3,8 @@ using System.ComponentModel;
 namespace Terminal.Gui;
 
 /// <summary>
-///     Aligns items within a container based on the specified <see cref="Gui.Alignment"/>. Both horizontal and vertical alignments are supported.
+///     Aligns items within a container based on the specified <see cref="Gui.Alignment"/>. Both horizontal and vertical
+///     alignments are supported.
 /// </summary>
 public class Aligner : INotifyPropertyChanged
 {
@@ -61,7 +62,8 @@ public class Aligner : INotifyPropertyChanged
     public event PropertyChangedEventHandler PropertyChanged;
 
     /// <summary>
-    ///     Takes a list of item sizes and returns a list of the positions of those items when aligned within <see name="ContainerSize"/>
+    ///     Takes a list of item sizes and returns a list of the positions of those items when aligned within
+    ///     <see name="ContainerSize"/>
     ///     using the <see cref="Alignment"/> and <see cref="AlignmentModes"/> settings.
     /// </summary>
     /// <param name="sizes">The sizes of the items to align.</param>
@@ -69,7 +71,8 @@ public class Aligner : INotifyPropertyChanged
     public int [] Align (int [] sizes) { return Align (Alignment, AlignmentModes, ContainerSize, sizes); }
 
     /// <summary>
-    ///     Takes a list of item sizes and returns a list of the  positions of those items when aligned within <paramref name="containerSize"/>
+    ///     Takes a list of item sizes and returns a list of the  positions of those items when aligned within
+    ///     <paramref name="containerSize"/>
     ///     using specified parameters.
     /// </summary>
     /// <param name="alignment">Specifies how the items will be aligned.</param>
@@ -77,52 +80,43 @@ public class Aligner : INotifyPropertyChanged
     /// <param name="containerSize">The size of the container.</param>
     /// <param name="sizes">The sizes of the items to align.</param>
     /// <returns>The positions of the items, from left/top to right/bottom.</returns>
-    public static int [] Align (in Alignment alignment, in AlignmentModes alignmentMode, int containerSize, int [] sizes)
+    public static int [] Align (in Alignment alignment, in AlignmentModes alignmentMode, in int containerSize, in int [] sizes)
     {
+        if (alignmentMode.HasFlag (AlignmentModes.EndToStart))
+        {
+            throw new NotImplementedException ("EndToStart is not implemented.");
+        }
+
         if (sizes.Length == 0)
         {
-            return new int [] { };
+            return [];
         }
 
         int maxSpaceBetweenItems = alignmentMode.HasFlag (AlignmentModes.AddSpaceBetweenItems) ? 1 : 0;
-
-        var positions = new int [sizes.Length]; // positions of the items. the return value.
         int totalItemsSize = sizes.Sum ();
         int totalGaps = sizes.Length - 1; // total gaps between items
-        int totalItemsAndSpaces = totalItemsSize + totalGaps * maxSpaceBetweenItems; // total size of items and spaces if we had enough room
-        int spaces = totalGaps * maxSpaceBetweenItems; // We'll decrement this below to place one space between each item until we run out
+        int totalItemsAndSpaces = totalItemsSize + totalGaps * maxSpaceBetweenItems; // total size of items and spacesToGive if we had enough room
+        int spacesToGive = totalGaps * maxSpaceBetweenItems; // We'll decrement this below to place one space between each item until we run out
 
         if (totalItemsSize >= containerSize)
         {
-            spaces = 0;
+            spacesToGive = 0;
         }
         else if (totalItemsAndSpaces > containerSize)
         {
-            spaces = containerSize - totalItemsSize;
+            spacesToGive = containerSize - totalItemsSize;
         }
 
-        var currentPosition = 0;
-
         switch (alignment)
         {
             case Alignment.Start:
                 switch (alignmentMode & ~AlignmentModes.AddSpaceBetweenItems)
                 {
                     case AlignmentModes.StartToEnd:
-                        Start (sizes, positions, ref spaces, maxSpaceBetweenItems);
-
-                        break;
+                        return Start (in sizes, maxSpaceBetweenItems, spacesToGive);
 
                     case AlignmentModes.StartToEnd | AlignmentModes.IgnoreFirstOrLast:
-                        IgnoreLast (sizes, containerSize, positions, maxSpaceBetweenItems, totalItemsSize, spaces, currentPosition);
-
-                        break;
-
-                    case AlignmentModes.EndToStart:
-                    case AlignmentModes.EndToStart | AlignmentModes.IgnoreFirstOrLast:
-                        throw new NotImplementedException ("EndToStart is not implemented.");
-
-                        break;
+                        return IgnoreLast (in sizes, containerSize, totalItemsSize, maxSpaceBetweenItems, spacesToGive);
                 }
 
                 break;
@@ -131,44 +125,31 @@ public class Aligner : INotifyPropertyChanged
                 switch (alignmentMode & ~AlignmentModes.AddSpaceBetweenItems)
                 {
                     case AlignmentModes.StartToEnd:
-                        End (containerSize, sizes, totalItemsSize, spaces, maxSpaceBetweenItems, positions);
-
-                        break;
+                        return End (in sizes, containerSize, totalItemsSize, maxSpaceBetweenItems, spacesToGive);
 
                     case AlignmentModes.StartToEnd | AlignmentModes.IgnoreFirstOrLast:
-                        IgnoreFirst (sizes, containerSize, positions, maxSpaceBetweenItems, totalItemsSize, spaces, currentPosition);
-
-                        break;
-
-                    case AlignmentModes.EndToStart:
-                    case AlignmentModes.EndToStart | AlignmentModes.IgnoreFirstOrLast:
-                        throw new NotImplementedException ("EndToStart is not implemented.");
-
-                        break;
-
+                        return IgnoreFirst (in sizes, containerSize, totalItemsSize, maxSpaceBetweenItems, spacesToGive);
                 }
 
                 break;
 
             case Alignment.Center:
-                Center (containerSize, sizes, totalItemsSize, spaces, positions, maxSpaceBetweenItems);
-
-                break;
+                return Center (in sizes, containerSize, totalItemsSize, maxSpaceBetweenItems, spacesToGive);
 
             case Alignment.Fill:
-                Fill (containerSize, sizes, totalItemsSize, positions);
-
-                break;
+                return Fill (in sizes, containerSize, totalItemsSize);
 
             default:
                 throw new ArgumentOutOfRangeException (nameof (alignment), alignment, null);
         }
 
-        return positions;
+        return [];
     }
 
-    private static void Start (int [] sizes, int [] positions, ref int spaces, int maxSpaceBetweenItems)
+    private static int [] Start (ref readonly int [] sizes, int maxSpaceBetweenItems, int spacesToGive)
     {
+        var positions = new int [sizes.Length]; // positions of the items. the return value.
+
         for (var i = 0; i < sizes.Length; i++)
         {
             CheckSizeCannotBeNegative (i, sizes);
@@ -180,18 +161,28 @@ public class Aligner : INotifyPropertyChanged
                 continue;
             }
 
-            int spaceBefore = spaces-- > 0 ? maxSpaceBetweenItems : 0;
+            int spaceBefore = spacesToGive-- > 0 ? maxSpaceBetweenItems : 0;
 
             // subsequent items are placed one space after the previous item
             positions [i] = positions [i - 1] + sizes [i - 1] + spaceBefore;
         }
+
+        return positions;
     }
 
-    private static void IgnoreFirst (int [] sizes, int containerSize, int [] positions, int maxSpaceBetweenItems, int totalItemsSize, int spaces, int currentPosition)
+    private static int [] IgnoreFirst (
+        ref readonly int [] sizes,
+        int containerSize,
+        int totalItemsSize,
+        int maxSpaceBetweenItems,
+        int spacesToGive
+    )
     {
+        var positions = new int [sizes.Length]; // positions of the items. the return value.
+
         if (sizes.Length > 1)
         {
-            currentPosition = 0;
+            var currentPosition = 0;
             positions [0] = currentPosition; // first item is flush left
 
             for (int i = sizes.Length - 1; i >= 0; i--)
@@ -207,7 +198,7 @@ public class Aligner : INotifyPropertyChanged
 
                 if (i < sizes.Length - 1 && i > 0)
                 {
-                    int spaceBefore = spaces-- > 0 ? maxSpaceBetweenItems : 0;
+                    int spaceBefore = spacesToGive-- > 0 ? maxSpaceBetweenItems : 0;
 
                     positions [i] = currentPosition - sizes [i] - spaceBefore;
                     currentPosition = positions [i];
@@ -219,19 +210,26 @@ public class Aligner : INotifyPropertyChanged
             CheckSizeCannotBeNegative (0, sizes);
             positions [0] = 0; // single item is flush left
         }
+
+        return positions;
     }
 
-    private static void IgnoreLast (int [] sizes, int containerSize, int [] positions, int maxSpaceBetweenItems, int totalItemsSize, int spaces, int currentPosition)
+    private static int [] IgnoreLast (
+        ref readonly int [] sizes,
+        int containerSize,
+        int totalItemsSize,
+        int maxSpaceBetweenItems,
+        int spacesToGive
+    )
     {
+        var positions = new int [sizes.Length]; // positions of the items. the return value.
+
         if (sizes.Length > 1)
         {
+            var currentPosition = 0;
             if (totalItemsSize > containerSize)
             {
-                currentPosition = containerSize - totalItemsSize - spaces;
-            }
-            else
-            {
-                currentPosition = 0;
+                currentPosition = containerSize - totalItemsSize - spacesToGive;
             }
 
             for (var i = 0; i < sizes.Length; i++)
@@ -240,7 +238,7 @@ public class Aligner : INotifyPropertyChanged
 
                 if (i < sizes.Length - 1)
                 {
-                    int spaceBefore = spaces-- > 0 ? maxSpaceBetweenItems : 0;
+                    int spaceBefore = spacesToGive-- > 0 ? maxSpaceBetweenItems : 0;
 
                     positions [i] = currentPosition;
                     currentPosition += sizes [i] + spaceBefore;
@@ -255,14 +253,17 @@ public class Aligner : INotifyPropertyChanged
 
             positions [0] = containerSize - sizes [0]; // single item is flush right
         }
+
+        return positions;
     }
 
-    private static void Fill (int containerSize, int [] sizes, int totalItemsSize, int [] positions)
+    private static int [] Fill (ref readonly int [] sizes, int containerSize, int totalItemsSize)
     {
-        int currentPosition;
+        var positions = new int [sizes.Length]; // positions of the items. the return value.
+
         int spaceBetween = sizes.Length > 1 ? (containerSize - totalItemsSize) / (sizes.Length - 1) : 0;
         int remainder = sizes.Length > 1 ? (containerSize - totalItemsSize) % (sizes.Length - 1) : 0;
-        currentPosition = 0;
+        var currentPosition = 0;
 
         for (var i = 0; i < sizes.Length; i++)
         {
@@ -271,14 +272,18 @@ public class Aligner : INotifyPropertyChanged
             int extraSpace = i < remainder ? 1 : 0;
             currentPosition += sizes [i] + spaceBetween + extraSpace;
         }
+
+        return positions;
     }
 
-    private static void Center (int containerSize, int [] sizes, int totalItemsSize, int spaces, int [] positions, int maxSpaceBetweenItems)
+    private static int [] Center (ref readonly int [] sizes, int containerSize, int totalItemsSize, int maxSpaceBetweenItems, int spacesToGive)
     {
+        var positions = new int [sizes.Length]; // positions of the items. the return value.
+
         if (sizes.Length > 1)
         {
             // remaining space to be distributed before first and after the items
-            int remainingSpace = Math.Max (0, containerSize - totalItemsSize - spaces);
+            int remainingSpace = Math.Max (0, containerSize - totalItemsSize - spacesToGive);
 
             for (var i = 0; i < sizes.Length; i++)
             {
@@ -291,7 +296,7 @@ public class Aligner : INotifyPropertyChanged
                     continue;
                 }
 
-                int spaceBefore = spaces-- > 0 ? maxSpaceBetweenItems : 0;
+                int spaceBefore = spacesToGive-- > 0 ? maxSpaceBetweenItems : 0;
 
                 // subsequent items are placed one space after the previous item
                 positions [i] = positions [i - 1] + sizes [i - 1] + spaceBefore;
@@ -302,24 +307,28 @@ public class Aligner : INotifyPropertyChanged
             CheckSizeCannotBeNegative (0, sizes);
             positions [0] = (containerSize - sizes [0]) / 2; // single item is centered
         }
+
+        return positions;
     }
 
-    private static void End (int containerSize, int [] sizes, int totalItemsSize, int spaces, int maxSpaceBetweenItems, int [] positions)
+    private static int [] End (ref readonly int [] sizes, int containerSize, int totalItemsSize, int maxSpaceBetweenItems, int spacesToGive)
     {
-        int currentPosition;
-        currentPosition = containerSize - totalItemsSize - spaces;
+        var positions = new int [sizes.Length]; // positions of the items. the return value.
+        int currentPosition = containerSize - totalItemsSize - spacesToGive;
 
         for (var i = 0; i < sizes.Length; i++)
         {
             CheckSizeCannotBeNegative (i, sizes);
-            int spaceBefore = spaces-- > 0 ? maxSpaceBetweenItems : 0;
+            int spaceBefore = spacesToGive-- > 0 ? maxSpaceBetweenItems : 0;
 
             positions [i] = currentPosition;
             currentPosition += sizes [i] + spaceBefore;
         }
+
+        return positions;
     }
 
-    private static void CheckSizeCannotBeNegative (int i, int [] sizes)
+    private static void CheckSizeCannotBeNegative (int i, IReadOnlyList<int> sizes)
     {
         if (sizes [i] < 0)
         {

+ 14 - 6
Terminal.Gui/View/Layout/Pos.cs

@@ -1,6 +1,4 @@
 #nullable enable
-using System.ComponentModel;
-
 namespace Terminal.Gui;
 
 /// <summary>
@@ -136,24 +134,34 @@ namespace Terminal.Gui;
 public abstract class Pos
 {
     #region static Pos creation methods
+
     /// <summary>Creates a <see cref="Pos"/> object that is an absolute position based on the specified integer value.</summary>
     /// <returns>The Absolute <see cref="Pos"/>.</returns>
     /// <param name="position">The value to convert to the <see cref="Pos"/>.</param>
     public static Pos Absolute (int position) { return new PosAbsolute (position); }
 
     /// <summary>
-    ///     Creates a <see cref="Pos"/> object that aligns a set of views according to the specified alignment setting.
+    ///     Creates a <see cref="Pos"/> object that aligns a set of views according to the specified <see cref="Alignment"/>
+    ///     and <see cref="AlignmentModes"/>.
     /// </summary>
     /// <param name="alignment">The alignment.</param>
     /// <param name="modes">The optional alignment modes.</param>
     /// <param name="groupId">
-    ///     The optional, unique identifier for the set of views to align according to
-    ///     <paramref name="alignment"/>.
+    ///     The optional identifier of a set of views that should be aligned together. When only a single
+    ///     set of views in a SuperView is aligned, this parameter is optional.
     /// </param>
     /// <returns></returns>
     public static Pos Align (Alignment alignment, AlignmentModes modes = AlignmentModes.StartToEnd | AlignmentModes.AddSpaceBetweenItems, int groupId = 0)
     {
-        return new PosAlign (alignment, modes, groupId);
+        return new PosAlign
+        {
+            Aligner = new ()
+            {
+                Alignment = alignment,
+                AlignmentModes = modes
+            },
+            GroupId = groupId
+        };
     }
 
     /// <summary>

+ 64 - 62
Terminal.Gui/View/Layout/PosAlign.cs

@@ -9,64 +9,75 @@ namespace Terminal.Gui;
 /// </summary>
 /// <remarks>
 ///     <para>
-///         The Group ID is used to identify a set of views that should be alignment together. When only a single
-///         set of views is aligned, setting the Group ID is not needed because it defaults to 0.
+///         Updating the properties of <see cref="Aligner"/> is supported, but will not automatically cause re-layout to
+///         happen. <see cref="View.LayoutSubviews"/>
+///         must be called on the SuperView.
 ///     </para>
 ///     <para>
-///         The first view added to the Superview with a given Group ID is used to determine the alignment of the group.
-///         The alignment is applied to all views with the same Group ID.
+///         Views that should be aligned together must have a distinct <see cref="GroupId"/>. When only a single
+///         set of views is aligned within a SuperView, setting <see cref="GroupId"/> is optional because it defaults to 0.
+///     </para>
+///     <para>
+///         The first view added to the Superview with a given <see cref="GroupId"/> is used to determine the alignment of
+///         the group.
+///         The alignment is applied to all views with the same <see cref="GroupId"/>.
 ///     </para>
 /// </remarks>
 public class PosAlign : Pos
 {
     /// <summary>
-    ///     The cached location. Used to store the calculated location to avoid recalculating it.
+    ///     The cached location. Used to store the calculated location to minimize recalculating it.
     /// </summary>
-    private int? _location;
+    private int? _cachedLocation;
 
     /// <summary>
     ///     Gets the identifier of a set of views that should be aligned together. When only a single
-    ///     set of views is aligned, setting the <see cref="_groupId"/> is not needed because it defaults to 0.
+    ///     set of views in a SuperView is aligned, setting <see cref="GroupId"/> is not needed because it defaults to 0.
     /// </summary>
-    private readonly int _groupId;
+    public int GroupId { get; init; }
 
-    public int GroupId
-    {
-        get => _groupId;
-        init => _groupId = value;
-    }
+    private readonly Aligner? _aligner;
 
     /// <summary>
     ///     Gets the alignment settings.
     /// </summary>
-    public Aligner Aligner { get; } = new ();
+    public required Aligner Aligner
+    {
+        get => _aligner!;
+        init
+        {
+            if (_aligner is { })
+            {
+                _aligner.PropertyChanged -= Aligner_PropertyChanged;
+            }
+
+            _aligner = value;
+            _aligner.PropertyChanged += Aligner_PropertyChanged;
+        }
+    }
 
     /// <summary>
     ///     Aligns the views in <paramref name="views"/> that have the same group ID as <paramref name="groupId"/>.
+    ///     Updates each view's cached _location.
     /// </summary>
     /// <param name="groupId"></param>
     /// <param name="views"></param>
     /// <param name="dimension"></param>
     /// <param name="size"></param>
-    private static void AlignGroup (int groupId, IList<View> views, Dimension dimension, int size)
+    private static void AlignAndUpdateGroup (int groupId, IList<View> views, Dimension dimension, int size)
     {
-        Aligner? firstInGroup = null;
         List<int> dimensionsList = new ();
 
+        // PERF: If this proves a perf issue, consider caching a ref to this list in each item
         List<View> viewsInGroup = views.Where (
                                                v =>
                                                {
-                                                   if (dimension == Dimension.Width && v.X is PosAlign alignX)
-                                                   {
-                                                       return alignX._groupId == groupId;
-                                                   }
-
-                                                   if (dimension == Dimension.Height && v.Y is PosAlign alignY)
-                                                   {
-                                                       return alignY._groupId == groupId;
-                                                   }
-
-                                                   return false;
+                                                   return dimension switch
+                                                          {
+                                                              Dimension.Width when v.X is PosAlign alignX => alignX.GroupId == groupId,
+                                                              Dimension.Height when v.Y is PosAlign alignY => alignY.GroupId == groupId,
+                                                              _ => false
+                                                          };
                                                })
                                        .ToList ();
 
@@ -75,13 +86,19 @@ public class PosAlign : Pos
             return;
         }
 
-        foreach (View view in viewsInGroup)
+        // PERF: We iterate over viewsInGroup multiple times here.
+
+        Aligner? firstInGroup = null;
+
+        // Update the dimensionList with the sizes of the views
+        for (var index = 0; index < viewsInGroup.Count; index++)
         {
+            View view = viewsInGroup [index];
             PosAlign? posAlign = dimension == Dimension.Width ? view.X as PosAlign : view.Y as PosAlign;
 
             if (posAlign is { })
             {
-                if (firstInGroup is null)
+                if (index == 0)
                 {
                     firstInGroup = posAlign.Aligner;
                 }
@@ -90,14 +107,13 @@ public class PosAlign : Pos
             }
         }
 
-        if (firstInGroup is null)
-        {
-            return;
-        }
+        // Update the first item in the group with the new container size.
+        firstInGroup!.ContainerSize = size;
 
-        firstInGroup.ContainerSize = size;
+        // Align
         int [] locations = firstInGroup.Align (dimensionsList.ToArray ());
 
+        // Update the cached location for each item
         for (var index = 0; index < viewsInGroup.Count; index++)
         {
             View view = viewsInGroup [index];
@@ -105,49 +121,35 @@ public class PosAlign : Pos
 
             if (align is { })
             {
-                align._location = locations [index];
+                align._cachedLocation = locations [index];
             }
         }
     }
 
-    /// <summary>
-    ///     Enables alignment of a set of views.
-    /// </summary>
-    /// <param name="alignment"></param>
-    /// <param name="modes"></param>
-    /// <param name="groupId">The unique identifier for the set of views to align according to <paramref name="alignment"/>.</param>
-    public PosAlign (Alignment alignment, AlignmentModes modes = AlignmentModes.StartToEnd | AlignmentModes.AddSpaceBetweenItems, int groupId = 0)
-    {
-        Aligner.Alignment = alignment;
-        Aligner.AlignmentModes = modes;
-        _groupId = groupId;
-        Aligner.PropertyChanged += Aligner_PropertyChanged;
-    }
-
-    private void Aligner_PropertyChanged (object? sender, PropertyChangedEventArgs e) { _location = null; }
+    private void Aligner_PropertyChanged (object? sender, PropertyChangedEventArgs e) { _cachedLocation = null; }
 
     /// <inheritdoc/>
     public override bool Equals (object? other)
     {
-        return other is PosAlign align &&
-               _groupId == align._groupId &&
-               _location == align._location &&
-               align.Aligner.Alignment == Aligner.Alignment;
+        return other is PosAlign align
+               && GroupId == align.GroupId
+               && align.Aligner.Alignment == Aligner.Alignment
+               && align.Aligner.AlignmentModes == Aligner.AlignmentModes;
     }
 
     /// <inheritdoc/>
-    public override int GetHashCode () { return HashCode.Combine (Aligner, _groupId); }
+    public override int GetHashCode () { return HashCode.Combine (Aligner, GroupId); }
 
     /// <inheritdoc/>
-    public override string ToString () { return $"Align(groupId={_groupId}, alignment={Aligner.Alignment})"; }
+    public override string ToString () { return $"Align(alignment={Aligner.Alignment},modes={Aligner.AlignmentModes},groupId={GroupId})"; }
 
-    internal override int GetAnchor (int width) { return _location ?? 0 - width; }
+    internal override int GetAnchor (int width) { return _cachedLocation ?? 0 - width; }
 
     internal override int Calculate (int superviewDimension, Dim dim, View us, Dimension dimension)
     {
-        if (_location.HasValue && Aligner.ContainerSize == superviewDimension)
+        if (_cachedLocation.HasValue && Aligner.ContainerSize == superviewDimension)
         {
-            return _location.Value;
+            return _cachedLocation.Value;
         }
 
         if (us?.SuperView is null)
@@ -155,11 +157,11 @@ public class PosAlign : Pos
             return 0;
         }
 
-        AlignGroup (_groupId, us.SuperView.Subviews, dimension, superviewDimension);
+        AlignAndUpdateGroup (GroupId, us.SuperView.Subviews, dimension, superviewDimension);
 
-        if (_location.HasValue)
+        if (_cachedLocation.HasValue)
         {
-            return _location.Value;
+            return _cachedLocation.Value;
         }
 
         return 0;

+ 84 - 77
UICatalog/Scenarios/PosAlignDemo.cs

@@ -63,24 +63,26 @@ public sealed class PosAlignDemo : Scenario
                                                    if (dimension == Dimension.Width)
                                                    {
                                                        _horizAligner.Alignment =
-                                                         (Alignment)Enum.Parse (typeof (Alignment), alignRadioGroup.RadioLabels [alignRadioGroup.SelectedItem]);
+                                                           (Alignment)Enum.Parse (
+                                                                                  typeof (Alignment),
+                                                                                  alignRadioGroup.RadioLabels [alignRadioGroup.SelectedItem]);
                                                        UpdatePosAlignObjects (appWindow, dimension, _horizAligner);
                                                    }
                                                    else
                                                    {
                                                        _vertAligner.Alignment =
-                                                         (Alignment)Enum.Parse (typeof (Alignment), alignRadioGroup.RadioLabels [alignRadioGroup.SelectedItem]);
+                                                           (Alignment)Enum.Parse (
+                                                                                  typeof (Alignment),
+                                                                                  alignRadioGroup.RadioLabels [alignRadioGroup.SelectedItem]);
                                                        UpdatePosAlignObjects (appWindow, dimension, _vertAligner);
                                                    }
-
                                                };
         appWindow.Add (alignRadioGroup);
 
-
         CheckBox ignoreFirstOrLast = new ()
         {
             ColorScheme = colorScheme,
-            Text = "IgnoreFirstOrLast",
+            Text = "IgnoreFirstOrLast"
         };
 
         if (dimension == Dimension.Width)
@@ -97,30 +99,30 @@ public sealed class PosAlignDemo : Scenario
         }
 
         ignoreFirstOrLast.Toggled += (s, e) =>
-                               {
-                                   if (dimension == Dimension.Width)
-                                   {
-                                       _horizAligner.AlignmentModes = e.NewValue is { } &&
-                                                                 e.NewValue.Value ?
-                                                                     _horizAligner.AlignmentModes | AlignmentModes.IgnoreFirstOrLast :
-                                                                     _horizAligner.AlignmentModes & ~AlignmentModes.IgnoreFirstOrLast;
-                                       UpdatePosAlignObjects (appWindow, dimension, _horizAligner);
-                                   }
-                                   else
-                                   {
-                                       _vertAligner.AlignmentModes = e.NewValue is { } &&
-                                                                 e.NewValue.Value ?
-                                                                     _vertAligner.AlignmentModes | AlignmentModes.IgnoreFirstOrLast :
-                                                                     _vertAligner.AlignmentModes & ~AlignmentModes.IgnoreFirstOrLast;
-                                       UpdatePosAlignObjects (appWindow, dimension, _vertAligner);
-                                   }
-                               };
+                                     {
+                                         if (dimension == Dimension.Width)
+                                         {
+                                             _horizAligner.AlignmentModes =
+                                                 e.NewValue is { } && e.NewValue.Value
+                                                     ? _horizAligner.AlignmentModes | AlignmentModes.IgnoreFirstOrLast
+                                                     : _horizAligner.AlignmentModes & ~AlignmentModes.IgnoreFirstOrLast;
+                                             UpdatePosAlignObjects (appWindow, dimension, _horizAligner);
+                                         }
+                                         else
+                                         {
+                                             _vertAligner.AlignmentModes =
+                                                 e.NewValue is { } && e.NewValue.Value
+                                                     ? _vertAligner.AlignmentModes | AlignmentModes.IgnoreFirstOrLast
+                                                     : _vertAligner.AlignmentModes & ~AlignmentModes.IgnoreFirstOrLast;
+                                             UpdatePosAlignObjects (appWindow, dimension, _vertAligner);
+                                         }
+                                     };
         appWindow.Add (ignoreFirstOrLast);
 
         CheckBox addSpacesBetweenItems = new ()
         {
             ColorScheme = colorScheme,
-            Text = "AddSpaceBetweenItems",
+            Text = "AddSpaceBetweenItems"
         };
 
         if (dimension == Dimension.Width)
@@ -137,25 +139,24 @@ public sealed class PosAlignDemo : Scenario
         }
 
         addSpacesBetweenItems.Toggled += (s, e) =>
-                             {
-                                 if (dimension == Dimension.Width)
-                                 {
-                                     _horizAligner.AlignmentModes = e.NewValue is { } &&
-                                                             e.NewValue.Value ?
-                                                                 _horizAligner.AlignmentModes | AlignmentModes.AddSpaceBetweenItems :
-                                                                 _horizAligner.AlignmentModes & ~AlignmentModes.AddSpaceBetweenItems;
-                                     UpdatePosAlignObjects (appWindow, dimension, _horizAligner);
-                                 }
-                                 else
-                                 {
-                                     _vertAligner.AlignmentModes = e.NewValue is { } &&
-                                                             e.NewValue.Value ?
-                                                                 _vertAligner.AlignmentModes | AlignmentModes.AddSpaceBetweenItems :
-                                                                 _vertAligner.AlignmentModes & ~AlignmentModes.AddSpaceBetweenItems;
-                                     UpdatePosAlignObjects (appWindow, dimension, _vertAligner);
-                                 }
-
-                             };
+                                         {
+                                             if (dimension == Dimension.Width)
+                                             {
+                                                 _horizAligner.AlignmentModes =
+                                                     e.NewValue is { } && e.NewValue.Value
+                                                         ? _horizAligner.AlignmentModes | AlignmentModes.AddSpaceBetweenItems
+                                                         : _horizAligner.AlignmentModes & ~AlignmentModes.AddSpaceBetweenItems;
+                                                 UpdatePosAlignObjects (appWindow, dimension, _horizAligner);
+                                             }
+                                             else
+                                             {
+                                                 _vertAligner.AlignmentModes =
+                                                     e.NewValue is { } && e.NewValue.Value
+                                                         ? _vertAligner.AlignmentModes | AlignmentModes.AddSpaceBetweenItems
+                                                         : _vertAligner.AlignmentModes & ~AlignmentModes.AddSpaceBetweenItems;
+                                                 UpdatePosAlignObjects (appWindow, dimension, _vertAligner);
+                                             }
+                                         };
 
         appWindow.Add (addSpacesBetweenItems);
 
@@ -194,14 +195,14 @@ public sealed class PosAlignDemo : Scenario
         List<Button> addedViews =
         [
             new ()
-                                                       {
-                                                           X = dimension == Dimension.Width ? Pos.Align (_horizAligner.Alignment) : Pos.Left (alignRadioGroup),
-                                                           Y = dimension == Dimension.Width ? Pos.Top(alignRadioGroup): Pos.Align (_vertAligner.Alignment),
-                                                           Text = NumberToWords.Convert (0)
-                                                       }
+            {
+                X = dimension == Dimension.Width ? Pos.Align (_horizAligner.Alignment) : Pos.Left (alignRadioGroup),
+                Y = dimension == Dimension.Width ? Pos.Top (alignRadioGroup) : Pos.Align (_vertAligner.Alignment),
+                Text = NumberToWords.Convert (0)
+            }
         ];
 
-        Buttons.NumericUpDown<int> addedViewsUpDown = new Buttons.NumericUpDown<int>
+        Buttons.NumericUpDown<int> addedViewsUpDown = new()
         {
             Width = 9,
             Title = "Added",
@@ -279,22 +280,22 @@ public sealed class PosAlignDemo : Scenario
                 // BUGBUG: of changes in the Pos object. See https://github.com/gui-cs/Terminal.Gui/issues/3485
                 if (dimension == Dimension.Width)
                 {
-                    PosAlign posAlign = view.X as PosAlign;
-                    view.X = new PosAlign (
-                                           aligner.Alignment,
-                                           aligner.AlignmentModes,
-                                           posAlign!.GroupId);
-                    view.Margin.Thickness = new (_leftMargin, 0, 0, 0);
+                    var posAlign = view.X as PosAlign;
 
+                    view.X = Pos.Align (
+                                        aligner.Alignment,
+                                        aligner.AlignmentModes,
+                                        posAlign!.GroupId);
+                    view.Margin.Thickness = new (_leftMargin, 0, 0, 0);
                 }
                 else
                 {
-                    PosAlign posAlign = view.Y as PosAlign;
-                    view.Y = new PosAlign (
-                                           aligner.Alignment,
-                                           aligner.AlignmentModes,
-                                           posAlign!.GroupId);
+                    var posAlign = view.Y as PosAlign;
 
+                    view.Y = Pos.Align (
+                                        aligner.Alignment,
+                                        aligner.AlignmentModes,
+                                        posAlign!.GroupId);
 
                     view.Margin.Thickness = new (0, _topMargin, 0, 0);
                 }
@@ -305,13 +306,13 @@ public sealed class PosAlignDemo : Scenario
     }
 
     /// <summary>
-    /// Creates a 3x3 grid of views with two GroupIds: One for aligning X and one for aligning Y.
-    /// Demonstrates using PosAlign to create a grid of views that flow.
+    ///     Creates a 3x3 grid of views with two GroupIds: One for aligning X and one for aligning Y.
+    ///     Demonstrates using PosAlign to create a grid of views that flow.
     /// </summary>
     /// <param name="appWindow"></param>
     private void Setup3By3Grid (View appWindow)
     {
-        var container = new FrameView ()
+        var container = new FrameView
         {
             Title = "3 by 3",
             X = Pos.AnchorEnd (),
@@ -319,26 +320,30 @@ public sealed class PosAlignDemo : Scenario
             Width = Dim.Percent (40),
             Height = Dim.Percent (40)
         };
-        container.Padding.Thickness = new Thickness (8, 1, 0, 0);
+        container.Padding.Thickness = new (8, 1, 0, 0);
         container.Padding.ColorScheme = Colors.ColorSchemes ["error"];
 
         Aligner widthAligner = new () { AlignmentModes = AlignmentModes.StartToEnd };
+
         RadioGroup widthAlignRadioGroup = new ()
         {
             RadioLabels = Enum.GetNames<Alignment> (),
             Orientation = Orientation.Horizontal,
-            X = Pos.Center (),
+            X = Pos.Center ()
         };
         container.Padding.Add (widthAlignRadioGroup);
 
         widthAlignRadioGroup.SelectedItemChanged += (sender, e) =>
-        {
-            widthAligner.Alignment =
-                (Alignment)Enum.Parse (typeof (Alignment), widthAlignRadioGroup.RadioLabels [widthAlignRadioGroup.SelectedItem]);
-            UpdatePosAlignObjects (container, Dimension.Width, widthAligner);
-        };
+                                                    {
+                                                        widthAligner.Alignment =
+                                                            (Alignment)Enum.Parse (
+                                                                                   typeof (Alignment),
+                                                                                   widthAlignRadioGroup.RadioLabels [widthAlignRadioGroup.SelectedItem]);
+                                                        UpdatePosAlignObjects (container, Dimension.Width, widthAligner);
+                                                    };
 
         Aligner heightAligner = new () { AlignmentModes = AlignmentModes.StartToEnd };
+
         RadioGroup heightAlignRadioGroup = new ()
         {
             RadioLabels = Enum.GetNames<Alignment> (),
@@ -348,11 +353,13 @@ public sealed class PosAlignDemo : Scenario
         container.Padding.Add (heightAlignRadioGroup);
 
         heightAlignRadioGroup.SelectedItemChanged += (sender, e) =>
-        {
-            heightAligner.Alignment =
-                (Alignment)Enum.Parse (typeof (Alignment), heightAlignRadioGroup.RadioLabels [heightAlignRadioGroup.SelectedItem]);
-            UpdatePosAlignObjects (container, Dimension.Height, heightAligner);
-        };
+                                                     {
+                                                         heightAligner.Alignment =
+                                                             (Alignment)Enum.Parse (
+                                                                                    typeof (Alignment),
+                                                                                    heightAlignRadioGroup.RadioLabels [heightAlignRadioGroup.SelectedItem]);
+                                                         UpdatePosAlignObjects (container, Dimension.Height, heightAligner);
+                                                     };
 
         for (var i = 0; i < 9; i++)
 
@@ -365,8 +372,8 @@ public sealed class PosAlignDemo : Scenario
                 Width = 5
             };
 
-            v.X = Pos.Align (widthAligner.Alignment, widthAligner.AlignmentModes, groupId: i / 3);
-            v.Y = Pos.Align (heightAligner.Alignment,heightAligner.AlignmentModes, groupId: i % 3 + 10);
+            v.X = Pos.Align (widthAligner.Alignment, widthAligner.AlignmentModes, i / 3);
+            v.Y = Pos.Align (heightAligner.Alignment, heightAligner.AlignmentModes, i % 3 + 10);
 
             container.Add (v);
         }

+ 56 - 13
UnitTests/View/Layout/Pos.AlignTests.cs

@@ -1,4 +1,6 @@
 
+using static Unix.Terminal.Delegates;
+
 namespace Terminal.Gui.PosDimTests;
 
 public class PosAlignTests ()
@@ -6,29 +8,70 @@ public class PosAlignTests ()
     [Fact]
     public void PosAlign_Constructor ()
     {
-        var posAlign = new PosAlign (Alignment.Fill);
+        var posAlign = new PosAlign ()
+        {
+            Aligner = new Aligner(),
+        };
         Assert.NotNull (posAlign);
     }
 
     [Theory]
-    [InlineData (Alignment.Start, Alignment.Start, true)]
-    [InlineData (Alignment.Center, Alignment.Center, true)]
-    [InlineData (Alignment.Start, Alignment.Center, false)]
-    [InlineData (Alignment.Center, Alignment.Start, false)]
-    public void PosAlign_Equals (Alignment align1, Alignment align2, bool expectedEquals)
+    [InlineData (Alignment.Start, Alignment.Start, AlignmentModes.AddSpaceBetweenItems, AlignmentModes.AddSpaceBetweenItems, true)]
+    [InlineData (Alignment.Center, Alignment.Center, AlignmentModes.AddSpaceBetweenItems, AlignmentModes.AddSpaceBetweenItems, true)]
+    [InlineData (Alignment.Start, Alignment.Center, AlignmentModes.AddSpaceBetweenItems, AlignmentModes.AddSpaceBetweenItems, false)]
+    [InlineData (Alignment.Center, Alignment.Start, AlignmentModes.AddSpaceBetweenItems, AlignmentModes.AddSpaceBetweenItems, false)]
+    [InlineData (Alignment.Start, Alignment.Start, AlignmentModes.StartToEnd, AlignmentModes.AddSpaceBetweenItems, false)]
+    public void PosAlign_Equals (Alignment align1, Alignment align2, AlignmentModes mode1, AlignmentModes mode2, bool expectedEquals)
     {
-        var posAlign1 = new PosAlign (align1);
-        var posAlign2 = new PosAlign (align2);
+        var posAlign1 = new PosAlign ()
+        {
+            Aligner = new Aligner ()
+            {
+                Alignment = align1,
+                AlignmentModes = mode1
+            }
+        };
+        var posAlign2 = new PosAlign ()
+        {
+            Aligner = new Aligner ()
+            {
+                Alignment = align2,
+                AlignmentModes = mode2
+            }
+        };
 
         Assert.Equal (expectedEquals, posAlign1.Equals (posAlign2));
         Assert.Equal (expectedEquals, posAlign2.Equals (posAlign1));
     }
 
+    [Fact]
+    public void PosAlign_Equals_CachedLocation_Not_Used ()
+    {
+        View superView = new ()
+        {
+            Width = 10,
+            Height = 25
+        };
+        View view = new ();
+        superView.Add (view);
+
+        var posAlign1 = Pos.Align (Alignment.Center, AlignmentModes.AddSpaceBetweenItems);
+        view.X = posAlign1;
+        var pos1 =  posAlign1.Calculate (10, Dim.Absolute(0)!, view, Dimension.Width);
+
+        var posAlign2 = Pos.Align (Alignment.Center, AlignmentModes.AddSpaceBetweenItems);
+        view.Y = posAlign2;
+        var pos2 = posAlign2.Calculate (25, Dim.Absolute (0)!, view, Dimension.Height);
+
+        Assert.NotEqual(pos1, pos2);
+        Assert.Equal (posAlign1, posAlign2);
+    }
+
     [Fact]
     public void PosAlign_ToString ()
     {
-        var posAlign = new PosAlign (Alignment.Fill);
-        var expectedString = "Align(groupId=0, alignment=Fill)";
+        var posAlign = Pos.Align (Alignment.Fill);
+        var expectedString = "Align(alignment=Fill,modes=AddSpaceBetweenItems,groupId=0)";
 
         Assert.Equal (expectedString, posAlign.ToString ());
     }
@@ -36,7 +79,7 @@ public class PosAlignTests ()
     [Fact]
     public void PosAlign_Anchor ()
     {
-        var posAlign = new PosAlign (Alignment.Start);
+        var posAlign = Pos.Align (Alignment.Start);
         var width = 50;
         var expectedAnchor = -width;
 
@@ -50,7 +93,7 @@ public class PosAlignTests ()
         Assert.IsType<PosAlign> (pos);
     }
 
-    // Tests that test Left alignment
+    // TODO: Test scenarios where views with matching GroupId's are added/removed from a Superview
 
-    // 
+    // TODO: Make AlignAndUpdateGroup internal and write low-level unit tests for it
 }