[Android] Map: Stop reloading a pin's icon on every cluster pass (#37774)

Fixes #37773

### Description of Change

`ReclusterPins` re-runs `AddPins` on every zoom change, and
`AddPinAsync` reloaded `Pin.ImageSource` unconditionally each time — a
Glide fetch plus a decode and rescale for every un-clustered pin, on
every pass. The `MarkerOptions` it writes the icon to already outlive
the markers built from them, so the work was redundant as soon as the
source had not changed.

`MapPinHandler` now records which image source produced the icon
currently on its `MarkerOptions`, and `AddPinAsync` skips the load while
it still matches.

The key comes from `GetClusterIconCacheKey`, the helper the cluster icon
cache already uses, rather than a reference comparison. That matters
twice:

- it compares by **value**, so a new but equivalent `ImageSource`
instance is recognised instead of forcing a reload;
- it returns `null` for a source that opted out of caching — a
`UriImageSource` with `CachingEnabled` false, or a non-positive
`CacheValidity` — and those always reload rather than being frozen for
the handler's lifetime. The helper's own comment states that rule for
the cluster cache; this reuses it rather than restating it.

The record is invalidated by the `ImageSource` mapper, which is the one
hook that always runs when the icon on the `MarkerOptions` goes stale —
on every `SetVirtualView`, including a handler reconnected to a
different pin, and on every `ImageSource` change.

The source is captured before the await **and re-checked after it**
before the icon and the key are written: `UpdatePinImageSourceAsync`
guards its own write the same way, and without the symmetric guard the
two writers race and a marker can be published with the older icon while
the change notification that would correct it has already been consumed.
`UpdatePinImageSourceAsync` writes the icon to the `MarkerOptions` as
well as the live marker and updates the key, reaching the platform
element through the non-throwing accessor since it resumes after an
await.

A cleared `ImageSource` now takes the previous icon off the reused
`MarkerOptions` (`SetIcon(null)`, Android's "use the default marker")
rather than leaving the pin rendering an image its source no longer
names.

Skipping the load also removes the await before `Map.AddMarker` on the
common path, so an un-clustered pin with an unchanged icon is added
synchronously instead of disappearing until its image resolves.

### Note on ordering

Split out of #37769 so the bug fix and this caching behaviour stay
separately reviewable. It applies independently, but #37769 is the one
that fixes broken behaviour — merge that first.

### Verification

Physical device (Samsung SM-X230, Android 15), clustering gallery using
pins whose icon is a PNG written to `FileSystem.CacheDirectory`, handler
instrumented at each boundary. Three zoom-in/zoom-out cycles over 25
clustered custom pins, merged with #37769: **19 image loads for 86
markers** instead of one load per marker, every de-clustered pin keeping
its icon, no Glide asserts, no recycled-bitmap errors, no crashes.
Re-measured after adding the mapper-based invalidation, to confirm it
does not fire on the recluster path — `ReclusterPins` reuses the
existing handler with an unchanged `VirtualView`, so the mapper does not
re-run there.

### Issues Fixed

Fixes #37773

---------

Co-authored-by: Kévin Baumeyer <kbaumeyer@divalto.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

Author: 3 people

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

@@ -1442,37 +1442,37 @@ public void ClusterInfoConstructorThrowsOnNullArguments()
 		}
 
 		[Fact]
-		public void GetClusterIconCacheKeyForFileImageSourceIsStableAcrossInstances()
+		public void GetIconCacheKeyForFileImageSourceIsStableAcrossInstances()
 		{
 			var first = new FileImageSource { File = "icon.png" };
 			var second = new FileImageSource { File = "icon.png" };
 
-			var firstKey = MapHandler.GetClusterIconCacheKey(first);
-			var secondKey = MapHandler.GetClusterIconCacheKey(second);
+			var firstKey = MapHandler.GetIconCacheKey(first);
+			var secondKey = MapHandler.GetIconCacheKey(second);
 
 			Assert.NotNull(firstKey);
 			Assert.StartsWith("file:", firstKey, StringComparison.Ordinal);
 			Assert.Equal(firstKey, secondKey);
 		}
 
 		[Fact]
