Fixed undrawn elements not updating their area correctly even if their children are drawn or accessed

(potentially not final)

Author: Ellpeck

CHANGELOG.md

@@ -11,6 +11,9 @@ Additions
 Improvements
 - Made the exception thrown after the element recursion limit is reached more helpful
 
+Fixes
+- Fixed undrawn elements not updating their area correctly even if their children are drawn or accessed
+
 ## 8.0.0
 
 ### MLEM

MLEM.Ui/Elements/Element.cs

@@ -556,6 +556,7 @@ protected IList<Element> SortedChildren {
         private bool treatSizeAsMaximum;
         private bool preventParentSpill;
         private int layoutRecursion;
+        private bool parentPotentiallyDirty = true;
 
         /// <summary>
         /// Creates a new element with the given anchor and size and sets up some default event reactions.
@@ -664,13 +665,22 @@ public void SetAreaDirty() {
             this.AreaDirty = true;
             this.System?.InvokeOnElementAreaDirty(this);
             this.Parent?.OnChildAreaDirty(this, false);
+            foreach (var child in this.Children)
+                child.OnParentAreaDirty(this, false);
         }
 
         /// <summary>
         /// Updates this element's <see cref="Area"/> and all of its <see cref="Children"/> by calling <see cref="ForceUpdateArea"/> if <see cref="AreaDirty"/> is true.
         /// </summary>
         /// <returns>Whether <see cref="AreaDirty"/> was true and <see cref="ForceUpdateArea"/> was called</returns>
         public bool UpdateAreaIfDirty() {
+            // ensure we are positioned correctly in our parent by updating its area if it is dirty.
+            // this ensures that area updates are correctly propagated even for elements that don't access their own areas while drawing or updating.
+            // if the parent's area was dirty and updated our area, our AreaDirty will have already been set to false before we reach the code below.
+            if (this.parentPotentiallyDirty) {
+                this.parentPotentiallyDirty = false;
+                this.Parent?.UpdateAreaIfDirty();
+            }
             if (this.AreaDirty) {
                 this.ForceUpdateArea();
                 return true;
@@ -688,6 +698,7 @@ public virtual void ForceUpdateArea() {
                 return;
             // if the parent's area is dirty, it would get updated anyway when querying its ChildPaddedArea,
             // which would cause our ForceUpdateArea code to be run twice, so we only update our parent instead
+            this.parentPotentiallyDirty = false;
             if (this.Parent != null && this.Parent.UpdateAreaIfDirty())
                 return;
             this.stopwatch.Restart();
@@ -1055,6 +1066,13 @@ protected virtual void OnChildAreaDirty(Element child, bool grandchild) {
             this.Parent?.OnChildAreaDirty(child, true);
         }
 
+        protected virtual void OnParentAreaDirty(Element parent, bool grandparent) {
+            if (!grandparent)
+                this.parentPotentiallyDirty = true;
+            foreach (var child in this.Children)
+                child.OnParentAreaDirty(parent, true);
+        }
+
         /// <summary>
         /// Transforms the given <paramref name="position"/> by the inverse of this element's <see cref="Transform"/> matrix.
         /// </summary>

MLEM.Ui/Elements/Group.cs

@@ -31,14 +31,5 @@ public Group(Anchor anchor, Vector2 size, bool setWidthBasedOnChildren, bool set
             base.CanBeSelected = false;
         }
 
-        /// <inheritdoc />
-        public override void Draw(GameTime time, SpriteBatch batch, float alpha, SpriteBatchContext context) {
-            // TODO since the group never accesses its own area when drawing, we have to update it manually :/
-            //  this is a little terrible but removing it causes a lot of issues when it comes to location updates for grouped elements
-            //  & this is a pretty good sign that we really need to streamline the element dirtying & updating order
-            this.UpdateAreaIfDirty();
-            base.Draw(time, batch, alpha, context);
-        }
-
     }
 }

MLEM.Ui/Elements/SquishingGroup.cs

@@ -30,20 +30,6 @@ public override void SetAreaAndUpdateChildren(RectangleF area) {
             }
         }
 
-        /// <inheritdoc />
-        public override void Draw(GameTime time, SpriteBatch batch, float alpha, SpriteBatchContext context) {
-            // be sure to update our area before drawing since our children may not have been squished yet
-            this.UpdateAreaIfDirty();
-            base.Draw(time, batch, alpha, context);
-        }
-
-        /// <inheritdoc />
-        public override Element GetElementUnderPos(Vector2 position) {
-            // be sure to update our area before since our children may not have been squished yet
-            this.UpdateAreaIfDirty();
-            return base.GetElementUnderPos(position);
-        }
-
         /// <inheritdoc />
         protected override void OnChildAreaDirty(Element child, bool grandchild) {
             base.OnChildAreaDirty(child, grandchild);

Tests/UiTests.cs

@@ -281,6 +281,22 @@ public void TestToString() {
         Assert.AreEqual(button.ToString(), "Test Button 0 @ Test Group 0 @ Panel Test");
     }
 
+    [Test]
+    public void TestGroupDirtyPropagation() {
+        var group = new Group(Anchor.BottomRight, Vector2.One) {SetWidthBasedOnChildren = true};
+        var btn = group.AddChild(new Button(Anchor.AutoInlineBottomIgnoreOverflow, new Vector2(12)));
+        var panel = group.AddChild(new Panel(Anchor.AutoRight, new Vector2(120, 175), Vector2.Zero, false, true) {PositionOffset = new Vector2(0, 1)});
+
+        this.AddAndUpdate(group, out _, out _);
+        Assert.AreEqual(btn.System.Viewport.Height - 175 - 1 - 12, btn.DisplayArea.Y);
+
+        // hiding the panel should update the button's display area,
+        // but due to elements that don't draw themselves previously not propagating updates correctly,
+        // this was not the case
+        panel.IsHidden = true;
+        Assert.AreEqual(btn.System.Viewport.Height - 12, btn.DisplayArea.Y);
+    }
+
     private void AddAndUpdate(Element element, out TimeSpan addTime, out TimeSpan updateTime) {
         foreach (var root in this.Game.UiSystem.GetRootElements())
             this.Game.UiSystem.Remove(root.Name);