Fix GeometryGroup.Children.Clear() leaking shared child geometries

src/Controls/src/Core/Shapes/GeometryGroup.cs

@@ -1,5 +1,6 @@
 #nullable disable
 using System;
+using System.Collections.Generic;
 using System.Collections.Specialized;
 using System.ComponentModel;
 
@@ -11,6 +12,8 @@ namespace Microsoft.Maui.Controls.Shapes
 	[ContentProperty("Children")]
 	public class GeometryGroup : Geometry
 	{
+		readonly Dictionary<Geometry, int> _subscriptionRefCounts = new();
+
 		/// <summary>Bindable property for <see cref="Children"/>.</summary>
 		public static readonly BindableProperty ChildrenProperty =
 			BindableProperty.Create(nameof(Children), typeof(GeometryCollection), typeof(GeometryGroup), null,
@@ -55,52 +58,146 @@ public FillRule FillRule
 
 		void UpdateChildren(GeometryCollection oldCollection, GeometryCollection newCollection)
 		{
-			if (oldCollection != null)
-			{
-				oldCollection.CollectionChanged -= OnChildrenCollectionChanged;
+			DetachCollection(oldCollection);
+			AttachCollection(newCollection);
+
+			Invalidate();
+		}
 
-				foreach (var oldChildren in oldCollection)
-				{
-					oldChildren.PropertyChanged -= OnChildrenPropertyChanged;
-				}
+		void AttachCollection(GeometryCollection collection)
+		{
+			if (collection == null)
+				return;
+
+			collection.CollectionChanged += OnChildrenCollectionChanged;
+
+			foreach (var geometry in collection)
+			{
+				SubscribeToGeometry(geometry);
 			}
+		}
 
-			if (newCollection == null)
+		void DetachCollection(GeometryCollection collection)
+		{
+			if (collection == null)
 				return;
 
-			newCollection.CollectionChanged += OnChildrenCollectionChanged;
+			collection.CollectionChanged -= OnChildrenCollectionChanged;
 
-			foreach (var newChildren in newCollection)
+			foreach (var geometry in collection)
 			{
-				newChildren.PropertyChanged += OnChildrenPropertyChanged;
+				UnsubscribeFromGeometry(geometry);
 			}
 		}
 
 		void OnChildrenCollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
 		{
-			if (e.OldItems != null)
+			switch (e.Action)
 			{
-				foreach (var oldItem in e.OldItems)
-				{
-					if (!(oldItem is Geometry oldGeometry))
-						continue;
+				case NotifyCollectionChangedAction.Add:
+					if (e.NewItems != null)
+					{
+						foreach (Geometry geometry in e.NewItems)
+						{
+							SubscribeToGeometry(geometry);
+						}
+					}
+					break;
+
+				case NotifyCollectionChangedAction.Remove:
+					if (e.OldItems != null)
+					{
+						foreach (Geometry geometry in e.OldItems)
+						{
+							UnsubscribeFromGeometry(geometry);
+						}
+					}
+					break;
+
+				case NotifyCollectionChangedAction.Replace:
+					if (e.OldItems != null)
+					{
+						foreach (Geometry geometry in e.OldItems)
+						{
+							UnsubscribeFromGeometry(geometry);
+						}
+					}
+
+					if (e.NewItems != null)
+					{
+						foreach (Geometry geometry in e.NewItems)
+						{
+							SubscribeToGeometry(geometry);
+						}
+					}
+					break;
+
+				case NotifyCollectionChangedAction.Move:
+					// No subscription changes required.
+					break;
+
+				case NotifyCollectionChangedAction.Reset:
+					ResubscribeCollection(sender as GeometryCollection);
+					break;
+			}
+
+			Invalidate();
+		}
 
-					oldGeometry.PropertyChanged -= OnChildrenPropertyChanged;
-				}
+		void ResubscribeCollection(GeometryCollection collection)
+		{
+			UnsubscribeFromAllChildren();
+
+			if (collection == null)
+				return;
+
+			foreach (var geometry in collection)
+			{
+				SubscribeToGeometry(geometry);
 			}
+		}
 
-			if (e.NewItems != null)
+		void SubscribeToGeometry(Geometry geometry)
+		{
+			if (geometry == null)
+				return;
+
+			if (_subscriptionRefCounts.TryGetValue(geometry, out var count))
 			{
-				foreach (var newItem in e.NewItems)
-				{
-					if (!(newItem is Geometry newGeometry))
-						continue;
+				_subscriptionRefCounts[geometry] = count + 1;
+				return;
+			}
 
-					newGeometry.PropertyChanged += OnChildrenPropertyChanged;
-				}
+			_subscriptionRefCounts[geometry] = 1;
+			geometry.PropertyChanged += OnChildrenPropertyChanged;
+		}
+
+		void UnsubscribeFromGeometry(Geometry geometry)
+		{
+			if (geometry == null)
+				return;
+
+			if (!_subscriptionRefCounts.TryGetValue(geometry, out var count))
+				return;
+
+			if (count > 1)
+			{
+				_subscriptionRefCounts[geometry] = count - 1;
+				return;
 			}
 
-			Invalidate();
+			_subscriptionRefCounts.Remove(geometry);
+			geometry.PropertyChanged -= OnChildrenPropertyChanged;
+		}
+
+		void UnsubscribeFromAllChildren()
+		{
+			foreach (var geometry in _subscriptionRefCounts.Keys)
+			{
+				geometry.PropertyChanged -= OnChildrenPropertyChanged;
+			}
+
+			_subscriptionRefCounts.Clear();
 		}
 
 		void OnChildrenPropertyChanged(object sender, PropertyChangedEventArgs e)

src/Controls/tests/Core.UnitTests/GeometryGroupTests.cs

@@ -0,0 +1,35 @@
+using Microsoft.Maui.Controls.Shapes;
+using Rect = Microsoft.Maui.Graphics.Rect;
+using Xunit;
+
+namespace Microsoft.Maui.Controls.Core.UnitTests;
+
+public class GeometryGroupTests : BaseTestFixture
+{
+	[Fact]
+	public void ClearUnsubscribesPreviousChildrenFromPropertyChanged()
+	{
+		var group = new GeometryGroup();
+		var oldChild = new RectangleGeometry(new Rect(0, 0, 10, 10));
+		var newChild = new RectangleGeometry(new Rect(0, 0, 5, 5));
+		var invalidations = 0;
+
+		group.InvalidateGeometryRequested += (_, _) => invalidations++;
+
+		group.Children.Add(oldChild);
+		invalidations = 0;
+
+		group.Children.Clear();
+		invalidations = 0;
+
+		// If Reset handling does not unsubscribe old items, this mutation incorrectly invalidates the group.
+		oldChild.Rect = new Rect(1, 1, 11, 11);
+		Assert.Equal(0, invalidations);
+
+		group.Children.Add(newChild);
+		invalidations = 0;
+
+		newChild.Rect = new Rect(2, 2, 6, 6);
+		Assert.Equal(1, invalidations);
+	}
+}