-		public void GetClusterIconCacheKeyForUriImageSourceDependsOnCachingEnabled()
+		public void GetIconCacheKeyForUriImageSourceDependsOnCachingEnabled()
 		{
 			var uri = new Uri("https://example.com/icon.png");
 
 			var cachingEnabled = new UriImageSource { Uri = uri, CachingEnabled = true };
 			var cachingDisabled = new UriImageSource { Uri = uri, CachingEnabled = false };
 
-			var enabledKey = MapHandler.GetClusterIconCacheKey(cachingEnabled);
-			var disabledKey = MapHandler.GetClusterIconCacheKey(cachingDisabled);
+			var enabledKey = MapHandler.GetIconCacheKey(cachingEnabled);
+			var disabledKey = MapHandler.GetIconCacheKey(cachingDisabled);
 
 			Assert.NotNull(enabledKey);
 			Assert.StartsWith("uri:", enabledKey, StringComparison.Ordinal);
 			Assert.Null(disabledKey);
 		}
 
 		[Fact]
-		public void GetClusterIconCacheKeyForUriImageSourceRequiresPositiveValidity()
+		public void GetIconCacheKeyForUriImageSourceRequiresPositiveValidity()
 		{
 			var source = new UriImageSource
 			{
@@ -1481,22 +1481,22 @@ public void GetClusterIconCacheKeyForUriImageSourceRequiresPositiveValidity()
 				CacheValidity = TimeSpan.Zero
 			};
 
-			Assert.Null(MapHandler.GetClusterIconCacheKey(source));
+			Assert.Null(MapHandler.GetIconCacheKey(source));
 		}
 
 		[Fact]
-		public void GetClusterIconCacheKeyForFontImageSourceContainsGlyph()
+		public void GetIconCacheKeyForFontImageSourceContainsGlyph()
 		{
 			var font = new FontImageSource { Glyph = "A", FontFamily = "F", Size = 24, Color = Colors.White };
 
-			var key = MapHandler.GetClusterIconCacheKey(font);
+			var key = MapHandler.GetIconCacheKey(font);
 
 			Assert.NotNull(key);
 			Assert.Contains("A", key, StringComparison.Ordinal);
 		}
 
 		[Fact]
-		public void GetClusterIconCacheKeyForFontImageSourceDistinguishesWeight()
+		public void GetIconCacheKeyForFontImageSourceDistinguishesWeight()
 		{
 			var regular = new FakeFontImageSource
 			{
@@ -1511,16 +1511,16 @@ public void GetClusterIconCacheKeyForFontImageSourceDistinguishesWeight()
 				Font = Font.OfSize("F", 24).WithWeight(FontWeight.Bold)
 			};
 
-			var regularKey = MapHandler.GetClusterIconCacheKey(regular);
-			var boldKey = MapHandler.GetClusterIconCacheKey(bold);
+			var regularKey = MapHandler.GetIconCacheKey(regular);
+			var boldKey = MapHandler.GetIconCacheKey(bold);
 
 			Assert.NotNull(regularKey);
 			Assert.NotNull(boldKey);
 			Assert.NotEqual(regularKey, boldKey);
 		}
 
 		[Fact]
-		public void GetClusterIconCacheKeyForFontImageSourceDistinguishesAutoScaling()
+		public void GetIconCacheKeyForFontImageSourceDistinguishesAutoScaling()
 		{
 			var scalingEnabled = new FakeFontImageSource
 			{
@@ -1535,27 +1535,27 @@ public void GetClusterIconCacheKeyForFontImageSourceDistinguishesAutoScaling()
 				Font = Font.OfSize("F", 24, enableScaling: false)
 			};
 
-			var enabledKey = MapHandler.GetClusterIconCacheKey(scalingEnabled);
-			var disabledKey = MapHandler.GetClusterIconCacheKey(scalingDisabled);
+			var enabledKey = MapHandler.GetIconCacheKey(scalingEnabled);
+			var disabledKey = MapHandler.GetIconCacheKey(scalingDisabled);
 
 			Assert.NotNull(enabledKey);
 			Assert.NotNull(disabledKey);
 			Assert.NotEqual(enabledKey, disabledKey);
 		}
 
 		[Fact]
