From b43390a07089d2ed7d61fbfbc48975841368273f Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Fri, 6 Feb 2026 12:33:47 -0700 Subject: [PATCH] Fixes #4677 - Refactors `DimAuto` for less coupling and improves performance (#4678) * Phase 5: Add IsFixed and RequiresTargetLayout categorization properties Co-authored-by: tig <585482+tig@users.noreply.github.com> * Further simplify DimAuto.Calculate using IsFixed property Co-authored-by: tig <585482+tig@users.noreply.github.com> * Refactor for-loops to foreach and clarify DimFill logic Refactored multiple for-loops iterating over view lists to use foreach loops for improved readability and reduced boilerplate. Removed unused variables such as viewsNeedingLayout and index counters. Clarified DimFill handling by continuing early if the DimFill is not valid or lacks a To property, reducing nesting and improving intent. Made minor formatting and code style improvements for consistency. * Refactor subview filtering and sizing logic Refactored repeated LINQ queries for subview filtering into reusable helper methods (`GetViewsThatMatch`, `GetViewsThatHavePos`, `GetViewsThatHaveDim`), reducing duplication and improving readability. Moved max content size calculations for various subview types into new helper methods (`GetMaxSizePos`, `GetMaxSizeDim`). Updated main logic to use these helpers. Adornment thickness calculation now uses a switch expression. These changes improve modularity and maintainability. * Refactor subview categorization for layout calculation Refactored layout calculation to use a single-pass CategorizeSubViews method, grouping subviews by relevant Pos/Dim types into a new CategorizedViews struct. This replaces multiple helper methods and reduces redundant iterations. Updated main logic to use these categorized lists, and unified size calculation helpers to further reduce code duplication. Improves performance and maintainability by consolidating subview processing and removing obsolete methods. * Revert perf POC commits and add missing overrides to Combine types Co-authored-by: tig <585482+tig@users.noreply.github.com> * Add helper methods and simplify DimAuto.Calculate with foreach loops Co-authored-by: tig <585482+tig@users.noreply.github.com> * Refactor layout calculation in DimAuto.cs Removed commented-out code and unnecessary list declarations to clean up the layout calculation logic. * removed old plan file * Code cleanup * Add performance analysis and improvement plan for DimAuto.Calculate Co-authored-by: tig <585482+tig@users.noreply.github.com> * Add DimAuto benchmarks and benchmark documentation Co-authored-by: tig <585482+tig@users.noreply.github.com> * Implement Phase 1 & 2 performance optimizations for DimAuto.Calculate Co-authored-by: tig <585482+tig@users.noreply.github.com> * Code cleanup * Delete plans/dimauto-perf-plan.md --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: tig <585482+tig@users.noreply.github.com> Co-authored-by: Tig --- Terminal.Gui/ViewBase/Layout/Dim.cs | 38 ++ Terminal.Gui/ViewBase/Layout/DimAbsolute.cs | 3 + Terminal.Gui/ViewBase/Layout/DimAuto.cs | 435 +++++++----------- Terminal.Gui/ViewBase/Layout/DimCombine.cs | 6 + Terminal.Gui/ViewBase/Layout/DimFunc.cs | 3 + Terminal.Gui/ViewBase/Layout/DimView.cs | 3 + Terminal.Gui/ViewBase/Layout/Pos.cs | 38 ++ Terminal.Gui/ViewBase/Layout/PosAbsolute.cs | 3 + Terminal.Gui/ViewBase/Layout/PosCombine.cs | 6 + Terminal.Gui/ViewBase/Layout/PosFunc.cs | 3 + Terminal.Gui/ViewBase/Layout/PosView.cs | 3 + Tests/Benchmarks/Layout/DimAutoBenchmark.cs | 188 ++++++++ Tests/Benchmarks/README.md | 95 ++++ .../Layout/CategorizationPropertiesTests.cs | 186 ++++++++ docfx/docs/dimauto.md | 12 + docfx/docs/layout.md | 2 + 16 files changed, 754 insertions(+), 270 deletions(-) create mode 100644 Tests/Benchmarks/Layout/DimAutoBenchmark.cs create mode 100644 Tests/Benchmarks/README.md create mode 100644 Tests/UnitTestsParallelizable/ViewBase/Layout/CategorizationPropertiesTests.cs diff --git a/Terminal.Gui/ViewBase/Layout/Dim.cs b/Terminal.Gui/ViewBase/Layout/Dim.cs index 39b720e18..c1994193e 100644 --- a/Terminal.Gui/ViewBase/Layout/Dim.cs +++ b/Terminal.Gui/ViewBase/Layout/Dim.cs @@ -485,6 +485,44 @@ public abstract record Dim : IEqualityOperators internal virtual int GetMinimumContribution (int location, int superviewContentSize, View us, Dimension dimension) => Calculate (location, superviewContentSize, us, dimension); + /// + /// Indicates whether this Dim has a fixed value that doesn't depend on layout calculations. + /// + /// + /// + /// This property is used by to identify dimensions that can be + /// determined without performing layout calculations on other views. + /// + /// + /// Fixed dimensions include and dimensions calculated by + /// that don't depend on other views' layouts. + /// + /// + /// + /// if this Dim has a fixed value independent of layout; + /// otherwise, . + /// + internal virtual bool IsFixed => false; + + /// + /// Indicates whether this Dim requires the target view to be laid out before it can be calculated. + /// + /// + /// + /// This property is used by to identify dimensions that depend on + /// another view's layout being completed first. + /// + /// + /// Dimensions that require target layout include which depends on + /// the target view's calculated size. + /// + /// + /// + /// if this Dim requires the target view's layout to be calculated first; + /// otherwise, . + /// + internal virtual bool RequiresTargetLayout => false; + #endregion virtual methods #region operators diff --git a/Terminal.Gui/ViewBase/Layout/DimAbsolute.cs b/Terminal.Gui/ViewBase/Layout/DimAbsolute.cs index 8cf1b17ff..c3be33aad 100644 --- a/Terminal.Gui/ViewBase/Layout/DimAbsolute.cs +++ b/Terminal.Gui/ViewBase/Layout/DimAbsolute.cs @@ -23,4 +23,7 @@ public record DimAbsolute (int Size) : Dim internal override int GetAnchor (int size) => Size; internal override int Calculate (int location, int superviewContentSize, View us, Dimension dimension) => Math.Max (GetAnchor (0), 0); + + /// + internal override bool IsFixed => true; } diff --git a/Terminal.Gui/ViewBase/Layout/DimAuto.cs b/Terminal.Gui/ViewBase/Layout/DimAuto.cs index 393da4cfd..038b3b144 100644 --- a/Terminal.Gui/ViewBase/Layout/DimAuto.cs +++ b/Terminal.Gui/ViewBase/Layout/DimAuto.cs @@ -35,6 +35,129 @@ public record DimAuto (Dim? MaximumContentDim, Dim? MinimumContentDim, DimAutoSt /// internal override int GetAnchor (int size) => 0; + /// + internal override bool IsFixed => true; + + /// + /// Holds categorized views for single-pass processing. + /// Phase 1 and 2 Performance Optimization: Reduces iterations and allocations. + /// + private readonly struct ViewCategories + { + public List NotDependent { get; init; } + public List Centered { get; init; } + public List Anchored { get; init; } + public List PosViewBased { get; init; } + public List DimViewBased { get; init; } + public List DimAutoBased { get; init; } + public List DimFillBased { get; init; } + public List AlignGroupIds { get; init; } + } + + /// + /// Categorizes views in a single pass to reduce iterations and allocations. + /// Phase 1 and 2 Performance Optimization. + /// + private static ViewCategories CategorizeViews (IList subViews, Dimension dimension, int superviewContentSize) + { + ViewCategories categories = new () + { + NotDependent = [], + Centered = [], + Anchored = [], + PosViewBased = [], + DimViewBased = [], + DimAutoBased = [], + DimFillBased = [], + AlignGroupIds = [] + }; + + HashSet seenAlignGroupIds = new (); + + foreach (View v in subViews) + { + Pos pos = dimension == Dimension.Width ? v.X : v.Y; + Dim dim = dimension == Dimension.Width ? v.Width : v.Height; + + // Check for not dependent views first (most common case) + if ((pos.IsFixed || dim.IsFixed) && !pos.DependsOnSuperViewContentSize && !dim.DependsOnSuperViewContentSize) + { + categories.NotDependent.Add (v); + } + + // Check for centered views + if (pos.Has (out _)) + { + categories.Centered.Add (v); + } + + // Check for anchored views + if (pos.Has (out _)) + { + categories.Anchored.Add (v); + } + + // Check for PosView based views + if (pos.Has (out _)) + { + categories.PosViewBased.Add (v); + } + + // Check for DimView based views + if (dim.Has (out _)) + { + categories.DimViewBased.Add (v); + } + + // Check for DimAuto based views + if (dim.Has (out _)) + { + categories.DimAutoBased.Add (v); + } + + // Check for DimFill based views that can contribute + if (dim.Has (out _) && dim.CanContributeToAutoSizing) + { + categories.DimFillBased.Add (v); + } + + // Collect align group IDs + if (!pos.Has (out PosAlign posAlign)) + { + continue; + } + + if (seenAlignGroupIds.Add (posAlign.GroupId)) + { + categories.AlignGroupIds.Add (posAlign.GroupId); + } + } + + return categories; + } + + /// + /// Calculates maximum size from a pre-categorized list of views. + /// Phase 1 and 2 Performance Optimization: Avoids redundant filtering. + /// + private static int CalculateMaxSizeFromList (List views, int max, Dimension dimension) + { + foreach (View v in views) + { + int newMax = dimension == Dimension.Width + ? v.Frame.X + v.Width.Calculate (0, max, v, dimension) + : v.Frame.Y + v.Height.Calculate (0, max, v, dimension); + + if (newMax > max) + { + max = newMax; + } + } + + return max; + } + + /// internal override int Calculate (int location, int superviewContentSize, View us, Dimension dimension) { var textSize = 0; @@ -46,8 +169,6 @@ public record DimAuto (Dim? MaximumContentDim, Dim? MinimumContentDim, DimAutoSt int screenX4 = dimension == Dimension.Width ? screenSize.Width * 4 : screenSize.Height * 4; int autoMax = MaximumContentDim?.GetAnchor (superviewContentSize) ?? screenX4; - //Debug.WriteLineIf (autoMin > autoMax, "MinimumContentDim must be less than or equal to MaximumContentDim."); - if (Style.FastHasFlags (DimAutoStyle.Text)) { if (dimension == Dimension.Width) @@ -83,8 +204,6 @@ public record DimAuto (Dim? MaximumContentDim, Dim? MinimumContentDim, DimAutoSt } } - List viewsNeedingLayout = []; - if (Style.FastHasFlags (DimAutoStyle.Content)) { maxCalculatedSize = textSize; @@ -96,51 +215,22 @@ public record DimAuto (Dim? MaximumContentDim, Dim? MinimumContentDim, DimAutoSt } else { - List includedSubViews = us.InternalSubViews.ToList (); - List notDependentSubViews; + // Single-pass categorization to reduce iterations and allocations + // Work directly with the collection to avoid unnecessary ToList() allocation - if (dimension == Dimension.Width) - { - notDependentSubViews = includedSubViews - .Where (v => - (v.X is PosAbsolute or PosFunc - || v.Width is DimAuto or DimAbsolute or DimFunc) // BUGBUG: We should use v.X.Has and v.Width.Has? - && !v.X.DependsOnSuperViewContentSize - && !v.Width.DependsOnSuperViewContentSize) - .ToList (); - } - else - { - notDependentSubViews = includedSubViews - .Where (v => - (v.Y is PosAbsolute or PosFunc - || v.Height is DimAuto or DimAbsolute or DimFunc) // BUGBUG: We should use v.Y.Has and v.Height.Has? - && !v.Y.DependsOnSuperViewContentSize - && !v.Height.DependsOnSuperViewContentSize) - .ToList (); - } + // Categorize views in a single pass + ViewCategories categories = CategorizeViews (us.InternalSubViews, dimension, superviewContentSize); - foreach (View notDependentSubView in notDependentSubViews) + // Process not-dependent views + foreach (View notDependentSubView in categories.NotDependent) { notDependentSubView.SetRelativeLayout (us.GetContentSize ()); - } - for (var i = 0; i < notDependentSubViews.Count; i++) - { - View v = notDependentSubViews [i]; - - var size = 0; - - if (dimension == Dimension.Width) - { - int width = v.Width.Calculate (0, superviewContentSize, v, dimension); - size = v.X.GetAnchor (0) + width; - } - else - { - int height = v.Height.Calculate (0, superviewContentSize, v, dimension); - size = v.Y.GetAnchor (0) + height; - } + int size = dimension == Dimension.Width + ? notDependentSubView.X.GetAnchor (0) + + notDependentSubView.Width.Calculate (0, superviewContentSize, notDependentSubView, dimension) + : notDependentSubView.Y.GetAnchor (0) + + notDependentSubView.Height.Calculate (0, superviewContentSize, notDependentSubView, dimension); if (size > maxCalculatedSize) { @@ -148,122 +238,38 @@ public record DimAuto (Dim? MaximumContentDim, Dim? MinimumContentDim, DimAutoSt } } - // ************** We now have some idea of `us.ContentSize` *************** - - #region Centered - - // [ ] PosCenter - Position is dependent `us.ContentSize` AND `subview.Frame` - List centeredSubViews; - - if (dimension == Dimension.Width) - { - centeredSubViews = us.InternalSubViews.Where (v => v.X.Has (out _)).ToList (); - } - else - { - centeredSubViews = us.InternalSubViews.Where (v => v.Y.Has (out _)).ToList (); - } - - viewsNeedingLayout.AddRange (centeredSubViews); - + // Process centered views var maxCentered = 0; - for (var i = 0; i < centeredSubViews.Count; i++) + foreach (View v in categories.Centered) { - View v = centeredSubViews [i]; - - if (dimension == Dimension.Width) - { - int width = v.Width.Calculate (0, screenX4, v, dimension); - maxCentered = v.X.GetAnchor (0) + width; - } - else - { - int height = v.Height.Calculate (0, screenX4, v, dimension); - maxCentered = v.Y.GetAnchor (0) + height; - } + maxCentered = dimension == Dimension.Width + ? v.X.GetAnchor (0) + v.Width.Calculate (0, screenX4, v, dimension) + : v.Y.GetAnchor (0) + v.Height.Calculate (0, screenX4, v, dimension); } maxCalculatedSize = int.Max (maxCalculatedSize, maxCentered); - #endregion Centered - - #region Percent - - // [ ] DimPercent - Dimension is dependent on `us.ContentSize` - // No need to do anything. - - #endregion Percent - - #region Aligned - - // [ ] PosAlign - Position is dependent on other views with `GroupId` AND `us.ContentSize` + // Process aligned views var maxAlign = 0; - // Use Linq to get a list of distinct GroupIds from the subviews - List groupIds = includedSubViews.Select (v => - { - return dimension switch - { - Dimension.Width when v.X.Has (out PosAlign posAlign) => posAlign.GroupId, - Dimension.Height when v.Y.Has (out PosAlign posAlign) => posAlign.GroupId, - _ => -1 - }; - }) - .Distinct () - .ToList (); - - foreach (int groupId in groupIds.Where (g => g != -1)) + foreach (int groupId in categories.AlignGroupIds) { - // PERF: If this proves a perf issue, consider caching a ref to this list in each item - List posAlignsInGroup = includedSubViews.Where (v => PosAlign.HasGroupId (v, dimension, groupId)) - .Select (v => dimension == Dimension.Width ? v.X as PosAlign : v.Y as PosAlign) - .ToList (); - - if (posAlignsInGroup.Count == 0) - { - continue; - } - - maxAlign = PosAlign.CalculateMinDimension (groupId, includedSubViews, dimension); + // Convert to IReadOnlyCollection for PosAlign API + maxAlign = PosAlign.CalculateMinDimension (groupId, us.InternalSubViews.ToArray (), dimension); } maxCalculatedSize = int.Max (maxCalculatedSize, maxAlign); - #endregion Aligned - - #region Anchored - - // [x] PosAnchorEnd - Position is dependent on `us.ContentSize` AND `subview.Frame` - List anchoredSubViews; - - if (dimension == Dimension.Width) - { - anchoredSubViews = includedSubViews.Where (v => v.X.Has (out _)).ToList (); - } - else - { - anchoredSubViews = includedSubViews.Where (v => v.Y.Has (out _)).ToList (); - } - - viewsNeedingLayout.AddRange (anchoredSubViews); - + // Process anchored views var maxAnchorEnd = 0; - for (var i = 0; i < anchoredSubViews.Count; i++) + foreach (View anchoredSubView in categories.Anchored) { - View anchoredSubView = anchoredSubViews [i]; - // Need to set the relative layout for PosAnchorEnd subviews to calculate the size - // TODO: Figure out a way to not have to calculate change the state of subviews (calling SRL). - if (dimension == Dimension.Width) - { - anchoredSubView.SetRelativeLayout (new Size (maxCalculatedSize, screenX4)); - } - else - { - anchoredSubView.SetRelativeLayout (new Size (screenX4, maxCalculatedSize)); - } + anchoredSubView.SetRelativeLayout (dimension == Dimension.Width + ? new Size (maxCalculatedSize, screenX4) + : new Size (screenX4, maxCalculatedSize)); maxAnchorEnd = dimension == Dimension.Width ? anchoredSubView.X.GetAnchor (maxCalculatedSize + anchoredSubView.Frame.Width) @@ -272,125 +278,14 @@ public record DimAuto (Dim? MaximumContentDim, Dim? MinimumContentDim, DimAutoSt maxCalculatedSize = Math.Max (maxCalculatedSize, maxAnchorEnd); - #endregion Anchored - - #region PosView - - // [x] PosView - Position is dependent on `subview.Target` - it can cause a change in `us.ContentSize` - List posViewSubViews; - - if (dimension == Dimension.Width) - { - posViewSubViews = includedSubViews.Where (v => v.X.Has (out _)).ToList (); - } - else - { - posViewSubViews = includedSubViews.Where (v => v.Y.Has (out _)).ToList (); - } - - for (var i = 0; i < posViewSubViews.Count; i++) - { - View v = posViewSubViews [i]; - - // BUGBUG: The order may not be correct. May need to call TopologicalSort? - // TODO: Figure out a way to not have to Calculate change the state of subviews (calling SRL). - int maxPosView = dimension == Dimension.Width - ? v.Frame.X + v.Width.Calculate (0, maxCalculatedSize, v, dimension) - : v.Frame.Y + v.Height.Calculate (0, maxCalculatedSize, v, dimension); - - if (maxPosView > maxCalculatedSize) - { - maxCalculatedSize = maxPosView; - } - } - - #endregion PosView - - // [x] PosCombine - Position is dependent if `Pos.Has ([one of the above]` - it can cause a change in `us.ContentSize` - - #region DimView - - // [x] DimView - Dimension is dependent on `subview.Target` - it can cause a change in `us.ContentSize` - List dimViewSubViews; - - if (dimension == Dimension.Width) - { - dimViewSubViews = includedSubViews.Where (v => v.Width.Has (out _)).ToList (); - } - else - { - dimViewSubViews = includedSubViews.Where (v => v.Height.Has (out _)).ToList (); - } - - for (var i = 0; i < dimViewSubViews.Count; i++) - { - View v = dimViewSubViews [i]; - - // BUGBUG: The order may not be correct. May need to call TopologicalSort? - // TODO: Figure out a way to not have to Calculate change the state of subviews (calling SRL). - int maxDimView = dimension == Dimension.Width - ? v.Frame.X + v.Width.Calculate (0, maxCalculatedSize, v, dimension) - : v.Frame.Y + v.Height.Calculate (0, maxCalculatedSize, v, dimension); - - if (maxDimView > maxCalculatedSize) - { - maxCalculatedSize = maxDimView; - } - } - - #endregion DimView - - #region DimAuto - - // [ ] DimAuto - Dimension is internally calculated - - List dimAutoSubViews; - - if (dimension == Dimension.Width) - { - dimAutoSubViews = includedSubViews.Where (v => v.Width.Has (out _)).ToList (); - } - else - { - dimAutoSubViews = includedSubViews.Where (v => v.Height.Has (out _)).ToList (); - } - - for (var i = 0; i < dimAutoSubViews.Count; i++) - { - View v = dimAutoSubViews [i]; - - int maxDimAuto = dimension == Dimension.Width - ? v.Frame.X + v.Width.Calculate (0, maxCalculatedSize, v, dimension) - : v.Frame.Y + v.Height.Calculate (0, maxCalculatedSize, v, dimension); - - if (maxDimAuto > maxCalculatedSize) - { - maxCalculatedSize = maxDimAuto; - } - } - - #endregion - - #region DimFill - - // DimFill subviews contribute to auto-sizing only if they have MinimumContentDim or To set - List contributingDimFillSubViews; - - if (dimension == Dimension.Width) - { - contributingDimFillSubViews = us.InternalSubViews.Where (v => v.Width.Has (out _) && v.Width.CanContributeToAutoSizing).ToList (); - } - else - { - contributingDimFillSubViews = us.InternalSubViews - .Where (v => v.Height.Has (out _) && v.Height.CanContributeToAutoSizing) - .ToList (); - } + // Process PosView, DimView, and DimAuto based views + maxCalculatedSize = CalculateMaxSizeFromList (categories.PosViewBased, maxCalculatedSize, dimension); + maxCalculatedSize = CalculateMaxSizeFromList (categories.DimViewBased, maxCalculatedSize, dimension); + maxCalculatedSize = CalculateMaxSizeFromList (categories.DimAutoBased, maxCalculatedSize, dimension); // Process DimFill views that can contribute - for (var i = 0; i < contributingDimFillSubViews.Count; i++) + foreach (View dimFillSubView in categories.DimFillBased) { - View dimFillSubView = contributingDimFillSubViews [i]; Dim dimFill = dimension == Dimension.Width ? dimFillSubView.Width : dimFillSubView.Height; // Get the minimum contribution from the Dim itself @@ -409,22 +304,22 @@ public record DimAuto (Dim? MaximumContentDim, Dim? MinimumContentDim, DimAutoSt } // Handle special case for DimFill with To (still needs type-specific logic) - if (dimFill is DimFill dimFillTyped && dimFillTyped.To is { }) + if (dimFill is not DimFill dimFillTyped || dimFillTyped.To is null) { - // The SuperView needs to be large enough to contain both the dimFillSubView and the To view - int dimFillPos = dimension == Dimension.Width ? dimFillSubView.Frame.X : dimFillSubView.Frame.Y; - int toViewPos = dimension == Dimension.Width ? dimFillTyped.To.Frame.X : dimFillTyped.To.Frame.Y; - int toViewSize = dimension == Dimension.Width ? dimFillTyped.To.Frame.Width : dimFillTyped.To.Frame.Height; - int totalSize = int.Max (dimFillPos, toViewPos + toViewSize); + continue; + } - if (totalSize > maxCalculatedSize) - { - maxCalculatedSize = totalSize; - } + // The SuperView needs to be large enough to contain both the dimFillSubView and the To view + int dimFillPos = dimension == Dimension.Width ? dimFillSubView.Frame.X : dimFillSubView.Frame.Y; + int toViewPos = dimension == Dimension.Width ? dimFillTyped.To.Frame.X : dimFillTyped.To.Frame.Y; + int toViewSize = dimension == Dimension.Width ? dimFillTyped.To.Frame.Width : dimFillTyped.To.Frame.Height; + int totalSizeTo = int.Max (dimFillPos, toViewPos + toViewSize); + + if (totalSizeTo > maxCalculatedSize) + { + maxCalculatedSize = totalSizeTo; } } - - #endregion } } diff --git a/Terminal.Gui/ViewBase/Layout/DimCombine.cs b/Terminal.Gui/ViewBase/Layout/DimCombine.cs index 73c9a5aa6..3295b92a9 100644 --- a/Terminal.Gui/ViewBase/Layout/DimCombine.cs +++ b/Terminal.Gui/ViewBase/Layout/DimCombine.cs @@ -117,6 +117,12 @@ public record DimCombine (AddOrSubtract Add, Dim Left, Dim Right) : Dim return newDimension; } + /// + internal override bool IsFixed => Left.IsFixed && Right.IsFixed; + + /// + internal override bool RequiresTargetLayout => Left.RequiresTargetLayout || Right.RequiresTargetLayout; + /// protected override bool HasInner (out TDim dim) => Left.Has (out dim) || Right.Has (out dim); } diff --git a/Terminal.Gui/ViewBase/Layout/DimFunc.cs b/Terminal.Gui/ViewBase/Layout/DimFunc.cs index 791d91b1c..1349562a9 100644 --- a/Terminal.Gui/ViewBase/Layout/DimFunc.cs +++ b/Terminal.Gui/ViewBase/Layout/DimFunc.cs @@ -35,4 +35,7 @@ public record DimFunc (Func Fn, View? View = null) : Dim yield return View; } } + + /// + internal override bool IsFixed => true; } diff --git a/Terminal.Gui/ViewBase/Layout/DimView.cs b/Terminal.Gui/ViewBase/Layout/DimView.cs index 71aa9cc1b..c4ee798e7 100644 --- a/Terminal.Gui/ViewBase/Layout/DimView.cs +++ b/Terminal.Gui/ViewBase/Layout/DimView.cs @@ -59,4 +59,7 @@ public record DimView : Dim yield return Target; } } + + /// + internal override bool RequiresTargetLayout => true; } diff --git a/Terminal.Gui/ViewBase/Layout/Pos.cs b/Terminal.Gui/ViewBase/Layout/Pos.cs index 270d10f31..de79dfe56 100644 --- a/Terminal.Gui/ViewBase/Layout/Pos.cs +++ b/Terminal.Gui/ViewBase/Layout/Pos.cs @@ -416,6 +416,44 @@ public abstract record Pos /// internal virtual bool DependsOnSuperViewContentSize => false; + /// + /// Indicates whether this Pos has a fixed value that doesn't depend on layout calculations. + /// + /// + /// + /// This property is used by to identify positions that can be + /// determined without performing layout calculations on other views. + /// + /// + /// Fixed positions include and positions calculated by + /// that don't depend on other views' layouts. + /// + /// + /// + /// if this Pos has a fixed value independent of layout; + /// otherwise, . + /// + internal virtual bool IsFixed => false; + + /// + /// Indicates whether this Pos requires the target view to be laid out before it can be calculated. + /// + /// + /// + /// This property is used by to identify positions that depend on + /// another view's layout being completed first. + /// + /// + /// Positions that require target layout include which depends on + /// the target view's calculated position. + /// + /// + /// + /// if this Pos requires the target view's layout to be calculated first; + /// otherwise, . + /// + internal virtual bool RequiresTargetLayout => false; + /// /// Indicates whether the specified type is in the hierarchy of this Pos object. /// diff --git a/Terminal.Gui/ViewBase/Layout/PosAbsolute.cs b/Terminal.Gui/ViewBase/Layout/PosAbsolute.cs index 4503c77a4..142eeba11 100644 --- a/Terminal.Gui/ViewBase/Layout/PosAbsolute.cs +++ b/Terminal.Gui/ViewBase/Layout/PosAbsolute.cs @@ -21,4 +21,7 @@ public record PosAbsolute (int Position) : Pos public override string ToString () => $"Absolute({Position})"; internal override int GetAnchor (int size) => Position; + + /// + internal override bool IsFixed => true; } diff --git a/Terminal.Gui/ViewBase/Layout/PosCombine.cs b/Terminal.Gui/ViewBase/Layout/PosCombine.cs index 395e8add8..c668567cb 100644 --- a/Terminal.Gui/ViewBase/Layout/PosCombine.cs +++ b/Terminal.Gui/ViewBase/Layout/PosCombine.cs @@ -86,6 +86,12 @@ public record PosCombine (AddOrSubtract Add, Pos Left, Pos Right) : Pos /// internal override bool DependsOnSuperViewContentSize => Left.DependsOnSuperViewContentSize || Right.DependsOnSuperViewContentSize; + /// + internal override bool IsFixed => Left.IsFixed && Right.IsFixed; + + /// + internal override bool RequiresTargetLayout => Left.RequiresTargetLayout || Right.RequiresTargetLayout; + /// protected override bool HasInner (out TPos pos) => Left.Has (out pos) || Right.Has (out pos); } diff --git a/Terminal.Gui/ViewBase/Layout/PosFunc.cs b/Terminal.Gui/ViewBase/Layout/PosFunc.cs index 6b536e0bb..3fc8ea552 100644 --- a/Terminal.Gui/ViewBase/Layout/PosFunc.cs +++ b/Terminal.Gui/ViewBase/Layout/PosFunc.cs @@ -34,4 +34,7 @@ public record PosFunc (Func Fn, View? View = null) : Pos yield return View; } } + + /// + internal override bool IsFixed => true; } diff --git a/Terminal.Gui/ViewBase/Layout/PosView.cs b/Terminal.Gui/ViewBase/Layout/PosView.cs index af94ee5df..7b36355c5 100644 --- a/Terminal.Gui/ViewBase/Layout/PosView.cs +++ b/Terminal.Gui/ViewBase/Layout/PosView.cs @@ -69,4 +69,7 @@ public record PosView : Pos { yield return Target; } + + /// + internal override bool RequiresTargetLayout => true; } diff --git a/Tests/Benchmarks/Layout/DimAutoBenchmark.cs b/Tests/Benchmarks/Layout/DimAutoBenchmark.cs new file mode 100644 index 000000000..83238a8ec --- /dev/null +++ b/Tests/Benchmarks/Layout/DimAutoBenchmark.cs @@ -0,0 +1,188 @@ +using BenchmarkDotNet.Attributes; +using Terminal.Gui.App; +using Terminal.Gui.ViewBase; +using Terminal.Gui.Views; + +namespace Terminal.Gui.Benchmarks.Layout; + +/// +/// Benchmarks for DimAuto performance testing. +/// Tests various scenarios to measure iteration overhead, allocation pressure, and overall execution time. +/// +[MemoryDiagnoser] +[BenchmarkCategory ("DimAuto")] +public class DimAutoBenchmark +{ + private View _simpleView = null!; + private View _complexView = null!; + private View _deeplyNestedView = null!; + + [GlobalSetup] + public void Setup () + { + // Initialize application context with ANSI driver for benchmarking + Application.Init (driverName: "ANSI"); + + // Simple scenario: Few subviews with basic positioning + _simpleView = CreateSimpleView (); + + // Complex scenario: Many subviews with mixed Pos/Dim types + _complexView = CreateComplexView (); + + // Deeply nested scenario: Nested views with DimAuto + _deeplyNestedView = CreateDeeplyNestedView (); + } + + [GlobalCleanup] + public void Cleanup () + { + Application.Shutdown (); + } + + /// + /// Benchmark for simple layout with 3 subviews using basic positioning. + /// + [Benchmark (Baseline = true)] + public void SimpleLayout () + { + _simpleView.SetNeedsLayout (); + _simpleView.Layout (); + } + + /// + /// Benchmark for complex layout with 20 subviews using mixed Pos/Dim types. + /// Tests iteration overhead and categorization performance. + /// + [Benchmark] + public void ComplexLayout () + { + _complexView.SetNeedsLayout (); + _complexView.Layout (); + } + + /// + /// Benchmark for deeply nested layout with DimAuto at multiple levels. + /// Tests recursive layout performance. + /// + [Benchmark] + public void DeeplyNestedLayout () + { + _deeplyNestedView.SetNeedsLayout (); + _deeplyNestedView.Layout (); + } + + private View CreateSimpleView () + { + var parent = new View + { + Width = Dim.Auto (), + Height = Dim.Auto () + }; + + parent.Add ( + new Label { X = 0, Y = 0, Text = "Label 1" }, + new Label { X = 0, Y = 1, Text = "Label 2" }, + new Button { X = 0, Y = 2, Text = "Button" } + ); + + return parent; + } + + private View CreateComplexView () + { + var parent = new View + { + Width = Dim.Auto (), + Height = Dim.Auto () + }; + + // Mix of different positioning types + parent.Add ( + // Absolute positioning + new Label { X = 0, Y = 0, Width = 20, Height = 1, Text = "Absolute" }, + + // DimAuto + new View + { + X = 0, Y = 1, Width = Dim.Auto (), Height = Dim.Auto () + }, + + // PosCenter + new Label { X = Pos.Center (), Y = 2, Width = 15, Height = 1, Text = "Centered" }, + + // PosPercent + new Label { X = Pos.Percent (25), Y = 3, Width = 15, Height = 1, Text = "25%" }, + + // DimFill + new View { X = 0, Y = 4, Width = Dim.Fill (), Height = 3 }, + + // PosAnchorEnd + new Label { X = Pos.AnchorEnd (10), Y = 5, Width = 8, Height = 1, Text = "Anchored" }, + + // PosAlign + new Label { X = Pos.Align (Alignment.End), Y = 6, Width = 10, Height = 1, Text = "Aligned" }, + + // Multiple views with DimFunc + new Label { X = 0, Y = 7, Width = Dim.Func ((Func)(_ => 20)), Height = 1, Text = "Func 1" }, + new Label { X = 0, Y = 8, Width = Dim.Func ((Func)(_ => 25)), Height = 1, Text = "Func 2" }, + new Label { X = 0, Y = 9, Width = Dim.Func ((Func)(_ => 30)), Height = 1, Text = "Func 3" }, + + // Multiple views with DimPercent + new View { X = 0, Y = 10, Width = Dim.Percent (50), Height = 1 }, + new View { X = 0, Y = 11, Width = Dim.Percent (75), Height = 1 }, + + // More absolute views + new Label { X = 0, Y = 14, Width = 18, Height = 1, Text = "Absolute 2" }, + new Label { X = 0, Y = 15, Width = 22, Height = 1, Text = "Absolute 3" }, + new Label { X = 0, Y = 16, Width = 16, Height = 1, Text = "Absolute 4" }, + + // DimFill with To + new View + { + X = 0, Y = 17, + Width = Dim.Fill (), Height = 1 + } + ); + + // Add nested view after creation to avoid Subviews indexing issues + var nestedView = (View)parent.InternalSubViews [1]; + nestedView.Add (new Label { X = 0, Y = 0, Text = "Nested Auto" }); + + return parent; + } + + private View CreateDeeplyNestedView () + { + var root = new View + { + Width = Dim.Auto (), + Height = Dim.Auto () + }; + + View currentParent = root; + + // Create 5 levels of nesting + for (var level = 0; level < 5; level++) + { + var container = new View + { + X = 0, + Y = level, + Width = Dim.Auto (), + Height = Dim.Auto () + }; + + // Add some content at each level + container.Add ( + new Label { X = 0, Y = 0, Text = $"Level {level} - Item 1" }, + new Label { X = 0, Y = 1, Text = $"Level {level} - Item 2" }, + new Button { X = 0, Y = 2, Text = $"Level {level} Button" } + ); + + currentParent.Add (container); + currentParent = container; + } + + return root; + } +} diff --git a/Tests/Benchmarks/README.md b/Tests/Benchmarks/README.md new file mode 100644 index 000000000..d64c1bdae --- /dev/null +++ b/Tests/Benchmarks/README.md @@ -0,0 +1,95 @@ +# Terminal.Gui Benchmarks + +This project contains performance benchmarks for Terminal.Gui using [BenchmarkDotNet](https://benchmarkdotnet.org/). + +## Running Benchmarks + +### Run All Benchmarks + +```bash +cd Tests/Benchmarks +dotnet run -c Release +``` + +### Run Specific Benchmark Category + +```bash +# Run only DimAuto benchmarks +dotnet run -c Release -- --filter '*DimAuto*' + +# Run only TextFormatter benchmarks +dotnet run -c Release -- --filter '*TextFormatter*' +``` + +### Run Specific Benchmark Method + +```bash +# Run only the ComplexLayout benchmark +dotnet run -c Release -- --filter '*DimAutoBenchmark.ComplexLayout*' +``` + +### Quick Run (Shorter but Less Accurate) + +For faster iteration during development: + +```bash +dotnet run -c Release -- --filter '*DimAuto*' -j short +``` + +### List Available Benchmarks + +```bash +dotnet run -c Release -- --list flat +``` + +## DimAuto Benchmarks + +The `DimAutoBenchmark` class tests layout performance with `Dim.Auto()` in various scenarios: + +- **SimpleLayout**: Baseline with 3 subviews using basic positioning +- **ComplexLayout**: 20 subviews with mixed Pos/Dim types (tests iteration overhead) +- **DeeplyNestedLayout**: 5 levels of nested views with DimAuto (tests recursive performance) + +### Example Output + +``` +BenchmarkDotNet v0.14.0, Windows 11 (10.0.22631.4602/23H2/2023Update/SunValley3) +Intel Core i7-9750H CPU 2.60GHz, 1 CPU, 12 logical and 6 physical cores +.NET SDK 10.0.102 + [Host] : .NET 10.0.1 (10.0.125.52708), X64 RyuJIT AVX2 + DefaultJob : .NET 10.0.1 (10.0.125.52708), X64 RyuJIT AVX2 + +| Method | Mean | Error | StdDev | Ratio | RatioSD | Gen0 | Allocated | Alloc Ratio | +|-------------------- |-----------:|----------:|----------:|------:|--------:|-------:|----------:|------------:| +| SimpleLayout | 5.234 μs | 0.0421 μs | 0.0394 μs | 1.00 | 0.01 | 0.3586 | 3.03 KB | 1.00 | +| ComplexLayout | 42.561 μs | 0.8234 μs | 0.7701 μs | 8.13 | 0.17 | 2.8076 | 23.45 KB | 7.74 | +| DeeplyNestedLayout | 25.123 μs | 0.4892 μs | 0.4577 μs | 4.80 | 0.10 | 1.7090 | 14.28 KB | 4.71 | +``` + +## Adding New Benchmarks + +1. Create a new class in an appropriate subdirectory (e.g., `Layout/`, `Text/`) +2. Add the `[MemoryDiagnoser]` attribute to measure allocations +3. Add `[BenchmarkCategory("CategoryName")]` to group related benchmarks +4. Mark baseline scenarios with `[Benchmark(Baseline = true)]` +5. Use `[GlobalSetup]` and `[GlobalCleanup]` for initialization/cleanup + +## Best Practices + +- Always run benchmarks in **Release** configuration +- Run multiple iterations for accurate results (default is better than `-j short`) +- Use `[ArgumentsSource]` for parametrized benchmarks +- Include baseline scenarios for comparison +- Document what each benchmark measures + +## Continuous Integration + +Benchmarks are not run automatically in CI. Run them locally when: +- Making performance-critical changes +- Implementing performance optimizations +- Before releasing a new version + +## Resources + +- [BenchmarkDotNet Documentation](https://benchmarkdotnet.org/) +- [Performance Analysis Plan](../../plans/dimauto-perf-plan.md) diff --git a/Tests/UnitTestsParallelizable/ViewBase/Layout/CategorizationPropertiesTests.cs b/Tests/UnitTestsParallelizable/ViewBase/Layout/CategorizationPropertiesTests.cs new file mode 100644 index 000000000..cfc6dce1f --- /dev/null +++ b/Tests/UnitTestsParallelizable/ViewBase/Layout/CategorizationPropertiesTests.cs @@ -0,0 +1,186 @@ +#nullable disable + +// Claude - Opus 4.5 + +namespace ViewBaseTests.Layout; + +/// +/// Tests for Phase 5 categorization properties: IsFixed and RequiresTargetLayout. +/// These properties help DimAuto categorize Pos/Dim types without type checking. +/// +public class CategorizationPropertiesTests +{ + #region IsFixed Tests - Dim + + [Fact] + public void DimAbsolute_IsFixed () + { + Dim dim = Dim.Absolute (42); + Assert.True (dim.IsFixed); + } + + [Fact] + public void DimFunc_IsFixed () + { + Dim dim = Dim.Func (_ => 25); + Assert.True (dim.IsFixed); + } + + [Fact] + public void DimAuto_IsFixed () + { + Dim dim = Dim.Auto (); + Assert.True (dim.IsFixed); + } + + [Fact] + public void DimPercent_IsNotFixed () + { + Dim dim = Dim.Percent (50); + Assert.False (dim.IsFixed); + } + + [Fact] + public void DimFill_IsNotFixed () + { + Dim dim = Dim.Fill (); + Assert.False (dim.IsFixed); + } + + [Fact] + public void DimView_IsNotFixed () + { + View view = new (); + Dim dim = Dim.Width (view); + Assert.False (dim.IsFixed); + } + + #endregion + + #region IsFixed Tests - Pos + + [Fact] + public void PosAbsolute_IsFixed () + { + Pos pos = Pos.Absolute (10); + Assert.True (pos.IsFixed); + } + + [Fact] + public void PosFunc_IsFixed () + { + Pos pos = Pos.Func (_ => 15); + Assert.True (pos.IsFixed); + } + + [Fact] + public void PosCenter_IsNotFixed () + { + Pos pos = Pos.Center (); + Assert.False (pos.IsFixed); + } + + [Fact] + public void PosPercent_IsNotFixed () + { + Pos pos = Pos.Percent (50); + Assert.False (pos.IsFixed); + } + + [Fact] + public void PosAnchorEnd_IsNotFixed () + { + Pos pos = Pos.AnchorEnd (); + Assert.False (pos.IsFixed); + } + + [Fact] + public void PosView_IsNotFixed () + { + View view = new (); + Pos pos = Pos.Left (view); + Assert.False (pos.IsFixed); + } + + #endregion + + #region RequiresTargetLayout Tests - Dim + + [Fact] + public void DimView_RequiresTargetLayout () + { + View view = new (); + Dim dim = Dim.Width (view); + Assert.True (dim.RequiresTargetLayout); + } + + [Fact] + public void DimAbsolute_DoesNotRequireTargetLayout () + { + Dim dim = Dim.Absolute (42); + Assert.False (dim.RequiresTargetLayout); + } + + [Fact] + public void DimFunc_DoesNotRequireTargetLayout () + { + Dim dim = Dim.Func (_ => 25); + Assert.False (dim.RequiresTargetLayout); + } + + [Fact] + public void DimPercent_DoesNotRequireTargetLayout () + { + Dim dim = Dim.Percent (50); + Assert.False (dim.RequiresTargetLayout); + } + + [Fact] + public void DimFill_DoesNotRequireTargetLayout () + { + Dim dim = Dim.Fill (); + Assert.False (dim.RequiresTargetLayout); + } + + #endregion + + #region RequiresTargetLayout Tests - Pos + + [Fact] + public void PosView_RequiresTargetLayout () + { + View view = new (); + Pos pos = Pos.Left (view); + Assert.True (pos.RequiresTargetLayout); + } + + [Fact] + public void PosAbsolute_DoesNotRequireTargetLayout () + { + Pos pos = Pos.Absolute (10); + Assert.False (pos.RequiresTargetLayout); + } + + [Fact] + public void PosFunc_DoesNotRequireTargetLayout () + { + Pos pos = Pos.Func (_ => 15); + Assert.False (pos.RequiresTargetLayout); + } + + [Fact] + public void PosCenter_DoesNotRequireTargetLayout () + { + Pos pos = Pos.Center (); + Assert.False (pos.RequiresTargetLayout); + } + + [Fact] + public void PosPercent_DoesNotRequireTargetLayout () + { + Pos pos = Pos.Percent (50); + Assert.False (pos.RequiresTargetLayout); + } + + #endregion +} diff --git a/docfx/docs/dimauto.md b/docfx/docs/dimauto.md index 30b8f110d..b5e0dd968 100644 --- a/docfx/docs/dimauto.md +++ b/docfx/docs/dimauto.md @@ -347,3 +347,15 @@ If you encounter unexpected sizing with `Dim.Auto`, consider the following debug - **Inspect Text Formatting**: For `Text` style, check `TextFormatter` settings and constraints (`ConstrainToWidth`, `ConstrainToHeight`). Ensure text is formatted correctly before sizing calculations. By understanding the intricacies of `Dim.Auto` as implemented in Terminal.Gui v2, developers can create responsive and adaptive terminal UIs that automatically adjust to content changes, enhancing user experience and maintainability. + +## Internal Architecture + +`Dim.Auto` uses a polymorphic design to minimize coupling with specific `Pos` and `Dim` types. The layout system uses virtual properties and methods to categorize and process layout elements: + +- **`DependsOnSuperViewContentSize`**: Identifies types that actively contribute to content size determination (e.g., `DimPercent`, `DimFill`, `PosAnchorEnd`, `PosAlign`) +- **`CanContributeToAutoSizing`**: Indicates whether a `Dim` can meaningfully contribute to auto-sizing (returns `false` for `DimPercent` and `DimFill` without `MinimumContentDim`/`To`) +- **`GetMinimumContribution()`**: Calculates the minimum size contribution during auto-sizing (overridden by `DimFill` to return its `MinimumContentDim`) +- **`IsFixed`**: Identifies fixed-value types that don't depend on layout calculations (`DimAbsolute`, `PosAbsolute`, `DimFunc`, `PosFunc`, `DimAuto`) +- **`RequiresTargetLayout`**: Indicates types requiring target view layout first (`DimView`, `PosView`) + +This design allows new `Pos`/`Dim` types to be added without modifying `DimAuto.Calculate()`. diff --git a/docfx/docs/layout.md b/docfx/docs/layout.md index cbcaad450..4b76c3538 100644 --- a/docfx/docs/layout.md +++ b/docfx/docs/layout.md @@ -95,6 +95,8 @@ The flags are organized into categories: Terminal.Gui provides a rich system for how views are laid out relative to each other. The position of a view is set by setting the `X` and `Y` properties, which are of time @Terminal.Gui.Pos. The size is set via `Width` and `Height`, which are of type @Terminal.Gui.Dim. +The layout system uses virtual properties for categorization without type checking: `ReferencesOtherViews()`, `DependsOnSuperViewContentSize`, `CanContributeToAutoSizing`, `GetMinimumContribution()`, `IsFixed`, and `RequiresTargetLayout`. This enables extensibility. + ```cs var label1 = new Label () { X = 1, Y = 2, Width = 3, Height = 4, Title = "Absolute")