Add Custom Pin Icons support for Maps#33950
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds first-class support for custom pin icons in .NET MAUI Maps by introducing a new ImageSource property on map pins and wiring platform renderers to apply that image when creating the native marker/annotation view.
Changes:
- Added
IMapPin.ImageSource(Core Maps) andPin.ImageSourcebindable property (Controls Maps). - Implemented custom icon application for pins on Android (via
MarkerOptions.SetIcon) and iOS/MacCatalyst (viaMKAnnotationView.Image). - Added unit tests for the new
Pin.ImageSourceproperty and a new sample gallery page to demonstrate usage.
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Core/maps/src/Core/IMapPin.cs | Adds IMapPin.ImageSource to the Core Maps pin contract. |
| src/Core/maps/src/Handlers/MapPin/MapPinHandler.cs (+ platform partials) | Adds ImageSource to the pin handler mapper and introduces per-platform MapImageSource methods (currently no-ops on some platforms). |
| src/Core/maps/src/Handlers/Map/MapHandler.Android.cs | Loads/scales an image and sets a custom marker icon before calling Map.AddMarker(). |
| src/Core/maps/src/Platform/iOS/MauiMKMapView.cs | Chooses a custom annotation view and asynchronously loads/scales an image for the annotation view. |
| src/Controls/Maps/src/Pin.cs + HandlerImpl/Pin.Impl.cs | Adds bindable Pin.ImageSource and exposes it via explicit IMapPin.ImageSource. |
| src/Controls/tests/Core.UnitTests/PinTests.cs | Adds unit tests validating default value, property change notifications, and interface projection. |
| src/Controls/samples/.../CustomPinIconGallery.xaml(.cs) + MapsGallery.cs | Adds a sample page and navigation entry demonstrating custom pin icons. |
| PublicAPI.Unshipped.txt (multiple TFMs) | Declares the newly shipped APIs across target frameworks. |
| void AddPins(IList pins) | ||
| { | ||
| //Mapper could be called before we have a Map ready | ||
| _pins = pins; | ||
| if (Map == null || MauiContext == null) | ||
| return; | ||
|
|
||
| if (_markers == null) | ||
| _markers = new List<Marker>(); | ||
|
|
||
| foreach (var p in pins) | ||
| { | ||
| IMapPin pin = (IMapPin)p; | ||
| Marker? marker; | ||
| AddPinAsync(pin).FireAndForget(); | ||
| } | ||
| _pins = null; | ||
| } |
There was a problem hiding this comment.
AddPins kicks off AddPinAsync(pin).FireAndForget() for each pin, but there’s no cancellation/generation check. If MapPins runs again (e.g., pins collection changes) and removes markers while earlier AddPinAsync calls are still awaiting image loads, those tasks can still add markers afterward, leaving stale/duplicate markers on the map. Consider tracking a version/cancellation token and bailing out before Map.AddMarker if a newer MapPins run occurred or the pin is no longer present.
src/Controls/samples/Controls.Sample/Pages/Controls/MapsGalleries/CustomPinIconGallery.xaml.cs
Outdated
Show resolved
Hide resolved
7f1df92 to
545d695
Compare
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
Independent Code ReviewSummary: Adds ✅ What Looks Good
🔴 Issues1. Thread-safety bug in if (_markers == null)
_markers = new List<Marker>();
_markers.Add(marker);
2. Android indentation error // Re-check after async operation since handler may have been disconnected
if (Map == null || MauiContext == null)
return;The 3. 4. No bitmap disposal after 5. iOS 🟡 Nits
|
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 33950Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 33950" |
|
Thanks for the thorough review @kubaflo! Addressed several issues: Fixed in latest commit:
Responses to other points: Thread-safety in _markers.Add — The \AddPinAsync\ calls all originate from the UI thread (\AddPins\ is called from the handler mapper). \FireAndForget\ resumes on the captured sync context (UI thread), so the \Map.AddMarker\ call and _markers.Add\ always run on the UI thread. The concurrent image loading happens off-thread, but the marker addition is serialized through the UI thread. Bitmap disposal after SetIcon — \BitmapDescriptorFactory.FromBitmap()\ copies the bitmap data internally, so the intermediate bitmap can be safely recycled. However, calling \�itmap.Recycle()\ immediately after might cause issues on some Android versions. The bitmap will be GC'd naturally. This is consistent with how Google Maps samples handle it. iOS UIGraphics.BeginImageContextWithOptions deprecated — Good observation. \UIGraphicsImageRenderer\ is the modern replacement. Filed for a follow-up to avoid scope creep. MapPinHandler.MapImageSource no-op — Correct observation. The mapper entry exists for API completeness (property changes go through the pin handler's mapper), but the actual work is done in \MapHandler.AddPinAsync\ during pin creation. This is a design trade-off: loading the image during \GetViewForAnnotation\ / \AddMarker\ is more efficient than re-loading on every property change. |
🔍 Round 2 Review — PR #33950 (Custom Pin Icons)Updated with author response analysis ✅ Fixed Since Round 1
|
Response to Round 2 ReviewGood catch on the iOS variable shadowing @kubaflo! 1. iOS variable shadowing - Fixed Thanks for the thorough second look! |
|
/azp run maui-pr |
|
Azure Pipelines successfully started running 1 pipeline(s). |
🔍 Round 3 — PR #33950 (Custom Pin Icons)✅ iOS Variable Shadowing — FixedVerified the fix: 📋 Final Status — Ready to merge ✅All original issues are now resolved:
Recommendation: Merge when CI passes. The |
Adds ImageSource property to Pin for custom map marker icons. - Add IImageSource? ImageSource to IMapPin interface - Add bindable ImageSource property to Pin class - iOS: Use MKAnnotationView with custom image instead of MKMarkerAnnotationView - Android: Load image before creating marker via BitmapDescriptorFactory - Add 5 unit tests for ImageSource property - Add CustomPinIconGallery sample page Fixes #10400
- Fix race condition: verify annotation view hasn't been reused after async load - Clear annotation image on reuse before loading new image - Dispose intermediate bitmap and recycle after scaling (Android) - Re-check Map/MauiContext after await (Android) - Log warnings instead of silently swallowing image load failures - Fix XML docs: image is scaled, not displayed at natural size - Fix comment: image is from file, not embedded resource - Fix comment: pixels, not dp
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ion type - Qualify System.Exception in Android handler (vs Java.Lang.Exception) - Add using System for EventArgs - Fix nullable event handler parameters - Fully qualify Location type to avoid XAML sourcegen conflict Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fix indentation error in AddPinAsync re-check after async - Add CancellationToken to AddPinAsync to prevent stale markers when pin collection changes during async image loading - Replace Debug.WriteLine with ILogger for image load failures (Android/iOS) - Use null-coalescing assignment for _markers Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Rename 'handler' to 'currentHandler' in catch block to avoid CS0136 conflict with outer scope 'handler' variable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
36424e7 to
3989ba6
Compare
|
🚨 API change(s) detected @davidortinau FYI |
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Description
Adds the ability to use custom images for map pins instead of the default platform markers.
Fixes #10400
Changes
API Changes
IImageSource? ImageSourceproperty toIMapPininterfaceImageSourcebindable property toPinclassPlatform Implementation
MKAnnotationViewwith custom image (scaled to 32x32 points)BitmapDescriptorFactory.FromBitmap()(scaled to 64x64 pixels)Usage
Testing
ImageSourcepropertyCustomPinIconGallerysample page