-		public void GetClusterIconCacheKeyIsNullForStreamOrMissingSource()
+		public void GetIconCacheKeyIsNullForStreamOrMissingSource()
 		{
 			var stream = new StreamImageSource();
 
-			Assert.Null(MapHandler.GetClusterIconCacheKey(stream));
-			Assert.Null(MapHandler.GetClusterIconCacheKey(null));
+			Assert.Null(MapHandler.GetIconCacheKey(stream));
+			Assert.Null(MapHandler.GetIconCacheKey(null));
 		}
 
 		[Fact]
-		public async Task ClusterIconCacheCoalescesConcurrentLoads()
+		public async Task IconCacheCoalescesConcurrentLoads()
 		{
-			var cache = new ClusterIconCache<object>(2);
+			var cache = new IconCache<object>(2);
 			var release = new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously);
 			var loadCount = 0;
 
@@ -1576,9 +1576,9 @@ Task<object> Load()
 		}
 
 		[Fact]
-		public async Task ClusterIconCacheEvictsLeastRecentlyUsedEntry()
+		public async Task IconCacheEvictsLeastRecentlyUsedEntry()
 		{
-			var cache = new ClusterIconCache<object>(2);
+			var cache = new IconCache<object>(2);
 			var first = new object();
 			var second = new object();
 			var third = new object();
@@ -1595,6 +1595,52 @@ public async Task ClusterIconCacheEvictsLeastRecentlyUsedEntry()
 			Assert.Same(third, cachedThird);
 		}
 
+		// The recluster path re-runs AddPins on every zoom step, so this is what stops a pin's icon
+		// being decoded and rescaled again on each pass.
+		[Fact]
+		public async Task IconCacheDoesNotReloadAKeyedSourceOnALaterPass()
+		{
+			var cache = new IconCache<object>(2);
+			var icon = new object();
+			var loadCount = 0;
+
+			Task<object> Load()
+			{
+				loadCount++;
+				return Task.FromResult(icon);
+			}
+
+			var first = await cache.GetOrCreateAsync("file:pin.png", Load, () => DateTime.MaxValue);
+			var second = await cache.GetOrCreateAsync("file:pin.png", Load, () => DateTime.MaxValue);
+
+			Assert.Equal(1, loadCount);
+			Assert.Same(icon, first);
+			Assert.Same(second, first);
+		}
+
+		// GetIconCacheKey returns null for a source that can't be keyed stably, or that opted out of
+		// caching (see GetIconCacheKeyForUriImageSourceDependsOnCachingEnabled). Those must never be
+		// frozen by the handler-level cache - they reload on every pass instead.
+		[Fact]
+		public async Task IconCacheAlwaysReloadsWhenTheKeyIsNull()
+		{
+			var cache = new IconCache<object>(2);
+			var loadCount = 0;
+
+			Task<object> Load()
+			{
+				loadCount++;
+				return Task.FromResult(new object());
+			}
+
+			var first = await cache.GetOrCreateAsync(null, Load, () => DateTime.MaxValue);
+			var second = await cache.GetOrCreateAsync(null, Load, () => DateTime.MaxValue);
+
+			Assert.Equal(2, loadCount);
+			Assert.NotSame(first, second);
+			Assert.Equal(0, cache.Count);
+		}
+
 		class FakeFontImageSource : IFontImageSource
 		{
 			public Color Color { get; set; }

src/Core/maps/src/Handlers/Map/MapHandler.Android.cs

@@ -48,8 +48,8 @@ public partial class MapHandler : ViewHandler<IMap, MapView>
 		bool _isClusteringEnabled;
 		List<MapCluster>? _clusters;
 		Dictionary<string, MapCluster>? _clusterMarkers;
-		const int MaxClusterIconCacheSize = 64;
-		readonly ClusterIconCache<BitmapDescriptor> _clusterIconCache = new(MaxClusterIconCacheSize);
+		const int MaxIconCacheSize = 64;
+		readonly IconCache<BitmapDescriptor> _iconCache = new(MaxIconCacheSize);
 		WeakReference<IMap>? _clusterImageOwner;
 		int _clusterImageVersion = int.MinValue;
 
@@ -135,7 +135,7 @@ protected override void DisconnectHandler(MapView platformView)
 			_circles = null;
 			_clusters?.Clear();
 			_clusterMarkers?.Clear();
-			_clusterIconCache.Clear();
+			_iconCache.Clear();
 			_clusterImageOwner = null;
 			_clusterImageVersion = int.MinValue;
 			_init = true;
@@ -414,7 +414,7 @@ void UpdateClusterImageVersion(IMap map)
 
 			_clusterImageOwner = new WeakReference<IMap>(map);
 			_clusterImageVersion = version;
-			_clusterIconCache.Clear();
+			_iconCache.Clear();
 		}
 
 		/// <summary>
