fixed some memory management issues in MLEM.Ui

CHANGELOG.md

@@ -68,6 +68,10 @@ Fixes
 - Fixed an exception when trying to force-update the area of an element without a ui system
 - Fixed the scroll bar of an empty panel being positioned incorrectly
 - Fixed UiControls maintaining old input states when input types are toggled off
+- Fixed an occasional deadlock when a game is disposed with a scrolling Panel present
+
+Removals
+- Marked Element.OnDisposed as obsolete in favor of the more predictable OnRemovedFromUi
 
 ### MLEM.Data
 Additions

MLEM.Ui/Elements/Element.cs

@@ -425,9 +425,10 @@ namespace MLEM.Ui.Elements {
         /// </summary>
         public GenericCallback OnRemovedFromUi;
         /// <summary>
-        /// Event that is called when this element's <see cref="Dispose"/> method is called, which also happens in <see cref="Finalize"/>.
+        /// Event that is called when this element's <see cref="Dispose"/> method is called.
         /// This event is useful for unregistering global event handlers when this object should be destroyed.
         /// </summary>
+        [Obsolete("OnDisposed will be removed in a future update. To unregister custom event handlers, use OnRemovedFromUi instead.")]
         public GenericCallback OnDisposed;
 
         /// <summary>
@@ -488,6 +489,7 @@ namespace MLEM.Ui.Elements {
         }
 
         /// <inheritdoc />
+        [Obsolete("Dispose will be removed in a future update. To unregister custom event handlers, use OnRemovedFromUi instead.")]
         ~Element() {
             this.Dispose();
         }
@@ -1098,6 +1100,7 @@ namespace MLEM.Ui.Elements {
         }
 
         /// <summary>Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources.</summary>
+        [Obsolete("Dispose will be removed in a future update. To unregister custom event handlers, use OnRemovedFromUi instead.")]
         public virtual void Dispose() {
             this.OnDisposed?.Invoke(this);
             GC.SuppressFinalize(this);

MLEM.Ui/Elements/Panel.cs

@@ -70,6 +70,13 @@ namespace MLEM.Ui.Elements {
             this.scrollOverflow = scrollOverflow;
             this.CanBeSelected = false;
 
+            // we dispose our render target when removing so that it doesn't cause a memory leak
+            // if we're added back afterwards, it'll be recreated in ScrollSetup anyway
+            this.OnRemovedFromUi += _ => {
+                this.renderTarget?.Dispose();
+                this.renderTarget = null;
+            };
+
             if (scrollOverflow) {
                 this.ScrollBar = new ScrollBar(Anchor.TopRight, Vector2.Zero, 0, 0) {
                     OnValueChanged = (element, value) => this.ScrollChildren(),
@@ -177,15 +184,6 @@ namespace MLEM.Ui.Elements {
             return base.GetElementUnderPos(position);
         }
 
-        /// <inheritdoc />
-        public override void Dispose() {
-            if (this.renderTarget != null) {
-                this.renderTarget.Dispose();
-                this.renderTarget = null;
-            }
-            base.Dispose();
-        }
-
         /// <summary>
         /// Scrolls this panel's <see cref="ScrollBar"/> to the given <see cref="Element"/> in such a way that its center is positioned in the center of this panel.
         /// </summary>
@@ -274,11 +272,13 @@ namespace MLEM.Ui.Elements {
 
             // update the render target
             var targetArea = (Rectangle) this.GetRenderTargetArea();
-            if (targetArea.Width <= 0 || targetArea.Height <= 0)
+            if (targetArea.Width <= 0 || targetArea.Height <= 0) {
+                this.renderTarget?.Dispose();
+                this.renderTarget = null;
                 return;
+            }
             if (this.renderTarget == null || targetArea.Width != this.renderTarget.Width || targetArea.Height != this.renderTarget.Height) {
-                if (this.renderTarget != null)
+                this.renderTarget?.Dispose();
-                    this.renderTarget.Dispose();
                 this.renderTarget = targetArea.IsEmpty ? null : new RenderTarget2D(this.System.Game.GraphicsDevice, targetArea.Width, targetArea.Height);
                 this.relevantChildrenDirty = true;
             }

MLEM.Ui/Parsers/UiParser.cs

@@ -27,7 +27,7 @@ namespace MLEM.Ui.Parsers {
         public static readonly ElementType[] ElementTypes =
             #if NET6_0_OR_GREATER
             Enum.GetValues<ElementType>();
-        #else
+            #else
             (ElementType[]) Enum.GetValues(typeof(ElementType));
         #endif
 
@@ -145,9 +145,15 @@ namespace MLEM.Ui.Parsers {
                 throw new NullReferenceException("A UI parser requires a GraphicsDevice for parsing images");
 
             TextureRegion image = null;
-            LoadImageAsync();
             return new Image(Anchor.AutoLeft, new Vector2(1, -1), _ => image) {
-                OnDisposed = e => image?.Texture.Dispose()
+                OnAddedToUi = e => {
+                    if (image == null)
+                        LoadImageAsync();
+                },
+                OnRemovedFromUi = e => {
+                    image?.Texture.Dispose();
+                    image = null;
+                }
             };
 
             async void LoadImageAsync() {

MLEM/Graphics/StaticSpriteBatch.cs

@@ -411,7 +411,6 @@ namespace MLEM.Graphics {
             this.indices?.Dispose();
             foreach (var buffer in this.vertexBuffers)
                 buffer.Dispose();
-            GC.SuppressFinalize(this);
         }
 
         private Item Add(Texture2D texture, Vector2 pos, Vector2 offset, Vector2 size, float sin, float cos, Color color, Vector2 texTl, Vector2 texBr, float depth) {