revert experimental changes, #2 :(

Revert "potentially cleaner version of our experrimental changes"
This reverts commit 618584c.

Revert "combine panel changes with the "more robust" approach"
This reverts commit ef3bd12.

Author: Ellpeck

CHANGELOG.md

@@ -7,11 +7,9 @@ MLEM uses [semantic versioning](https://semver.org/). Potentially breaking chang
 Additions
 - Added UiSystem.OnElementAreaDirty and Element.OnAreaDirty events
 - Added Element.DebugName to allow describing ui elements in ToString
-- Added ILayoutItem.OnLayoutRecursionSettled which is called when recursive ui layouting has finished
 
 Improvements
 - Made the exception thrown after the element recursion limit is reached more helpful
-- **Implemented a more robust UiLayouter recursion tracking system**
 
 Fixes
 - Fixed undrawn elements not updating their area correctly even if their children are drawn or accessed

MLEM.Ui/Elements/Element.cs

@@ -555,7 +555,7 @@ protected IList<Element> SortedChildren {
         private bool treatSizeAsMinimum;
         private bool treatSizeAsMaximum;
         private bool preventParentSpill;
-        private (int, int) layoutRecursion;
+        private int layoutRecursion;
         private bool parentPotentiallyDirty;
 
         /// <summary>
@@ -1104,32 +1104,6 @@ public Vector2 TransformInverseAll(Vector2 position) {
             return this.TransformInverse(position);
         }
 
-        /// <summary>
-        /// A method called by <see cref="UiLayouter.Layout{T}"/> when a layout item's size is being recalculated based on its children.
-        /// </summary>
-        /// <param name="recursion">The current recursion depth.</param>
-        /// <param name="relevantChild">The child that triggered the layout recursion. May be <see langword="null"/> in case the source of the layout recursion is unknown.</param>
-        protected virtual void OnLayoutRecursion(int recursion, ILayoutItem relevantChild) {
-            this.System.Metrics.SummedRecursionDepth++;
-            if (recursion > this.System.Metrics.MaxRecursionDepth)
-                this.System.Metrics.MaxRecursionDepth = recursion;
-
-            if (recursion >= Element.RecursionLimit) {
-                var exceptionText = $"The area of {this} has recursively updated more often than the configured Element.RecursionLimit. This issue may occur due to this element or one of its children containing conflicting auto-sizing settings, a custom element setting its area dirty too frequently, or this element being part of a complex layout tree that should be split up into multiple groups.";
-                if (relevantChild != null)
-                    exceptionText += $" Does its child {relevantChild} contain any conflicting auto-sizing settings?";
-                throw new ArithmeticException(exceptionText);
-            }
-        }
-
-        /// <summary>
-        /// A method called by <see cref="UiLayouter.Layout{T}"/> when a layout item's size is being calculated, but recursive calculations have settled.
-        /// Also see <see cref="OnLayoutRecursion"/>, which is called for every recursive operation during element layouting.
-        /// </summary>
-        /// <param name="totalRecursion">The total reached recursion depth.</param>
-        /// <param name="elementInternal"><see langword="true"/> if the settled recursive operation was element-internal (ie related to properties like <see cref="SetWidthBasedOnChildren"/> and <see cref="SetHeightBasedOnChildren"/>); <see langword="false"/> if the settled recursive operation was related to recursively updated children or parents of this element.</param>
-        protected virtual void OnLayoutRecursionSettled(int totalRecursion, bool elementInternal) {}
-
         /// <summary>
         /// Called when this element is added to a <see cref="UiSystem"/> and, optionally, a given <see cref="RootElement"/>.
         /// This method is called in <see cref="AddChild{T}"/> for a parent whose <see cref="System"/> is set, as well as <see cref="UiSystem.Add"/>.
@@ -1157,11 +1131,16 @@ protected internal virtual void RemovedFromUi() {
         }
 
         void ILayoutItem.OnLayoutRecursion(int recursion, ILayoutItem relevantChild) {
-            this.OnLayoutRecursion(recursion, relevantChild);
-        }
+            this.System.Metrics.SummedRecursionDepth++;
+            if (recursion > this.System.Metrics.MaxRecursionDepth)
+                this.System.Metrics.MaxRecursionDepth = recursion;
 
-        void ILayoutItem.OnLayoutRecursionSettled(int totalRecursion, bool elementInternal) {
-            this.OnLayoutRecursionSettled(totalRecursion, elementInternal);
+            if (recursion >= Element.RecursionLimit) {
+                var exceptionText = $"The area of {this} has recursively updated more often than the configured Element.RecursionLimit. This issue may occur due to this element or one of its children containing conflicting auto-sizing settings, a custom element setting its area dirty too frequently, or this element being part of a complex layout tree that should be split up into multiple groups.";
+                if (relevantChild != null)
+                    exceptionText += $" Does its child {relevantChild} contain any conflicting auto-sizing settings?";
+                throw new ArithmeticException(exceptionText);
+            }
         }
 
         Vector2 ILayoutItem.CalcActualSize(RectangleF parentArea) {

MLEM.Ui/Elements/Panel.cs

@@ -172,10 +172,11 @@ public override void RemoveChildren(Func<Element, bool> condition = null) {
         }
 
         /// <inheritdoc />
-        protected override void OnLayoutRecursionSettled(int totalRecursion, bool elementInternal) {
-            base.OnLayoutRecursionSettled(totalRecursion, elementInternal);
-            if (!elementInternal)
-                this.scrollBarMaxHistory.Clear();
+        public override void Update(GameTime time) {
+            // reset the scroll bar's max history when an update happens, at which point we know that any scroll bar recursion has "settled"
+            // (this reset ensures that the max history is recursion-internal and old values aren't reused when elements get modified later)
+            this.scrollBarMaxHistory.Clear();
+            base.Update(time);
         }
 
         /// <inheritdoc />
@@ -335,13 +336,12 @@ protected virtual void ScrollSetup() {
             var scrollBarMax = Math.Max(0, (childrenHeight - this.ChildPaddedArea.Height) / this.Scale);
             // avoid an infinite show/hide oscillation that occurs while updating our area by simply using the maximum recent height in that case
             if (this.scrollBarMaxHistory.Count(v => v.Equals(scrollBarMax, Element.Epsilon)) >= 2)
-                scrollBarMax = Math.Max(scrollBarMax, this.scrollBarMaxHistory.Max());
+                scrollBarMax = this.scrollBarMaxHistory.Max();
             if (!this.ScrollBar.MaxValue.Equals(scrollBarMax, Element.Epsilon)) {
                 this.scrollBarMaxHistory.Add(scrollBarMax);
                 if (this.scrollBarMaxHistory.Count > 8)
                     this.scrollBarMaxHistory.RemoveAt(0);
 
-                var wasHidden = this.ScrollBar.IsHidden;
                 this.ScrollBar.MaxValue = scrollBarMax;
                 this.relevantChildrenDirty = true;
 
@@ -353,11 +353,6 @@ protected virtual void ScrollSetup() {
                     this.ChildPadding += new Padding(0, childOffsetDelta, 0, 0);
                 }
 
-                // if we know we'll have to update our area, do it now so that the UiLayouter recursion tracker counts this change as "recursion-internal"
-                // (we use our child padded area below when changing the scroll bar's scroller size, so force update before that!)
-                if (this.ScrollBar.IsHidden != wasHidden || !childOffsetDelta.Equals(0, Element.Epsilon))
-                    this.ForceUpdateArea();
-
                 // the scroller height has the same relation to the scroll bar height as the visible area has to the total height of the panel's content
                 var scrollerHeight = Math.Min(this.ChildPaddedArea.Height / childrenHeight / this.Scale, 1) * this.ScrollBar.Area.Height;
                 this.ScrollBar.ScrollerSize = new Vector2(this.ScrollerSize.Value.X, Math.Max(this.ScrollerSize.Value.Y, scrollerHeight));

MLEM.Ui/UiLayouter.cs

@@ -15,25 +15,20 @@ public static class UiLayouter {
         /// Lays out the given <paramref name="item"/> based on the information specified in its <see cref="ILayoutItem"/> interface.
         /// </summary>
         /// <param name="item">The item to lay out.</param>
-        /// <param name="recursionTracker">A reference to a field in the <paramref name="item"/> that is used by the layouter to track recursion counts across <see cref="Layout"/> calls.</param>
+        /// <param name="layoutRecursionTracker">A reference to a field in the <paramref name="item"/> that is used by the layouter to track recursion counts across <see cref="Layout"/> calls.</param>
         /// <param name="epsilon">An epsilon value used in layout item size, position and resulting area calculations to mitigate floating point rounding inaccuracies.</param>
         /// <typeparam name="T"></typeparam>
-        public static void Layout<T>(T item, ref (int Layer, int Depth) recursionTracker, float epsilon = 0) where T : class, ILayoutItem {
-            if (recursionTracker.Depth > 0)
-                item.OnLayoutRecursion(recursionTracker.Depth, null);
-            recursionTracker.Layer++;
-            recursionTracker.Depth++;
+        public static void Layout<T>(T item, ref int layoutRecursionTracker, float epsilon = 0) where T : class, ILayoutItem {
+            var initiatedLayouting = layoutRecursionTracker <= 0;
+            if (!initiatedLayouting)
+                item.OnLayoutRecursion(layoutRecursionTracker, null);
+            layoutRecursionTracker++;
 
             var internalRecursion = 0;
             UpdateDisplayArea();
-            item.OnLayoutRecursionSettled(internalRecursion, true);
 
-            recursionTracker.Layer--;
-            if (recursionTracker.Layer <= 0) {
-                item.OnLayoutRecursionSettled(recursionTracker.Depth, false);
-                recursionTracker.Layer = 0;
-                recursionTracker.Depth = 0;
-            }
+            if (initiatedLayouting)
+                layoutRecursionTracker = 0;
 
             void UpdateDisplayArea(Vector2? overrideSize = null) {
                 var parentArea = item.ParentArea;
@@ -381,10 +376,11 @@ public interface ILayoutItem {
         bool CanAutoAnchorsAttach { get; }
 
         /// <summary>
-        /// Calculates the actual total size that this element should take up.
+        /// Calculates the actual size that this layout item should take up, based on the area that its parent encompasses.
+        /// By default, this is based on the information specified in <see cref="System.Drawing.Size"/>'s documentation.
         /// </summary>
-        /// <param name="parentArea">This parent's area, or the ui system's viewport if it has no parent.</param>
-        /// <returns>The actual size of this element.</returns>
+        /// <param name="parentArea">This parent's area, or the ui system's viewport if it has no parent</param>
+        /// <returns>The actual size of this layout item, any scaling into account</returns>
         Vector2 CalcActualSize(RectangleF parentArea);
 
         /// <summary>
@@ -405,19 +401,10 @@ public interface ILayoutItem {
 
         /// <summary>
         /// A method called by <see cref="UiLayouter.Layout{T}"/> when a layout item's size is being recalculated based on its children.
-        /// Also see <see cref="OnLayoutRecursionSettled"/>, which is called after recursive operations during element layouting have completed.
         /// </summary>
         /// <param name="recursion">The current recursion depth.</param>
         /// <param name="relevantChild">The child that triggered the layout recursion. May be <see langword="null"/> in case the source of the layout recursion is unknown.</param>
         void OnLayoutRecursion(int recursion, ILayoutItem relevantChild);
 
-        /// <summary>
-        /// A method called by <see cref="UiLayouter.Layout{T}"/> when a layout item's size is being calculated, but recursive calculations have settled.
-        /// Also see <see cref="OnLayoutRecursion"/>, which is called for every recursive operation during element layouting.
-        /// </summary>
-        /// <param name="totalRecursion">The total reached recursion depth.</param>
-        /// <param name="elementInternal"><see langword="true"/> if the settled recursive operation was element-internal (ie related to properties like <see cref="SetWidthBasedOnChildren"/> and <see cref="SetHeightBasedOnChildren"/>); <see langword="false"/> if the settled recursive operation was related to recursively updated children or parents of this element.</param>
-        void OnLayoutRecursionSettled(int totalRecursion, bool elementInternal);
-
     }
 }