@@ -907,11 +907,27 @@ async Task AddPinAsync(IMapPin pin, CancellationToken ct)
 
 			var markerOptions = iMapPinHandler.PlatformView;
 
-			// Load custom image if specified
-			if (pin.ImageSource != null)
+			// Captured before the await: if ImageSource changes while the load is running, the write
+			// below has to be dropped rather than publish a marker with the older icon.
+			var requestedSource = pin.ImageSource;
+
+			if (requestedSource is null)
+			{
+				// The MarkerOptions are reused, so a cleared ImageSource has to take the previous
+				// icon off them - null is Android's "use the default marker".
+				markerOptions.SetIcon(null);
+			}
+			else
 			{
-				var icon = await LoadPinIconAsync(pin.ImageSource, MauiContext, ct);
-				if (icon != null)
+				// Clustering re-runs AddPins on every zoom change, so without the cache the icon of
+				// every un-clustered pin is decoded and rescaled again on each pass.
+				var icon = await GetIconAsync(requestedSource, MauiContext, ct);
+
+				// Re-checked after the await: UpdatePinImageSourceAsync can have applied a newer
+				// source in the meantime, and it guards its own write the same way. Without this
+				// the two writers race and the marker can be published with the older icon while
+				// the change notification that would fix it has already been consumed.
+				if (icon != null && ReferenceEquals(pin.ImageSource, requestedSource))
 					markerOptions.SetIcon(icon);
 			}
 
@@ -955,9 +971,35 @@ async Task UpdatePinImageSourceAsync(IMapPin pin, Marker marker, CancellationTok
 			if (!ct.IsCancellationRequested && ReferenceEquals(pin.ImageSource, requestedSource) && (pin.MarkerId as string) == marker.Id)
 			{
 				marker.SetIcon(icon);
+
+				// The MarkerOptions are reused the next time this pin is added, so update them too -
+				// otherwise the next cluster pass would restore the previous icon. Reached through
+				// the non-throwing accessor: this resumes after an await, and the typed PlatformView
+				// throws once the handler has been disconnected.
+				if (pin.Handler is MapPinHandler mapPinHandler &&
+					((IElementHandler)mapPinHandler).PlatformView is MarkerOptions handlerOptions)
+				{
+					handlerOptions.SetIcon(icon);
+				}
 			}
 		}
 
+		// Shared by pin and cluster markers: one decoded descriptor backs every marker naming the same
+		// image, so a recluster pass reuses it instead of decoding per marker. GetIconCacheKey returns
+		// null for a source that can't be keyed stably, or that opted out of caching - those load fresh
+		// every time, which is also why they keep this caller's ct: nobody else is waiting on them.
+		// A keyable load is shared, so it must not be cancelled by whichever caller happened to start it.
+		Task<BitmapDescriptor?> GetIconAsync(IImageSource imageSource, IMauiContext mauiContext, CancellationToken ct)
+		{
+			var cacheKey = GetIconCacheKey(imageSource);
+			var loadToken = cacheKey is null ? ct : CancellationToken.None;
+
+			return _iconCache.GetOrCreateAsync(
+				cacheKey,
+				() => LoadPinIconAsync(imageSource, mauiContext, loadToken),
+				() => GetIconCacheExpiry(imageSource));
+		}
+
 		// Loads imageSource into a BitmapDescriptor
 		// Returns null on cancel/failure/no drawable.
 		static async Task<BitmapDescriptor?> LoadPinIconAsync(IImageSource imageSource, IMauiContext mauiContext, CancellationToken ct)
@@ -1009,14 +1051,7 @@ async Task AddClusterMarkerAsync(MapCluster cluster, CancellationToken ct)
 
 			BitmapDescriptor? icon = null;
 			if (image != null)
-			{
-				var cacheKey = GetClusterIconCacheKey(image);
-				var loadToken = cacheKey is null ? ct : CancellationToken.None;
-				icon = await _clusterIconCache.GetOrCreateAsync(
-					cacheKey,
-					() => LoadPinIconAsync(image, mauiContext, loadToken),
-					() => GetClusterIconCacheExpiry(image));
-			}
+				icon = await GetIconAsync(image, mauiContext, ct);
 
 			if (ct.IsCancellationRequested || !ReferenceEquals(Map, map))
 				return;

src/Core/maps/src/Handlers/Map/MapHandler.cs

@@ -104,13 +104,13 @@ public static void MapHideInfoWindow(IMapHandler handler, IMap map, object? arg)
 				mapHandler.HideInfoWindow(pin);
 		}
 
-		// Builds a stable cache key for a cluster icon so logically identical images (same file, URI,
-		// or font glyph) share one decoded/rasterized bitmap even when the provider hands back a fresh
-		// ImageSource instance on every recluster. Returns null for sources that can't be keyed stably
-		// (e.g. streams), so those are simply loaded fresh instead of being cached forever.
+		// Builds a stable cache key for a marker icon - cluster or pin - so logically identical images
+		// (same file, URI, or font glyph) share one decoded/rasterized bitmap even when the caller hands
+		// back a fresh ImageSource instance on every recluster. Returns null for sources that can't be
+		// keyed stably (e.g. streams), so those are simply loaded fresh instead of being cached forever.
 		// A URI source with CachingEnabled == false has explicitly opted out of caching, so it must not
 		// be frozen by this handler-level cache either.
-		internal static string? GetClusterIconCacheKey(IImageSource? source) =>
+		internal static string? GetIconCacheKey(IImageSource? source) =>
 			source switch
 			{
 				IFileImageSource file when !string.IsNullOrEmpty(file.File) => $"file:{file.File}",
@@ -124,7 +124,7 @@ public static void MapHideInfoWindow(IMapHandler handler, IMap map, object? arg)
 		// URI sources carry an explicit CacheValidity; other stable sources never expire on their own
 		// (the cache is bounded and cleared during handler cleanup). Clamped so a large
 		// validity like TimeSpan.MaxValue ("cache forever") can't overflow DateTime arithmetic.
-		internal static DateTime GetClusterIconCacheExpiry(IImageSource? source)
+		internal static DateTime GetIconCacheExpiry(IImageSource? source)
 		{
 			if (source is not IUriImageSource uri)
 				return DateTime.MaxValue;
@@ -134,7 +134,10 @@ internal static DateTime GetClusterIconCacheExpiry(IImageSource? source)
 		}
 	}
 
-	internal sealed class ClusterIconCache<T>
+	// Bounded LRU of decoded marker icons, shared by cluster and pin markers so one decode serves
+	// every marker naming the same image. Keys come from MapHandler.GetIconCacheKey; a null key
+	// bypasses the cache entirely rather than being stored under a fabricated one.
+	internal sealed class IconCache<T>
 		where T : class
 	{
 		readonly Dictionary<string, (T Value, DateTime ExpiresAtUtc, long AccessTick)> _entries = new();
@@ -145,7 +148,7 @@ internal sealed class ClusterIconCache<T>
 		long _accessCounter;
 		int _generation;
 
-		internal ClusterIconCache(int capacity, Action<T>? disposeValue = null)
+		internal IconCache(int capacity, Action<T>? disposeValue = null)
 		{
 			if (capacity <= 0)
 				throw new ArgumentOutOfRangeException(nameof(capacity));